From 531734a547f16de08ce94ec64d58cf94c2230893 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 14 Mar 2016 16:13:24 +0100 Subject: 04.08: apply new bitmask functions, fix bitmask use Replace hardcoded protocol discriminator and message type bitmasks with function calls recently introduced in libosmocore. Note that the release 98 bitmasks slightly differ from the release 99 bitmasks. This patch uses the "default" gsm48_hdr_msg_type invocation, thus it depends on libosmocore whether 98 or 99 bitmasks are used. In some places, use of the bitmask was erratic. Fix these implicitly by employing the bitmask functions: * silent_call.c: silent_call_reroute(): add missing bitmask for MM. * bsc_msg_filter.c: bsc_msg_filter_initial(): RR vs. MM messages. * osmo_bsc_filter.c: bsc_find_msc() and bsc_scan_bts_msg(): RR vs. MM messages. * bsc_nat_rewrite.c: bsc_nat_rewrite_msg(): SMS vs. CC messages. * bsc_ussd.c: no bitmask is applicable for the message types used here. * gb_proxy.c: gbproxy_imsi_acquisition(): missing bit mask for pdisc. In gprs_gb_parse.c: gprs_gb_parse_dtap(), add a log notice for unexpected message types. --- openbsc/src/gprs/gb_proxy.c | 4 ++-- openbsc/src/gprs/gprs_gb_parse.c | 13 ++++++++++--- openbsc/src/gprs/gprs_gmm.c | 2 +- openbsc/src/libbsc/bsc_api.c | 8 +++++--- openbsc/src/libfilter/bsc_msg_filter.c | 10 +++++----- openbsc/src/libmsc/gsm_04_08.c | 11 +++++------ openbsc/src/libmsc/silent_call.c | 5 +++-- openbsc/src/osmo-bsc/osmo_bsc_api.c | 8 ++++---- openbsc/src/osmo-bsc/osmo_bsc_filter.c | 10 +++++----- openbsc/src/osmo-bsc_nat/bsc_nat.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_ussd.c | 4 ++-- 12 files changed, 46 insertions(+), 37 deletions(-) diff --git a/openbsc/src/gprs/gb_proxy.c b/openbsc/src/gprs/gb_proxy.c index 955133562..6e6b03b86 100644 --- a/openbsc/src/gprs/gb_proxy.c +++ b/openbsc/src/gprs/gb_proxy.c @@ -462,8 +462,8 @@ static int gbproxy_imsi_acquisition(struct gbproxy_peer *peer, if (link_info->imsi_acq_pending && link_info->imsi_len > 0) { int is_ident_resp = parse_ctx->g48_hdr && - parse_ctx->g48_hdr->proto_discr == GSM48_PDISC_MM_GPRS && - parse_ctx->g48_hdr->msg_type == GSM48_MT_GMM_ID_RESP; + gsm48_hdr_pdisc(parse_ctx->g48_hdr) == GSM48_PDISC_MM_GPRS && + gsm48_hdr_msg_type(parse_ctx->g48_hdr) == GSM48_MT_GMM_ID_RESP; /* The IMSI is now available */ gbproxy_flush_stored_messages(peer, msg, now, link_info, diff --git a/openbsc/src/gprs/gprs_gb_parse.c b/openbsc/src/gprs/gprs_gb_parse.c index 609685407..63ac9028d 100644 --- a/openbsc/src/gprs/gprs_gb_parse.c +++ b/openbsc/src/gprs/gprs_gb_parse.c @@ -329,17 +329,20 @@ int gprs_gb_parse_dtap(uint8_t *data, size_t data_len, struct gprs_gb_parse_context *parse_ctx) { struct gsm48_hdr *g48h; + uint8_t pdisc; + uint8_t msg_type; if (gprs_shift_v_fixed(&data, &data_len, sizeof(*g48h), (uint8_t **)&g48h) <= 0) return 0; parse_ctx->g48_hdr = g48h; - if ((g48h->proto_discr & 0x0f) != GSM48_PDISC_MM_GPRS && - (g48h->proto_discr & 0x0f) != GSM48_PDISC_SM_GPRS) + pdisc = gsm48_hdr_pdisc(g48h); + if (pdisc != GSM48_PDISC_MM_GPRS && pdisc != GSM48_PDISC_SM_GPRS) return 1; - switch (g48h->msg_type) { + msg_type = gsm48_hdr_msg_type(g48h); + switch (msg_type) { case GSM48_MT_GMM_ATTACH_REQ: return gprs_gb_parse_gmm_attach_req(data, data_len, parse_ctx); @@ -376,6 +379,10 @@ int gprs_gb_parse_dtap(uint8_t *data, size_t data_len, break; default: + LOGP(DLLC, LOGL_NOTICE, + "Unknown GSM 04.08 message type 0x%02hhx for protocol" + " discriminator 0x%02hhx.\n", + msg_type, pdisc); break; }; diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c index d93ba3f77..9d94c24fe 100644 --- a/openbsc/src/gprs/gprs_gmm.c +++ b/openbsc/src/gprs/gprs_gmm.c @@ -2089,7 +2089,7 @@ int gsm0408_gprs_force_reattach(struct sgsn_mm_ctx *mmctx) int gsm0408_gprs_rcvmsg(struct msgb *msg, struct gprs_llc_llme *llme) { struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; + uint8_t pdisc = gsm48_hdr_pdisc(gh); struct sgsn_mm_ctx *mmctx; struct gprs_ra_id ra_id; int rc = -EINVAL; diff --git a/openbsc/src/libbsc/bsc_api.c b/openbsc/src/libbsc/bsc_api.c index 504f044fb..e6d820d7e 100644 --- a/openbsc/src/libbsc/bsc_api.c +++ b/openbsc/src/libbsc/bsc_api.c @@ -563,6 +563,7 @@ static void dispatch_dtap(struct gsm_subscriber_connection *conn, struct bsc_api *api = msg->lchan->ts->trx->bts->network->bsc_api; struct gsm48_hdr *gh; uint8_t pdisc; + uint8_t msg_type; int rc; if (msgb_l3len(msg) < sizeof(*gh)) { @@ -571,7 +572,8 @@ static void dispatch_dtap(struct gsm_subscriber_connection *conn, } gh = msgb_l3(msg); - pdisc = gh->proto_discr & 0x0f; + pdisc = gsm48_hdr_pdisc(gh); + msg_type = gsm48_hdr_msg_type(gh); /* the idea is to handle all RR messages here, and only hand * MM/CC/SMS-CP/LCS up to the MSC. Some messages like PAGING @@ -581,7 +583,7 @@ static void dispatch_dtap(struct gsm_subscriber_connection *conn, * will call api->compl_l3() for it */ switch (pdisc) { case GSM48_PDISC_RR: - switch (gh->msg_type) { + switch (msg_type) { case GSM48_MT_RR_GPRS_SUSP_REQ: DEBUGP(DRR, "GRPS SUSPEND REQUEST\n"); break; @@ -640,7 +642,7 @@ static void dispatch_dtap(struct gsm_subscriber_connection *conn, * messages, but we'd rather forward what we * don't know than drop it... */ LOGP(DRR, LOGL_NOTICE, "BSC: Passing unknown 04.08 " - "RR message type 0x%02x to MSC\n", gh->msg_type); + "RR message type 0x%02x to MSC\n", msg_type); if (api->dtap) api->dtap(conn, link_id, msg); } diff --git a/openbsc/src/libfilter/bsc_msg_filter.c b/openbsc/src/libfilter/bsc_msg_filter.c index eafeff4db..115d376cb 100644 --- a/openbsc/src/libfilter/bsc_msg_filter.c +++ b/openbsc/src/libfilter/bsc_msg_filter.c @@ -339,15 +339,15 @@ int bsc_msg_filter_initial(struct gsm48_hdr *hdr48, size_t hdr48_len, cause->lu_reject_cause = GSM48_REJECT_PLMN_NOT_ALLOWED; *imsi = NULL; - proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = gsm48_hdr_pdisc(hdr48); + msg_type = gsm48_hdr_msg_type(hdr48); if (proto == GSM48_PDISC_MM && msg_type == GSM48_MT_MM_LOC_UPD_REQUEST) { *con_type = FLT_CON_TYPE_LU; ret = _cr_check_loc_upd(req->ctx, &hdr48->data[0], hdr48_len - sizeof(*hdr48), imsi); } else if (proto == GSM48_PDISC_MM && - msg_type == GSM48_MT_MM_CM_SERV_REQ) { + msg_type == GSM48_MT_MM_CM_SERV_REQ) { *con_type = FLT_CON_TYPE_CM_SERV_REQ; ret = _cr_check_cm_serv_req(req->ctx, &hdr48->data[0], hdr48_len - sizeof(*hdr48), @@ -388,8 +388,8 @@ int bsc_msg_filter_data(struct gsm48_hdr *hdr48, size_t len, if (state->imsi_checked) return 0; - proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = gsm48_hdr_pdisc(hdr48); + msg_type = gsm48_hdr_msg_type(hdr48); if (proto != GSM48_PDISC_MM || msg_type != GSM48_MT_MM_ID_RESP) return 0; diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index d9d739032..9e70ba922 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -124,13 +124,12 @@ static int gsm48_conn_sendmsg(struct msgb *msg, struct gsm_subscriber_connection msg->lchan = trans->conn->lchan; } - if (msg->lchan) { struct e1inp_sign_link *sign_link = msg->lchan->ts->trx->rsl_link; msg->dst = sign_link; - if ((gh->proto_discr & GSM48_PDISC_MASK) == GSM48_PDISC_CC) + if (gsm48_hdr_pdisc(gh) == GSM48_PDISC_CC) DEBUGP(DCC, "(bts %d trx %d ts %d ti %02x) " "Sending '%s' to MS.\n", sign_link->trx->bts->nr, @@ -1131,7 +1130,7 @@ static int gsm0408_rcv_mm(struct gsm_subscriber_connection *conn, struct msgb *m struct gsm48_hdr *gh = msgb_l3(msg); int rc = 0; - switch (gh->msg_type & 0xbf) { + switch (gsm48_hdr_msg_type(gh)) { case GSM48_MT_MM_LOC_UPD_REQUEST: DEBUGP(DMM, "LOCATION UPDATING REQUEST: "); rc = mm_rx_loc_upd_req(conn, msg); @@ -1860,7 +1859,7 @@ static void gsm48_start_cc_timer(struct gsm_trans *trans, int current, static int gsm48_cc_rx_setup(struct gsm_trans *trans, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t msg_type = gh->msg_type & 0xbf; + uint8_t msg_type = gsm48_hdr_msg_type(gh); unsigned int payload_len = msgb_l3len(msg) - sizeof(*gh); struct tlv_parsed tp; struct gsm_mncc setup; @@ -3487,7 +3486,7 @@ static struct datastate { static int gsm0408_rcv_cc(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t msg_type = gh->msg_type & 0xbf; + uint8_t msg_type = gsm48_hdr_msg_type(gh); uint8_t transaction_id = ((gh->proto_discr & 0xf0) ^ 0x80) >> 4; /* flip */ struct gsm_trans *trans = NULL; int i, rc = 0; @@ -3578,7 +3577,7 @@ int gsm0408_new_conn(struct gsm_subscriber_connection *conn) int gsm0408_dispatch(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; + uint8_t pdisc = gsm48_hdr_pdisc(gh); int rc = 0; LOGP(DRLL, LOGL_DEBUG, "Dispatching 04.08 message, pdisc=%d\n", pdisc); diff --git a/openbsc/src/libmsc/silent_call.c b/openbsc/src/libmsc/silent_call.c index e9ece1835..131a1786b 100644 --- a/openbsc/src/libmsc/silent_call.c +++ b/openbsc/src/libmsc/silent_call.c @@ -95,7 +95,8 @@ static const struct msg_match silent_call_accept[] = { int silent_call_reroute(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; + uint8_t pdisc = gsm48_hdr_pdisc(gh); + uint8_t msg_type = gsm48_hdr_msg_type(gh); int i; /* if we're not part of a silent call, never reroute */ @@ -105,7 +106,7 @@ int silent_call_reroute(struct gsm_subscriber_connection *conn, struct msgb *msg /* check if we are a special message that is handled in openbsc */ for (i = 0; i < ARRAY_SIZE(silent_call_accept); i++) { if (silent_call_accept[i].pdisc == pdisc && - silent_call_accept[i].msg_type == gh->msg_type) + silent_call_accept[i].msg_type == msg_type) return 0; } diff --git a/openbsc/src/osmo-bsc/osmo_bsc_api.c b/openbsc/src/osmo-bsc/osmo_bsc_api.c index fbeed77b7..d31e6c152 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_api.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_api.c @@ -180,8 +180,8 @@ static void bsc_send_ussd_no_srv(struct gsm_subscriber_connection *conn, return; gh = msgb_l3(msg); - pdisc = gh->proto_discr & 0x0f; - mtype = gh->msg_type & 0xbf; + pdisc = gsm48_hdr_pdisc(gh); + mtype = gsm48_hdr_msg_type(gh); /* Is CM service request? */ if (pdisc == GSM48_PDISC_MM && mtype == GSM48_MT_MM_CM_SERV_REQ) { @@ -341,8 +341,8 @@ static int handle_cc_setup(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; - uint8_t mtype = gh->msg_type & 0xbf; + uint8_t pdisc = gsm48_hdr_pdisc(gh); + uint8_t mtype = gsm48_hdr_msg_type(gh); struct osmo_msc_data *msc; struct gsm_mncc_number called; diff --git a/openbsc/src/osmo-bsc/osmo_bsc_filter.c b/openbsc/src/osmo-bsc/osmo_bsc_filter.c index 389a124fd..a71871f77 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_filter.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_filter.c @@ -141,8 +141,8 @@ struct osmo_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn, } gh = msgb_l3(msg); - pdisc = gh->proto_discr & 0x0f; - mtype = gh->msg_type & 0xbf; + pdisc = gsm48_hdr_pdisc(gh); + mtype = gsm48_hdr_msg_type(gh); /* * We are asked to select a MSC here but they are not equal. We @@ -212,8 +212,8 @@ paging: int bsc_scan_bts_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) { struct gsm48_hdr *gh = msgb_l3(msg); - uint8_t pdisc = gh->proto_discr & 0x0f; - uint8_t mtype = gh->msg_type & 0xbf; + uint8_t pdisc = gsm48_hdr_pdisc(gh); + uint8_t mtype = gsm48_hdr_msg_type(gh); if (pdisc == GSM48_PDISC_MM) { if (mtype == GSM48_MT_MM_LOC_UPD_REQUEST) @@ -347,7 +347,7 @@ int bsc_scan_msc_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) gh = (struct gsm48_hdr *) msgb_l3(msg); length -= (const char *)&gh->data[0] - (const char *)gh; - mtype = gh->msg_type & 0xbf; + mtype = gsm48_hdr_msg_type(gh); net = conn->bts->network; msc = conn->sccp_con->msc; diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c index cdab4064d..cacb9199d 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c @@ -670,8 +670,8 @@ static void update_con_authorize(struct nat_sccp_connection *con, if (!hdr48) return; - proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = gsm48_hdr_pdisc(hdr48); + msg_type = gsm48_hdr_msg_type(hdr48); if (proto == GSM48_PDISC_MM && msg_type == GSM48_MT_MM_CM_SERV_ACC) con->authorized = 1; diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c b/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c index ca5670ca4..58667fe8c 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat_rewrite.c @@ -594,8 +594,8 @@ struct msgb *bsc_nat_rewrite_msg(struct bsc_nat *nat, struct msgb *msg, struct b return msg; link_id = msg->l3h[1]; - proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = gsm48_hdr_pdisc(hdr48); + msg_type = gsm48_hdr_msg_type(hdr48); if (proto == GSM48_PDISC_CC && msg_type == GSM48_MT_CC_SETUP) new_msg = rewrite_setup(nat, msg, parsed, imsi, hdr48, len); diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c index 108241421..2905c85b0 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c +++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c @@ -407,8 +407,8 @@ int bsc_ussd_check(struct nat_sccp_connection *con, struct bsc_nat_parsed *parse if (!hdr48) return 0; - proto = hdr48->proto_discr & 0x0f; - msg_type = hdr48->msg_type & 0xbf; + proto = gsm48_hdr_pdisc(hdr48); + msg_type = gsm48_hdr_msg_type(hdr48); ti = (hdr48->proto_discr & 0x70) >> 4; if (proto != GSM48_PDISC_NC_SS) return 0; -- cgit v1.2.3 From 961bd0b121d604612ea27bd2c83edd73290b2cb6 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 14 Mar 2016 16:13:25 +0100 Subject: 04.08: apply new transaction id inline functions libosmocore recently added inline functions to relieve callers from applying bitmasks and bit shifts to access the transaction id of a GSM 04.08 header. Apply these functions. --- openbsc/src/gprs/gprs_gmm.c | 8 ++++---- openbsc/src/libmsc/gsm_04_08.c | 2 +- openbsc/src/libmsc/gsm_04_11.c | 2 +- openbsc/src/osmo-bsc_nat/bsc_ussd.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c index 9d94c24fe..5f0a5fda1 100644 --- a/openbsc/src/gprs/gprs_gmm.c +++ b/openbsc/src/gprs/gprs_gmm.c @@ -1753,7 +1753,7 @@ static int do_act_pdp_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg) uint8_t req_qos_len, req_pdpa_len; uint8_t *req_qos, *req_pdpa; struct tlv_parsed tp; - uint8_t transaction_id = (gh->proto_discr >> 4); + uint8_t transaction_id = gsm48_hdr_trans_id(gh); struct sgsn_ggsn_ctx *ggsn; struct sgsn_pdp_ctx *pdp; enum gsm48_gsm_cause gsm_cause; @@ -1923,7 +1923,7 @@ static int gsm48_rx_gsm_act_pdp_req(struct sgsn_mm_ctx *mmctx, msg = gprs_msgb_copy(_msg, __func__); if (!msg) { struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(_msg); - uint8_t transaction_id = (gh->proto_discr >> 4); + uint8_t transaction_id = gsm48_hdr_trans_id(gh); LOGMMCTXP(LOGL_ERROR, mmctx, "-> ACTIVATE PDP CONTEXT REQ failed copy.\n"); /* Send reject with GSM_CAUSE_INV_MAND_INFO */ @@ -1941,7 +1941,7 @@ static int gsm48_rx_gsm_act_pdp_req(struct sgsn_mm_ctx *mmctx, static int gsm48_rx_gsm_deact_pdp_req(struct sgsn_mm_ctx *mm, struct msgb *msg) { struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg); - uint8_t transaction_id = (gh->proto_discr >> 4); + uint8_t transaction_id = gsm48_hdr_trans_id(gh); struct sgsn_pdp_ctx *pdp; LOGMMCTXP(LOGL_INFO, mm, "-> DEACTIVATE PDP CONTEXT REQ (cause: %s)\n", @@ -1962,7 +1962,7 @@ static int gsm48_rx_gsm_deact_pdp_req(struct sgsn_mm_ctx *mm, struct msgb *msg) static int gsm48_rx_gsm_deact_pdp_ack(struct sgsn_mm_ctx *mm, struct msgb *msg) { struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg); - uint8_t transaction_id = (gh->proto_discr >> 4); + uint8_t transaction_id = gsm48_hdr_trans_id(gh); struct sgsn_pdp_ctx *pdp; LOGMMCTXP(LOGL_INFO, mm, "-> DEACTIVATE PDP CONTEXT ACK\n"); diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 9e70ba922..1a1d6f2d4 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -3487,7 +3487,7 @@ static int gsm0408_rcv_cc(struct gsm_subscriber_connection *conn, struct msgb *m { struct gsm48_hdr *gh = msgb_l3(msg); uint8_t msg_type = gsm48_hdr_msg_type(gh); - uint8_t transaction_id = ((gh->proto_discr & 0xf0) ^ 0x80) >> 4; /* flip */ + uint8_t transaction_id = gsm48_hdr_trans_id_flip_ti(gh); struct gsm_trans *trans = NULL; int i, rc = 0; diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index bf256c32a..20d18a959 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -780,7 +780,7 @@ int gsm0411_rcv_sms(struct gsm_subscriber_connection *conn, { struct gsm48_hdr *gh = msgb_l3(msg); uint8_t msg_type = gh->msg_type; - uint8_t transaction_id = ((gh->proto_discr >> 4) ^ 0x8); /* flip */ + uint8_t transaction_id = gsm48_hdr_trans_id_flip_ti(gh); struct gsm_trans *trans; int new_trans = 0; int rc = 0; diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c index 2905c85b0..e0809059a 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c +++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c @@ -409,7 +409,7 @@ int bsc_ussd_check(struct nat_sccp_connection *con, struct bsc_nat_parsed *parse proto = gsm48_hdr_pdisc(hdr48); msg_type = gsm48_hdr_msg_type(hdr48); - ti = (hdr48->proto_discr & 0x70) >> 4; + ti = gsm48_hdr_trans_id_no_ti(hdr48); if (proto != GSM48_PDISC_NC_SS) return 0; -- cgit v1.2.3 From 8c515272c3e82c2400b15b5bfefa9dd883b86b96 Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Tue, 15 Mar 2016 14:21:49 +0100 Subject: meas: Do not retry to close the database There is no concurrency involved and if it failed the first time, it will fail the second, third, ... time as well. Simply print that we will leak the database instance. --- openbsc/src/utils/meas_db.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/openbsc/src/utils/meas_db.c b/openbsc/src/utils/meas_db.c index 6c7e7ae6e..a3b694e6b 100644 --- a/openbsc/src/utils/meas_db.c +++ b/openbsc/src/utils/meas_db.c @@ -314,7 +314,6 @@ err_io: void meas_db_close(struct meas_db_state *st) { - int retries; if (sqlite3_finalize(st->stmt_ins_mr) != SQLITE_OK) fprintf(stderr, "DB insert measurement report finalize error: %s\n", sqlite3_errmsg(st->db)); @@ -324,16 +323,8 @@ void meas_db_close(struct meas_db_state *st) if (sqlite3_finalize(st->stmt_upd_mr) != SQLITE_OK) fprintf(stderr, "DB update measurement report finalize error: %s\n", sqlite3_errmsg(st->db)); - retries = 0; - while (1) { - if (sqlite3_close(st->db) == SQLITE_OK) - break; - if ((++retries) >= 3) { - fprintf(stderr, "Unable to close DB, abandoning.\n"); - break; - } - sleep(1); - } + if (sqlite3_close(st->db) != SQLITE_OK) + fprintf(stderr, "Unable to close DB, abandoning.\n"); talloc_free(st); -- cgit v1.2.3 From 10cd11345c2dd3f38793e7dd7456e7882ab95dd9 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 14 Mar 2016 16:15:02 +0100 Subject: bsc_scan_msc_msg: check protocol discriminator The function assumed an MM protocol discriminator without verifying it. --- openbsc/src/osmo-bsc/osmo_bsc_filter.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openbsc/src/osmo-bsc/osmo_bsc_filter.c b/openbsc/src/osmo-bsc/osmo_bsc_filter.c index a71871f77..14e0b7144 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_filter.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_filter.c @@ -336,6 +336,7 @@ int bsc_scan_msc_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) struct gsm_network *net; struct gsm48_loc_area_id *lai; struct gsm48_hdr *gh; + uint8_t pdisc; uint8_t mtype; int length = msgb_l3len(msg); @@ -347,6 +348,10 @@ int bsc_scan_msc_msg(struct gsm_subscriber_connection *conn, struct msgb *msg) gh = (struct gsm48_hdr *) msgb_l3(msg); length -= (const char *)&gh->data[0] - (const char *)gh; + pdisc = gsm48_hdr_pdisc(gh); + if (pdisc != GSM48_PDISC_MM) + return 0; + mtype = gsm48_hdr_msg_type(gh); net = conn->bts->network; msc = conn->sccp_con->msc; -- cgit v1.2.3 From f4afcf0b2345a51d3e69c4dbb61279af233388cf Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Wed, 16 Mar 2016 11:15:19 +0100 Subject: mgcp: Fix compiler warnings on size_t on AMD64 mgcp_transcode.c: In function 'decode_audio': mgcp_transcode.c:332:4: warning: format '%d' expects argument of type 'int', but argument 7 has type 'size_t' [-Wformat=] LOGP(DMGCP, LOGL_ERROR, ^ mgcp_transcode.c:332:4: warning: format '%d' expects argument of type 'int', but argument 8 has type 'long unsigned int' [-Wformat=] mgcp_transcode.c: In function 'encode_audio': mgcp_transcode.c:390:4: warning: format '%d' expects argument of type 'int', but argument 7 has type 'size_t' [-Wformat=] LOGP(DMGCP, LOGL_INFO, ^ mgcp_transcode.c:390:4: warning: format '%d' expects argument of type 'int', but argument 8 has type 'size_t' [-Wformat=] mgcp_transcode.c: In function 'mgcp_transcoding_process_rtp': mgcp_transcode.c:542:5: warning: format '%d' expects argument of type 'int', but argument 9 has type 'size_t' [-Wformat=] LOGP(DMGCP, LOGL_NOTICE, ^ mgcp_transcode.c:571:4: warning: format '%d' expects argument of type 'int', but argument 7 has type 'size_t' [-Wformat=] LOGP(DMGCP, LOGL_NOTICE, ^ --- openbsc/src/libmgcp/mgcp_transcode.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openbsc/src/libmgcp/mgcp_transcode.c b/openbsc/src/libmgcp/mgcp_transcode.c index c994d3291..f31e7aefb 100644 --- a/openbsc/src/libmgcp/mgcp_transcode.c +++ b/openbsc/src/libmgcp/mgcp_transcode.c @@ -330,7 +330,7 @@ static int decode_audio(struct mgcp_process_rtp_state *state, while (*nbytes >= state->src_frame_size) { if (state->sample_cnt + state->src_samples_per_frame > ARRAY_SIZE(state->samples)) { LOGP(DMGCP, LOGL_ERROR, - "Sample buffer too small: %d > %d.\n", + "Sample buffer too small: %zu > %zu.\n", state->sample_cnt + state->src_samples_per_frame, ARRAY_SIZE(state->samples)); return -ENOSPC; @@ -388,7 +388,7 @@ static int encode_audio(struct mgcp_process_rtp_state *state, /* Not even one frame fits into the buffer */ LOGP(DMGCP, LOGL_INFO, - "Encoding (RTP) buffer too small: %d > %d.\n", + "Encoding (RTP) buffer too small: %zu > %zu.\n", nbytes + state->dst_frame_size, buf_size); return -ENOSPC; } @@ -540,7 +540,7 @@ int mgcp_transcoding_process_rtp(struct mgcp_endpoint *endp, * instead if the delta is small enough. */ LOGP(DMGCP, LOGL_NOTICE, - "0x%x dropping sample buffer due delta=%d sample_cnt=%d\n", + "0x%x dropping sample buffer due delta=%d sample_cnt=%zu\n", ENDPOINT_NUMBER(endp), delta, state->sample_cnt); state->sample_cnt = 0; state->next_time = ts_no; @@ -569,7 +569,7 @@ int mgcp_transcoding_process_rtp(struct mgcp_endpoint *endp, if (nbytes > 0) LOGP(DMGCP, LOGL_NOTICE, - "Skipped audio frame in RTP packet: %d octets\n", + "Skipped audio frame in RTP packet: %zu octets\n", nbytes); } else ts_no = state->next_time; -- cgit v1.2.3 From d2fa7a509a989bb775b56932502d128ec974330d Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Thu, 10 Mar 2016 23:30:37 +0100 Subject: fix confusing typo in constant (THAN -> THEN) --- openbsc/include/openbsc/auth.h | 2 +- openbsc/src/libmsc/auth.c | 2 +- openbsc/src/libmsc/gsm_04_08.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openbsc/include/openbsc/auth.h b/openbsc/include/openbsc/auth.h index 2364fb3d2..d41d1419b 100644 --- a/openbsc/include/openbsc/auth.h +++ b/openbsc/include/openbsc/auth.h @@ -6,7 +6,7 @@ struct gsm_subscriber; enum auth_action { AUTH_NOT_AVAIL = 0, /* No auth tuple available */ - AUTH_DO_AUTH_THAN_CIPH = 1, /* Firsth authenticate, then cipher */ + AUTH_DO_AUTH_THEN_CIPH = 1, /* Firsth authenticate, then cipher */ AUTH_DO_CIPH = 2, /* Only ciphering */ AUTH_DO_AUTH = 3, /* Only authentication, no ciphering */ }; diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index 0a2f59e36..65a9b03c4 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -132,6 +132,6 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, db_sync_lastauthtuple_for_subscr(atuple, subscr); DEBUGP(DMM, "Need to do authentication and ciphering\n"); - return AUTH_DO_AUTH_THAN_CIPH; + return AUTH_DO_AUTH_THEN_CIPH; } diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 1a1d6f2d4..1524ec44f 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -231,7 +231,7 @@ int gsm48_secure_channel(struct gsm_subscriber_connection *conn, int key_seq, /* FIXME: Should start a timer for completion ... */ /* Then do whatever is needed ... */ - if (rc == AUTH_DO_AUTH_THAN_CIPH) { + if (rc == AUTH_DO_AUTH_THEN_CIPH) { /* Start authentication */ return gsm48_tx_mm_auth_req(conn, op->atuple.rand, op->atuple.key_seq); } else if (rc == AUTH_DO_CIPH) { -- cgit v1.2.3 From 044fbe6568f82a12bf4e3addc7e3d6db529b6548 Mon Sep 17 00:00:00 2001 From: Vadim Yanitskiy Date: Wed, 16 Mar 2016 21:14:03 +0600 Subject: move to hex TMSI representation In OpenBSC, we traditionally displayed a TMSI in its integer representation, which is quite unusual in the telecom world. A TMSI is normally printed as a series of 8 hex digits. This patch aligns OpenBSC with the telecom industry standard. Signed-off-by: Vadim Yanitskiy --- openbsc/include/openbsc/gsm_subscriber.h | 2 +- openbsc/src/libmsc/db.c | 15 ++++++++------- openbsc/tests/db/db_test.c | 4 ++-- openbsc/tests/gsm0408/gsm0408_test.c | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h index 7d6c776bc..785dc3617 100644 --- a/openbsc/include/openbsc/gsm_subscriber.h +++ b/openbsc/include/openbsc/gsm_subscriber.h @@ -14,7 +14,7 @@ #define GSM_SUBSCRIBER_FIRST_CONTACT 0x00000001 /* gprs_sgsn.h defines additional flags including and above bit 16 (0x10000) */ -#define tmsi_from_string(str) strtoul(str, NULL, 10) +#define tmsi_from_string(str) strtoul(str + 2, NULL, 16) #define GSM_SUBSCRIBER_NO_EXPIRATION 0x0 diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 0935fc54d..952151e90 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -893,9 +893,10 @@ struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field field, subscr->id = dbi_result_get_ulonglong(result, "id"); db_set_from_query(subscr, result); - DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', TMSI %u, EXTEN '%s', LAC %hu, AUTH %u\n", - subscr->id, subscr->imsi, subscr->name, subscr->tmsi, subscr->extension, - subscr->lac, subscr->authorized); + DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', " + "TMSI 0x%08x, EXTEN '%s', LAC %hu, AUTH %u\n", + subscr->id, subscr->imsi, subscr->name, subscr->tmsi, + subscr->extension, subscr->lac, subscr->authorized); dbi_result_free(result); get_equipment_by_subscr(subscr); @@ -935,7 +936,7 @@ int db_subscriber_update(struct gsm_subscriber *subscr) int db_sync_subscriber(struct gsm_subscriber *subscriber) { dbi_result result; - char tmsi[14]; + char tmsi[11]; char *q_tmsi, *q_name, *q_extension; dbi_conn_quote_string_copy(conn, @@ -944,7 +945,7 @@ int db_sync_subscriber(struct gsm_subscriber *subscriber) subscriber->extension, &q_extension); if (subscriber->tmsi != GSM_RESERVED_TMSI) { - sprintf(tmsi, "%u", subscriber->tmsi); + sprintf(tmsi, "0x%08x", subscriber->tmsi); dbi_conn_quote_string_copy(conn, tmsi, &q_tmsi); @@ -1194,7 +1195,7 @@ int db_subscriber_expire(void *priv, void (*callback)(void *priv, long long unsi int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber) { dbi_result result = NULL; - char tmsi[14]; + char tmsi[11]; char *tmsi_quoted; for (;;) { @@ -1205,7 +1206,7 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber) if (subscriber->tmsi == GSM_RESERVED_TMSI) continue; - sprintf(tmsi, "%u", subscriber->tmsi); + sprintf(tmsi, "0x%08x", subscriber->tmsi); dbi_conn_quote_string_copy(conn, tmsi, &tmsi_quoted); result = dbi_conn_queryf(conn, "SELECT * FROM Subscriber " diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c index a02d1f801..faea82055 100644 --- a/openbsc/tests/db/db_test.c +++ b/openbsc/tests/db/db_test.c @@ -200,7 +200,7 @@ int main() alice->lac=42; db_sync_subscriber(alice); /* Get by TMSI */ - snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi); + snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi); alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str); COMPARE(alice, alice_db); SUBSCR_PUT(alice_db); @@ -227,7 +227,7 @@ int main() db_subscriber_assoc_imei(alice, "1234567890"); db_subscriber_assoc_imei(alice, "6543560920"); /* Get by TMSI */ - snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi); + snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi); alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str); COMPARE(alice, alice_db); SUBSCR_PUT(alice_db); diff --git a/openbsc/tests/gsm0408/gsm0408_test.c b/openbsc/tests/gsm0408/gsm0408_test.c index 781ef6147..8ed57ca17 100644 --- a/openbsc/tests/gsm0408/gsm0408_test.c +++ b/openbsc/tests/gsm0408/gsm0408_test.c @@ -93,7 +93,7 @@ static void test_mi_functionality(void) /* tmsi code */ mi_len = gsm48_generate_mid_from_tmsi(mi, tmsi); gsm48_mi_to_string(mi_parsed, sizeof(mi_parsed), mi + 2, mi_len - 2); - COMPARE((uint32_t)strtoul(mi_parsed, NULL, 10), ==, tmsi); + COMPARE((uint32_t)tmsi_from_string(mi_parsed), ==, tmsi); /* imsi code */ mi_len = gsm48_generate_mid_from_imsi(mi, imsi_odd); -- cgit v1.2.3 From 3ad0346f00c4d135f7d4d15017ce2941e13e2c54 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Thu, 17 Mar 2016 14:41:26 +0100 Subject: Revert "move to hex TMSI representation" This reverts commit 044fbe6568f82a12bf4e3addc7e3d6db529b6548. --- openbsc/include/openbsc/gsm_subscriber.h | 2 +- openbsc/src/libmsc/db.c | 15 +++++++-------- openbsc/tests/db/db_test.c | 4 ++-- openbsc/tests/gsm0408/gsm0408_test.c | 2 +- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h index 785dc3617..7d6c776bc 100644 --- a/openbsc/include/openbsc/gsm_subscriber.h +++ b/openbsc/include/openbsc/gsm_subscriber.h @@ -14,7 +14,7 @@ #define GSM_SUBSCRIBER_FIRST_CONTACT 0x00000001 /* gprs_sgsn.h defines additional flags including and above bit 16 (0x10000) */ -#define tmsi_from_string(str) strtoul(str + 2, NULL, 16) +#define tmsi_from_string(str) strtoul(str, NULL, 10) #define GSM_SUBSCRIBER_NO_EXPIRATION 0x0 diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 952151e90..0935fc54d 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -893,10 +893,9 @@ struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field field, subscr->id = dbi_result_get_ulonglong(result, "id"); db_set_from_query(subscr, result); - DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', " - "TMSI 0x%08x, EXTEN '%s', LAC %hu, AUTH %u\n", - subscr->id, subscr->imsi, subscr->name, subscr->tmsi, - subscr->extension, subscr->lac, subscr->authorized); + DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', TMSI %u, EXTEN '%s', LAC %hu, AUTH %u\n", + subscr->id, subscr->imsi, subscr->name, subscr->tmsi, subscr->extension, + subscr->lac, subscr->authorized); dbi_result_free(result); get_equipment_by_subscr(subscr); @@ -936,7 +935,7 @@ int db_subscriber_update(struct gsm_subscriber *subscr) int db_sync_subscriber(struct gsm_subscriber *subscriber) { dbi_result result; - char tmsi[11]; + char tmsi[14]; char *q_tmsi, *q_name, *q_extension; dbi_conn_quote_string_copy(conn, @@ -945,7 +944,7 @@ int db_sync_subscriber(struct gsm_subscriber *subscriber) subscriber->extension, &q_extension); if (subscriber->tmsi != GSM_RESERVED_TMSI) { - sprintf(tmsi, "0x%08x", subscriber->tmsi); + sprintf(tmsi, "%u", subscriber->tmsi); dbi_conn_quote_string_copy(conn, tmsi, &q_tmsi); @@ -1195,7 +1194,7 @@ int db_subscriber_expire(void *priv, void (*callback)(void *priv, long long unsi int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber) { dbi_result result = NULL; - char tmsi[11]; + char tmsi[14]; char *tmsi_quoted; for (;;) { @@ -1206,7 +1205,7 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber) if (subscriber->tmsi == GSM_RESERVED_TMSI) continue; - sprintf(tmsi, "0x%08x", subscriber->tmsi); + sprintf(tmsi, "%u", subscriber->tmsi); dbi_conn_quote_string_copy(conn, tmsi, &tmsi_quoted); result = dbi_conn_queryf(conn, "SELECT * FROM Subscriber " diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c index faea82055..a02d1f801 100644 --- a/openbsc/tests/db/db_test.c +++ b/openbsc/tests/db/db_test.c @@ -200,7 +200,7 @@ int main() alice->lac=42; db_sync_subscriber(alice); /* Get by TMSI */ - snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi); + snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi); alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str); COMPARE(alice, alice_db); SUBSCR_PUT(alice_db); @@ -227,7 +227,7 @@ int main() db_subscriber_assoc_imei(alice, "1234567890"); db_subscriber_assoc_imei(alice, "6543560920"); /* Get by TMSI */ - snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi); + snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi); alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str); COMPARE(alice, alice_db); SUBSCR_PUT(alice_db); diff --git a/openbsc/tests/gsm0408/gsm0408_test.c b/openbsc/tests/gsm0408/gsm0408_test.c index 8ed57ca17..781ef6147 100644 --- a/openbsc/tests/gsm0408/gsm0408_test.c +++ b/openbsc/tests/gsm0408/gsm0408_test.c @@ -93,7 +93,7 @@ static void test_mi_functionality(void) /* tmsi code */ mi_len = gsm48_generate_mid_from_tmsi(mi, tmsi); gsm48_mi_to_string(mi_parsed, sizeof(mi_parsed), mi + 2, mi_len - 2); - COMPARE((uint32_t)tmsi_from_string(mi_parsed), ==, tmsi); + COMPARE((uint32_t)strtoul(mi_parsed, NULL, 10), ==, tmsi); /* imsi code */ mi_len = gsm48_generate_mid_from_imsi(mi, imsi_odd); -- cgit v1.2.3 From 56ea30ff3f9f98a4903d060196b185ce124d7eaa Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Sat, 26 Mar 2016 21:35:11 +0100 Subject: osmo-bsc: fix compiler warning: store struct in vty->index Don't store an MSC index number in the vty->index void* value. Instead, store the osmo_msc_data struct directly. Thus avoid warnings about differences in int vs void* sizes, and save some index lookups. --- openbsc/src/osmo-bsc/osmo_bsc_vty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openbsc/src/osmo-bsc/osmo_bsc_vty.c b/openbsc/src/osmo-bsc/osmo_bsc_vty.c index d871f015a..e623c9c10 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_vty.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_vty.c @@ -43,7 +43,7 @@ static struct osmo_bsc_data *osmo_bsc_data(struct vty *vty) static struct osmo_msc_data *osmo_msc_data(struct vty *vty) { - return osmo_msc_data_find(bsc_gsmnet, (int) vty->index); + return vty->index; } static struct cmd_node bsc_node = { @@ -70,7 +70,7 @@ DEFUN(cfg_net_msc, cfg_net_msc_cmd, return CMD_WARNING; } - vty->index = (void *) index; + vty->index = msc; vty->node = MSC_NODE; return CMD_SUCCESS; } -- cgit v1.2.3 From 37984bdb1b507446421e5aa6131ccdf117dd269f Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:24 +0200 Subject: Add MM Auth test; add auth_action_str() function Add basic MM Authentication test setup, with fake DB access and RAND_bytes(). So far implement simple tests for IO error during DB access and missing auth entry. To print the auth action during tests, add struct auth_action_names and auth_action_str() inline function in auth.[hc]. --- openbsc/.gitignore | 1 + openbsc/configure.ac | 1 + openbsc/include/openbsc/auth.h | 8 +++ openbsc/src/libmsc/auth.c | 9 +++ openbsc/tests/Makefile.am | 2 +- openbsc/tests/mm_auth/Makefile.am | 21 ++++++ openbsc/tests/mm_auth/mm_auth_test.c | 119 ++++++++++++++++++++++++++++++++++ openbsc/tests/mm_auth/mm_auth_test.ok | 8 +++ openbsc/tests/testsuite.at | 7 ++ 9 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 openbsc/tests/mm_auth/Makefile.am create mode 100644 openbsc/tests/mm_auth/mm_auth_test.c create mode 100644 openbsc/tests/mm_auth/mm_auth_test.ok diff --git a/openbsc/.gitignore b/openbsc/.gitignore index 55f4a3189..28fdcc8c4 100644 --- a/openbsc/.gitignore +++ b/openbsc/.gitignore @@ -80,6 +80,7 @@ tests/sgsn/sgsn_test tests/subscr/subscr_test tests/oap/oap_test tests/gtphub/gtphub_test +tests/mm_auth/mm_auth_test tests/atconfig tests/atlocal diff --git a/openbsc/configure.ac b/openbsc/configure.ac index 24dbc30f7..60601fe7a 100644 --- a/openbsc/configure.ac +++ b/openbsc/configure.ac @@ -216,6 +216,7 @@ AC_OUTPUT( tests/subscr/Makefile tests/oap/Makefile tests/gtphub/Makefile + tests/mm_auth/Makefile doc/Makefile doc/examples/Makefile Makefile) diff --git a/openbsc/include/openbsc/auth.h b/openbsc/include/openbsc/auth.h index d41d1419b..90495bb58 100644 --- a/openbsc/include/openbsc/auth.h +++ b/openbsc/include/openbsc/auth.h @@ -1,6 +1,8 @@ #ifndef _AUTH_H #define _AUTH_H +#include + struct gsm_auth_tuple; struct gsm_subscriber; @@ -11,6 +13,12 @@ enum auth_action { AUTH_DO_AUTH = 3, /* Only authentication, no ciphering */ }; +extern const struct value_string auth_action_names[]; +static inline const char *auth_action_str(enum auth_action a) +{ + return get_value_string(auth_action_names, a); +} + int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, struct gsm_subscriber *subscr, int key_seq); diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index 65a9b03c4..85123167b 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -31,6 +31,15 @@ #include +const struct value_string auth_action_names[] = { +#define AUTH_ACTION_STR(X) { X, #X } + { -1, "(internal error)" }, /* soon to be fixed with an enum val */ + AUTH_ACTION_STR(AUTH_NOT_AVAIL), + AUTH_ACTION_STR(AUTH_DO_AUTH_THEN_CIPH), + AUTH_ACTION_STR(AUTH_DO_CIPH), + AUTH_ACTION_STR(AUTH_DO_AUTH), +#undef AUTH_ACTION_STR +}; static int _use_xor(struct gsm_auth_info *ainfo, struct gsm_auth_tuple *atuple) diff --git a/openbsc/tests/Makefile.am b/openbsc/tests/Makefile.am index 04b8e345f..09298a35c 100644 --- a/openbsc/tests/Makefile.am +++ b/openbsc/tests/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = gsm0408 db channel mgcp gprs abis gbproxy trau subscr +SUBDIRS = gsm0408 db channel mgcp gprs abis gbproxy trau subscr mm_auth if BUILD_NAT SUBDIRS += bsc-nat bsc-nat-trie diff --git a/openbsc/tests/mm_auth/Makefile.am b/openbsc/tests/mm_auth/Makefile.am new file mode 100644 index 000000000..516df0007 --- /dev/null +++ b/openbsc/tests/mm_auth/Makefile.am @@ -0,0 +1,21 @@ +AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include +AM_CFLAGS=-Wall \ + $(LIBOSMOCORE_CFLAGS) \ + $(LIBOSMOGSM_CFLAGS) \ + $(LIBCRYPTO_CFLAGS) + +noinst_PROGRAMS = mm_auth_test + +EXTRA_DIST = mm_auth_test.ok + +mm_auth_test_SOURCES = mm_auth_test.c + +mm_auth_test_LDFLAGS = \ + -Wl,--wrap=db_get_authinfo_for_subscr \ + -Wl,--wrap=db_get_lastauthtuple_for_subscr \ + -Wl,--wrap=db_sync_lastauthtuple_for_subscr + +mm_auth_test_LDADD = $(top_builddir)/src/libmsc/libmsc.a \ + $(top_builddir)/src/libcommon/libcommon.a \ + $(LIBOSMOCORE_LIBS) \ + $(LIBOSMOGSM_LIBS) diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c new file mode 100644 index 000000000..d8e44758c --- /dev/null +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -0,0 +1,119 @@ +#include + +#include +#include + +#include +#include +#include +#include + +/* override, requires '-Wl,--wrap=db_get_authinfo_for_subscr' */ +int __real_db_get_authinfo_for_subscr(struct gsm_auth_info *ainfo, + struct gsm_subscriber *subscr); + +int test_get_authinfo_rc = 0; +struct gsm_auth_info test_auth_info = {0}; +struct gsm_auth_info default_auth_info = { + .auth_algo = AUTH_ALGO_COMP128v1, + .a3a8_ki_len = 16, + .a3a8_ki = { 0 } +}; + +int __wrap_db_get_authinfo_for_subscr(struct gsm_auth_info *ainfo, + struct gsm_subscriber *subscr) +{ + *ainfo = test_auth_info; + printf("wrapped: db_get_authinfo_for_subscr(): rc = %d\n", test_get_authinfo_rc); + return test_get_authinfo_rc; +} + +/* override, requires '-Wl,--wrap=db_get_lastauthtuple_for_subscr' */ +int __real_db_get_lastauthtuple_for_subscr(struct gsm_auth_tuple *atuple, + struct gsm_subscriber *subscr); + +int test_get_lastauthtuple_rc = 0; +struct gsm_auth_tuple test_last_auth_tuple = { 0 }; +struct gsm_auth_tuple default_auth_tuple = { 0 }; + +int __wrap_db_get_lastauthtuple_for_subscr(struct gsm_auth_tuple *atuple, + struct gsm_subscriber *subscr) +{ + *atuple = test_last_auth_tuple; + printf("wrapped: db_get_lastauthtuple_for_subscr(): rc = %d\n", test_get_lastauthtuple_rc); + return test_get_lastauthtuple_rc; +} + +/* override, requires '-Wl,--wrap=db_sync_lastauthtuple_for_subscr' */ +int __real_db_sync_lastauthtuple_for_subscr(struct gsm_auth_tuple *atuple, + struct gsm_subscriber *subscr); +int test_sync_lastauthtuple_rc = 0; +int __wrap_db_sync_lastauthtuple_for_subscr(struct gsm_auth_tuple *atuple, + struct gsm_subscriber *subscr) +{ + test_last_auth_tuple = *atuple; + printf("wrapped: db_sync_lastauthtuple_for_subscr(): rc = %d\n", test_sync_lastauthtuple_rc); + return test_sync_lastauthtuple_rc; +} + +int auth_get_tuple_for_subscr_verbose(struct gsm_auth_tuple *atuple, + struct gsm_subscriber *subscr, + int key_seq) +{ + int auth_action; + auth_action = auth_get_tuple_for_subscr(atuple, subscr, key_seq); + printf("auth_get_tuple_for_subscr(key_seq=%d) --> auth_action == %s\n", + key_seq, auth_action_str(auth_action)); + return auth_action; +} + +/* override libssl RAND_bytes() to get testable crypto results */ +int RAND_bytes(uint8_t *rand, int len) +{ + memset(rand, 23, len); + return 1; +} + +static void test_error() +{ + int auth_action; + + struct gsm_auth_tuple atuple = {0}; + struct gsm_subscriber subscr = {0}; + int key_seq = 0; + + printf("\n* test_error()\n"); + + /* any error (except -ENOENT) */ + test_get_authinfo_rc = -EIO; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == -1); +} + +static void test_auth_not_avail() +{ + int auth_action; + + struct gsm_auth_tuple atuple = {0}; + struct gsm_subscriber subscr = {0}; + int key_seq = 0; + + printf("\n* test_auth_not_avail()\n"); + + /* no entry */ + test_get_authinfo_rc = -ENOENT; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_NOT_AVAIL); +} + +int main(void) +{ + osmo_init_logging(&log_info); + log_set_log_level(osmo_stderr_target, LOGL_INFO); + + test_error(); + test_auth_not_avail(); + return 0; +} diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok new file mode 100644 index 000000000..5efb3deca --- /dev/null +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -0,0 +1,8 @@ + +* test_error() +wrapped: db_get_authinfo_for_subscr(): rc = -5 +auth_get_tuple_for_subscr(key_seq=0) --> auth_action == (internal error) + +* test_auth_not_avail() +wrapped: db_get_authinfo_for_subscr(): rc = -2 +auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_NOT_AVAIL diff --git a/openbsc/tests/testsuite.at b/openbsc/tests/testsuite.at index 6a1c77f54..dab956888 100644 --- a/openbsc/tests/testsuite.at +++ b/openbsc/tests/testsuite.at @@ -117,3 +117,10 @@ AT_CHECK([test "$enable_gtphub_test" != no || exit 77]) cat $abs_srcdir/gtphub/gtphub_test.ok > expout AT_CHECK([$abs_top_builddir/tests/gtphub/gtphub_test], [], [expout], [ignore]) AT_CLEANUP + +AT_SETUP([mm_auth]) +AT_KEYWORDS([mm_auth]) +cat $abs_srcdir/mm_auth/mm_auth_test.ok > expout +AT_CHECK([$abs_top_builddir/tests/mm_auth/mm_auth_test], [], [expout], [ignore]) +AT_CLEANUP + -- cgit v1.2.3 From 4554a62c4d16bd95ae4fb38ede0fd9f64a4d1429 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:25 +0200 Subject: MM Auth test: add two tests for AUTH_THEN_CIPH Test two situations for AUTH_DO_AUTH_THEN_CIPH: - when no auth tuple is available - when the key sequence from LU is marked invalid Add convenience auth tuple comparison function using stringification. --- openbsc/tests/mm_auth/mm_auth_test.c | 136 ++++++++++++++++++++++++++++++++++ openbsc/tests/mm_auth/mm_auth_test.ok | 16 ++++ 2 files changed, 152 insertions(+) diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c index d8e44758c..c0b8da424 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.c +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -8,6 +8,59 @@ #include #include +#define min(A,B) ((A)>(B)? (B) : (A)) + +static char *auth_tuple_str(struct gsm_auth_tuple *atuple) +{ + static char buf[256]; + char *pos = buf; + int len = sizeof(buf); + int l; + +#define print2buf(FMT, args...) do {\ + l = snprintf(pos, len, FMT, ## args); \ + pos += l;\ + len -= l;\ + } while (0) + + print2buf("gsm_auth_tuple {\n"); + print2buf(" .use_count = %d\n", atuple->use_count); + print2buf(" .key_seq = %d\n", atuple->key_seq); + print2buf(" .rand = %s\n", osmo_hexdump(atuple->rand, sizeof(atuple->rand))); + print2buf(" .sres = %s\n", osmo_hexdump(atuple->sres, sizeof(atuple->sres))); + print2buf(" .kc = %s\n", osmo_hexdump(atuple->kc, sizeof(atuple->kc))); + print2buf("}\n"); +#undef print2buf + + return buf; +} + +static bool auth_tuple_is(struct gsm_auth_tuple *atuple, + const char *expect_str) +{ + int l, l1, l2; + int i; + char *tuple_str = auth_tuple_str(atuple); + bool same = (strcmp(expect_str, tuple_str) == 0); + if (!same) { + l1 = strlen(expect_str); + l2 = strlen(tuple_str); + printf("Expected %d:\n%s\nGot %d:\n%s\n", + l1, expect_str, l2, tuple_str); + l = min(l1, l2); + for (i = 0; i < l; i++) { + if (expect_str[i] != tuple_str[i]) { + printf("Difference at pos %d" + " (%c 0x%0x != %c 0x%0x)\n", + i, expect_str[i], expect_str[i], + tuple_str[i], tuple_str[i]); + break; + } + } + } + return same; +} + /* override, requires '-Wl,--wrap=db_get_authinfo_for_subscr' */ int __real_db_get_authinfo_for_subscr(struct gsm_auth_info *ainfo, struct gsm_subscriber *subscr); @@ -108,6 +161,87 @@ static void test_auth_not_avail() OSMO_ASSERT(auth_action == AUTH_NOT_AVAIL); } +static void test_auth_then_ciph1() +{ + int auth_action; + + struct gsm_auth_tuple atuple = {0}; + struct gsm_subscriber subscr = {0}; + int key_seq; + + printf("\n* test_auth_then_ciph1()\n"); + + /* Ki entry, but no auth tuple negotiated yet */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = -ENOENT; + key_seq = 0; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_AUTH_THEN_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 1\n" + " .key_seq = 1\n" + " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" + " .sres = a1 ab c6 90 \n" + " .kc = 0f 27 ed f3 ac 97 ac 00 \n" + "}\n" + )); +} + +static void test_auth_then_ciph2() +{ + int auth_action; + + struct gsm_auth_tuple atuple = {0}; + struct gsm_subscriber subscr = {0}; + int key_seq; + + printf("\n* test_auth_then_ciph2()\n"); + + /* Ki entry, auth tuple negotiated, but invalid incoming key_seq */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_last_auth_tuple.key_seq = 2; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = 0; + key_seq = GSM_KEY_SEQ_INVAL; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_AUTH_THEN_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 1\n" + " .key_seq = 3\n" + " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" + " .sres = a1 ab c6 90 \n" + " .kc = 0f 27 ed f3 ac 97 ac 00 \n" + "}\n" + )); + + /* Change the last saved key_seq, expect last_auth_tuple.key_seq + 1 */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_last_auth_tuple.key_seq = 3; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = 0; + key_seq = GSM_KEY_SEQ_INVAL; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_AUTH_THEN_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 1\n" + " .key_seq = 4\n" + " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" + " .sres = a1 ab c6 90 \n" + " .kc = 0f 27 ed f3 ac 97 ac 00 \n" + "}\n" + )); +} + int main(void) { osmo_init_logging(&log_info); @@ -115,5 +249,7 @@ int main(void) test_error(); test_auth_not_avail(); + test_auth_then_ciph1(); + test_auth_then_ciph2(); return 0; } diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok index 5efb3deca..52feb3679 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.ok +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -6,3 +6,19 @@ auth_get_tuple_for_subscr(key_seq=0) --> auth_action == (internal error) * test_auth_not_avail() wrapped: db_get_authinfo_for_subscr(): rc = -2 auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_NOT_AVAIL + +* test_auth_then_ciph1() +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = -2 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_DO_AUTH_THEN_CIPH + +* test_auth_then_ciph2() +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = 0 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=7) --> auth_action == AUTH_DO_AUTH_THEN_CIPH +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = 0 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=7) --> auth_action == AUTH_DO_AUTH_THEN_CIPH -- cgit v1.2.3 From d617c5d3ac65b5090f787da3e2911fb6742f515c Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:26 +0200 Subject: MM Auth test: add test to re-use existing auth --- openbsc/tests/mm_auth/mm_auth_test.c | 31 +++++++++++++++++++++++++++++++ openbsc/tests/mm_auth/mm_auth_test.ok | 6 ++++++ 2 files changed, 37 insertions(+) diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c index c0b8da424..e54189834 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.c +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -242,6 +242,36 @@ static void test_auth_then_ciph2() )); } +static void test_auth_reuse() +{ + int auth_action; + struct gsm_auth_tuple atuple = {0}; + struct gsm_subscriber subscr = {0}; + int key_seq; + + printf("\n* test_auth_reuse()\n"); + + /* Ki entry, auth tuple negotiated, valid+matching incoming key_seq */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_last_auth_tuple.key_seq = key_seq = 3; + test_last_auth_tuple.use_count = 1; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = 0; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 2\n" + " .key_seq = 3\n" + " .rand = 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \n" + " .sres = 00 00 00 00 \n" + " .kc = 00 00 00 00 00 00 00 00 \n" + "}\n" + )); +} + int main(void) { osmo_init_logging(&log_info); @@ -251,5 +281,6 @@ int main(void) test_auth_not_avail(); test_auth_then_ciph1(); test_auth_then_ciph2(); + test_auth_reuse(); return 0; } diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok index 52feb3679..cc0e76960 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.ok +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -22,3 +22,9 @@ wrapped: db_get_authinfo_for_subscr(): rc = 0 wrapped: db_get_lastauthtuple_for_subscr(): rc = 0 wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 auth_get_tuple_for_subscr(key_seq=7) --> auth_action == AUTH_DO_AUTH_THEN_CIPH + +* test_auth_reuse() +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = 0 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=3) --> auth_action == AUTH_DO_CIPH -- cgit v1.2.3 From f9b212fabd0d5c37dac9639fc9d9ecd73688e3a3 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:27 +0200 Subject: MM Auth: introduce AUTH_ERROR constant. Instead of using hardcoded -1 for errors, include -1 in the enum auth_action type; apply its use. In the mm_auth test, the string output changes from '(internal error)' to 'AUTH_ERROR', since now the proper enum value is used in auth_action_names[]. --- openbsc/include/openbsc/auth.h | 1 + openbsc/src/libmsc/auth.c | 6 +++--- openbsc/tests/mm_auth/mm_auth_test.c | 2 +- openbsc/tests/mm_auth/mm_auth_test.ok | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/openbsc/include/openbsc/auth.h b/openbsc/include/openbsc/auth.h index 90495bb58..61811316b 100644 --- a/openbsc/include/openbsc/auth.h +++ b/openbsc/include/openbsc/auth.h @@ -7,6 +7,7 @@ struct gsm_auth_tuple; struct gsm_subscriber; enum auth_action { + AUTH_ERROR = -1, /* Internal error */ AUTH_NOT_AVAIL = 0, /* No auth tuple available */ AUTH_DO_AUTH_THEN_CIPH = 1, /* Firsth authenticate, then cipher */ AUTH_DO_CIPH = 2, /* Only ciphering */ diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index 85123167b..99e3a243e 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -33,7 +33,7 @@ const struct value_string auth_action_names[] = { #define AUTH_ACTION_STR(X) { X, #X } - { -1, "(internal error)" }, /* soon to be fixed with an enum val */ + AUTH_ACTION_STR(AUTH_ERROR), AUTH_ACTION_STR(AUTH_NOT_AVAIL), AUTH_ACTION_STR(AUTH_DO_AUTH_THEN_CIPH), AUTH_ACTION_STR(AUTH_DO_CIPH), @@ -93,7 +93,7 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, if (rc < 0) { LOGP(DMM, LOGL_NOTICE, "No retrievable Ki for subscriber, skipping auth\n"); - return rc == -ENOENT ? AUTH_NOT_AVAIL : -1; + return rc == -ENOENT ? AUTH_NOT_AVAIL : AUTH_ERROR; } /* If possible, re-use the last tuple and skip auth */ @@ -114,7 +114,7 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, if (RAND_bytes(atuple->rand, sizeof(atuple->rand)) != 1) { LOGP(DMM, LOGL_NOTICE, "RAND_bytes failed, can't generate new auth tuple\n"); - return -1; + return AUTH_ERROR; } switch (ainfo.auth_algo) { diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c index e54189834..1d65984cb 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.c +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -141,7 +141,7 @@ static void test_error() test_get_authinfo_rc = -EIO; auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, key_seq); - OSMO_ASSERT(auth_action == -1); + OSMO_ASSERT(auth_action == AUTH_ERROR); } static void test_auth_not_avail() diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok index cc0e76960..7dedadc38 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.ok +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -1,7 +1,7 @@ * test_error() wrapped: db_get_authinfo_for_subscr(): rc = -5 -auth_get_tuple_for_subscr(key_seq=0) --> auth_action == (internal error) +auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_ERROR * test_auth_not_avail() wrapped: db_get_authinfo_for_subscr(): rc = -2 -- cgit v1.2.3 From 4e875aec0fda55bc93ea76c6992aaf3d90931d2d Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:28 +0200 Subject: MM Auth: return AUTH_NOT_AVAIL instead of hardcoded zero AUTH_NOT_AVAIL == 0, so this is no functional change. --- openbsc/src/libmsc/auth.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index 99e3a243e..4ce183935 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -120,22 +120,22 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, switch (ainfo.auth_algo) { case AUTH_ALGO_NONE: DEBUGP(DMM, "No authentication for subscriber\n"); - return 0; + return AUTH_NOT_AVAIL; case AUTH_ALGO_XOR: if (_use_xor(&ainfo, atuple)) - return 0; + return AUTH_NOT_AVAIL; break; case AUTH_ALGO_COMP128v1: if (_use_comp128_v1(&ainfo, atuple)) - return 0; + return AUTH_NOT_AVAIL; break; default: DEBUGP(DMM, "Unsupported auth type algo_id=%d\n", ainfo.auth_algo); - return 0; + return AUTH_NOT_AVAIL; } db_sync_lastauthtuple_for_subscr(atuple, subscr); -- cgit v1.2.3 From 0d929be8264ba592313f2cdd9bc4bd9b2579df00 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:29 +0200 Subject: Fix MM Auth: disallow key_seq mismatch In auth_get_tuple_for_subscr(), add missing condition to match incoming key_seq with stored key_seq, so that re-authentication is requested for mismatching key_seqs. Add test for this issue. --- openbsc/src/libmsc/auth.c | 1 + openbsc/tests/mm_auth/mm_auth_test.c | 32 ++++++++++++++++++++++++++++++++ openbsc/tests/mm_auth/mm_auth_test.ok | 6 ++++++ 3 files changed, 39 insertions(+) diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index 4ce183935..ca39d0118 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -100,6 +100,7 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, rc = db_get_lastauthtuple_for_subscr(atuple, subscr); if ((rc == 0) && (key_seq != GSM_KEY_SEQ_INVAL) && + (key_seq == atuple->key_seq) && (atuple->use_count < 3)) { atuple->use_count++; diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c index 1d65984cb..2b4586101 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.c +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -272,6 +272,37 @@ static void test_auth_reuse() )); } +static void test_auth_reuse_key_seq_mismatch() +{ + int auth_action; + struct gsm_auth_tuple atuple = {0}; + struct gsm_subscriber subscr = {0}; + int key_seq; + + printf("\n* test_auth_reuse_key_seq_mismatch()\n"); + + /* Ki entry, auth tuple negotiated, valid+matching incoming key_seq */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_last_auth_tuple.key_seq = 3; + key_seq = 4; + test_last_auth_tuple.use_count = 1; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = 0; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_AUTH_THEN_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 1\n" + " .key_seq = 4\n" + " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" + " .sres = a1 ab c6 90 \n" + " .kc = 0f 27 ed f3 ac 97 ac 00 \n" + "}\n" + )); +} + int main(void) { osmo_init_logging(&log_info); @@ -282,5 +313,6 @@ int main(void) test_auth_then_ciph1(); test_auth_then_ciph2(); test_auth_reuse(); + test_auth_reuse_key_seq_mismatch(); return 0; } diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok index 7dedadc38..9d89bfb84 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.ok +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -28,3 +28,9 @@ wrapped: db_get_authinfo_for_subscr(): rc = 0 wrapped: db_get_lastauthtuple_for_subscr(): rc = 0 wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 auth_get_tuple_for_subscr(key_seq=3) --> auth_action == AUTH_DO_CIPH + +* test_auth_reuse_key_seq_mismatch() +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = 0 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=4) --> auth_action == AUTH_DO_AUTH_THEN_CIPH -- cgit v1.2.3 From cf1302e4cb4875816615a23e5d7e2e9f7bcb5bca Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Wed, 30 Mar 2016 11:22:30 +0200 Subject: Fix MM Auth: zero-initialize auth tuple before first use Make sure a new auth tuple is initialized after db_get_lastauthtuple_for_subscr() returns an error, i.e. if no tuple is present for the subscriber yet. Before this patch, the first key_seq depended on the typically uninitialized value that was present in auth tuple's key_seq upon calling auth_get_tuple_for_subscr(). The very first key_seq used for a new subscriber will now always be 0. Before, it used to be mostly 1 ("(0 + 1) % 7"), but depended on whether the key_seq was indeed initialized with 0, actually by random. --- openbsc/src/libmsc/auth.c | 11 ++++++++++- openbsc/tests/mm_auth/mm_auth_test.c | 24 +++++++++++++++++++++++- openbsc/tests/mm_auth/mm_auth_test.ok | 4 ++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index ca39d0118..f30d56dce 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -110,8 +110,17 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, } /* Generate a new one */ + if (rc != 0) { + /* If db_get_lastauthtuple_for_subscr() returned nothing, make + * sure the atuple memory is initialized to zero and thus start + * off with key_seq = 0. */ + memset(atuple, 0, sizeof(*atuple)); + } else { + /* If db_get_lastauthtuple_for_subscr() returned a previous + * tuple, use the next key_seq. */ + atuple->key_seq = (atuple->key_seq + 1) % 7; + } atuple->use_count = 1; - atuple->key_seq = (atuple->key_seq + 1) % 7; if (RAND_bytes(atuple->rand, sizeof(atuple->rand)) != 1) { LOGP(DMM, LOGL_NOTICE, "RAND_bytes failed, can't generate new auth tuple\n"); diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c index 2b4586101..34d96f187 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.c +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -183,7 +183,29 @@ static void test_auth_then_ciph1() OSMO_ASSERT(auth_tuple_is(&atuple, "gsm_auth_tuple {\n" " .use_count = 1\n" - " .key_seq = 1\n" + " .key_seq = 0\n" + " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" + " .sres = a1 ab c6 90 \n" + " .kc = 0f 27 ed f3 ac 97 ac 00 \n" + "}\n" + )); + + /* With a different last saved key_seq stored in the out-arg of + * db_get_lastauthtuple_for_subscr() by coincidence, expect absolutely + * the same as above. */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_last_auth_tuple.key_seq = 3; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = -ENOENT; + key_seq = 0; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_AUTH_THEN_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 1\n" + " .key_seq = 0\n" " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" " .sres = a1 ab c6 90 \n" " .kc = 0f 27 ed f3 ac 97 ac 00 \n" diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok index 9d89bfb84..6c49f97b7 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.ok +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -12,6 +12,10 @@ wrapped: db_get_authinfo_for_subscr(): rc = 0 wrapped: db_get_lastauthtuple_for_subscr(): rc = -2 wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_DO_AUTH_THEN_CIPH +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = -2 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_DO_AUTH_THEN_CIPH * test_auth_then_ciph2() wrapped: db_get_authinfo_for_subscr(): rc = 0 -- cgit v1.2.3