Change in osmo-msc[master]: store classmark in vlr_subscr, not conn
Harald Welte has submitted this change and it was merged. ( 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(-) Approvals: Jenkins Builder: Verified Vadim Yanitskiy: Looks good to me, but someone else must approve Harald Welte: Looks good to me, approved 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(>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 @@ _lai, _lai, is_utran || conn->network->authentication_required, is_utran || conn->network->a5_encryption_mask > 0x01, - classmark_is_r99(>classmark), + classmark1_is_r99(>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; +
Change in osmo-msc[master]: store classmark in vlr_subscr, not conn
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/10985 ) Change subject: store classmark in vlr_subscr, not conn .. Patch Set 1: Code-Review+2 -- 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: comment Gerrit-Change-Id: I27081bf6e9e017923b2d02607f7ea06beddad82a Gerrit-Change-Number: 10985 Gerrit-PatchSet: 1 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Mon, 17 Sep 2018 10:33:10 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: store classmark in vlr_subscr, not conn
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/10985 ) Change subject: store classmark in vlr_subscr, not conn .. Patch Set 1: Code-Review+1 -- 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: comment Gerrit-Change-Id: I27081bf6e9e017923b2d02607f7ea06beddad82a Gerrit-Change-Number: 10985 Gerrit-PatchSet: 1 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Mon, 17 Sep 2018 05:38:08 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: store classmark in vlr_subscr, not conn
Neels 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(>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 @@ _lai, _lai, is_utran || conn->network->authentication_required, is_utran || conn->network->a5_encryption_mask > 0x01, - classmark_is_r99(>classmark), + classmark1_is_r99(>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 @@