[PATCH] osmo-msc[master]: fix: clear vlr_subscr->msc_conn_ref when the conn is discarded

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7176

to look at the new patch set (#3).

fix: clear vlr_subscr->msc_conn_ref when the conn is discarded

Before this, it was for example possible to crash the MSC by the vty 'show
subscriber' command, which would dereference a potentially stale
vsub->msc_conn_ref pointer.

Related: OS#3050
Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
---
M src/libmsc/osmo_msc.c
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/76/7176/3

diff --git a/src/libmsc/osmo_msc.c b/src/libmsc/osmo_msc.c
index 1478c19..f6df0d2 100644
--- a/src/libmsc/osmo_msc.c
+++ b/src/libmsc/osmo_msc.c
@@ -228,6 +228,7 @@
DEBUGP(DRLL, "subscr %s: Freeing subscriber connection\n",
   vlr_subscr_name(conn->vsub));
msc_subscr_cleanup(conn->vsub);
+   conn->vsub->msc_conn_ref = NULL;
vlr_subscr_put(conn->vsub);
conn->vsub = NULL;
} else

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder


osmo-msc[master]: vlr auth: gracefully reject malformed auth response

2018-03-09 Thread Neels Hofmeyr

Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/7188/3/src/libmsc/gsm_04_08.c
File src/libmsc/gsm_04_08.c:

Line 978:   return vlr_subscr_rx_auth_fail(conn->vsub, NULL);
hmm, this is actually meant for the MM Authentication Failure message from the 
MS, which also is capable of triggering an UMTS AKA Auth Resync. But I guess 
handling it like this is nicer than passing zero RES into 
vlr_subscr_rx_auth_resp() below, like in an earlier patch set? What do you guys 
think?

We could also go for a timeout, in the sense of dropping invalid messages on 
the floor. But here we can invalidate the conn fast. (just not conn_close() 
directly, which loses the responses we're meant to send in case of failure)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4179a290069ac61d0662de4ec7ca3edb76988899
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: Yes


[PATCH] osmo-msc[master]: cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7193

to look at the new patch set (#3).

cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA

The code deciding on whether UMTS AKA is used was cascaded and convoluted. By
flattening the decisions, they become easier to read and possibly catch more
weird corner cases / log information more clearly.

- First decide what AKA the RES length reflects.
- Then decide whether all prerequisites for UMTS AKA are satisfied.
- Finally, on UTRAN, turn down the auth if we don't have UMTS AKA, and neatly
  log all of the potential causes.

One corner case that should never occur is that the UMTS AKA RES length is
actually the same length as the GSM AKA SRES. If this nevertheless occurs, log
this as an error, though not turning down authentication because of it. (The
effect is that we would favor UMTS AKA when it has a res_len == sizeof(sres)
and would not succeed to GSM AKA. At least the log will tell us why, now.)

Adjust an expected test output, trivial logging difference.

Change-Id: I43f7f301ea85e518bac91f707391a53182e54fab
---
M src/libvlr/vlr_auth_fsm.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 47 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/93/7193/3

diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 51e22c9..0d0df11 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -136,34 +136,57 @@
struct gsm_auth_tuple *at = vsub->last_tuple;
struct osmo_auth_vector *vec = >vec;
bool check_umts;
+   bool res_is_umts_aka;
OSMO_ASSERT(at);
 
LOGVSUBP(LOGL_DEBUG, vsub, "received res: %s\n",
 osmo_hexdump(res, res_len));
 
/* RES must be present and at least 32bit */
-   if (!res || res_len < sizeof(vec->sres)) {
-   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH RES missing or too short "
-"(%u)\n", res_len);
+   if (!res || !res_len) {
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH SRES/RES missing\n");
goto out_false;
}
 
-   check_umts = false;
-   if (is_r99 && (vec->auth_types & OSMO_AUTH_TYPE_UMTS)) {
-   check_umts = true;
-   /* We have a R99 capable UE and have a UMTS AKA capable USIM.
-* However, the ME may still choose to only perform GSM AKA, as
-* long as the bearer is GERAN */
-   if (res_len != vec->res_len) {
-   if (is_utran) {
-   LOGVSUBP(LOGL_NOTICE, vsub,
-"AUTH via UTRAN but "
-"res_len(%u) != vec->res_len(%u)\n",
-res_len, vec->res_len);
-   goto out_false;
-   }
-   check_umts = false;
-   }
+   /* We're deciding the UMTS AKA-ness of the response by the RES size. So 
let's make sure we can't
+* mix them up by size. On UTRAN, we expect full length RES always, no 
way to mix up there. */
+   if (!is_utran && vec->res_len == sizeof(vec->sres))
+   LOGVSUBP(LOGL_ERROR, vsub, "Unforeseen situation: UMTS AKA's 
RES length"
+" equals the size of SRES: %u -- this code wants to 
differentiate"
+" the two by their size, which won't work properly 
now.\n", vec->res_len);
+
+   /* RES must be either vec->res_len (UMTS AKA) or sizeof(sres) (GSM AKA) 
*/
+   if (res_len == vec->res_len)
+   res_is_umts_aka = true;
+   else if (res_len == sizeof(vec->sres))
+   res_is_umts_aka = false;
+   else {
+   if (is_utran)
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH RES has invalid 
length: %u."
+" Expected %u (UMTS AKA)\n",
+res_len, vec->res_len);
+   else
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH SRES/RES has invalid 
length: %u."
+" Expected either %zu (GSM AKA) or %u (UMTS 
AKA)\n",
+res_len, sizeof(vec->sres), vec->res_len);
+   goto out_false;
+   }
+
+   check_umts = (is_r99
+ && (vec->auth_types & OSMO_AUTH_TYPE_UMTS)
+ && res_is_umts_aka);
+
+   /* 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 -- never on UTRAN: */
+   if (is_utran && !check_umts) {
+   LOGVSUBP(LOGL_ERROR, vsub,
+"AUTH via UTRAN, cannot allow GSM AKA"
+" (MS is %sR99 capable, vec has %sUMTS AKA tokens, 
res_len=%u is %s)\n",
+   

[PATCH] osmo-msc[master]: cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7194

to look at the new patch set (#3).

cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES

Change-Id: Ib0f9f573ffac2302fbd3ee28f48ccd8fce5fe286
---
M src/libvlr/vlr_auth_fsm.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
8 files changed, 69 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/94/7194/3

diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 0d0df11..224bc9f 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -139,8 +139,10 @@
bool res_is_umts_aka;
OSMO_ASSERT(at);
 
-   LOGVSUBP(LOGL_DEBUG, vsub, "received res: %s\n",
-osmo_hexdump(res, res_len));
+   LOGVSUBP(LOGL_DEBUG, vsub, "AUTH on %s received %s: %s (%u bytes)\n",
+is_utran ? "UTRAN" : "GERAN",
+is_utran ? "RES" : "SRES/RES",
+osmo_hexdump_nospc(res, res_len), res_len);
 
/* RES must be present and at least 32bit */
if (!res || !res_len) {
diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.err 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
index 843e0d5..df91789 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.err
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
@@ -52,7 +52,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM IMSI:90170010650: MM UMTS AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(IMSI:90170010650) AUTH on GERAN received SRES/RES: 
e229c19e791f2e41 (8 bytes)
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -219,7 +219,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(MSISDN:42342) AUTH on GERAN received SRES/RES: e229c19e791f2e41 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -340,7 +340,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: 7d b4 7c f7 f8 1e 4d c7 
+DVLR SUBSCR(MSISDN:42342) AUTH on GERAN received SRES/RES: 7db47cf7f81e4dc7 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -480,7 +480,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM IMSI:90170010650: MM UMTS AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(IMSI:90170010650) AUTH on UTRAN received RES: e229c19e791f2e41 
(8 bytes)
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -659,7 +659,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(MSISDN:42342) AUTH on UTRAN received RES: e229c19e791f2e41 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with too long RES

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7191

to look at the new patch set (#3).

msc_vlr_test_umts_authen: test response with too long RES

Change-Id: Ie5473f06fc2d04c6a9f343da5764ec95b292a5f9
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 289 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/91/7191/3

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index 66eacd7..861a615 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -671,6 +671,105 @@
comment_end();
 }
 
+static void _test_umts_authen_too_long_res(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" "2105" "791f2e4123" /* added one byte 
*/);
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+   ASSERT_RELEASE_CLEAR(via_ran);
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+}
+
+static void 

[PATCH] osmo-msc[master]: gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not res...

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7189

to look at the new patch set (#3).

gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not response size

Do not interpret the SRES/RES length returned in the auth response as the R99
capability bit, instead determine it from the actual Classmark information
associated with the conn.

This fixes the is_r99 flag passed in to vlr_subscr_rx_auth_resp(), which ends
up in the struct vlr_auth_resp_par dispatched to the auth_fi and influences the
authentication acceptance.

Though the effect of a wrongly-set-to-false R99 flag is not harmful in this
code path, let's not get this confused.

Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
---
M src/libmsc/gsm_04_08.c
1 file changed, 8 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/89/7189/3

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 95a5079..7eaeabd 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -952,7 +952,7 @@
uint8_t res[16];
uint8_t res_len;
int rc;
-   bool is_r99;
+   bool is_umts;
 
if (!conn->vsub) {
LOGP(DMM, LOGL_ERROR,
@@ -961,29 +961,27 @@
return -EINVAL;
}
 
-   if (msgb_l3len(msg) >
-   sizeof(struct gsm48_hdr) + sizeof(struct gsm48_auth_resp)) {
+   is_umts = (msgb_l3len(msg) > sizeof(struct gsm48_hdr) + sizeof(struct 
gsm48_auth_resp));
+
+   if (is_umts)
rc = parse_umts_auth_resp(res, _len, conn, msg);
-   is_r99 = true;
-   } else {
+   else
rc = parse_gsm_auth_resp(res, _len, conn, msg);
-   is_r99 = false;
-   }
 
if (rc) {
LOGP(DMM, LOGL_ERROR,
 "%s: MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA 
Auth Response"
 " failed with rc=%d\n",
-vlr_subscr_name(conn->vsub), is_r99 ? "UMTS" : "GSM", rc);
+vlr_subscr_name(conn->vsub), is_umts ? "UMTS" : "GSM", rc);
return vlr_subscr_rx_auth_fail(conn->vsub, NULL);
}
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
   vlr_subscr_name(conn->vsub),
-  is_r99 ? "R99" : "GSM", is_r99 ? "res" : "sres",
+  is_umts ? "R99" : "GSM", is_umts ? "res" : "sres",
   osmo_hexdump_nospc(res, res_len));
 
-   return vlr_subscr_rx_auth_resp(conn->vsub, is_r99,
+   return vlr_subscr_rx_auth_resp(conn->vsub, 
classmark_is_r99(>classmark),
   conn->via_ran == RAN_UTRAN_IU,
   res, res_len);
 }

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder


[PATCH] osmo-msc[master]: vlr auth: gracefully reject malformed auth response

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7188

to look at the new patch set (#3).

vlr auth: gracefully reject malformed auth response

Instead of just closing down the conn hard, signal auth failure to the VLR in
order to trigger all the actions we want to see with a failed authentication:
- a GSUP signal that the auth failed,
- a LU reject.
Verify this in new test_wrong_sres_length() in msc_vlr_test_gsm_authen.c.

Note that in gsm48_rx_mm_auth_resp(), the is_r99 flag is falsely derived from
the RES length, which upcoming commit Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
will fix.

Change-Id: I4179a290069ac61d0662de4ec7ca3edb76988899
---
M src/libmsc/gsm_04_08.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
3 files changed, 175 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/88/7188/3

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index f87a4c6..95a5079 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -971,8 +971,11 @@
}
 
if (rc) {
-   msc_subscr_conn_close(conn, GSM_CAUSE_AUTH_FAILED);
-   return -EINVAL;
+   LOGP(DMM, LOGL_ERROR,
+"%s: MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA 
Auth Response"
+" failed with rc=%d\n",
+vlr_subscr_name(conn->vsub), is_r99 ? "UMTS" : "GSM", rc);
+   return vlr_subscr_rx_auth_fail(conn->vsub, NULL);
}
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.c 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.c
index 9c09aa4..b0db12d 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.c
@@ -914,11 +914,70 @@
comment_end();
 }
 
+static void test_wrong_sres_length()
+{
+   comment_start();
+   fake_time_start();
+
+   net->authentication_required = true;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("0801080971004026f0");
+   ms_sends_msg("05080200816800013008991007006402");
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   auth_request_sent = false;
+   auth_request_expect_rand = "585df1ae287f6e273dce07090d61320b";
+   auth_request_expect_autn = NULL;
+   /* Based on a Ki of 000102030405060708090a0b0c0d0e0f */
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971004026f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0322"  "2010" "585df1ae287f6e273dce07090d61320b"
+   /*   TL sres   TL kc */
+   "2104" "2d8b2c3e" "2208" "61855fb81fc2a800"
+   "0322"  "2010" "12aca96fb4ffdea5c985cbafa9b6e18b"
+   "2104" "20bde240" "2208" "07fa7502e07e1c00"
+   "0322"  "2010" "e7c03ba7cf0e2fde82b2dc4d63077d42"
+   "2104" "a29514ae" "2208" "e2b234f807886400"
+   "0322"  "2010" "fa8f20b781b5881329d4fea26b1a3c51"
+   "2104" "5afc8d72" "2208" "2392f14f709ae000"
+   "0322"  "2010" "0fd4cc8dbe8715d1f439e304edfd68dc"
+   "2104" "bc8d1c5b" "2208" "da7cdd6bfe2d7000",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("If the HLR were to send a GSUP _UPDATE_LOCATION_RESULT we'd still 
reject");
+   gsup_rx("0601080971004026f0", NULL);
+   EXPECT_ACCEPTED(false);
+
+   thwart_rx_non_initial_requests();
+
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response with too short SRES data, auth is 
thwarted.");
+   gsup_expect_tx("0b01080971004026f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_bssap_clear();
+   ms_sends_msg("05542d8b2c");
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+   comment_end();
+}
+
 msc_vlr_test_func_t msc_vlr_tests[] = {
test_gsm_authen,
test_gsm_authen_tmsi,
test_gsm_authen_imei,
test_gsm_authen_tmsi_imei,
test_gsm_milenage_authen,
+   test_wrong_sres_length,
NULL
 };
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.err 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
index a46a838..006b2b3 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.err
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
@@ -1998,6 +1998,117 @@
 full talloc report on 'msgb' (total  0 bytes in   1 blocks)
 talloc_total_blocks(tall_bsc_ctx) == 7
 
+= 

[PATCH] osmo-msc[master]: cosmetic: gsm48_rx_mm_auth_resp(): log 'UMTS AKA', not 'R99 ...

2018-03-09 Thread Neels Hofmeyr

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

cosmetic: gsm48_rx_mm_auth_resp(): log 'UMTS AKA', not 'R99 AKA'

Change-Id: Iba43c685cbe238d96175267e9cc954b2f2f3e7fc
---
M src/libmsc/gsm_04_08.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
4 files changed, 31 insertions(+), 31 deletions(-)


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

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 7eaeabd..36f7a7b 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -978,7 +978,7 @@
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
   vlr_subscr_name(conn->vsub),
-  is_umts ? "R99" : "GSM", is_umts ? "res" : "sres",
+  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(>classmark),
diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.err 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
index 269cd35..843e0d5 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.err
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
@@ -50,7 +50,7 @@
   MSC <--RAN_GERAN_A-- MS: GSM48_MT_MM_AUTH_RESP
 DREF IMSI:90170010650: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM IMSI:90170010650: MM R99 AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
+DMM IMSI:90170010650: MM UMTS AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
 DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
@@ -217,7 +217,7 @@
   MSC <--RAN_GERAN_A-- MS: GSM48_MT_MM_AUTH_RESP
 DREF MSISDN:42342: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
+DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
 DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
@@ -338,7 +338,7 @@
   MSC <--RAN_GERAN_A-- MS: GSM48_MT_MM_AUTH_RESP
 DREF MSISDN:42342: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
+DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
 DVLR SUBSCR(MSISDN:42342) received res: 7d b4 7c f7 f8 1e 4d c7 
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
@@ -478,7 +478,7 @@
   MSC <--RAN_UTRAN_IU-- MS: GSM48_MT_MM_AUTH_RESP
 DREF IMSI:90170010650: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM IMSI:90170010650: MM R99 AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
+DMM IMSI:90170010650: MM UMTS AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
 DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
@@ -657,7 +657,7 @@
   MSC <--RAN_UTRAN_IU-- MS: GSM48_MT_MM_AUTH_RESP
 DREF MSISDN:42342: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
+DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
 DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
@@ -788,7 +788,7 @@
   MSC <--RAN_UTRAN_IU-- MS: GSM48_MT_MM_AUTH_RESP
 DREF MSISDN:42342: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
+DMM MSISDN:42342: MM UMTS AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
 DVLR SUBSCR(MSISDN:42342) received res: 7d b4 7c f7 f8 1e 4d c7 
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
@@ -938,7 +938,7 @@
   MSC <--RAN_GERAN_A-- MS: GSM48_MT_MM_AUTH_RESP
 DREF IMSI:90170010650: MSC conn use + dtap == 2 (0x6)
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
-DMM IMSI:90170010650: MM R99 AUTHENTICATION 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with too short RES

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7190

to look at the new patch set (#3).

msc_vlr_test_umts_authen: test response with too short RES

Change-Id: Ia1bc57b3dc1f3c3c654ba2d907b16ba925cd03e8
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 290 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/90/7190/3

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index e6dc445..66eacd7 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -572,10 +572,112 @@
comment_end();
 }
 
+static void _test_umts_authen_too_short_res(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" "2103" "791f2e" /* nipped one byte */);
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+   ASSERT_RELEASE_CLEAR(via_ran);
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+}
+
+static void 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with only SRES half ...

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7192

to look at the new patch set (#2).

msc_vlr_test_umts_authen: test response with only SRES half of RES

Change-Id: I0e9099625bd9d3de3db5ee29fbf81b2d8a30071d
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 294 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index 861a615..b5a55fc 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -770,6 +770,110 @@
comment_end();
 }
 
+static void _test_umts_authen_only_sres(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   if (via_ran == RAN_GERAN_A)
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts:"
+   " GERAN reports an SRES mismatch");
+   else
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts:"
+   " UTRAN disallows GSM AKA altogether");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   

[PATCH] osmo-msc[master]: cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7193

to look at the new patch set (#2).

cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA

The code deciding on whether UMTS AKA is used was cascaded and convoluted. By
flattening the decisions, they become easier to read and possibly catch more
weird corner cases / log information more clearly.

- First decide what AKA the RES length reflects.
- Then decide whether all prerequisites for UMTS AKA are satisfied.
- Finally, on UTRAN, turn down the auth if we don't have UMTS AKA, and neatly
  log all of the potential causes.

One corner case that should never occur is that the UMTS AKA RES length is
actually the same length as the GSM AKA SRES. If this nevertheless occurs, log
this as an error, though not turning down authentication because of it. (The
effect is that we would favor UMTS AKA when it has a res_len == sizeof(sres)
and would not succeed to GSM AKA. At least the log will tell us why, now.)

Adjust an expected test output, trivial logging difference.

Change-Id: I43f7f301ea85e518bac91f707391a53182e54fab
---
M src/libvlr/vlr_auth_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
3 files changed, 48 insertions(+), 25 deletions(-)


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

diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 51e22c9..0d0df11 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -136,34 +136,57 @@
struct gsm_auth_tuple *at = vsub->last_tuple;
struct osmo_auth_vector *vec = >vec;
bool check_umts;
+   bool res_is_umts_aka;
OSMO_ASSERT(at);
 
LOGVSUBP(LOGL_DEBUG, vsub, "received res: %s\n",
 osmo_hexdump(res, res_len));
 
/* RES must be present and at least 32bit */
-   if (!res || res_len < sizeof(vec->sres)) {
-   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH RES missing or too short "
-"(%u)\n", res_len);
+   if (!res || !res_len) {
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH SRES/RES missing\n");
goto out_false;
}
 
-   check_umts = false;
-   if (is_r99 && (vec->auth_types & OSMO_AUTH_TYPE_UMTS)) {
-   check_umts = true;
-   /* We have a R99 capable UE and have a UMTS AKA capable USIM.
-* However, the ME may still choose to only perform GSM AKA, as
-* long as the bearer is GERAN */
-   if (res_len != vec->res_len) {
-   if (is_utran) {
-   LOGVSUBP(LOGL_NOTICE, vsub,
-"AUTH via UTRAN but "
-"res_len(%u) != vec->res_len(%u)\n",
-res_len, vec->res_len);
-   goto out_false;
-   }
-   check_umts = false;
-   }
+   /* We're deciding the UMTS AKA-ness of the response by the RES size. So 
let's make sure we can't
+* mix them up by size. On UTRAN, we expect full length RES always, no 
way to mix up there. */
+   if (!is_utran && vec->res_len == sizeof(vec->sres))
+   LOGVSUBP(LOGL_ERROR, vsub, "Unforeseen situation: UMTS AKA's 
RES length"
+" equals the size of SRES: %u -- this code wants to 
differentiate"
+" the two by their size, which won't work properly 
now.\n", vec->res_len);
+
+   /* RES must be either vec->res_len (UMTS AKA) or sizeof(sres) (GSM AKA) 
*/
+   if (res_len == vec->res_len)
+   res_is_umts_aka = true;
+   else if (res_len == sizeof(vec->sres))
+   res_is_umts_aka = false;
+   else {
+   if (is_utran)
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH RES has invalid 
length: %u."
+" Expected %u (UMTS AKA)\n",
+res_len, vec->res_len);
+   else
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH SRES/RES has invalid 
length: %u."
+" Expected either %zu (GSM AKA) or %u (UMTS 
AKA)\n",
+res_len, sizeof(vec->sres), vec->res_len);
+   goto out_false;
+   }
+
+   check_umts = (is_r99
+ && (vec->auth_types & OSMO_AUTH_TYPE_UMTS)
+ && res_is_umts_aka);
+
+   /* 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 -- never on UTRAN: */
+   if (is_utran && !check_umts) {
+   LOGVSUBP(LOGL_ERROR, vsub,
+"AUTH via UTRAN, cannot allow GSM AKA"
+" (MS is %sR99 capable, vec has %sUMTS AKA tokens, 

[PATCH] osmo-msc[master]: cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7194

to look at the new patch set (#2).

cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES

Change-Id: Ib0f9f573ffac2302fbd3ee28f48ccd8fce5fe286
---
M src/libvlr/vlr_auth_fsm.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
8 files changed, 70 insertions(+), 68 deletions(-)


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

diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 0d0df11..224bc9f 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -139,8 +139,10 @@
bool res_is_umts_aka;
OSMO_ASSERT(at);
 
-   LOGVSUBP(LOGL_DEBUG, vsub, "received res: %s\n",
-osmo_hexdump(res, res_len));
+   LOGVSUBP(LOGL_DEBUG, vsub, "AUTH on %s received %s: %s (%u bytes)\n",
+is_utran ? "UTRAN" : "GERAN",
+is_utran ? "RES" : "SRES/RES",
+osmo_hexdump_nospc(res, res_len), res_len);
 
/* RES must be present and at least 32bit */
if (!res || !res_len) {
diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.err 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
index 269cd35..474fea1 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.err
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
@@ -52,7 +52,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM IMSI:90170010650: MM R99 AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(IMSI:90170010650) AUTH on GERAN received SRES/RES: 
e229c19e791f2e41 (8 bytes)
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -219,7 +219,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(MSISDN:42342) AUTH on GERAN received SRES/RES: e229c19e791f2e41 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -340,7 +340,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: 7d b4 7c f7 f8 1e 4d c7 
+DVLR SUBSCR(MSISDN:42342) AUTH on GERAN received SRES/RES: 7db47cf7f81e4dc7 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -480,7 +480,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM IMSI:90170010650: MM R99 AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(IMSI:90170010650) AUTH on UTRAN received RES: e229c19e791f2e41 
(8 bytes)
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -659,7 +659,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(MSISDN:42342) AUTH on UTRAN received RES: e229c19e791f2e41 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with too long RES

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7191

to look at the new patch set (#2).

msc_vlr_test_umts_authen: test response with too long RES

Change-Id: Ie5473f06fc2d04c6a9f343da5764ec95b292a5f9
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 289 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index 66eacd7..861a615 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -671,6 +671,105 @@
comment_end();
 }
 
+static void _test_umts_authen_too_long_res(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" "2105" "791f2e4123" /* added one byte 
*/);
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+   ASSERT_RELEASE_CLEAR(via_ran);
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+}
+
+static void 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with too short RES

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7190

to look at the new patch set (#2).

msc_vlr_test_umts_authen: test response with too short RES

Change-Id: Ia1bc57b3dc1f3c3c654ba2d907b16ba925cd03e8
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 290 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index e6dc445..66eacd7 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -572,10 +572,112 @@
comment_end();
 }
 
+static void _test_umts_authen_too_short_res(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" "2103" "791f2e" /* nipped one byte */);
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+   ASSERT_RELEASE_CLEAR(via_ran);
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+}
+
+static void 

[PATCH] osmo-msc[master]: gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not res...

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7189

to look at the new patch set (#2).

gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not response size

Do not interpret the SRES/RES length returned in the auth response as the R99
capability bit, instead determine it from the actual Classmark information
associated with the conn.

This fixes the is_r99 flag passed in to vlr_subscr_rx_auth_resp(), which ends
up in the struct vlr_auth_resp_par dispatched to the auth_fi and influences the
authentication acceptance.

Though the effect of a wrongly-set-to-false R99 flag is not harmful in this
code path, let's not get this confused.

Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
---
M src/libmsc/gsm_04_08.c
1 file changed, 8 insertions(+), 10 deletions(-)


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

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index ceef2d8..4564f8e 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -952,7 +952,7 @@
uint8_t res[16];
uint8_t res_len;
int rc;
-   bool is_r99;
+   bool is_umts;
 
if (!conn->vsub) {
LOGP(DMM, LOGL_ERROR,
@@ -961,30 +961,28 @@
return -EINVAL;
}
 
-   if (msgb_l3len(msg) >
-   sizeof(struct gsm48_hdr) + sizeof(struct gsm48_auth_resp)) {
+   is_umts = (msgb_l3len(msg) > sizeof(struct gsm48_hdr) + sizeof(struct 
gsm48_auth_resp));
+
+   if (is_umts)
rc = parse_umts_auth_resp(res, _len, conn, msg);
-   is_r99 = true;
-   } else {
+   else
rc = parse_gsm_auth_resp(res, _len, conn, msg);
-   is_r99 = false;
-   }
 
if (rc) {
LOGP(DMM, LOGL_ERROR,
 "%s: MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA 
Auth Response"
 " failed with rc=%d; dispatching zero length SRES/RES to 
trigger failure\n",
-vlr_subscr_name(conn->vsub), is_r99 ? "UMTS" : "GSM", rc);
+vlr_subscr_name(conn->vsub), is_umts ? "UMTS" : "GSM", rc);
memset(res, 0, sizeof(res));
res_len = 0;
}
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
   vlr_subscr_name(conn->vsub),
-  is_r99 ? "R99" : "GSM", is_r99 ? "res" : "sres",
+  is_umts ? "R99" : "GSM", is_umts ? "res" : "sres",
   osmo_hexdump_nospc(res, res_len));
 
-   return vlr_subscr_rx_auth_resp(conn->vsub, is_r99,
+   return vlr_subscr_rx_auth_resp(conn->vsub, 
classmark_is_r99(>classmark),
   conn->via_ran == RAN_UTRAN_IU,
   res, res_len);
 }

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder


[PATCH] osmo-msc[master]: vlr auth: gracefully reject malformed auth response

2018-03-09 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7188

to look at the new patch set (#2).

vlr auth: gracefully reject malformed auth response

Instead of just closing down the conn hard, actually feed invalid auth response
data to vlr_subscr_rc_auth_resp() in order to trigger all the actions we want
to see with a failed authentication:
- a GSUP signal that the auth failed,
- a LU reject.

Verify this in new test_wrong_sres_length() in msc_vlr_test_gsm_authen.c.

Note that in this function, the is_r99 flag is falsely derived from the RES
length, which upcoming commit Ib7f7d89a8b9455d2c022d53d74328fa7488577f4 will
fix.

Change-Id: I4179a290069ac61d0662de4ec7ca3edb76988899
---
M src/libmsc/gsm_04_08.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
3 files changed, 179 insertions(+), 2 deletions(-)


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

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index f87a4c6..ceef2d8 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -971,8 +971,12 @@
}
 
if (rc) {
-   msc_subscr_conn_close(conn, GSM_CAUSE_AUTH_FAILED);
-   return -EINVAL;
+   LOGP(DMM, LOGL_ERROR,
+"%s: MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA 
Auth Response"
+" failed with rc=%d; dispatching zero length SRES/RES to 
trigger failure\n",
+vlr_subscr_name(conn->vsub), is_r99 ? "UMTS" : "GSM", rc);
+   memset(res, 0, sizeof(res));
+   res_len = 0;
}
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.c 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.c
index 9c09aa4..b0db12d 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.c
@@ -914,11 +914,70 @@
comment_end();
 }
 
+static void test_wrong_sres_length()
+{
+   comment_start();
+   fake_time_start();
+
+   net->authentication_required = true;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("0801080971004026f0");
+   ms_sends_msg("05080200816800013008991007006402");
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   auth_request_sent = false;
+   auth_request_expect_rand = "585df1ae287f6e273dce07090d61320b";
+   auth_request_expect_autn = NULL;
+   /* Based on a Ki of 000102030405060708090a0b0c0d0e0f */
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971004026f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0322"  "2010" "585df1ae287f6e273dce07090d61320b"
+   /*   TL sres   TL kc */
+   "2104" "2d8b2c3e" "2208" "61855fb81fc2a800"
+   "0322"  "2010" "12aca96fb4ffdea5c985cbafa9b6e18b"
+   "2104" "20bde240" "2208" "07fa7502e07e1c00"
+   "0322"  "2010" "e7c03ba7cf0e2fde82b2dc4d63077d42"
+   "2104" "a29514ae" "2208" "e2b234f807886400"
+   "0322"  "2010" "fa8f20b781b5881329d4fea26b1a3c51"
+   "2104" "5afc8d72" "2208" "2392f14f709ae000"
+   "0322"  "2010" "0fd4cc8dbe8715d1f439e304edfd68dc"
+   "2104" "bc8d1c5b" "2208" "da7cdd6bfe2d7000",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("If the HLR were to send a GSUP _UPDATE_LOCATION_RESULT we'd still 
reject");
+   gsup_rx("0601080971004026f0", NULL);
+   EXPECT_ACCEPTED(false);
+
+   thwart_rx_non_initial_requests();
+
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response with too short SRES data, auth is 
thwarted.");
+   gsup_expect_tx("0b01080971004026f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_bssap_clear();
+   ms_sends_msg("05542d8b2c");
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+   comment_end();
+}
+
 msc_vlr_test_func_t msc_vlr_tests[] = {
test_gsm_authen,
test_gsm_authen_tmsi,
test_gsm_authen_imei,
test_gsm_authen_tmsi_imei,
test_gsm_milenage_authen,
+   test_wrong_sres_length,
NULL
 };
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.err 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
index a46a838..a454e2f 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.err
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
@@ -1998,6 +1998,120 @@
 full talloc report on 

[PATCH] osmo-msc[master]: msc_vlr_test_gsm_ciph: add test for GSM AKA in UMTS environment

2018-03-09 Thread Neels Hofmeyr

msc_vlr_test_gsm_ciph: add test for GSM AKA in UMTS environment

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. In that case, we must
make sure to send the GSM AKA Kc for ciphering.

Add test_gsm_ciph_in_umts_env() to msc_vlr_test_gsm_ciph.c to answer an Auth
Request with a GSM AKA response (see the log stating "AUTH established GSM
security context" after we sent a UMTS AKA challenge).

In the test, show that we currently send the *wrong* Kc, i.e. the UMTS AKA
derived Kc for GERAN, instead of the correct Kc for GSM AKA (which was received
from the HLR in the auth tuples).

Subsequent patch I42ce51ae979f42d173a45ae69273071c426bf97c will fix this and
correct the test expectations.

Related: OS#2793
Change-Id: I85f12a20dcd701e671188e56811ec7b58d84da82
---
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
2 files changed, 725 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c 
b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
index 1e6bf7c..d8c0441 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
@@ -794,11 +794,271 @@
comment_end();
 }
 
+static void test_gsm_ciph_in_umts_env()
+{
+   struct vlr_subscr *vsub;
+   const char *imsi = "90170010650";
+   const char *sms =
+   "09" /* SMS messages */
+   "01" /* CP-DATA */
+   "58" /* length */
+   "01" /* Network to MS */
+   "00" /* reference */
+   /* originator (gsm411_send_sms() hardcodes this weird nr) */
+   "0791" "447758100650" /* 447785016005 */
+   "00" /* dest */
+   /* SMS TPDU */
+   "4c" /* len */
+   "00" /* SMS deliver */
+   "05802443f2" /* originating address 42342 */
+   "00" /* TP-PID */
+   "00" /* GSM default alphabet */
+   "071010" /* Y-M-D (from wrapped gsm340_gen_scts())*/
+   "00" /* H-M-S */
+   "00" /* GMT+0 */
+   "44" /* data length */
+   "5079da1e1ee7416937485e9ea7c965373d1d6683c270383b3d0e"
+   "d3d36ff71c949e83c22072799e9687c5ec32a81d96afcbf4b4fb"
+   "0c7ac3e9e9b7db05";
+
+   comment_start();
+
+   /* implicit: net->authentication_required = true; */
+   net->a5_encryption_mask = (1 << 1);
+   rx_from_ran = RAN_GERAN_A;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends *UMTS AKA* Auth Req 
to MS");
+   /* based on
+* 2G auth: COMP128v1
+*  KI=7bcd108be4c3d551ee6c67faaf52bd68
+* 3G auth: MILENAGE
+*  K=7bcd108be4c3d551ee6c67faaf52bd68
+*  OPC=6e23f641ce724679b73d933515a8589d
+*  IND-bitlen=5 last-SQN=641
+* Note that the SRES will be calculated by COMP128v1, separately from 
3G tokens;
+* the resulting Kc to use for ciphering returned by the HLR is also 
calculated from COMP128v1.
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "4ac8d1cd1a51937597ca1016fe69a0fa";
+   auth_request_expect_autn = "2d837d2b0d6f4b282d5acf23428d";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362" "2010" "4ac8d1cd1a51937597ca1016fe69a0fa"
+   /*   TL sres   TL kc */
+  "2104" "dacc4b26" "2208" "7a75f0ac9b844400"
+   /*   TL 3G IK */
+  "2310" "3747da4e31545baa2db59e500bdae047"
+   /*   TL 3G CK */
+  "2410" "8544d35b945ccba01a7f1293575291c3"
+   /*   TL AUTN */
+  "2510" "2d837d2b0d6f4b282d5acf23428d"
+   /*   TL RES */
+  "2708" "37527064741f8ddb"
+   /* TLTL rand */
+   "0362" "2010" "b2661531b97b12c5a2edc21a0ed16fc5"
+  "2104" "2fb4cfad" "2208" "da149b11d473f400"
+

[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]: msc_vlr_test_umts_authen: test response with too long RES

2018-03-09 Thread Neels Hofmeyr

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

msc_vlr_test_umts_authen: test response with too long RES

Change-Id: Ie5473f06fc2d04c6a9f343da5764ec95b292a5f9
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 289 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index 66eacd7..861a615 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -671,6 +671,105 @@
comment_end();
 }
 
+static void _test_umts_authen_too_long_res(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" "2105" "791f2e4123" /* added one byte 
*/);
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+   ASSERT_RELEASE_CLEAR(via_ran);
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+}
+
+static void test_umts_authen_too_long_res_geran()
+{
+   comment_start();
+   _test_umts_authen_too_long_res(RAN_GERAN_A);
+   

[PATCH] osmo-msc[master]: msc_vlr_test_gsm_ciph: add test for GSM AKA in UMTS environment

2018-03-09 Thread Neels Hofmeyr

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

msc_vlr_test_gsm_ciph: add test for GSM AKA in UMTS environment

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. In that case, we must
make sure to send the GSM AKA Kc for ciphering.

Add test_gsm_ciph_in_umts_env() to msc_vlr_test_gsm_ciph.c to answer an Auth
Request with a GSM AKA response (see the log stating "AUTH established GSM
security context" after we sent a UMTS AKA challenge).

In the test, show that we currently send the *wrong* Kc, i.e. the UMTS AKA
derived Kc for GERAN, instead of the correct Kc for GSM AKA (which was received
from the HLR in the auth tuples).

Subsequent patch I42ce51ae979f42d173a45ae69273071c426bf97c will fix this and
correct the test expectations.

Related: OS#2793
Change-Id: I85f12a20dcd701e671188e56811ec7b58d84da82
---
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
2 files changed, 725 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c 
b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
index 1e6bf7c..d8c0441 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_ciph.c
@@ -794,11 +794,271 @@
comment_end();
 }
 
+static void test_gsm_ciph_in_umts_env()
+{
+   struct vlr_subscr *vsub;
+   const char *imsi = "90170010650";
+   const char *sms =
+   "09" /* SMS messages */
+   "01" /* CP-DATA */
+   "58" /* length */
+   "01" /* Network to MS */
+   "00" /* reference */
+   /* originator (gsm411_send_sms() hardcodes this weird nr) */
+   "0791" "447758100650" /* 447785016005 */
+   "00" /* dest */
+   /* SMS TPDU */
+   "4c" /* len */
+   "00" /* SMS deliver */
+   "05802443f2" /* originating address 42342 */
+   "00" /* TP-PID */
+   "00" /* GSM default alphabet */
+   "071010" /* Y-M-D (from wrapped gsm340_gen_scts())*/
+   "00" /* H-M-S */
+   "00" /* GMT+0 */
+   "44" /* data length */
+   "5079da1e1ee7416937485e9ea7c965373d1d6683c270383b3d0e"
+   "d3d36ff71c949e83c22072799e9687c5ec32a81d96afcbf4b4fb"
+   "0c7ac3e9e9b7db05";
+
+   comment_start();
+
+   /* implicit: net->authentication_required = true; */
+   net->a5_encryption_mask = (1 << 1);
+   rx_from_ran = RAN_GERAN_A;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends *UMTS AKA* Auth Req 
to MS");
+   /* based on
+* 2G auth: COMP128v1
+*  KI=7bcd108be4c3d551ee6c67faaf52bd68
+* 3G auth: MILENAGE
+*  K=7bcd108be4c3d551ee6c67faaf52bd68
+*  OPC=6e23f641ce724679b73d933515a8589d
+*  IND-bitlen=5 last-SQN=641
+* Note that the SRES will be calculated by COMP128v1, separately from 
3G tokens;
+* the resulting Kc to use for ciphering returned by the HLR is also 
calculated from COMP128v1.
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "4ac8d1cd1a51937597ca1016fe69a0fa";
+   auth_request_expect_autn = "2d837d2b0d6f4b282d5acf23428d";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362" "2010" "4ac8d1cd1a51937597ca1016fe69a0fa"
+   /*   TL sres   TL kc */
+  "2104" "dacc4b26" "2208" "7a75f0ac9b844400"
+   /*   TL 3G IK */
+  "2310" "3747da4e31545baa2db59e500bdae047"
+   /*   TL 3G CK */
+  "2410" "8544d35b945ccba01a7f1293575291c3"
+   /*   TL AUTN */
+  "2510" "2d837d2b0d6f4b282d5acf23428d"
+   /*   TL RES */
+  "2708" "37527064741f8ddb"
+   /* TLTL rand */
+   "0362" "2010" "b2661531b97b12c5a2edc21a0ed16fc5"
+  "2104" 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with too short RES

2018-03-09 Thread Neels Hofmeyr

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

msc_vlr_test_umts_authen: test response with too short RES

Change-Id: Ia1bc57b3dc1f3c3c654ba2d907b16ba925cd03e8
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 290 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index e6dc445..66eacd7 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -572,10 +572,112 @@
comment_end();
 }
 
+static void _test_umts_authen_too_short_res(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" "2103" "791f2e" /* nipped one byte */);
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+   ASSERT_RELEASE_CLEAR(via_ran);
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+}
+
+static void test_umts_authen_too_short_res_geran()
+{
+   comment_start();
+   _test_umts_authen_too_short_res(RAN_GERAN_A);
+   

[PATCH] osmo-msc[master]: cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES

2018-03-09 Thread Neels Hofmeyr

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

cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES

Change-Id: Ib0f9f573ffac2302fbd3ee28f48ccd8fce5fe286
---
M src/libvlr/vlr_auth_fsm.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.err
M tests/msc_vlr/msc_vlr_test_hlr_reject.err
M tests/msc_vlr/msc_vlr_test_ms_timeout.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
8 files changed, 70 insertions(+), 68 deletions(-)


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

diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 0d0df11..224bc9f 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -139,8 +139,10 @@
bool res_is_umts_aka;
OSMO_ASSERT(at);
 
-   LOGVSUBP(LOGL_DEBUG, vsub, "received res: %s\n",
-osmo_hexdump(res, res_len));
+   LOGVSUBP(LOGL_DEBUG, vsub, "AUTH on %s received %s: %s (%u bytes)\n",
+is_utran ? "UTRAN" : "GERAN",
+is_utran ? "RES" : "SRES/RES",
+osmo_hexdump_nospc(res, res_len), res_len);
 
/* RES must be present and at least 32bit */
if (!res || !res_len) {
diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.err 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
index 269cd35..474fea1 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.err
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
@@ -52,7 +52,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM IMSI:90170010650: MM R99 AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(IMSI:90170010650) AUTH on GERAN received SRES/RES: 
e229c19e791f2e41 (8 bytes)
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -219,7 +219,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(MSISDN:42342) AUTH on GERAN received SRES/RES: e229c19e791f2e41 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -340,7 +340,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = 7db47cf7f81e4dc7)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: 7d b4 7c f7 f8 1e 4d c7 
+DVLR SUBSCR(MSISDN:42342) AUTH on GERAN received SRES/RES: 7db47cf7f81e4dc7 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -480,7 +480,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM IMSI:90170010650: MM R99 AUTHENTICATION RESPONSE (res = 
e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(IMSI:90170010650) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(IMSI:90170010650) AUTH on UTRAN received RES: e229c19e791f2e41 
(8 bytes)
 DVLR SUBSCR(IMSI:90170010650) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result VLR_AUTH_RES_PASSED
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: state_chg to 
VLR_SUB_AS_AUTHENTICATED
@@ -659,7 +659,7 @@
 DRLL Dispatching 04.08 message GSM48_MT_MM_AUTH_RESP (0x5:0x14)
 DMM MSISDN:42342: MM R99 AUTHENTICATION RESPONSE (res = e229c19e791f2e41)
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Received Event 
VLR_AUTH_E_MS_AUTH_RESP
-DVLR SUBSCR(MSISDN:42342) received res: e2 29 c1 9e 79 1f 2e 41 
+DVLR SUBSCR(MSISDN:42342) AUTH on UTRAN received RES: e229c19e791f2e41 (8 
bytes)
 DVLR SUBSCR(MSISDN:42342) AUTH established UMTS security context
 DVLR VLR_Authenticate(90170010650){VLR_SUB_AS_WAIT_RESP}: Authentication 
terminating with result 

[PATCH] osmo-msc[master]: cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA

2018-03-09 Thread Neels Hofmeyr

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

cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA

The code deciding on whether UMTS AKA is used was cascaded and convoluted. By
flattening the decisions, they become easier to read and possibly catch more
weird corner cases / log information more clearly.

- First decide what AKA the RES length reflects.
- Then decide whether all prerequisites for UMTS AKA are satisfied.
- Finally, on UTRAN, turn down the auth if we don't have UMTS AKA, and neatly
  log all of the potential causes.

One corner case that should never occur is that the UMTS AKA RES length is
actually the same length as the GSM AKA SRES. If this nevertheless occurs, log
this as an error, though not turning down authentication because of it. (The
effect is that we would favor UMTS AKA when it has a res_len == sizeof(sres)
and would not succeed to GSM AKA. At least the log will tell us why, now.)

Adjust an expected test output, trivial logging difference.

Change-Id: I43f7f301ea85e518bac91f707391a53182e54fab
---
M src/libvlr/vlr_auth_fsm.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
M tests/msc_vlr/msc_vlr_test_umts_authen.err
3 files changed, 48 insertions(+), 25 deletions(-)


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

diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 51e22c9..0d0df11 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -136,34 +136,57 @@
struct gsm_auth_tuple *at = vsub->last_tuple;
struct osmo_auth_vector *vec = >vec;
bool check_umts;
+   bool res_is_umts_aka;
OSMO_ASSERT(at);
 
LOGVSUBP(LOGL_DEBUG, vsub, "received res: %s\n",
 osmo_hexdump(res, res_len));
 
/* RES must be present and at least 32bit */
-   if (!res || res_len < sizeof(vec->sres)) {
-   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH RES missing or too short "
-"(%u)\n", res_len);
+   if (!res || !res_len) {
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH SRES/RES missing\n");
goto out_false;
}
 
-   check_umts = false;
-   if (is_r99 && (vec->auth_types & OSMO_AUTH_TYPE_UMTS)) {
-   check_umts = true;
-   /* We have a R99 capable UE and have a UMTS AKA capable USIM.
-* However, the ME may still choose to only perform GSM AKA, as
-* long as the bearer is GERAN */
-   if (res_len != vec->res_len) {
-   if (is_utran) {
-   LOGVSUBP(LOGL_NOTICE, vsub,
-"AUTH via UTRAN but "
-"res_len(%u) != vec->res_len(%u)\n",
-res_len, vec->res_len);
-   goto out_false;
-   }
-   check_umts = false;
-   }
+   /* We're deciding the UMTS AKA-ness of the response by the RES size. So 
let's make sure we can't
+* mix them up by size. On UTRAN, we expect full length RES always, no 
way to mix up there. */
+   if (!is_utran && vec->res_len == sizeof(vec->sres))
+   LOGVSUBP(LOGL_ERROR, vsub, "Unforeseen situation: UMTS AKA's 
RES length"
+" equals the size of SRES: %u -- this code wants to 
differentiate"
+" the two by their size, which won't work properly 
now.\n", vec->res_len);
+
+   /* RES must be either vec->res_len (UMTS AKA) or sizeof(sres) (GSM AKA) 
*/
+   if (res_len == vec->res_len)
+   res_is_umts_aka = true;
+   else if (res_len == sizeof(vec->sres))
+   res_is_umts_aka = false;
+   else {
+   if (is_utran)
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH RES has invalid 
length: %u."
+" Expected %u (UMTS AKA)\n",
+res_len, vec->res_len);
+   else
+   LOGVSUBP(LOGL_NOTICE, vsub, "AUTH SRES/RES has invalid 
length: %u."
+" Expected either %zu (GSM AKA) or %u (UMTS 
AKA)\n",
+res_len, sizeof(vec->sres), vec->res_len);
+   goto out_false;
+   }
+
+   check_umts = (is_r99
+ && (vec->auth_types & OSMO_AUTH_TYPE_UMTS)
+ && res_is_umts_aka);
+
+   /* 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 -- never on UTRAN: */
+   if (is_utran && !check_umts) {
+   LOGVSUBP(LOGL_ERROR, vsub,
+"AUTH via UTRAN, cannot allow GSM AKA"
+" (MS is %sR99 capable, vec has %sUMTS AKA tokens, 
res_len=%u is %s)\n",
+is_r99 ? "" : "NOT ",
+

[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 @@

[PATCH] osmo-msc[master]: msc_vlr_tests: clearly separate Ciph Mode from Security Mode...

2018-03-09 Thread Neels Hofmeyr

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

msc_vlr_tests: clearly separate Ciph Mode from Security Mode checking

Clearly distinguish between Ciphering Mode Command on GERAN and Security Mode
Control on UTRAN.

Cosmetic: explicitly verify the key strings in the testing code (not only in
the expected output).

Change-Id: Ica93ed06c4c63dc6768736d25231de8068001114
---
M tests/msc_vlr/msc_vlr_test_authen_reuse.c
M tests/msc_vlr/msc_vlr_test_authen_reuse.err
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
M tests/msc_vlr/msc_vlr_test_gsm_ciph.c
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
M tests/msc_vlr/msc_vlr_tests.c
M tests/msc_vlr/msc_vlr_tests.h
9 files changed, 98 insertions(+), 48 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.c 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.c
index e78ab06..6fd1e66 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.c
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.c
@@ -105,9 +105,9 @@
} else {
/* On UTRAN */
btw("MS sends Authen Response, VLR accepts and sends 
SecurityModeControl");
-   cipher_mode_cmd_sent = false;
+   expect_security_mode_ctrl(NULL, 
"27497388b6cb044648f396aa155b95ef");
ms_sends_msg("0554" "e229c19e" "2104" "791f2e41");
-   VERBOSE_ASSERT(cipher_mode_cmd_sent, == true, "%d");
+   VERBOSE_ASSERT(security_mode_ctrl_sent, == true, "%d");
VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
 
btw("MS sends SecurityModeControl acceptance, VLR accepts and 
sends GSUP LU Req to HLR");
@@ -179,9 +179,9 @@
} else {
/* On UTRAN */
btw("MS sends Authen Response, VLR accepts and sends 
SecurityModeControl");
-   cipher_mode_cmd_sent = false;
+   expect_security_mode_ctrl(NULL, 
"27497388b6cb044648f396aa155b95ef");
ms_sends_msg("0554" "e229c19e" "2104" "791f2e41");
-   VERBOSE_ASSERT(cipher_mode_cmd_sent, == true, "%d");
+   VERBOSE_ASSERT(security_mode_ctrl_sent, == true, "%d");
VERBOSE_ASSERT(cm_service_result_sent, == RES_NONE, 
"%d");
 
btw("MS sends SecurityModeControl acceptance, VLR 
accepts; above Ciphering is an implicit CM Service Accept");
@@ -253,9 +253,9 @@
} else {
/* On UTRAN */
btw("MS sends Authen Response, VLR accepts and sends 
SecurityModeControl");
-   cipher_mode_cmd_sent = false;
+   expect_security_mode_ctrl(NULL, 
"1159ec926a50e98c034a6b7d7c9f418d");
ms_sends_msg("0554" "7db47cf7" "2104" "f81e4dc7"); /* 
2nd vector's res, s.a. */
-   VERBOSE_ASSERT(cipher_mode_cmd_sent, == true, "%d");
+   VERBOSE_ASSERT(security_mode_ctrl_sent, == true, "%d");
VERBOSE_ASSERT(cm_service_result_sent, == RES_NONE, 
"%d");
 
btw("MS sends SecurityModeControl acceptance, VLR 
accepts; above Ciphering is an implicit CM Service Accept");
diff --git a/tests/msc_vlr/msc_vlr_test_authen_reuse.err 
b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
index ea3b989..269cd35 100644
--- a/tests/msc_vlr/msc_vlr_test_authen_reuse.err
+++ b/tests/msc_vlr/msc_vlr_test_authen_reuse.err
@@ -497,7 +497,7 @@
 DVLR vlr_lu_fsm(90170010650){VLR_ULA_S_WAIT_AUTH}: state_chg to 
VLR_ULA_S_WAIT_CIPH
 DMM IMSI:90170010650: bump: conn still being established 
(SUBSCR_CONN_S_NEW)
 DREF IMSI:90170010650: MSC conn use - dtap == 1 (0x4)
-  cipher_mode_cmd_sent == 1
+  security_mode_ctrl_sent == 1
   lu_result_sent == 0
 - MS sends SecurityModeControl acceptance, VLR accepts and sends GSUP LU Req 
to HLR
 DMM <- SECURITY MODE COMPLETE IMSI:90170010650
@@ -677,7 +677,7 @@
 DVLR Process_Access_Request_VLR(90170010650){PR_ARQ_S_WAIT_AUTH}: 
state_chg to PR_ARQ_S_WAIT_CIPH
 DMM MSISDN:42342: bump: conn still being established (SUBSCR_CONN_S_NEW)
 DREF MSISDN:42342: MSC conn use - dtap == 1 (0x4)
-  cipher_mode_cmd_sent == 1
+  security_mode_ctrl_sent == 1
   cm_service_result_sent == 0
 - MS sends SecurityModeControl acceptance, VLR accepts; above Ciphering is an 
implicit CM Service Accept
 DMM <- SECURITY MODE COMPLETE MSISDN:42342
@@ -808,7 +808,7 @@
 DVLR Process_Access_Request_VLR(90170010650){PR_ARQ_S_WAIT_AUTH}: 
state_chg to PR_ARQ_S_WAIT_CIPH
 DMM MSISDN:42342: bump: conn still being established (SUBSCR_CONN_S_NEW)
 DREF MSISDN:42342: MSC conn use - dtap == 1 (0x4)
-  cipher_mode_cmd_sent == 1
+  security_mode_ctrl_sent == 1
   cm_service_result_sent == 0
 - MS sends SecurityModeControl acceptance, VLR accepts; above Ciphering is an 
implicit CM 

[PATCH] osmo-msc[master]: gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not res...

2018-03-09 Thread Neels Hofmeyr

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

gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not response size

Do not interpret the SRES/RES length returned in the auth response as the R99
capability bit, instead determine it from the actual Classmark information
associated with the conn.

This fixes the is_r99 flag passed in to vlr_subscr_rx_auth_resp(), which ends
up in the struct vlr_auth_resp_par dispatched to the auth_fi and influences the
authentication acceptance.

Though the effect of a wrongly-set-to-false R99 flag is not harmful in this
code path, let's not get this confused.

Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
---
M src/libmsc/gsm_04_08.c
1 file changed, 8 insertions(+), 10 deletions(-)


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

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index ceef2d8..4564f8e 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -952,7 +952,7 @@
uint8_t res[16];
uint8_t res_len;
int rc;
-   bool is_r99;
+   bool is_umts;
 
if (!conn->vsub) {
LOGP(DMM, LOGL_ERROR,
@@ -961,30 +961,28 @@
return -EINVAL;
}
 
-   if (msgb_l3len(msg) >
-   sizeof(struct gsm48_hdr) + sizeof(struct gsm48_auth_resp)) {
+   is_umts = (msgb_l3len(msg) > sizeof(struct gsm48_hdr) + sizeof(struct 
gsm48_auth_resp));
+
+   if (is_umts)
rc = parse_umts_auth_resp(res, _len, conn, msg);
-   is_r99 = true;
-   } else {
+   else
rc = parse_gsm_auth_resp(res, _len, conn, msg);
-   is_r99 = false;
-   }
 
if (rc) {
LOGP(DMM, LOGL_ERROR,
 "%s: MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA 
Auth Response"
 " failed with rc=%d; dispatching zero length SRES/RES to 
trigger failure\n",
-vlr_subscr_name(conn->vsub), is_r99 ? "UMTS" : "GSM", rc);
+vlr_subscr_name(conn->vsub), is_umts ? "UMTS" : "GSM", rc);
memset(res, 0, sizeof(res));
res_len = 0;
}
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
   vlr_subscr_name(conn->vsub),
-  is_r99 ? "R99" : "GSM", is_r99 ? "res" : "sres",
+  is_umts ? "R99" : "GSM", is_umts ? "res" : "sres",
   osmo_hexdump_nospc(res, res_len));
 
-   return vlr_subscr_rx_auth_resp(conn->vsub, is_r99,
+   return vlr_subscr_rx_auth_resp(conn->vsub, 
classmark_is_r99(>classmark),
   conn->via_ran == RAN_UTRAN_IU,
   res, res_len);
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


[PATCH] osmo-msc[master]: vlr auth: gracefully reject malformed auth response

2018-03-09 Thread Neels Hofmeyr

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

vlr auth: gracefully reject malformed auth response

Instead of just closing down the conn hard, actually feed invalid auth response
data to vlr_subscr_rc_auth_resp() in order to trigger all the actions we want
to see with a failed authentication:
- a GSUP signal that the auth failed,
- a LU reject.

Verify this in new test_wrong_sres_length() in msc_vlr_test_gsm_authen.c.

Change-Id: I4179a290069ac61d0662de4ec7ca3edb76988899
---
M src/libmsc/gsm_04_08.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.c
M tests/msc_vlr/msc_vlr_test_gsm_authen.err
3 files changed, 179 insertions(+), 2 deletions(-)


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

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index f87a4c6..ceef2d8 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -971,8 +971,12 @@
}
 
if (rc) {
-   msc_subscr_conn_close(conn, GSM_CAUSE_AUTH_FAILED);
-   return -EINVAL;
+   LOGP(DMM, LOGL_ERROR,
+"%s: MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA 
Auth Response"
+" failed with rc=%d; dispatching zero length SRES/RES to 
trigger failure\n",
+vlr_subscr_name(conn->vsub), is_r99 ? "UMTS" : "GSM", rc);
+   memset(res, 0, sizeof(res));
+   res_len = 0;
}
 
DEBUGP(DMM, "%s: MM %s AUTHENTICATION RESPONSE (%s = %s)\n",
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.c 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.c
index 9c09aa4..b0db12d 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.c
@@ -914,11 +914,70 @@
comment_end();
 }
 
+static void test_wrong_sres_length()
+{
+   comment_start();
+   fake_time_start();
+
+   net->authentication_required = true;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("0801080971004026f0");
+   ms_sends_msg("05080200816800013008991007006402");
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   auth_request_sent = false;
+   auth_request_expect_rand = "585df1ae287f6e273dce07090d61320b";
+   auth_request_expect_autn = NULL;
+   /* Based on a Ki of 000102030405060708090a0b0c0d0e0f */
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971004026f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0322"  "2010" "585df1ae287f6e273dce07090d61320b"
+   /*   TL sres   TL kc */
+   "2104" "2d8b2c3e" "2208" "61855fb81fc2a800"
+   "0322"  "2010" "12aca96fb4ffdea5c985cbafa9b6e18b"
+   "2104" "20bde240" "2208" "07fa7502e07e1c00"
+   "0322"  "2010" "e7c03ba7cf0e2fde82b2dc4d63077d42"
+   "2104" "a29514ae" "2208" "e2b234f807886400"
+   "0322"  "2010" "fa8f20b781b5881329d4fea26b1a3c51"
+   "2104" "5afc8d72" "2208" "2392f14f709ae000"
+   "0322"  "2010" "0fd4cc8dbe8715d1f439e304edfd68dc"
+   "2104" "bc8d1c5b" "2208" "da7cdd6bfe2d7000",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("If the HLR were to send a GSUP _UPDATE_LOCATION_RESULT we'd still 
reject");
+   gsup_rx("0601080971004026f0", NULL);
+   EXPECT_ACCEPTED(false);
+
+   thwart_rx_non_initial_requests();
+
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("MS sends Authen Response with too short SRES data, auth is 
thwarted.");
+   gsup_expect_tx("0b01080971004026f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_bssap_clear();
+   ms_sends_msg("05542d8b2c");
+   VERBOSE_ASSERT(lu_result_sent, == RES_REJECT, "%d");
+
+   EXPECT_CONN_COUNT(0);
+   clear_vlr();
+   comment_end();
+}
+
 msc_vlr_test_func_t msc_vlr_tests[] = {
test_gsm_authen,
test_gsm_authen_tmsi,
test_gsm_authen_imei,
test_gsm_authen_tmsi_imei,
test_gsm_milenage_authen,
+   test_wrong_sres_length,
NULL
 };
diff --git a/tests/msc_vlr/msc_vlr_test_gsm_authen.err 
b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
index a46a838..a454e2f 100644
--- a/tests/msc_vlr/msc_vlr_test_gsm_authen.err
+++ b/tests/msc_vlr/msc_vlr_test_gsm_authen.err
@@ -1998,6 +1998,120 @@
 full talloc report on 'msgb' (total  0 bytes in   1 blocks)
 talloc_total_blocks(tall_bsc_ctx) == 7
 
+= test_wrong_sres_length
+- Total time passed: 0.00 s
+- Location Update request causes a GSUP Send Auth Info request to HLR
+  MSC <--RAN_GERAN_A-- MS: 

[PATCH] osmo-msc[master]: msc_vlr_test_umts_authen: test response with only SRES half ...

2018-03-09 Thread Neels Hofmeyr

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

msc_vlr_test_umts_authen: test response with only SRES half of RES

Change-Id: I0e9099625bd9d3de3db5ee29fbf81b2d8a30071d
---
M tests/msc_vlr/msc_vlr_test_umts_authen.c
M tests/msc_vlr/msc_vlr_test_umts_authen.err
2 files changed, 294 insertions(+), 0 deletions(-)


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

diff --git a/tests/msc_vlr/msc_vlr_test_umts_authen.c 
b/tests/msc_vlr/msc_vlr_test_umts_authen.c
index 861a615..b5a55fc 100644
--- a/tests/msc_vlr/msc_vlr_test_umts_authen.c
+++ b/tests/msc_vlr/msc_vlr_test_umts_authen.c
@@ -770,6 +770,110 @@
comment_end();
 }
 
+static void _test_umts_authen_only_sres(enum ran_type via_ran)
+{
+   net->authentication_required = true;
+   net->vlr->cfg.assign_tmsi = true;
+   rx_from_ran = via_ran;
+
+   btw("Location Update request causes a GSUP Send Auth Info request to 
HLR");
+   lu_result_sent = RES_NONE;
+   gsup_expect_tx("080108" "0971000156f0");
+   ms_sends_msg("0508" /* MM LU */
+"7" /* ciph key seq: no key available */
+"0" /* LU type: normal */
+"ff" "" /* LAI, LAC */
+"57" /* classmark 1: R99, early classmark, no power lvl */
+"08991007106005" /* IMSI */
+"3303575886" /* classmark 2 */
+);
+   OSMO_ASSERT(gsup_tx_confirmed);
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   btw("from HLR, rx _SEND_AUTH_INFO_RESULT; VLR sends Auth Req to MS");
+   /* based on auc_3g:
+* K = 'EB215756028D60E3275E613320AEC880',
+* OPC = 'FB2A3D1B360F599ABAB99DB8669F8308'
+* SQN = 0
+*/
+   auth_request_sent = false;
+   auth_request_expect_rand = "39fa2f4e3d523d8619a73b4f65c3e14d";
+   auth_request_expect_autn = "8704f5ba55f3d2ee44b22c8ea919";
+   gsup_rx("0a"
+   /* imsi */
+   "0108" "0971000156f0"
+   /* 5 auth vectors... */
+   /* TLTL rand */
+   "0362"  "2010" "39fa2f4e3d523d8619a73b4f65c3e14d"
+   /*   TL sres   TL kc */
+   "2104" "9b36efdf" "2208" "059a4f668f6fbe39"
+   /*   TL 3G IK */
+   "2310" "27497388b6cb044648f396aa155b95ef"
+   /*   TL 3G CK */
+   "2410" "f64735036e5871319c679f4742a75ea1"
+   /*   TL AUTN */
+   "2510" "8704f5ba55f3d2ee44b22c8ea919"
+   /*   TL RES */
+   "2708" "e229c19e791f2e41"
+   /* TLTL rand */
+   "0362"  "2010" "c187a53a5e6b9d573cac7c74451fd46d"
+   "2104" "85aa3130" "2208" "d3d50a000bf04f6e"
+   "2310" "1159ec926a50e98c034a6b7d7c9f418d"
+   "2410" "df3a03d9ca5335641efc8e36d76cd20b"
+   "2510" "1843a645b98d5b2d666af46c45d9"
+   "2708" "7db47cf7f81e4dc7"
+   "0362"  "2010" "efa9c29a9742148d5c9070348716e1bb"
+   "2104" "69d5f9fb" "2208" "3df176f0c29f1a3d"
+   "2310" "eb50e770ddcc3060101d2f43b6c2b884"
+   "2410" "76542abce5ff9345b0e8947f4c6e019c"
+   "2510" "f9375e6d41e196e7fe4ff1c27e39"
+   "2708" "706f996719ba609c"
+   "0362"  "2010" "f023d5a3b24726e0631b64b3840f8253"
+   "2104" "d570c03f" "2208" "ec011be8919883d6"
+   "2310" "c4e58af4ba43f3bcd904e16984f086d7"
+   "2410" "0593f65e752e5cb7f473862bda05aa0a"
+   "2510" "541ff1f07727c5ea00d658bc7e9a"
+   "2708" "3fd26072eaa2a04d"
+   "0362"  "2010" "2f8f90c780d6a9c0c53da7ac57b6707e"
+   "2104" "b072446f220823f39f9f425ad6e6"
+   "2310" "65af0527fda95b0dc5ae4aa515cdf32f"
+   "2410" "537c3b35a3b13b08d08eeb28098f45cc"
+   "2510" "4bf4e564f7539bc796706bc65744"
+   "2708" "0edb0eadbea94ac2",
+   NULL);
+   VERBOSE_ASSERT(auth_request_sent, == true, "%d");
+   VERBOSE_ASSERT(lu_result_sent, == RES_NONE, "%d");
+
+   if (via_ran == RAN_GERAN_A)
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts:"
+   " GERAN reports an SRES mismatch");
+   else
+   btw("MS sends Authen Response of wrong RES size, VLR thwarts:"
+   " UTRAN disallows GSM AKA altogether");
+   gsup_expect_tx("0b01080971000156f0"); /* 
OSMO_GSUP_MSGT_AUTH_FAIL_REPORT */
+   expect_release_clear(via_ran);
+   ms_sends_msg("0554" "e229c19e" /* Only the SRES half of the RES */);
+   VERBOSE_ASSERT(lu_result_sent, == 

[PATCH] osmo-bts[master]: sysinfo.c: SI1 is optional; Send SI2 at TC=0 if no SI1 exists

2018-03-09 Thread Harald Welte

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

sysinfo.c: SI1 is optional; Send SI2 at TC=0 if no SI1 exists

SI1 is only required if frequency hopping is used or if NCH is used. So it's 
optional.

If OsmoBTS has no SI1 configured, it will transmit the empty SI1 buffer
at TC=0, and as a result no valid SI will be broadcast at TC=0.

Change-Id: I41ab885c00e943199b2e939e98f30e267ecffbee
Closes: OS#3051
---
M src/common/sysinfo.c
1 file changed, 3 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/84/7184/1

diff --git a/src/common/sysinfo.c b/src/common/sysinfo.c
index cf86380..6f7a387 100644
--- a/src/common/sysinfo.c
+++ b/src/common/sysinfo.c
@@ -73,7 +73,9 @@
 * present in a cell. If the MS finds another message
 * when TC = 0, it can assume that System Information
 * Type 1 is not in use.  */
-   return GSM_BTS_SI(bts, SYSINFO_TYPE_1);
+   if (GSM_BTS_HAS_SI(bts, SYSINFO_TYPE_1))
+   return GSM_BTS_SI(bts, SYSINFO_TYPE_1);
+   return GSM_BTS_SI(bts, SYSINFO_TYPE_2);
case 1:
/* A SI 2 message will be sent at least every time TC = 1. */
return GSM_BTS_SI(bts, SYSINFO_TYPE_2);

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I41ab885c00e943199b2e939e98f30e267ecffbee
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Harald Welte 


[MERGED] osmo-gsm-tester[master]: contrib: build-osmo-bsc: Add aibsip-find and ipaccess-config...

2018-03-09 Thread Pau Espin Pedrol
Pau Espin Pedrol has submitted this change and it was merged.

Change subject: contrib: build-osmo-bsc: Add aibsip-find and ipaccess-config 
binaries to archive
..


contrib: build-osmo-bsc: Add aibsip-find and ipaccess-config binaries to archive

They will be required by Nanobts class.

Change-Id: Ib0e003f74603c3146aa76d581ab493f960f73ab5
---
M contrib/jenkins-build-osmo-bsc.sh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/contrib/jenkins-build-osmo-bsc.sh 
b/contrib/jenkins-build-osmo-bsc.sh
index ee58c9d..28a1f5e 100755
--- a/contrib/jenkins-build-osmo-bsc.sh
+++ b/contrib/jenkins-build-osmo-bsc.sh
@@ -11,4 +11,4 @@
 build_repo osmo-mgw
 build_repo osmo-bsc
 
-create_bin_tgz osmo-bsc
+create_bin_tgz osmo-bsc abisip-find ipaccess-config

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0e003f74603c3146aa76d581ab493f960f73ab5
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 


osmo-gsm-tester[master]: contrib: build-osmo-bts: Remove dropped dependency openbsc

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a1918971388afad41308629c1851614d1381f25
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


[MERGED] osmo-gsm-tester[master]: contrib: build-osmo-bts: Remove dropped dependency openbsc

2018-03-09 Thread Pau Espin Pedrol
Pau Espin Pedrol has submitted this change and it was merged.

Change subject: contrib: build-osmo-bts: Remove dropped dependency openbsc
..


contrib: build-osmo-bts: Remove dropped dependency openbsc

Since osmo-bts ec33b0397f5d71248c5834513d4be7b9b0e46366, it doesn't
require openbsc anymore to build.

Change-Id: I8a1918971388afad41308629c1851614d1381f25
---
M contrib/jenkins-build-osmo-bts.sh
1 file changed, 0 insertions(+), 2 deletions(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/contrib/jenkins-build-osmo-bts.sh 
b/contrib/jenkins-build-osmo-bts.sh
index 7349ce5..462bce0 100755
--- a/contrib/jenkins-build-osmo-bts.sh
+++ b/contrib/jenkins-build-osmo-bts.sh
@@ -4,8 +4,6 @@
 name="osmo-bts"
 . "$(dirname "$0")/jenkins-build-common.sh"
 
-# for gsm_data_shared.*
-have_repo openbsc
 have_repo octphy-2g-headers
 
 build_repo libosmocore --disable-doxygen

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8a1918971388afad41308629c1851614d1381f25
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 


osmo-gsm-tester[master]: contrib: build-osmo-bsc: Add aibsip-find and ipaccess-config...

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0e003f74603c3146aa76d581ab493f960f73ab5
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


[PATCH] osmo-gsm-tester[master]: contrib: build-osmo-bsc: Add aibsip-find and ipaccess-config...

2018-03-09 Thread Pau Espin Pedrol

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

contrib: build-osmo-bsc: Add aibsip-find and ipaccess-config binaries to archive

They will be required by Nanobts class.

Change-Id: Ib0e003f74603c3146aa76d581ab493f960f73ab5
---
M contrib/jenkins-build-osmo-bsc.sh
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/83/7183/1

diff --git a/contrib/jenkins-build-osmo-bsc.sh 
b/contrib/jenkins-build-osmo-bsc.sh
index ee58c9d..28a1f5e 100755
--- a/contrib/jenkins-build-osmo-bsc.sh
+++ b/contrib/jenkins-build-osmo-bsc.sh
@@ -11,4 +11,4 @@
 build_repo osmo-mgw
 build_repo osmo-bsc
 
-create_bin_tgz osmo-bsc
+create_bin_tgz osmo-bsc abisip-find ipaccess-config

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0e003f74603c3146aa76d581ab493f960f73ab5
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 


[PATCH] osmo-gsm-tester[master]: contrib: build-osmo-bts: Remove dropped dependency openbsc

2018-03-09 Thread Pau Espin Pedrol

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

contrib: build-osmo-bts: Remove dropped dependency openbsc

Since osmo-bts ec33b0397f5d71248c5834513d4be7b9b0e46366, it doesn't
require openbsc anymore to build.

Change-Id: I8a1918971388afad41308629c1851614d1381f25
---
M contrib/jenkins-build-osmo-bts.sh
1 file changed, 0 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/82/7182/1

diff --git a/contrib/jenkins-build-osmo-bts.sh 
b/contrib/jenkins-build-osmo-bts.sh
index 7349ce5..462bce0 100755
--- a/contrib/jenkins-build-osmo-bts.sh
+++ b/contrib/jenkins-build-osmo-bts.sh
@@ -4,8 +4,6 @@
 name="osmo-bts"
 . "$(dirname "$0")/jenkins-build-common.sh"
 
-# for gsm_data_shared.*
-have_repo openbsc
 have_repo octphy-2g-headers
 
 build_repo libosmocore --disable-doxygen

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a1918971388afad41308629c1851614d1381f25
Gerrit-PatchSet: 1
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 


[PATCH] osmo-gsm-tester[master]: WIP: Introduce ip.access nanobts support

2018-03-09 Thread Pau Espin Pedrol
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7168

to look at the new patch set (#2).

WIP: Introduce ip.access nanobts support

Change-Id: Ibaea025b3a503dfe897d36701234445de6d49f82
---
M example/resources.conf
A example/scenarios/nanobts.conf
A src/osmo_gsm_tester/bts_nanobts.py
M src/osmo_gsm_tester/resource.py
4 files changed, 129 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/68/7168/2

diff --git a/example/resources.conf b/example/resources.conf
index fb2ac9f..500eb71 100644
--- a/example/resources.conf
+++ b/example/resources.conf
@@ -44,6 +44,16 @@
   - hw_addr: 00:0c:90:2e:80:1e
 net_device: enp2s0
 
+- label: NanoBTS
+  type: nanobts
+  ipa_unit_id: 9
+  addr: 10.42.42.53
+  band: GSM-1800
+  power_supply:
+type: 'sispm'
+device: '01:01:55:2e:b6'
+port: '1'
+
 arfcn:
   - arfcn: 512
 band: GSM-1800
diff --git a/example/scenarios/nanobts.conf b/example/scenarios/nanobts.conf
new file mode 100644
index 000..7322288
--- /dev/null
+++ b/example/scenarios/nanobts.conf
@@ -0,0 +1,3 @@
+resources:
+  bts:
+  - type: nanobts
diff --git a/src/osmo_gsm_tester/bts_nanobts.py 
b/src/osmo_gsm_tester/bts_nanobts.py
new file mode 100644
index 000..f45dea9
--- /dev/null
+++ b/src/osmo_gsm_tester/bts_nanobts.py
@@ -0,0 +1,111 @@
+# osmo_gsm_tester: specifics for running an ip.access nanoBTS
+#
+# Copyright (C) 2018 by sysmocom - s.f.m.c. GmbH
+#
+# Author: Pau Espin Pedrol 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+import os
+import pprint
+import tempfile
+from abc import ABCMeta, abstractmethod
+from . import log, config, util, template, process, event_loop, bts, pcu
+from . import powersupply
+
+class NanoBts(bts.Bts):
+
+pwsup = None
+pcu = None
+##
+# PROTECTED
+##
+def __init__(self, suite_run, conf):
+super().__init__(suite_run, conf, 'nanobts')
+
+def configure(self):
+if self.bsc is None:
+raise RuntimeError('BTS needs to be added to a BSC or NITB before 
it can be configured')
+
+pwsup_opt = self.conf.get('power_supply', {})
+if not pwsup_opt:
+raise log.Error('No power_supply attribute provided in conf!')
+pwsup_type = pwsup_opt.get('type')
+if not pwsup_type:
+raise log.Error('No type attribute provided in power_supply conf!')
+
+self.pwsup = powersupply.get_instance_by_type(pwsup_type, pwsup_opt)
+
+
+# PUBLIC - INTERNAL API
+
+
+def conf_for_bsc(self):
+values = config.get_defaults('bsc_bts')
+config.overlay(values, config.get_defaults('nanobts'))
+if self.lac is not None:
+config.overlay(values, { 'location_area_code': self.lac })
+if self.rac is not None:
+config.overlay(values, { 'routing_area_code': self.rac })
+if self.cellid is not None:
+config.overlay(values, { 'cell_identity': self.cellid })
+if self.bvci is not None:
+config.overlay(values, { 'bvci': self.bvci })
+config.overlay(values, self.conf)
+
+sgsn_conf = {} if self.sgsn is None else self.sgsn.conf_for_client()
+config.overlay(values, sgsn_conf)
+
+config.overlay(values, { 'osmobsc_bts_type': 'nanobts' })
+
+self.dbg(conf=values)
+return values
+
+
+def cleanup(self):
+if self.pwsup:
+self.dbg('Powering off NanoBTS')
+self.pwsup.power_set(False)
+
+###
+# PUBLIC (test API included)
+###
+
+def start(self):
+self.run_dir = 
util.Dir(self.suite_run.get_test_run_dir().new_dir(self.name()))
+self.configure()
+
+self.pwsup.power_cycle(1.0)
+
+self.log('Starting to connect to', self.bsc)
+
+#According to roh, it can be configured to use a static IP in a 
permanent way:
+# 1- use abisip-find to find the default address
+# 2- use ./ipaccess-config --ip-address IP/MASK
+# 3- use ./ipaccess-config  --ip-gateway IP to set the IP of the main 
unit
+# 4- use ./ipaccess-config --restart to restart and apply the changes
+
+#Start must do the following:
+# 1- use abisip-find to find the default address
+

[PATCH] osmo-gsm-tester[master]: Create Pcu abstract class and make OsmoPcu inherit from it

2018-03-09 Thread Pau Espin Pedrol
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7158

to look at the new patch set (#2).

Create Pcu abstract class and make OsmoPcu inherit from it

This base class will be used to describe the required accessors for all
PCU objects.

It is introduced in this commit and will be further used in the future
when adding a Dummy PCU object which will be used by NanoBts object.

Change-Id: Ia3fd4551d1f2932362f99f7d44d65f8ae4fd1979
---
A src/osmo_gsm_tester/pcu.py
M src/osmo_gsm_tester/pcu_osmo.py
2 files changed, 50 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/58/7158/2

diff --git a/src/osmo_gsm_tester/pcu.py b/src/osmo_gsm_tester/pcu.py
new file mode 100644
index 000..c43ee16
--- /dev/null
+++ b/src/osmo_gsm_tester/pcu.py
@@ -0,0 +1,47 @@
+# osmo_gsm_tester: specifics pcu base abstract class
+#
+# Copyright (C) 2018 by sysmocom - s.f.m.c. GmbH
+#
+# Author: Pau Espin Pedrol 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+from abc import ABCMeta, abstractmethod
+from . import log
+
+class Pcu(log.Origin, metaclass=ABCMeta):
+suite_run = None
+run_dir = None
+bts = None
+
+##
+# PROTECTED
+##
+
+def __init__(self, suite_run, bts, conf, name):
+super().__init__(log.C_RUN, name)
+self.suite_run = suite_run
+self.bts = bts
+self.conf = conf
+
+###
+# PUBLIC (test API included)
+###
+
+@abstractmethod
+def start(self):
+'Start the PCU'
+pass
+
+# vim: expandtab tabstop=4 shiftwidth=4
diff --git a/src/osmo_gsm_tester/pcu_osmo.py b/src/osmo_gsm_tester/pcu_osmo.py
index 23c93eb..deaeb80 100644
--- a/src/osmo_gsm_tester/pcu_osmo.py
+++ b/src/osmo_gsm_tester/pcu_osmo.py
@@ -20,11 +20,9 @@
 import os
 import pprint
 import tempfile
-from . import log, config, util, template, process, event_loop
+from . import log, config, util, template, process, event_loop, pcu
 
-class OsmoPcu(log.Origin):
-suite_run = None
-run_dir = None
+class OsmoPcu(pcu.Pcu):
 inst = None
 env = None
 
@@ -32,9 +30,7 @@
 PCU_OSMO_CFG = 'osmo-pcu.cfg'
 
 def __init__(self, suite_run, bts, conf):
-super().__init__(log.C_RUN, OsmoPcu.BIN_PCU)
-self.suite_run = suite_run
-self.bts = bts
+super().__init__(suite_run, bts, conf, OsmoPcu.BIN_PCU)
 self.conf = conf
 self.env = {}
 

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3fd4551d1f2932362f99f7d44d65f8ae4fd1979
Gerrit-PatchSet: 2
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder


[PATCH] osmo-gsm-tester[master]: Create Bts abstract class and make OsmoBts inherit from it

2018-03-09 Thread Pau Espin Pedrol
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7160

to look at the new patch set (#2).

Create Bts abstract class and make OsmoBts inherit from it

This base class will be used to describe the required accessors for all
BTS objects, be it an osmocom BTS or not.

It is introduced in this commit and will be further used in the future
when adding a NanoBts object.

Change-Id: Ic13133e61abda73a8b507c1a1bd7b98c677460f9
---
A src/osmo_gsm_tester/bts.py
M src/osmo_gsm_tester/bts_osmo.py
2 files changed, 102 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/60/7160/2

diff --git a/src/osmo_gsm_tester/bts.py b/src/osmo_gsm_tester/bts.py
new file mode 100644
index 000..289b697
--- /dev/null
+++ b/src/osmo_gsm_tester/bts.py
@@ -0,0 +1,95 @@
+# osmo_gsm_tester: base classes to share code among BTS subclasses.
+#
+# Copyright (C) 2018 by sysmocom - s.f.m.c. GmbH
+#
+# Author: Pau Espin Pedrol 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+import os
+import pprint
+import tempfile
+from abc import ABCMeta, abstractmethod
+from . import log, config, util, template, process, event_loop, pcu_osmo
+
+class Bts(log.Origin, metaclass=ABCMeta):
+suite_run = None
+conf = None
+bsc = None
+sgsn = None
+lac = None
+rac = None
+cellid = None
+bvci = None
+
+##
+# PROTECTED
+##
+def __init__(self, suite_run, conf, name):
+super().__init__(log.C_RUN, name)
+self.suite_run = suite_run
+self.conf = conf
+
+
+# PUBLIC - INTERNAL API
+
+@abstractmethod
+def conf_for_bsc(self):
+'Used by bsc objects to get path to socket.'
+pass
+
+def remote_addr(self):
+return self.conf.get('addr')
+
+def cleanup(self):
+'Nothing to do by default. Subclass can override if required.'
+pass
+
+###
+# PUBLIC (test API included)
+###
+@abstractmethod
+def start(self):
+'Starts BTS proccess and sets self.proc_bts with an object of Process 
interface'
+pass
+
+@abstractmethod
+def ready_for_pcu(self):
+'True if the BTS is prepared to have a PCU connected, false otherwise'
+pass
+
+@abstractmethod
+def pcu(self):
+'Get the Pcu object associated with the BTS'
+pass
+
+def set_bsc(self, bsc):
+self.bsc = bsc
+
+def set_sgsn(self, sgsn):
+self.sgsn = sgsn
+
+def set_lac(self, lac):
+self.lac = lac
+
+def set_rac(self, rac):
+self.rac = rac
+
+def set_cellid(self, cellid):
+self.cellid = cellid
+
+def set_bvci(self, bvci):
+self.bvci = bvci
+
+# vim: expandtab tabstop=4 shiftwidth=4
diff --git a/src/osmo_gsm_tester/bts_osmo.py b/src/osmo_gsm_tester/bts_osmo.py
index 5ae3392..60b9695 100644
--- a/src/osmo_gsm_tester/bts_osmo.py
+++ b/src/osmo_gsm_tester/bts_osmo.py
@@ -21,26 +21,17 @@
 import pprint
 import tempfile
 from abc import ABCMeta, abstractmethod
-from . import log, config, util, template, process, event_loop, pcu_osmo
+from . import log, config, util, template, process, event_loop, bts, pcu_osmo
 
-class OsmoBts(log.Origin, metaclass=ABCMeta):
-suite_run = None
+class OsmoBts(bts.Bts, metaclass=ABCMeta):
 proc_bts = None
-bsc = None
-sgsn = None
-lac = None
-rac = None
-cellid = None
-bvci = None
 _pcu = None
 
 ##
 # PROTECTED
 ##
 def __init__(self, suite_run, conf, name):
-super().__init__(log.C_RUN, name)
-self.suite_run = suite_run
-self.conf = conf
+super().__init__(suite_run, conf, name)
 if len(self.pcu_socket_path().encode()) > 107:
 raise log.Error('Path for pcu socket is longer than max allowed 
len for unix socket path (107):', self.pcu_socket_path())
 
@@ -49,7 +40,7 @@
 
 @abstractmethod
 def conf_for_bsc(self):
-'Used by bsc objects to get path to socket.'
+# coming from bts.Bts, we forward the implementation to children.
 pass
 
 @abstractmethod
@@ -62,19 +53,12 @@
 'Used by base class. Subclass can create different pcu 
implementations.'
 pass
 
- 

[PATCH] osmo-gsm-tester[master]: Introduce PowerSupply interface and PowerSupplySispm

2018-03-09 Thread Pau Espin Pedrol
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7167

to look at the new patch set (#2).

Introduce PowerSupply interface and PowerSupplySispm

File powersupply.py defines the interface to be used by child classes
implementing it. It also provides helpers to allocate a child class
based on configuration provided ('type' field).

File powersupply_sispm.py is an implementation using pysispm [1], as it's
the one used to control the programmable power socket we have right now.

This kind of class will be used in later commits by Nanobts class, as we
want to poweroff the Nanobts completelly when not in use.

Using it requires the following extra dependencies:
$ apt-get install python3-usb
$ pip3 install pysispm

Related: OS#3040

[1] https://github.com/xypron/pysispm

Change-Id: I981c260eca1a61657147e6d83b4226618088223c
---
A src/osmo_gsm_tester/powersupply.py
A src/osmo_gsm_tester/powersupply_sispm.py
2 files changed, 144 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/67/7167/2

diff --git a/src/osmo_gsm_tester/powersupply.py 
b/src/osmo_gsm_tester/powersupply.py
new file mode 100644
index 000..69dd51b
--- /dev/null
+++ b/src/osmo_gsm_tester/powersupply.py
@@ -0,0 +1,69 @@
+# osmo_gsm_tester: class defining a Power Supply object
+#
+# Copyright (C) 2018 by sysmocom - s.f.m.c. GmbH
+#
+# Author: Pau Espin Pedrol 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+from abc import ABCMeta, abstractmethod
+from . import log, event_loop
+
+class PowerSupply(log.Origin, metaclass=ABCMeta):
+
+##
+# PROTECTED
+##
+def __init__(self, conf, name):
+super().__init__(log.C_RUN, name)
+self.conf = conf
+
+
+# PUBLIC - INTERNAL API
+
+@abstractmethod
+def powered(self):
+'Get whether the device is powered on or off'
+pass
+
+@abstractmethod
+def power_set(self, onoff):
+'Turn on (onoff=True) or off (onof=False) the device'
+pass
+
+def power_cycle(self, sleep=0):
+'Turns off the device, waits N.N seconds, then turn on the device'
+self.power_set(False)
+event_loop.sleep(self, sleep)
+self.power_set(True)
+
+
+from . import powersupply_sispm
+
+KNOWN_PWSUPPLY_TYPES = {
+'sispm' : powersupply_sispm.PowerSupplySispm,
+}
+
+def register_type(name, clazz):
+KNOWN_PWSUPPLY_TYPES[name] = clazz
+
+def get_instance_by_type(pwsupply_type, pwsupply_opt):
+obj = KNOWN_PWSUPPLY_TYPES.get(pwsupply_type, None)
+if not obj:
+raise log.Error('PowerSupply type not supported:', pwsupply_type)
+return obj(pwsupply_opt)
+
+
+
+# vim: expandtab tabstop=4 shiftwidth=4
diff --git a/src/osmo_gsm_tester/powersupply_sispm.py 
b/src/osmo_gsm_tester/powersupply_sispm.py
new file mode 100644
index 000..3bc6c29
--- /dev/null
+++ b/src/osmo_gsm_tester/powersupply_sispm.py
@@ -0,0 +1,75 @@
+# osmo_gsm_tester: class defining a Power Supply object
+#
+# Copyright (C) 2018 by sysmocom - s.f.m.c. GmbH
+#
+# Author: Pau Espin Pedrol 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+import sispm
+
+from . import log
+from .powersupply import PowerSupply
+
+class PowerSupplySispm(PowerSupply):
+
+device = None
+
+##
+# PROTECTED
+##
+def __init__(self, conf):
+super().__init__(conf, 'sispm')
+mydevid = conf.get('device', None)
+if mydevid is None:
+raise log.Error('No "device" attribute provided in supply conf!')
+self.set_name('sispm-'+mydevid)
+myport = conf.get('port', None)
+if myport is None:
+

[PATCH] osmo-msc[master]: silent call: clarify rc and error messages logged on vty

2018-03-09 Thread Neels Hofmeyr

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

silent call: clarify rc and error messages logged on vty

In gsm_silent_call_{start,stop}(), return meaningful error codes and interpret
them on the VTY to clearly indicate the result.

Change-Id: Id5abb8f2ba901689e03040af8e51483b6c618e7f
---
M src/libmsc/silent_call.c
M src/libmsc/vty_interface_layer3.c
2 files changed, 40 insertions(+), 17 deletions(-)


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

diff --git a/src/libmsc/silent_call.c b/src/libmsc/silent_call.c
index f3291d7..1ea2305 100644
--- a/src/libmsc/silent_call.c
+++ b/src/libmsc/silent_call.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* paging of the requested subscriber has completed */
 static int paging_cb_silent(unsigned int hooknum, unsigned int event,
@@ -130,7 +131,9 @@
 * A-interface. */
req = subscr_request_conn(vsub, paging_cb_silent, data,
  "establish silent call");
-   return req != NULL;
+   if (!req)
+   return -ENODEV;
+   return 0;
 }
 
 /* end a silent call with a given subscriber */
@@ -139,12 +142,18 @@
struct gsm_subscriber_connection *conn;
 
conn = connection_for_subscr(vsub);
-   if (!conn)
-   return -EINVAL;
+   if (!conn) {
+   LOGP(DMM, LOGL_ERROR, "%s: Cannot stop silent call, no 
connection for subscriber\n",
+vlr_subscr_name(vsub));
+   return -ENODEV;
+   }
 
/* did we actually establish a silent call for this guy? */
-   if (!conn->silent_call)
-   return -EINVAL;
+   if (!conn->silent_call) {
+   LOGP(DMM, LOGL_ERROR, "%s: Cannot stop silent call, subscriber 
has no active silent call\n",
+vlr_subscr_name(vsub));
+   return -ENOENT;
+   }
 
 #if BEFORE_MSCSPLIT
/* Re-enable this log output once we can obtain this information via
diff --git a/src/libmsc/vty_interface_layer3.c 
b/src/libmsc/vty_interface_layer3.c
index 06a4267..05f5b63 100644
--- a/src/libmsc/vty_interface_layer3.c
+++ b/src/libmsc/vty_interface_layer3.c
@@ -524,16 +524,20 @@
type = RSL_CHANNEED_ANY;/* Defaults to ANY */
 
rc = gsm_silent_call_start(vsub, vty, type);
-   if (rc <= 0) {
-   vty_out(vty, "%% Subscriber not attached%s",
-   VTY_NEWLINE);
-   vlr_subscr_put(vsub);
-   return CMD_WARNING;
+   switch (rc) {
+   case -ENODEV:
+   vty_out(vty, "%% Subscriber not attached%s", VTY_NEWLINE);
+   break;
+   default:
+   if (rc)
+   vty_out(vty, "%% Cannot start silent call (rc=%d)%s", 
rc, VTY_NEWLINE);
+   else
+   vty_out(vty, "%% Silent call initiated%s", VTY_NEWLINE);
+   break;
}
 
vlr_subscr_put(vsub);
-
-   return CMD_SUCCESS;
+   return rc ? CMD_WARNING : CMD_SUCCESS;
 }
 
 DEFUN(subscriber_silent_call_stop,
@@ -553,14 +557,24 @@
}
 
rc = gsm_silent_call_stop(vsub);
-   if (rc < 0) {
-   vlr_subscr_put(vsub);
-   return CMD_WARNING;
+   switch (rc) {
+   case -ENODEV:
+   vty_out(vty, "%% No active connection for subscriber%s", 
VTY_NEWLINE);
+   break;
+   case -ENOENT:
+   vty_out(vty, "%% Subscriber has no silent call active%s",
+   VTY_NEWLINE);
+   break;
+   default:
+   if (rc)
+   vty_out(vty, "%% Cannot stop silent call (rc=%d)%s", 
rc, VTY_NEWLINE);
+   else
+   vty_out(vty, "%% Silent call stopped%s", VTY_NEWLINE);
+   break;
}
 
vlr_subscr_put(vsub);
-
-   return CMD_SUCCESS;
+   return rc ? CMD_WARNING : CMD_SUCCESS;
 }
 
 DEFUN(subscriber_ussd_notify,

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id5abb8f2ba901689e03040af8e51483b6c618e7f
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


[PATCH] osmo-msc[master]: vty: add 'subscriber ... paging' cmd

2018-03-09 Thread Neels Hofmeyr

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

vty: add 'subscriber ... paging' cmd

Allow issuing an empty paging from the VTY, for debugging purposes.

Change-Id: I403904cb789ece699f14b4cbd52c336eb02d45e4
---
M src/libmsc/vty_interface_layer3.c
1 file changed, 26 insertions(+), 0 deletions(-)


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

diff --git a/src/libmsc/vty_interface_layer3.c 
b/src/libmsc/vty_interface_layer3.c
index 6def359..06a4267 100644
--- a/src/libmsc/vty_interface_layer3.c
+++ b/src/libmsc/vty_interface_layer3.c
@@ -608,6 +608,31 @@
return CMD_SUCCESS;
 }
 
+DEFUN(subscriber_paging,
+  subscriber_paging_cmd,
+  "subscriber " SUBSCR_TYPES " ID paging",
+  SUBSCR_HELP "Issue an empty Paging for the subscriber (for debugging)\n")
+{
+   struct gsm_network *gsmnet = gsmnet_from_vty(vty);
+   struct vlr_subscr *vsub = get_vsub_by_argv(gsmnet, argv[0], argv[1]);
+   struct subscr_request *req;
+
+   if (!vsub) {
+   vty_out(vty, "%% No subscriber found for %s %s%s",
+   argv[0], argv[1], VTY_NEWLINE);
+   return CMD_WARNING;
+   }
+
+   req = subscr_request_conn(vsub, NULL, NULL, "manual Paging from VTY");
+   if (req)
+   vty_out(vty, "%% paging subscriber%s", VTY_NEWLINE);
+   else
+   vty_out(vty, "%% paging subscriber failed%s", VTY_NEWLINE);
+
+   vlr_subscr_put(vsub);
+   return req ? CMD_SUCCESS : CMD_WARNING;
+}
+
 static int loop_by_char(uint8_t ch)
 {
switch (ch) {
@@ -992,6 +1017,7 @@
install_element_ve(_ussd_notify_cmd);
install_element_ve(_mstest_close_cmd);
install_element_ve(_mstest_open_cmd);
+   install_element_ve(_paging_cmd);
install_element_ve(_stats_cmd);
install_element_ve(_smsqueue_cmd);
install_element_ve(_fltr_imsi_cmd);

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I403904cb789ece699f14b4cbd52c336eb02d45e4
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


[PATCH] osmo-msc[master]: vty: drop unused commands

2018-03-09 Thread Neels Hofmeyr

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

vty: drop unused commands

Change-Id: I76d6777ebe9105b8abf37993b86c3749a7e18008
---
M src/libmsc/vty_interface_layer3.c
1 file changed, 0 insertions(+), 30 deletions(-)


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

diff --git a/src/libmsc/vty_interface_layer3.c 
b/src/libmsc/vty_interface_layer3.c
index 6ed2ade..b0243c3 100644
--- a/src/libmsc/vty_interface_layer3.c
+++ b/src/libmsc/vty_interface_layer3.c
@@ -726,34 +726,6 @@
return CMD_SUCCESS;
 }
 
-#define A3A8_ALG_TYPES "(none|xor|comp128v1)"
-#define A3A8_ALG_HELP  \
-   "Use No A3A8 algorithm\n"   \
-   "Use XOR algorithm\n"   \
-   "Use COMP128v1 algorithm\n"
-
-DEFUN(ena_subscr_a3a8,
-  ena_subscr_a3a8_cmd,
-  "subscriber " SUBSCR_TYPES " ID a3a8 " A3A8_ALG_TYPES " [KI]",
-  SUBSCR_HELP "Set a3a8 parameters for the subscriber\n"
-  A3A8_ALG_HELP "Encryption Key Ki\n")
-{
-   vty_out(vty, "%% 'subscriber a3a8' is no longer supported.%s"
-   "%% This is now up to osmo-hlr.%s",
-   VTY_NEWLINE, VTY_NEWLINE);
-   return CMD_WARNING;
-}
-
-DEFUN(subscriber_update,
-  subscriber_update_cmd,
-  "subscriber " SUBSCR_TYPES " ID update",
-  SUBSCR_HELP "Update the subscriber data from the dabase.\n")
-{
-   vty_out(vty, "%% 'subscriber update' is no longer supported.%s",
-   VTY_NEWLINE);
-   return CMD_WARNING;
-}
-
 static int scall_cbfn(unsigned int subsys, unsigned int signal,
void *handler_data, void *signal_data)
 {
@@ -1019,13 +991,11 @@
install_element_ve(_ussd_notify_cmd);
install_element_ve(_mstest_close_cmd);
install_element_ve(_mstest_open_cmd);
-   install_element_ve(_update_cmd);
install_element_ve(_stats_cmd);
install_element_ve(_smsqueue_cmd);
install_element_ve(_fltr_imsi_cmd);
 
install_element(ENABLE_NODE, _subscr_expire_cmd);
-   install_element(ENABLE_NODE, _subscr_a3a8_cmd);
install_element(ENABLE_NODE, _trigger_cmd);
install_element(ENABLE_NODE, _max_cmd);
install_element(ENABLE_NODE, _clear_cmd);

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I76d6777ebe9105b8abf37993b86c3749a7e18008
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


[PATCH] osmo-msc[master]: msc_main: do not say 'osmo-nitb' in the usage

2018-03-09 Thread Neels Hofmeyr

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

msc_main: do not say 'osmo-nitb' in the usage

Change-Id: I2090097dc0d7e0251c116b95b802076df3419455
---
M src/osmo-msc/msc_main.c
1 file changed, 1 insertion(+), 1 deletion(-)


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

diff --git a/src/osmo-msc/msc_main.c b/src/osmo-msc/msc_main.c
index 00f132b..aba74b6 100644
--- a/src/osmo-msc/msc_main.c
+++ b/src/osmo-msc/msc_main.c
@@ -112,7 +112,7 @@
 
 static void print_usage()
 {
-   printf("Usage: osmo-nitb\n");
+   printf("Usage: osmo-msc\n");
 }
 
 static void print_help()

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2090097dc0d7e0251c116b95b802076df3419455
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


[PATCH] osmo-msc[master]: vty: add 'msisdn' as alias for 'extension'

2018-03-09 Thread Neels Hofmeyr

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

vty: add 'msisdn' as alias for 'extension'

Rationale: in the HLR, it is called 'msisdn' after the database column, so a
user going back and forth between osmo-hlr and osmo-msc would appreciate being
able to type 'msisdn' in the MSC's vty as well.

Change-Id: I7b46f9736421e8edd8a95ae89e025ebe486fde4c
---
M src/libmsc/vty_interface_layer3.c
1 file changed, 3 insertions(+), 2 deletions(-)


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

diff --git a/src/libmsc/vty_interface_layer3.c 
b/src/libmsc/vty_interface_layer3.c
index b0243c3..6def359 100644
--- a/src/libmsc/vty_interface_layer3.c
+++ b/src/libmsc/vty_interface_layer3.c
@@ -343,9 +343,10 @@
 
return NULL;
 }
-#define SUBSCR_TYPES "(extension|imsi|tmsi|id)"
+#define SUBSCR_TYPES "(msisdn|extension|imsi|tmsi|id)"
 #define SUBSCR_HELP "Operations on a Subscriber\n" \
-   "Identify subscriber by extension (phone number)\n" \
+   "Identify subscriber by MSISDN (phone number)\n"\
+   "Legacy alias for 'msisdn'\n"   \
"Identify subscriber by IMSI\n" \
"Identify subscriber by TMSI\n" \
"Identify subscriber by database ID\n"  \

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b46f9736421e8edd8a95ae89e025ebe486fde4c
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


[PATCH] osmo-msc[master]: fix: clear vlr_subscr->msc_conn_ref when the conn is discarded

2018-03-09 Thread Neels Hofmeyr

fix: clear vlr_subscr->msc_conn_ref when the conn is discarded

Before this, it was for example possible to crash the MSC by the vty 'show
subscriber' command, which would dereference a potentially stale
vsub->msc_conn_ref pointer.

Related: OS#3050
Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
---
M src/libmsc/osmo_msc.c
1 file changed, 1 insertion(+), 0 deletions(-)


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

diff --git a/src/libmsc/osmo_msc.c b/src/libmsc/osmo_msc.c
index 1478c19..e9a3c5b 100644
--- a/src/libmsc/osmo_msc.c
+++ b/src/libmsc/osmo_msc.c
@@ -229,6 +229,7 @@
   vlr_subscr_name(conn->vsub));
msc_subscr_cleanup(conn->vsub);
vlr_subscr_put(conn->vsub);
+   conn->vsub->msc_conn_ref = NULL;
conn->vsub = NULL;
} else
DEBUGP(DRLL, "Freeing subscriber connection"

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder


[PATCH] osmo-msc[master]: fix: clear vlr_subscr->msc_conn_ref when the conn is discarded

2018-03-09 Thread Neels Hofmeyr

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

fix: clear vlr_subscr->msc_conn_ref when the conn is discarded

Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
---
M src/libmsc/osmo_msc.c
1 file changed, 1 insertion(+), 0 deletions(-)


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

diff --git a/src/libmsc/osmo_msc.c b/src/libmsc/osmo_msc.c
index 1478c19..e9a3c5b 100644
--- a/src/libmsc/osmo_msc.c
+++ b/src/libmsc/osmo_msc.c
@@ -229,6 +229,7 @@
   vlr_subscr_name(conn->vsub));
msc_subscr_cleanup(conn->vsub);
vlr_subscr_put(conn->vsub);
+   conn->vsub->msc_conn_ref = NULL;
conn->vsub = NULL;
} else
DEBUGP(DRLL, "Freeing subscriber connection"

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 


osmo-trx[master]: Transceiver.cpp: use pointer arithmetics for CMD parsing

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/7172/1/Transceiver52M/Transceiver.cpp
File Transceiver52M/Transceiver.cpp:

Line 672:   size_t cmd_len = strlen(cmd);
> Thanks!
You can add a macro:
#define MATCH_CMD(buf, cmd, params) match_cmd(buf, cmd, sizeof(cmd) - 1, params)

You can even stringify cmd in the macro and pass MATCH_CMD(buf, POWEROFF, 
).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I226ca0771e63228cf5e04ef9766057d4107fdd11
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


[PATCH] osmo-trx[master]: Transceiver.cpp: use pointer arithmetics for CMD parsing

2018-03-09 Thread Vadim Yanitskiy
Hello Pau Espin Pedrol, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7172

to look at the new patch set (#2).

Transceiver.cpp: use pointer arithmetics for CMD parsing

It looks like the author of control command parsing code was not
familar with simple pointer arithmetics, so excessive amount of
memory and useless memcopying was used to parse a single command.

Let's introduce two pointers, one of which will point to the
beginning of a command, another to the beginning of its arguments.
Also, let's simplify the command matching by using a separate
function called 'MATCH_CMD'.

Change-Id: I226ca0771e63228cf5e04ef9766057d4107fdd11
---
M Transceiver52M/Transceiver.cpp
1 file changed, 64 insertions(+), 51 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/72/7172/2

diff --git a/Transceiver52M/Transceiver.cpp b/Transceiver52M/Transceiver.cpp
index 8f41c5e..4b4fefe 100644
--- a/Transceiver52M/Transceiver.cpp
+++ b/Transceiver52M/Transceiver.cpp
@@ -659,10 +659,40 @@
 
 #define MAX_PACKET_LENGTH 100
 
+/**
+ * Matches a buffer with a command.
+ * @param  bufa buffer to look command in
+ * @param  cmda command to look in buffer
+ * @param  params pointer to arguments, or NULL
+ * @returntrue if command matches, otherwise false
+ */
+static bool match_cmd(char *buf,
+  const char *cmd, char **params)
+{
+  size_t cmd_len = strlen(cmd);
+
+  /* Check a command itself */
+  if (strncmp(buf, cmd, cmd_len))
+return false;
+
+  /* A command has arguments */
+  if (params != NULL) {
+/* Make sure there is a space */
+if (buf[cmd_len] != ' ')
+  return false;
+
+/* Update external pointer */
+*params = buf + cmd_len + 1;
+  }
+
+  return true;
+}
+
 void Transceiver::driveControl(size_t chan)
 {
   char buffer[MAX_PACKET_LENGTH + 1];
   char response[MAX_PACKET_LENGTH + 1];
+  char *command, *params;
   int msgLen;
 
   /* Attempt to read from control socket */
@@ -673,22 +703,20 @@
   /* Zero-terminate received string */
   buffer[msgLen] = '\0';
 
-  char cmdcheck[4];
-  char command[MAX_PACKET_LENGTH];
-
-  sscanf(buffer,"%3s %s",cmdcheck,command);
-
-  if (strcmp(cmdcheck,"CMD")!=0) {
+  /* Verify a command signature */
+  if (strncmp(buffer, "CMD ", 4)) {
 LOG(WARNING) << "bogus message on control interface";
 return;
   }
-  LOG(INFO) << "command is " << buffer;
 
-  if (strcmp(command,"POWEROFF")==0) {
+  /* Set command pointer */
+  command = buffer + 4;
+  LOG(INFO) << "command is " << command;
+
+  if (match_cmd(command, "POWEROFF", NULL)) {
 stop();
 sprintf(response,"RSP POWEROFF 0");
-  }
-  else if (strcmp(command,"POWERON")==0) {
+  } else if (match_cmd(command, "POWERON", NULL)) {
 if (!start()) {
   sprintf(response,"RSP POWERON 1");
 } else {
@@ -698,41 +726,35 @@
   mHandover[i][j] = false;
   }
 }
-  }
-  else if (strcmp(command,"HANDOVER")==0){
+  } else if (match_cmd(command, "HANDOVER", )) {
 int ts=0,ss=0;
-sscanf(buffer,"%3s %s %d %d",cmdcheck,command,,);
+sscanf(params, "%d %d", , );
 mHandover[ts][ss] = true;
 sprintf(response,"RSP HANDOVER 0 %d %d",ts,ss);
-  }
-  else if (strcmp(command,"NOHANDOVER")==0){
+  } else if (match_cmd(command, "NOHANDOVER", )) {
 int ts=0,ss=0;
-sscanf(buffer,"%3s %s %d %d",cmdcheck,command,,);
+sscanf(params, "%d %d", , );
 mHandover[ts][ss] = false;
 sprintf(response,"RSP NOHANDOVER 0 %d %d",ts,ss);
-  }
-  else if (strcmp(command,"SETMAXDLY")==0) {
+  } else if (match_cmd(command, "SETMAXDLY", )) {
 //set expected maximum time-of-arrival
 int maxDelay;
-sscanf(buffer,"%3s %s %d",cmdcheck,command,);
+sscanf(params, "%d", );
 mMaxExpectedDelayAB = maxDelay; // 1 GSM symbol is approx. 1 km
 sprintf(response,"RSP SETMAXDLY 0 %d",maxDelay);
-  }
-  else if (strcmp(command,"SETMAXDLYNB")==0) {
+  } else if (match_cmd(command, "SETMAXDLYNB", )) {
 //set expected maximum time-of-arrival
 int maxDelay;
-sscanf(buffer,"%3s %s %d",cmdcheck,command,);
+sscanf(params, "%d", );
 mMaxExpectedDelayNB = maxDelay; // 1 GSM symbol is approx. 1 km
 sprintf(response,"RSP SETMAXDLYNB 0 %d",maxDelay);
-  }
-  else if (strcmp(command,"SETRXGAIN")==0) {
+  } else if (match_cmd(command, "SETRXGAIN", )) {
 //set expected maximum time-of-arrival
 int newGain;
-sscanf(buffer,"%3s %s %d",cmdcheck,command,);
+sscanf(params, "%d", );
 newGain = mRadioInterface->setRxGain(newGain, chan);
 sprintf(response,"RSP SETRXGAIN 0 %d",newGain);
-  }
-  else if (strcmp(command,"NOISELEV")==0) {
+  } else if (match_cmd(command, "NOISELEV", NULL)) {
 if (mOn) {
   float lev = mStates[chan].mNoiseLev;
   sprintf(response,"RSP NOISELEV 0 %d",
@@ -741,26 +763,23 @@
 else {
   sprintf(response,"RSP NOISELEV 1  0");
 }
-  }
-  else if (!strcmp(command, "SETPOWER")) {
+  } else if (match_cmd(command, "SETPOWER", 

[PATCH] osmo-trx[master]: Transceiver.cpp: prevent out-of-range array access

2018-03-09 Thread Vadim Yanitskiy
Hello Pau Espin Pedrol, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7174

to look at the new patch set (#2).

Transceiver.cpp: prevent out-of-range array access

There was no a simple range check for both (NO)HANDOVER commands,
so an out-of-range access was possible. For example, a command:

  CMD HANDOVER 0 -3

might enable EDGE at run-time, because:

  a[i] == *(a + i)

Let's fix this.

Change-Id: I24a5f70e8e8097f218d7cbdef8cb10df2c35416f
---
M Transceiver52M/Transceiver.cpp
1 file changed, 16 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/74/7174/2

diff --git a/Transceiver52M/Transceiver.cpp b/Transceiver52M/Transceiver.cpp
index 859a1de..2d3771c 100644
--- a/Transceiver52M/Transceiver.cpp
+++ b/Transceiver52M/Transceiver.cpp
@@ -727,15 +727,23 @@
   }
 }
   } else if (match_cmd(command, "HANDOVER", )) {
-int ts=0,ss=0;
-sscanf(params, "%d %d", , );
-mHandover[ts][ss] = true;
-sprintf(response,"RSP HANDOVER 0 %d %d",ts,ss);
+unsigned ts = 0, ss = 0;
+sscanf(params, "%u %u", , );
+if (ts > 7 || ss > 7) {
+  sprintf(response, "RSP NOHANDOVER 1 %u %u", ts, ss);
+} else {
+  mHandover[ts][ss] = true;
+  sprintf(response, "RSP HANDOVER 0 %u %u", ts, ss);
+}
   } else if (match_cmd(command, "NOHANDOVER", )) {
-int ts=0,ss=0;
-sscanf(params, "%d %d", , );
-mHandover[ts][ss] = false;
-sprintf(response,"RSP NOHANDOVER 0 %d %d",ts,ss);
+unsigned ts = 0, ss = 0;
+sscanf(params, "%u %u", , );
+if (ts > 7 || ss > 7) {
+  sprintf(response, "RSP NOHANDOVER 1 %u %u", ts, ss);
+} else {
+  mHandover[ts][ss] = false;
+  sprintf(response, "RSP NOHANDOVER 0 %u %u", ts, ss);
+}
   } else if (match_cmd(command, "SETMAXDLY", )) {
 //set expected maximum time-of-arrival
 int maxDelay;

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I24a5f70e8e8097f218d7cbdef8cb10df2c35416f
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 


osmo-trx[master]: Transceiver.cpp: use pointer arithmetics for CMD parsing

2018-03-09 Thread Vadim Yanitskiy

Patch Set 1:

(3 comments)

https://gerrit.osmocom.org/#/c/7172/1/Transceiver52M/Transceiver.cpp
File Transceiver52M/Transceiver.cpp:

Line 672:   size_t cmd_len = strlen(cmd);
> as the string of cmd is known at compile time, better pass cmd_len as param
Thanks!

> mark it as static and let the compiler decide

Done.

> better use lower caps -> match_cmd

Done.

Still not sure about passing the length of a command.
I also don't like that 'cmd_len' will be recalculated
every time, and passing the length statically would be
better, but 'sizeof("POWEROFF")-1' looks ugly to me, sorry.

I am anyway open for any ideas ;)


Line 731: sscanf(params, "%d %d", , );
> We should be checking the return value of sscanf to make sure it matches th
A topic for a separate change, here I am only moving
to simple pointer arithmetics.


Line 733: sprintf(response,"RSP HANDOVER 0 %d %d",ts,ss);
> I'd use snprintf in here and all other responses. Probably you are already 
A topic for a separate change, here I am only moving
to simple pointer arithmetics.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I226ca0771e63228cf5e04ef9766057d4107fdd11
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: Yes


libasn1c[master]: configure: add --enable-werror

2018-03-09 Thread Harald Welte

Patch Set 1:

well, as nobody is proposign any feasible alternative, I'm inclined to say "go 
ahead mark all of those as +2 and merge it'.  I still don't like it, but I 
don't have time for a counter-proposal either.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcc8238584a585434b39a046cd2d7e18ddaf7f8c
Gerrit-PatchSet: 1
Gerrit-Project: libasn1c
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


[MERGED] libosmocore[master]: src/msgb.c: cosmetic: fix spelling mistakes

2018-03-09 Thread Harald Welte
Harald Welte has submitted this change and it was merged.

Change subject: src/msgb.c: cosmetic: fix spelling mistakes
..


src/msgb.c: cosmetic: fix spelling mistakes

Change-Id: I6b473aadaa22d95f2a8cc87580c638ccd7e531a4
---
M src/msgb.c
1 file changed, 3 insertions(+), 3 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/msgb.c b/src/msgb.c
index faef09e..48ef25a 100644
--- a/src/msgb.c
+++ b/src/msgb.c
@@ -38,7 +38,7 @@
  *   * headroom, i.e. space in front of the message, to allow
  * for additional headers being pushed in front of the current
  * data
- *   * the curently occupied data for the message
+ *   * the currently occupied data for the message
  *   * tailroom, i.e. space at the end of the message, to
  * allow more data to be added after the end of the current
  * data
@@ -98,7 +98,7 @@
 }
 
 /*! Release given message buffer
- * \param[in] m Message buffer to be free'd
+ * \param[in] m Message buffer to be freed
  */
 void msgb_free(struct msgb *m)
 {
@@ -144,7 +144,7 @@
  *  \param[in] msg message buffer that is to be resetted
  *
  * This will re-set the various internal pointers into the underlying
- * message buffer, i.e. remvoe all headroom and treat the msgb as
+ * message buffer, i.e. remove all headroom and treat the msgb as
  * completely empty.  It also initializes the control buffer to zero.
  */
 void msgb_reset(struct msgb *msg)

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6b473aadaa22d95f2a8cc87580c638ccd7e531a4
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder


[MERGED] libosmocore[master]: src/msgb.c: remove dead includes from OpenBSC

2018-03-09 Thread Harald Welte
Harald Welte has submitted this change and it was merged.

Change subject: src/msgb.c: remove dead includes from OpenBSC
..


src/msgb.c: remove dead includes from OpenBSC

The MSGB API is not a part of OpenBSC anymore, so let's remove
dead includes, which were probably left here during the
migration process.

Change-Id: Ief562a6e5b220a84902f95862d67279f953ee726
---
M src/msgb.c
1 file changed, 0 insertions(+), 3 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/msgb.c b/src/msgb.c
index 48ef25a..9efaea5 100644
--- a/src/msgb.c
+++ b/src/msgb.c
@@ -60,11 +60,8 @@
 #include 
 #include 
 
-
 #include 
-//#include 
 #include 
-//#include 
 
 void *tall_msgb_ctx = NULL;
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ief562a6e5b220a84902f95862d67279f953ee726
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder


[MERGED] libosmocore[master]: src/msgb.c: avoid using internal talloc API

2018-03-09 Thread Harald Welte
Harald Welte has submitted this change and it was merged.

Change subject: src/msgb.c: avoid using internal talloc API
..


src/msgb.c: avoid using internal talloc API

An internal symbol '_talloc_zero' of talloc library was used
during a msgb allocation. This is not actually good because:

  - it may be removed or modified by talloc developers;
  - the behaviour may be changed by talloc developers;
  - it's marked as internal using 'underscore';
  - there is public API to do the same.

So, let's use the public API.

Change-Id: I1080c9071e997944cc0f9fc3716129e9395437ad
---
M src/msgb.c
1 file changed, 4 insertions(+), 2 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/msgb.c b/src/msgb.c
index 82902b4..844cfc6 100644
--- a/src/msgb.c
+++ b/src/msgb.c
@@ -79,14 +79,16 @@
 {
struct msgb *msg;
 
-   msg = _talloc_zero(tall_msgb_ctx, sizeof(*msg) + size, name);
-
+   msg = talloc_named_const(tall_msgb_ctx, sizeof(*msg) + size, name);
if (!msg) {
LOGP(DLGLOBAL, LOGL_FATAL, "Unable to allocate a msgb: "
"name='%s', size=%u\n", name, size);
return NULL;
}
 
+   /* Manually zero-initialize allocated memory */
+   memset(msg, 0x00, sizeof(*msg) + size);
+
msg->data_len = size;
msg->len = 0;
msg->data = msg->_data;

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1080c9071e997944cc0f9fc3716129e9395437ad
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 


[MERGED] libosmocore[master]: src/msgb.c: print an error if msgb allocation failed

2018-03-09 Thread Harald Welte
Harald Welte has submitted this change and it was merged.

Change subject: src/msgb.c: print an error if msgb allocation failed
..


src/msgb.c: print an error if msgb allocation failed

Printing an error message when msgb allocation failed was initially
intended, but have been commented out for years. This would
facilitate the bug hunting process, especially on embedded
platforms with limited resources (e.g. amount of RAM).

The GLOBAL logging subsystem with FATAL level is used
for printing such messages.

Change-Id: I3e2d1beabd6936fc28a1ad664c083ff1698bb644
---
M src/msgb.c
1 file changed, 3 insertions(+), 1 deletion(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/msgb.c b/src/msgb.c
index 9efaea5..82902b4 100644
--- a/src/msgb.c
+++ b/src/msgb.c
@@ -62,6 +62,7 @@
 
 #include 
 #include 
+#include 
 
 void *tall_msgb_ctx = NULL;
 
@@ -81,7 +82,8 @@
msg = _talloc_zero(tall_msgb_ctx, sizeof(*msg) + size, name);
 
if (!msg) {
-   //LOGP(DRSL, LOGL_FATAL, "unable to allocate msgb\n");
+   LOGP(DLGLOBAL, LOGL_FATAL, "Unable to allocate a msgb: "
+   "name='%s', size=%u\n", name, size);
return NULL;
}
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3e2d1beabd6936fc28a1ad664c083ff1698bb644
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder


libosmocore[master]: src/msgb.c: avoid using internal talloc API

2018-03-09 Thread Harald Welte

Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1080c9071e997944cc0f9fc3716129e9395437ad
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No


libasn1c[master]: configure: add --enable-werror

2018-03-09 Thread Neels Hofmeyr

Patch Set 1:

> So clearly =pragma is wrong

Ah, you meant, not suppress #pragma, but use #pragma instead of #warning. Ok, 
possible, but with -Wno-error=cpp we can also keep the #warnings :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcc8238584a585434b39a046cd2d7e18ddaf7f8c
Gerrit-PatchSet: 1
Gerrit-Project: libasn1c
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


libasn1c[master]: configure: add --enable-werror

2018-03-09 Thread Neels Hofmeyr

Patch Set 1:

> Regarding deprecated, I think it's good that it fails when using a
 > deprecated symbol, this way it forces us to fix it.

That part is good, yes, but the pattern that has emerged recently is: our 
jenkins builds use -Werror. That means as soon as we create some foo2() and 
deprecate foo(), all of the builds that still use foo() flare up red. It's 
still ok for that other project to use foo(), we haven't yet seen a need for it 
to move the newer API and use the new, safer features / the clarified signature 
/ the more universal application. So what do I do? I *don't* mark foo() as 
deprecated. I make a note in my mind to first look at all the dependant 
projects and get rid of foo(), and *then* mark it deprecated. And then that 
never happens. The conclusion is we will refrain from marking anything 
deprecated if our builds fail because of deprecation warnings.

So my opinion is quite strongly against failing for deprecation. We really need 
all those other warnings as errors, yes, deprecation as error breaks the 
workflow.

 > Regarding the warning pragmas, I'd bet -Wno-error=cpp also disabled
 > other C preprocessor warnings which we may not want to disable. If
 > you want to add explicit warning messages to show during the build,
 > use #pragma message instead.

man gcc:

-Wno-cpp
   (C, Objective-C, C++, Objective-C++ and Fortran only)

   Suppress warning messages emitted by "#warning" directives.

and

   -Wno-pragmas
   Do not warn about misuses of pragmas, such as incorrect parameters, 
invalid syntax, or conflicts between pragmas.  See also -Wunknown-pragmas.

So clearly =pragma is wrong and =cpp has exactly the desired effect. (Unless 
the manpage is wrong.)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcc8238584a585434b39a046cd2d7e18ddaf7f8c
Gerrit-PatchSet: 1
Gerrit-Project: libasn1c
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


[MERGED] osmo-bts[master]: fix handover: handle_ph_ra_ind(): evaluate ra_ind before msg...

2018-03-09 Thread Harald Welte
Harald Welte has submitted this change and it was merged.

Change subject: fix handover: handle_ph_ra_ind(): evaluate ra_ind before 
msgb_trim()
..


fix handover: handle_ph_ra_ind(): evaluate ra_ind before msgb_trim()

Commit c2b4c668f3510b7b0baace749c5a310959010e90
I3b989580cb38082e3fd8fc50a11fedda13991092 introduces evaluation of ra_ind
members below the msgb_trim() call that actually invalidates ra_ind.
A symptom is that it breaks detection of Handover RACH, wich always ends up
with lchan == NULL and interpreting all RACH as chan_nr == 0x88.

Fix: do all evaluation of ra_ind before the msgb_trim(), for osmo-bts-sysmo,
litecell-15 and octphy.

To guard against similar mistakes in the future, set ra_ind = NULL before the
msgb_trim() call.

Related: OS#3045
Change-Id: I203021ee57f49cb963679ba8bec5943e2abb67fb
---
M src/osmo-bts-litecell15/l1_if.c
M src/osmo-bts-octphy/l1_if.c
M src/osmo-bts-sysmo/l1_if.c
3 files changed, 138 insertions(+), 117 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo-bts-litecell15/l1_if.c b/src/osmo-bts-litecell15/l1_if.c
index 9e122cd..ea652a1 100644
--- a/src/osmo-bts-litecell15/l1_if.c
+++ b/src/osmo-bts-litecell15/l1_if.c
@@ -997,10 +997,8 @@
struct gsm_bts_role_bts *btsb = bts->role;
struct gsm_lchan *lchan;
struct osmo_phsap_prim *l1sap;
-   uint32_t fn;
-   uint8_t acc_delay = 0;
-   uint16_t ra = 0, is_11bit = 0, burst_type = 0, temp = 0;
int rc;
+   struct ph_rach_ind_param rach_ind_param;
 
/* FIXME: this should be deprecated/obsoleted as it bypasses rach.busy 
counting */
if (ra_ind->measParam.fLinkQuality < btsb->min_qual_rach) {
@@ -1008,12 +1006,7 @@
return 0;
}
 
-   /* the old legacy full-bits acc_delay cannot express negative values */
-   if (ra_ind->measParam.i16BurstTiming > 0)
-   acc_delay = ra_ind->measParam.i16BurstTiming >> 2;
-
dump_meas_res(LOGL_DEBUG, _ind->measParam);
-   burst_type = ra_ind->burstType;
 
if ((ra_ind->msgUnitParam.u8Size != 1) &&
(ra_ind->msgUnitParam.u8Size != 2)) {
@@ -1022,60 +1015,74 @@
return 0;
}
 
+   /* We need to evaluate ra_ind before below msgb_trim(), since that 
invalidates *ra_ind. */
+   rach_ind_param = (struct ph_rach_ind_param) {
+   /* .chan_nr set below */
+   /* .ra set below */
+   .acc_delay = 0,
+   .fn = ra_ind->u32Fn,
+   /* .is_11bit set below */
+   /* .burst_type set below */
+   .rssi = (int8_t) ra_ind->measParam.fRssi,
+   .ber10k = (unsigned int) (ra_ind->measParam.fBer * 1.0),
+   .acc_delay_256bits = ra_ind->measParam.i16BurstTiming * 64,
+   };
+
+   lchan = l1if_hLayer_to_lchan(trx, (uint32_t)ra_ind->hLayer2);
+   if (!lchan || lchan->ts->pchan == GSM_PCHAN_CCCH ||
+   lchan->ts->pchan == GSM_PCHAN_CCCH_SDCCH4 ||
+   lchan->ts->pchan == GSM_PCHAN_CCCH_SDCCH4_CBCH)
+   rach_ind_param.chan_nr = 0x88;
+   else
+   rach_ind_param.chan_nr = gsm_lchan2chan_nr(lchan);
+
if (ra_ind->msgUnitParam.u8Size == 2) {
-   is_11bit = 1;
-   ra = ra_ind->msgUnitParam.u8Buffer[0];
+   uint16_t temp;
+   uint16_t ra = ra_ind->msgUnitParam.u8Buffer[0];
ra = ra << 3;
temp = (ra_ind->msgUnitParam.u8Buffer[1] & 0x7);
ra = ra | temp;
+   rach_ind_param.is_11bit = 1;
+   rach_ind_param.ra = ra;
} else {
-   is_11bit = 0;
-   ra = ra_ind->msgUnitParam.u8Buffer[0];
+   rach_ind_param.is_11bit = 0;
+   rach_ind_param.ra = ra_ind->msgUnitParam.u8Buffer[0];
}
 
-   fn = ra_ind->u32Fn;
+   /* the old legacy full-bits acc_delay cannot express negative values */
+   if (ra_ind->measParam.i16BurstTiming > 0)
+   rach_ind_param.acc_delay = ra_ind->measParam.i16BurstTiming >> 
2;
+
+   /* mapping of the burst type, the values are specific to
+* osmo-bts-litecell15 */
+   switch (ra_ind->burstType) {
+   case GsmL1_BurstType_Access_0:
+   rach_ind_param.burst_type =
+   GSM_L1_BURST_TYPE_ACCESS_0;
+   break;
+   case GsmL1_BurstType_Access_1:
+   rach_ind_param.burst_type =
+   GSM_L1_BURST_TYPE_ACCESS_1;
+   break;
+   case GsmL1_BurstType_Access_2:
+   rach_ind_param.burst_type =
+   GSM_L1_BURST_TYPE_ACCESS_2;
+   break;
+   default:
+   rach_ind_param.burst_type =
+   GSM_L1_BURST_TYPE_NONE;
+   break;
+   }
+
+   /* msgb_trim() 

osmo-bts[master]: fix handover: handle_ph_ra_ind(): evaluate ra_ind before msg...

2018-03-09 Thread Harald Welte

Patch Set 2:

merge this now, we can still improve further upon it later

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

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


osmo-bts[master]: fix handover: handle_ph_ra_ind(): evaluate ra_ind before msg...

2018-03-09 Thread Harald Welte

Patch Set 2: Code-Review+2

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

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


osmo-bsc[master]: debian: Move abisip-find from osmo-bsc to its own package

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1:

It would be nice if somebody with better knowledge on how the debian packages 
are being generated can have a look at this and test it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If3d476a1bd67abcb9cff241ab5989db923873986
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


[PATCH] osmo-bsc[master]: debian: Move abisip-find from osmo-bsc to its own package

2018-03-09 Thread Pau Espin Pedrol

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

debian: Move abisip-find from osmo-bsc to its own package

In some scenarios osmo-bsc is not required, only abisip-find to do the
initial set-up of the BTS, so no need to install osmo-bsc with it.

Change-Id: If3d476a1bd67abcb9cff241ab5989db923873986
---
A debian/abisip-find.install
M debian/control
M debian/osmo-bsc.install
3 files changed, 7 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/75/7175/1

diff --git a/debian/abisip-find.install b/debian/abisip-find.install
new file mode 100644
index 000..1e19459
--- /dev/null
+++ b/debian/abisip-find.install
@@ -0,0 +1 @@
+usr/bin/abisip-find
diff --git a/debian/control b/debian/control
index fae280a..c2627a1 100644
--- a/debian/control
+++ b/debian/control
@@ -36,6 +36,12 @@
 Depends: osmo-bsc (= ${binary:Version}), ${misc:Depends}
 Description: OsmoBSC: Osmocom's Base Station Controller for 2G 
circuit-switched mobile networks
 
+Package: abisip-find
+Architecture: any
+Multi-Arch: foreign
+Depends: ${misc:Depends}, ${shlibs:Depends}
+Description: Command line utility to find ip.access compatible BTS
+
 Package: osmo-bsc-ipaccess-utils
 Architecture: any
 Multi-Arch: foreign
diff --git a/debian/osmo-bsc.install b/debian/osmo-bsc.install
index 9a2bc3f..5dc1e0f 100644
--- a/debian/osmo-bsc.install
+++ b/debian/osmo-bsc.install
@@ -1,4 +1,3 @@
 usr/bin/osmo-bsc
-usr/bin/abisip-find
 usr/share/doc/osmo-bsc/examples/osmo-bsc/osmo-bsc_custom-sccp.cfg 
usr/share/doc/osmo-bsc/examples
 usr/share/doc/osmo-bsc/examples/osmo-bsc/osmo-bsc.cfg 
usr/share/doc/osmo-bsc/examples

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If3d476a1bd67abcb9cff241ab5989db923873986
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol 


osmo-trx[master]: Transceiver.cpp: use a define for the MAX_PACKET_LENGTH

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If9ffb03b9e7b642f45732ba5938977bca271f1c7
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


osmo-trx[master]: Transceiver.cpp: fix incorrect format string for SETTSC

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If69a478121a31aa7df945548cc17271c476d6a6b
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


osmo-trx[master]: Transceiver.cpp: properly zero-terminate received commands

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69f413f33156c38a853efc5a8cdc66fbfb0ca6af
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: No


osmo-trx[master]: Transceiver.cpp: use pointer arithmetics for CMD parsing

2018-03-09 Thread Pau Espin Pedrol

Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.osmocom.org/#/c/7172/1/Transceiver52M/Transceiver.cpp
File Transceiver52M/Transceiver.cpp:

Line 672:   size_t cmd_len = strlen(cmd);
as the string of cmd is known at compile time, better pass cmd_len as param and 
use eg. sizeof("POWEROFF")-1.

BTW, this looks like a lot of code for an explicit inline function. I'd better 
mark it as static and let the compiler decide. Then better use lower caps -> 
match_cmd.


Line 731: sscanf(params, "%d %d", , );
We should be checking the return value of sscanf to make sure it matches the 
amount of params expected.

Once we do that, there's no need to initialize ts and ss variables.


Line 733: sprintf(response,"RSP HANDOVER 0 %d %d",ts,ss);
I'd use snprintf in here and all other responses. Probably you are already 
doing that in next patches.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I226ca0771e63228cf5e04ef9766057d4107fdd11
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-HasComments: Yes