This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgNeels Hofmeyr has uploaded this change for review. ( https://gerrit.osmocom.org/10985 Change subject: store classmark in vlr_subscr, not conn ...................................................................... store classmark in vlr_subscr, not conn Store all Classmark information in the VLR. So, we now always know the Classmark 1 (mandatory IE for LU). This is visible in the msc_vlr_tests -- they no longer indicate "assuming A5/1 is supported" because classmark 1 is missing, because we now know the Classmark 1. Rationale: During Location Updating, we receive Classmark 1; during CM Service Request and Paging Response, we receive Classmark 2. So far we stored these only for the duration of the conn, so as soon as a LU is complete, we would forget CM1. In other words, for anything else than a LU Request, we had no Classmark 1 available at all. During Ciphering Mode Command, we rely on Classmark 1 to determine whether A5/1 is supported. That is moot if we don't even have a Classmark 1 for any CM Service Request or Paging Response initiated connections. The only reason that A5/1 worked is that we assume A5/1 to work if Classmark 1 is missing. To add to the confusion, if a phone indicated that it did *not* support A5/1 in the Classmark 1, according to spec we're supposed to not service it at all. A code comment however says that we instead want to heed the flag -- which so far was only present in a Location Updating initiated connection. Now we can make this decision without assuming things. This got my attention while hacking on sending a BSSMAP Classmark Request from the MSC if it finds missing Classmark information, and was surprised to see it it lacking CM1 to decide about A5/1. Change-Id: I27081bf6e9e017923b2d02607f7ea06beddad82a --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/vlr.h M src/libmsc/gsm_04_08.c M src/libmsc/osmo_msc.c M tests/msc_vlr/msc_vlr_test_gsm_ciph.err M tests/msc_vlr/msc_vlr_test_rest.err 6 files changed, 75 insertions(+), 69 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/85/10985/1 diff --git a/include/osmocom/msc/gsm_data.h b/include/osmocom/msc/gsm_data.h index 27f7fc5..ffe3afc 100644 --- a/include/osmocom/msc/gsm_data.h +++ b/include/osmocom/msc/gsm_data.h @@ -127,8 +127,6 @@ /* connected via 2G or 3G? */ enum ran_type via_ran; - struct gsm_classmark classmark; - uint16_t lac; struct gsm_encr encr; diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h index 386a548..d52713c 100644 --- a/include/osmocom/msc/vlr.h +++ b/include/osmocom/msc/vlr.h @@ -174,6 +174,8 @@ uint8_t lac; enum ran_type attached_via_ran; } cs; + + struct gsm_classmark classmark; }; enum vlr_ciph { diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c index 19b0572..b942a03 100644 --- a/src/libmsc/gsm_04_08.c +++ b/src/libmsc/gsm_04_08.c @@ -98,14 +98,25 @@ return gsm48_conn_sendmsg(msg, conn, NULL); } +static bool classmark1_is_r99(const struct gsm48_classmark1 *cm1) +{ + return cm1->rev_lev >= 2; +} + +static bool classmark2_is_r99(const uint8_t *cm2, uint8_t cm2_len) +{ + uint8_t rev_lev; + if (!cm2_len) + return false; + rev_lev = (cm2[0] >> 5) & 0x3; + return rev_lev >= 2; +} + static bool classmark_is_r99(struct gsm_classmark *cm) { - int rev_lev = 0; if (cm->classmark1_set) - rev_lev = cm->classmark1.rev_lev; - else if (cm->classmark2_len > 0) - rev_lev = (cm->classmark2[0] >> 5) & 0x3; - return rev_lev >= 2; + return classmark1_is_r99(&cm->classmark1); + return classmark2_is_r99(cm->classmark2, cm->classmark2_len); } /* Determine if the given CLASSMARK (1/2/3) value permits a given A5/n cipher */ @@ -345,9 +356,6 @@ msc_subscr_conn_update_id(conn, COMPLETE_LAYER3_LU, mi_string); - conn->classmark.classmark1 = lu->classmark1; - conn->classmark.classmark1_set = true; - DEBUGP(DMM, "LOCATION UPDATING REQUEST: MI(%s)=%s type=%s\n", gsm48_mi_type_name(mi_type), mi_string, get_value_string(lupd_names, lu->type)); @@ -402,7 +410,7 @@ &old_lai, &new_lai, is_utran || conn->network->authentication_required, is_utran || conn->network->a5_encryption_mask > 0x01, - classmark_is_r99(&conn->classmark), + classmark1_is_r99(&lu->classmark1), is_utran, net->vlr->cfg.assign_tmsi); if (!lu_fsm) { @@ -421,6 +429,9 @@ return -EIO; } + conn->vsub->classmark.classmark1 = lu->classmark1; + conn->vsub->classmark.classmark1_set = true; + msc_subscr_conn_complete_layer_3(conn); return 0; } @@ -773,8 +784,6 @@ msc_subscr_conn_update_id(conn, COMPLETE_LAYER3_CM_SERVICE_REQ, mi_string); osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, mi_p); - memcpy(conn->classmark.classmark2, classmark2, classmark2_len); - conn->classmark.classmark2_len = classmark2_len; is_utran = (conn->via_ran == RAN_UTRAN_IU); vlr_proc_acc_req(conn->fi, @@ -783,9 +792,20 @@ VLR_PR_ARQ_T_CM_SERV_REQ, mi-1, &lai, is_utran || conn->network->authentication_required, is_utran || conn->network->a5_encryption_mask > 0x01, - classmark_is_r99(&conn->classmark), + classmark2_is_r99(classmark2, classmark2_len), is_utran); + /* From vlr_proc_acc_req() we expect an implicit dispatch of PR_ARQ_E_START we expect + * msc_vlr_subscr_assoc() to already have been called and completed. Has an error occured? */ + if (!conn->vsub) { + LOGP(DRR, LOGL_ERROR, "%s: subscriber not allowed to do a CM Service Request\n", + mi_string); + return -EIO; + } + + memcpy(conn->vsub->classmark.classmark2, classmark2, classmark2_len); + conn->vsub->classmark.classmark2_len = classmark2_len; + msc_subscr_conn_complete_layer_3(conn); return 0; } @@ -846,11 +866,6 @@ break; } - /* TODO? We used to remember the subscriber's classmark1 here and - * stored it in the old sqlite db, but now we store it in a conn that - * will be discarded anyway: */ - conn->classmark.classmark1 = idi->classmark1; - if (!vsub) { LOGP(DMM, LOGL_ERROR, "IMSI DETACH for unknown subscriber MI(%s)=%s\n", gsm48_mi_type_name(mi_type), mi_string); @@ -860,6 +875,9 @@ if (vsub->cs.is_paging) subscr_paging_cancel(vsub, GSM_PAGING_EXPIRED); + /* We already got Classmark 1 during Location Updating ... but well, ok */ + vsub->classmark.classmark1 = idi->classmark1; + vlr_subscr_rx_imsi_detach(vsub); osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_DETACHED, vsub); vlr_subscr_put(vsub); @@ -986,7 +1004,7 @@ is_umts ? "UMTS" : "GSM", is_umts ? "res" : "sres", osmo_hexdump_nospc(res, res_len)); - return vlr_subscr_rx_auth_resp(conn->vsub, classmark_is_r99(&conn->classmark), + return vlr_subscr_rx_auth_resp(conn->vsub, classmark_is_r99(&conn->vsub->classmark), conn->via_ran == RAN_UTRAN_IU, res, res_len); } @@ -1128,27 +1146,15 @@ return rc; } -static uint8_t *gsm48_cm2_get_mi(uint8_t *classmark2_lv, unsigned int tot_len) -{ - /* Check the size for the classmark */ - if (tot_len < 1 + *classmark2_lv) - return NULL; - - uint8_t *mi_lv = classmark2_lv + *classmark2_lv + 1; - if (tot_len < 2 + *classmark2_lv + mi_lv[0]) - return NULL; - - return mi_lv; -} - /* Receive a PAGING RESPONSE message from the MS */ static int gsm48_rx_rr_pag_resp(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm_network *net = conn->network; struct gsm48_hdr *gh = msgb_l3(msg); struct gsm48_pag_resp *resp; - uint8_t *classmark2_lv = gh->data + 1; - uint8_t *mi_lv; + uint8_t classmark2_len = gh->data[1]; + uint8_t *classmark2 = gh->data+2; + uint8_t *mi_lv = classmark2 + classmark2_len; uint8_t mi_type; char mi_string[GSM48_MI_SIZE]; struct osmo_location_area_id lai; @@ -1158,8 +1164,11 @@ lai.lac = conn->lac; resp = (struct gsm48_pag_resp *) &gh->data[0]; - gsm48_paging_extract_mi(resp, msgb_l3len(msg) - sizeof(*gh), - mi_string, &mi_type); + + if (gsm48_paging_extract_mi(resp, msgb_l3len(msg) - sizeof(*gh), mi_string, &mi_type) <= 0) { + LOGP(DRR, LOGL_ERROR, "PAGING RESPONSE: invalid Mobile Identity\n"); + return -EINVAL; + } if (msc_subscr_conn_is_establishing_auth_ciph(conn)) { LOGP(DMM, LOGL_ERROR, @@ -1174,17 +1183,8 @@ DEBUGP(DRR, "PAGING RESPONSE: MI(%s)=%s\n", gsm48_mi_type_name(mi_type), mi_string); - mi_lv = gsm48_cm2_get_mi(classmark2_lv, msgb_l3len(msg) - sizeof(*gh)); - if (!mi_lv) { - LOGP(DRR, LOGL_ERROR, "PAGING RESPONSE: invalid Mobile Identity\n"); - return -1; - } - msc_subscr_conn_update_id(conn, COMPLETE_LAYER3_PAGING_RESP, mi_string); - memcpy(conn->classmark.classmark2, classmark2_lv+1, *classmark2_lv); - conn->classmark.classmark2_len = *classmark2_lv; - is_utran = (conn->via_ran == RAN_UTRAN_IU); vlr_proc_acc_req(conn->fi, SUBSCR_CONN_E_ACCEPTED, SUBSCR_CONN_E_CN_CLOSE, NULL, @@ -1192,9 +1192,20 @@ VLR_PR_ARQ_T_PAGING_RESP, mi_lv, &lai, is_utran || conn->network->authentication_required, is_utran || conn->network->a5_encryption_mask > 0x01, - classmark_is_r99(&conn->classmark), + classmark2_is_r99(classmark2, classmark2_len), is_utran); + /* From vlr_proc_acc_req() we expect an implicit dispatch of PR_ARQ_E_START we expect + * msc_vlr_subscr_assoc() to already have been called and completed. Has an error occured? */ + if (!conn->vsub) { + LOGP(DRR, LOGL_ERROR, "%s: subscriber not allowed to do a Paging Response\n", + mi_string); + return -EIO; + } + + memcpy(conn->vsub->classmark.classmark2, classmark2, classmark2_len); + conn->vsub->classmark.classmark2_len = classmark2_len; + msc_subscr_conn_complete_layer_3(conn); return 0; } @@ -1390,10 +1401,10 @@ gh = msgb_l3(msg); pdisc = gsm48_hdr_pdisc(gh); - if (conn->classmark.classmark1_set && conn->classmark.classmark1.rev_lev < 2) { - modulo = gsm0407_determine_nsd_ret_modulo_r98(pdisc, gh->msg_type, &n_sd); - } else { /* R99 */ + if (conn->vsub && classmark_is_r99(&conn->vsub->classmark)) { modulo = gsm0407_determine_nsd_ret_modulo_r99(pdisc, gh->msg_type, &n_sd); + } else { /* pre R99 */ + modulo = gsm0407_determine_nsd_ret_modulo_r98(pdisc, gh->msg_type, &n_sd); } if (modulo == 0) return false; @@ -1618,7 +1629,7 @@ for (i = 0; i < 8; i++) { if (net->a5_encryption_mask & (1 << i) && - classmark_supports_a5(&conn->classmark, i)) + classmark_supports_a5(&conn->vsub->classmark, i)) ei.perm_algo[j++] = vlr_ciph_to_gsm0808_alg_id(i); } ei.perm_algo_len = j; diff --git a/src/libmsc/osmo_msc.c b/src/libmsc/osmo_msc.c index a6618c0..1966043 100644 --- a/src/libmsc/osmo_msc.c +++ b/src/libmsc/osmo_msc.c @@ -147,23 +147,25 @@ const uint8_t *cm2, uint8_t cm2_len, const uint8_t *cm3, uint8_t cm3_len) { + struct gsm_classmark *cm = &conn->vsub->classmark; + if (cm2 && cm2_len) { - if (cm2_len > sizeof(conn->classmark.classmark2)) { + if (cm2_len > sizeof(cm->classmark2)) { LOGP(DRR, LOGL_NOTICE, "%s: classmark2 is %u bytes, truncating at %zu bytes\n", - vlr_subscr_name(conn->vsub), cm2_len, sizeof(conn->classmark.classmark2)); - cm2_len = sizeof(conn->classmark.classmark2); + vlr_subscr_name(conn->vsub), cm2_len, sizeof(cm->classmark2)); + cm2_len = sizeof(cm->classmark2); } - conn->classmark.classmark2_len = cm2_len; - memcpy(conn->classmark.classmark2, cm2, cm2_len); + cm->classmark2_len = cm2_len; + memcpy(cm->classmark2, cm2, cm2_len); } if (cm3 && cm3_len) { - if (cm3_len > sizeof(conn->classmark.classmark3)) { + if (cm3_len > sizeof(cm->classmark3)) { LOGP(DRR, LOGL_NOTICE, "%s: classmark3 is %u bytes, truncating at %zu bytes\n", - vlr_subscr_name(conn->vsub), cm3_len, sizeof(conn->classmark.classmark3)); - cm3_len = sizeof(conn->classmark.classmark3); + vlr_subscr_name(conn->vsub), cm3_len, sizeof(cm->classmark3)); + cm3_len = sizeof(cm->classmark3); } - conn->classmark.classmark3_len = cm3_len; - memcpy(conn->classmark.classmark3, cm3, cm3_len); + cm->classmark3_len = cm3_len; + memcpy(cm->classmark3, cm3, cm3_len); } } diff --git a/tests/msc_vlr/msc_vlr_test_gsm_ciph.err b/tests/msc_vlr/msc_vlr_test_gsm_ciph.err index 83723ab..71e34a8 100644 --- a/tests/msc_vlr/msc_vlr_test_gsm_ciph.err +++ b/tests/msc_vlr/msc_vlr_test_gsm_ciph.err @@ -237,7 +237,6 @@ DVLR Process_Access_Request_VLR(CM_SERVICE_REQ:901700000004620){PR_ARQ_S_WAIT_AUTH}: _proc_arq_vlr_node2() DVLR Process_Access_Request_VLR(CM_SERVICE_REQ:901700000004620){PR_ARQ_S_WAIT_AUTH}: Set Ciphering Mode DMM -> CIPHER MODE COMMAND MSISDN:46071 -DMSC CLASSMARK 1 unknown, assuming MS supports A5/1 - sending Ciphering Mode Command for MSISDN:46071: include_imeisv=0 - ...perm algo: 2 - ...key: 07fa7502e07e1c00 @@ -376,7 +375,6 @@ DVLR Process_Access_Request_VLR(PAGING_RESP:901700000004620){PR_ARQ_S_WAIT_AUTH}: _proc_arq_vlr_node2() DVLR Process_Access_Request_VLR(PAGING_RESP:901700000004620){PR_ARQ_S_WAIT_AUTH}: Set Ciphering Mode DMM -> CIPHER MODE COMMAND MSISDN:46071 -DMSC CLASSMARK 1 unknown, assuming MS supports A5/1 - sending Ciphering Mode Command for MSISDN:46071: include_imeisv=0 - ...perm algo: 2 - ...key: e2b234f807886400 @@ -779,7 +777,6 @@ DVLR Process_Access_Request_VLR(CM_SERVICE_REQ:50462976){PR_ARQ_S_WAIT_AUTH}: _proc_arq_vlr_node2() DVLR Process_Access_Request_VLR(CM_SERVICE_REQ:50462976){PR_ARQ_S_WAIT_AUTH}: Set Ciphering Mode DMM -> CIPHER MODE COMMAND MSISDN:46071 -DMSC CLASSMARK 1 unknown, assuming MS supports A5/1 - sending Ciphering Mode Command for MSISDN:46071: include_imeisv=0 - ...perm algo: 2 - ...key: 07fa7502e07e1c00 @@ -918,7 +915,6 @@ DVLR Process_Access_Request_VLR(PAGING_RESP:50462976){PR_ARQ_S_WAIT_AUTH}: _proc_arq_vlr_node2() DVLR Process_Access_Request_VLR(PAGING_RESP:50462976){PR_ARQ_S_WAIT_AUTH}: Set Ciphering Mode DMM -> CIPHER MODE COMMAND MSISDN:46071 -DMSC CLASSMARK 1 unknown, assuming MS supports A5/1 - sending Ciphering Mode Command for MSISDN:46071: include_imeisv=0 - ...perm algo: 2 - ...key: e2b234f807886400 @@ -2018,7 +2014,6 @@ DVLR Process_Access_Request_VLR(CM_SERVICE_REQ:901700000010650){PR_ARQ_S_WAIT_AUTH}: _proc_arq_vlr_node2() DVLR Process_Access_Request_VLR(CM_SERVICE_REQ:901700000010650){PR_ARQ_S_WAIT_AUTH}: Set Ciphering Mode DMM -> CIPHER MODE COMMAND MSISDN:42342 -DMSC CLASSMARK 1 unknown, assuming MS supports A5/1 - sending Ciphering Mode Command for MSISDN:42342: include_imeisv=0 - ...perm algo: 2 - ...key: da149b11d473f400 @@ -2147,7 +2142,6 @@ DVLR Process_Access_Request_VLR(PAGING_RESP:901700000010650){PR_ARQ_S_WAIT_AUTH}: _proc_arq_vlr_node2() DVLR Process_Access_Request_VLR(PAGING_RESP:901700000010650){PR_ARQ_S_WAIT_AUTH}: Set Ciphering Mode DMM -> CIPHER MODE COMMAND MSISDN:42342 -DMSC CLASSMARK 1 unknown, assuming MS supports A5/1 - sending Ciphering Mode Command for MSISDN:42342: include_imeisv=0 - ...perm algo: 2 - ...key: 26ec67fad3073000 diff --git a/tests/msc_vlr/msc_vlr_test_rest.err b/tests/msc_vlr/msc_vlr_test_rest.err index e71295a..395a138 100644 --- a/tests/msc_vlr/msc_vlr_test_rest.err +++ b/tests/msc_vlr/msc_vlr_test_rest.err @@ -71,8 +71,7 @@ DMM Subscr_Conn(CM_SERVICE_REQ:901700000004620){SUBSCR_CONN_S_NEW}: state_chg to SUBSCR_CONN_S_RELEASING DREF unknown: MSC conn use + release == 2 (0x101: compl_l3,release) - BSSAP Clear --RAN_GERAN_A--> MS -DMM Subscr_Conn(CM_SERVICE_REQ:901700000004620){SUBSCR_CONN_S_RELEASING}: Received Event SUBSCR_CONN_E_COMPLETE_LAYER_3 -DMM Subscr_Conn(CM_SERVICE_REQ:901700000004620){SUBSCR_CONN_S_RELEASING}: Event SUBSCR_CONN_E_COMPLETE_LAYER_3 not permitted +DRR 901700000004620: subscriber not allowed to do a CM Service Request DREF unknown: MSC conn use - compl_l3 == 1 (0x100: release) bssap_clear_sent == 1 - conn was released -- To view, visit https://gerrit.osmocom.org/10985 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I27081bf6e9e017923b2d02607f7ea06beddad82a Gerrit-Change-Number: 10985 Gerrit-PatchSet: 1 Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180917/74dc9637/attachment.htm>