[MERGED] osmo-msc[master]: vlr: fix GSM AKA in a UMTS AKA capable environment

2018-03-10 Thread Neels Hofmeyr 
Neels Hofmeyr has submitted this change and it was merged.

Change subject: vlr: fix GSM AKA in a UMTS AKA capable environment
..


vlr: fix GSM AKA in a UMTS AKA capable environment

Switch by vsub->sec_ctx to use the proper Kc for ciphering.

Even on an R99 capable MS with a UMTS AKA capable USIM, the MS may still choose
to only perform GSM AKA, as long as the bearer is GERAN. The VLR already stores
whether the MS replied with a GSM AKA SRES or a UMTS AKA RES in vsub->sec_ctx.
So far, though, we were always using the UMTS AKA Kc just because the USIM and
core net are capable of it, ignoring the choice the MS might have made in the
Authentication Response.

In msc_vlr_test_gsm_ciph, fix the test expectations to the correct GSM AKA Kc
keys, showing that all of LU, CM Service Request and Paging Response now
support MS choosing GSM AKA in a UMTS capable environment.

Related: OS#2793
Change-Id: I42ce51ae979f42d173a45ae69273071c426bf97c
---
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
4 files changed, 33 insertions(+), 14 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved; Verified



diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 556e694..95a618d 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -284,6 +284,7 @@
 {
struct proc_arq_priv *par = fi->priv;
struct vlr_subscr *vsub = par->vsub;
+   bool umts_aka;
 
LOGPFSM(fi, "%s()\n", __func__);
 
@@ -292,9 +293,22 @@
return;
}
 
+   switch (vsub->sec_ctx) {
+   case VLR_SEC_CTX_GSM:
+   umts_aka = false;
+   break;
+   case VLR_SEC_CTX_UMTS:
+   umts_aka = true;
+   break;
+   default:
+   LOGPFSML(fi, LOGL_ERROR, "Cannot start ciphering, security 
context is not established\n");
+   proc_arq_fsm_done(fi, VLR_PR_ARQ_RES_SYSTEM_FAILURE);
+   return;
+   }
+
if (vlr_set_ciph_mode(vsub->vlr, fi, par->msc_conn_ref,
  par->ciphering_required,
- vlr_use_umts_aka(>last_tuple->vec, 
par->is_r99),
+ umts_aka,
  vsub->vlr->cfg.retrieve_imeisv_ciphered)) {
LOGPFSML(fi, LOGL_ERROR,
 "Failed to send Ciphering Mode Command\n");
diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index b36e4e3..c6fd080 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -846,6 +846,7 @@
 {
struct lu_fsm_priv *lfp = lu_fsm_fi_priv(fi);
struct vlr_subscr *vsub = lfp->vsub;
+   bool umts_aka;
 
LOGPFSM(fi, "%s()\n", __func__);
 
@@ -862,9 +863,22 @@
return;
}
 
+   switch (vsub->sec_ctx) {
+   case VLR_SEC_CTX_GSM:
+   umts_aka = false;
+   break;
+   case VLR_SEC_CTX_UMTS:
+   umts_aka = true;
+   break;
+   default:
+   LOGPFSML(fi, LOGL_ERROR, "Cannot start ciphering, security 
context is not established\n");
+   lu_fsm_failure(fi, GSM48_REJECT_NETWORK_FAILURE);
+   return;
+   }
+
if (vlr_set_ciph_mode(vsub->vlr, fi, lfp->msc_conn_ref,
  lfp->ciphering_required,
- vlr_use_umts_aka(>last_tuple->vec, 
lfp->is_r99),
+ umts_aka,
  vsub->vlr->cfg.retrieve_imeisv_ciphered)) {
LOGPFSML(fi, LOGL_ERROR,
 "Failed to send Ciphering Mode Command\n");
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c 
b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
index d8c0441..57284a3 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
@@ -901,10 +901,7 @@
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
 
btw("MS sends *GSM AKA* Authen Response, VLR accepts and sends 
Ciphering Mode Command to MS");
-   /* EXPECTING ERROR: should be the GSM AKA kc:
expect_cipher_mode_cmd("7a75f0ac9b844400");
-* but instead is the UMTS AKA derived kc: */
-   expect_cipher_mode_cmd("85c985d6f980e18e");
ms_sends_msg("0554" "dacc4b26");
OSMO_ASSERT(cipher_mode_cmd_sent);
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
@@ -947,10 +944,7 @@
thwart_rx_non_initial_requests();
 
btw("MS sends *GSM AKA* Authen Response, VLR accepts and requests 
Ciphering");
-   /* EXPECTING ERROR: should be the GSM AKA kc:
expect_cipher_mode_cmd("da149b11d473f400");
-* but instead is the UMTS AKA derived kc: */
-   expect_cipher_mode_cmd("dec1351054200a58");
ms_sends_msg("0554" "2fb4cfad");

osmo-msc[master]: vlr: fix GSM AKA in a UMTS AKA capable environment

2018-03-10 Thread Harald Welte 

Patch Set 2: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.osmocom.org/7187
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I42ce51ae979f42d173a45ae69273071c426bf97c
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No

[PATCH] osmo-msc[master]: vlr: fix GSM AKA in a UMTS AKA capable environment

2018-03-09 Thread Neels Hofmeyr 

vlr: fix GSM AKA in a UMTS AKA capable environment

Switch by vsub->sec_ctx to use the proper Kc for ciphering.

Even on an R99 capable MS with a UMTS AKA capable USIM, the MS may still choose
to only perform GSM AKA, as long as the bearer is GERAN. The VLR already stores
whether the MS replied with a GSM AKA SRES or a UMTS AKA RES in vsub->sec_ctx.
So far, though, we were always using the UMTS AKA Kc just because the USIM and
core net are capable of it, ignoring the choice the MS might have made in the
Authentication Response.

In msc_vlr_test_gsm_ciph, fix the test expectations to the correct GSM AKA Kc
keys, showing that all of LU, CM Service Request and Paging Response now
support MS choosing GSM AKA in a UMTS capable environment.

Related: OS#2793
Change-Id: I42ce51ae979f42d173a45ae69273071c426bf97c
---
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
4 files changed, 33 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/87/7187/2

diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 556e694..95a618d 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -284,6 +284,7 @@
 {
struct proc_arq_priv *par = fi->priv;
struct vlr_subscr *vsub = par->vsub;
+   bool umts_aka;
 
LOGPFSM(fi, "%s()\n", __func__);
 
@@ -292,9 +293,22 @@
return;
}
 
+   switch (vsub->sec_ctx) {
+   case VLR_SEC_CTX_GSM:
+   umts_aka = false;
+   break;
+   case VLR_SEC_CTX_UMTS:
+   umts_aka = true;
+   break;
+   default:
+   LOGPFSML(fi, LOGL_ERROR, "Cannot start ciphering, security 
context is not established\n");
+   proc_arq_fsm_done(fi, VLR_PR_ARQ_RES_SYSTEM_FAILURE);
+   return;
+   }
+
if (vlr_set_ciph_mode(vsub->vlr, fi, par->msc_conn_ref,
  par->ciphering_required,
- vlr_use_umts_aka(>last_tuple->vec, 
par->is_r99),
+ umts_aka,
  vsub->vlr->cfg.retrieve_imeisv_ciphered)) {
LOGPFSML(fi, LOGL_ERROR,
 "Failed to send Ciphering Mode Command\n");
diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index b36e4e3..c6fd080 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -846,6 +846,7 @@
 {
struct lu_fsm_priv *lfp = lu_fsm_fi_priv(fi);
struct vlr_subscr *vsub = lfp->vsub;
+   bool umts_aka;
 
LOGPFSM(fi, "%s()\n", __func__);
 
@@ -862,9 +863,22 @@
return;
}
 
+   switch (vsub->sec_ctx) {
+   case VLR_SEC_CTX_GSM:
+   umts_aka = false;
+   break;
+   case VLR_SEC_CTX_UMTS:
+   umts_aka = true;
+   break;
+   default:
+   LOGPFSML(fi, LOGL_ERROR, "Cannot start ciphering, security 
context is not established\n");
+   lu_fsm_failure(fi, GSM48_REJECT_NETWORK_FAILURE);
+   return;
+   }
+
if (vlr_set_ciph_mode(vsub->vlr, fi, lfp->msc_conn_ref,
  lfp->ciphering_required,
- vlr_use_umts_aka(>last_tuple->vec, 
lfp->is_r99),
+ umts_aka,
  vsub->vlr->cfg.retrieve_imeisv_ciphered)) {
LOGPFSML(fi, LOGL_ERROR,
 "Failed to send Ciphering Mode Command\n");
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c 
b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
index d8c0441..57284a3 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
@@ -901,10 +901,7 @@
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
 
btw("MS sends *GSM AKA* Authen Response, VLR accepts and sends 
Ciphering Mode Command to MS");
-   /* EXPECTING ERROR: should be the GSM AKA kc:
expect_cipher_mode_cmd("7a75f0ac9b844400");
-* but instead is the UMTS AKA derived kc: */
-   expect_cipher_mode_cmd("85c985d6f980e18e");
ms_sends_msg("0554" "dacc4b26");
OSMO_ASSERT(cipher_mode_cmd_sent);
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
@@ -947,10 +944,7 @@
thwart_rx_non_initial_requests();
 
btw("MS sends *GSM AKA* Authen Response, VLR accepts and requests 
Ciphering");
-   /* EXPECTING ERROR: should be the GSM AKA kc:
expect_cipher_mode_cmd("da149b11d473f400");
-* but instead is the UMTS AKA derived kc: */
-   expect_cipher_mode_cmd("dec1351054200a58");
ms_sends_msg("0554" "2fb4cfad");
VERBOSE_ASSERT(cm_service_result_sent, == RES_NONE, "%d");
VERBOSE_ASSERT(cipher_mode_cmd_sent, == true, "%d");
@@ -1006,10 +1000,7 @@
thwart_rx_non_initial_requests();

[PATCH] osmo-msc[master]: vlr: fix GSM AKA in a UMTS AKA capable environment

2018-03-09 Thread Neels Hofmeyr 

Review at  https://gerrit.osmocom.org/7187

vlr: fix GSM AKA in a UMTS AKA capable environment

Switch by vsub->sec_ctx to use the proper Kc for ciphering.

Even on an R99 capable UE with a UMTS AKA capable USIM, the ME may still choose
to only perform GSM AKA, as long as the bearer is GERAN. The VLR already stores
whether the MS replied with a GSM AKA SRES or a UMTS AKA RES in vsub->sec_ctx.
So far, though, we were always using the UMTS AKA Kc just because the USIM and
core net are capable of it, ignoring the choice the MS might have made in the
Authentication Response.

In msc_vlr_test_gsm_ciph, fix the test expectations to the correct GSM AKA Kc
keys, showing that all of LU, CM Service Request and Paging Response now
support MS choosing GSM AKA in a UMTS capable environment.

Related: OS#2793
Change-Id: I42ce51ae979f42d173a45ae69273071c426bf97c
---
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_lu_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
4 files changed, 33 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/87/7187/1

diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 556e694..95a618d 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -284,6 +284,7 @@
 {
struct proc_arq_priv *par = fi->priv;
struct vlr_subscr *vsub = par->vsub;
+   bool umts_aka;
 
LOGPFSM(fi, "%s()\n", __func__);
 
@@ -292,9 +293,22 @@
return;
}
 
+   switch (vsub->sec_ctx) {
+   case VLR_SEC_CTX_GSM:
+   umts_aka = false;
+   break;
+   case VLR_SEC_CTX_UMTS:
+   umts_aka = true;
+   break;
+   default:
+   LOGPFSML(fi, LOGL_ERROR, "Cannot start ciphering, security 
context is not established\n");
+   proc_arq_fsm_done(fi, VLR_PR_ARQ_RES_SYSTEM_FAILURE);
+   return;
+   }
+
if (vlr_set_ciph_mode(vsub->vlr, fi, par->msc_conn_ref,
  par->ciphering_required,
- vlr_use_umts_aka(>last_tuple->vec, 
par->is_r99),
+ umts_aka,
  vsub->vlr->cfg.retrieve_imeisv_ciphered)) {
LOGPFSML(fi, LOGL_ERROR,
 "Failed to send Ciphering Mode Command\n");
diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index b36e4e3..c6fd080 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -846,6 +846,7 @@
 {
struct lu_fsm_priv *lfp = lu_fsm_fi_priv(fi);
struct vlr_subscr *vsub = lfp->vsub;
+   bool umts_aka;
 
LOGPFSM(fi, "%s()\n", __func__);
 
@@ -862,9 +863,22 @@
return;
}
 
+   switch (vsub->sec_ctx) {
+   case VLR_SEC_CTX_GSM:
+   umts_aka = false;
+   break;
+   case VLR_SEC_CTX_UMTS:
+   umts_aka = true;
+   break;
+   default:
+   LOGPFSML(fi, LOGL_ERROR, "Cannot start ciphering, security 
context is not established\n");
+   lu_fsm_failure(fi, GSM48_REJECT_NETWORK_FAILURE);
+   return;
+   }
+
if (vlr_set_ciph_mode(vsub->vlr, fi, lfp->msc_conn_ref,
  lfp->ciphering_required,
- vlr_use_umts_aka(>last_tuple->vec, 
lfp->is_r99),
+ umts_aka,
  vsub->vlr->cfg.retrieve_imeisv_ciphered)) {
LOGPFSML(fi, LOGL_ERROR,
 "Failed to send Ciphering Mode Command\n");
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c 
b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
index d8c0441..57284a3 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
@@ -901,10 +901,7 @@
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
 
btw("MS sends *GSM AKA* Authen Response, VLR accepts and sends 
Ciphering Mode Command to MS");
-   /* EXPECTING ERROR: should be the GSM AKA kc:
expect_cipher_mode_cmd("7a75f0ac9b844400");
-* but instead is the UMTS AKA derived kc: */
-   expect_cipher_mode_cmd("85c985d6f980e18e");
ms_sends_msg("0554" "dacc4b26");
OSMO_ASSERT(cipher_mode_cmd_sent);
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
@@ -947,10 +944,7 @@
thwart_rx_non_initial_requests();
 
btw("MS sends *GSM AKA* Authen Response, VLR accepts and requests 
Ciphering");
-   /* EXPECTING ERROR: should be the GSM AKA kc:
expect_cipher_mode_cmd("da149b11d473f400");
-* but instead is the UMTS AKA derived kc: */
-   expect_cipher_mode_cmd("dec1351054200a58");
ms_sends_msg("0554" "2fb4cfad");
VERBOSE_ASSERT(cm_service_result_sent, == RES_NONE, "%d");
VERBOSE_ASSERT(cipher_mode_cmd_sent, == true, "%d");
@@ -1006,10 +1000,7 @@