diff options
author | Neels Hofmeyr <neels@hofmeyr.de> | 2020-05-26 02:45:23 +0200 |
---|---|---|
committer | Neels Hofmeyr <neels@hofmeyr.de> | 2020-06-12 16:35:26 +0200 |
commit | d1ceca9d48eb3d8b212f386a1ebb35d8fc612297 (patch) | |
tree | ed3b0558378654bf1699c98b766b78520dde2d97 /src/gb | |
parent | 0b6a8c8446497da32970b8442c68eca796518b6d (diff) |
add osmo_mobile_identity API
Implement better API around 3GPP TS 24.008 Mobile Identity coding.
struct osmo_mobile_identity is a decoded representation of the raw Mobile
Identity, with a string representation as well as dedicated raw uint32_t TMSI.
The aim is to remove all uncertainty about decoded buffer sizes / data types.
I have patches ready for all osmo programs, completely replacing the Mobile
Identity coding with this new API. Hence deprecate the old MI API.
New API functions provide properly size-checking implementations of:
- decoding a raw MI from a bunch of MI octets;
- locating and decoding MI from a full 3GPP TS 24.008 Complete Layer 3 msgb;
- encoding to a buffer;
- encoding to the end of a msgb.
Other than the old gsm48_generate_mid(), omit a TLV tag and length from
encoding. Many callers manually stripped the tag and value after calling
gsm48_generate_mid(). The aim is to leave writing a TL to the caller entirely,
especially since some callers need to use a TvL, i.e. support a variable-size
length of 8 or 16 bit.
New validity checks so far not implemented anywhere else:
- stricter validation of number of digits of IMSI, IMEI, IMEI-SV MI.
- stricter on filler nibbles to be 0xf.
Rationale:
While implementing osmo-bsc's MSC pooling feature in osmo-bsc, this API will be
used to reduce the number of times a Mobile Identity is extracted from a raw
RSL message.
Extracting the Mobile Identity from messages has numerous duplicate
implementations across our code with various levels of specialization.
https://xkcd.com/927/
To name a few:
- libosmocore: gsm48_mi_to_string(), osmo_mi_name_buf()
- osmo-bsc: extract_sub()
- osmo-msc: mm_rx_loc_upd_req(), cm_serv_reuse_conn(), gsm48_rx_mm_serv_req(),
vlr_proc_acc_req()
We have existing functions to produce a human readable string from a Mobile
Identity, more or less awkward:
- gsm48_mi_to_string() decodes a TMSI as a decimal number. These days we use
hexadecimal TMSI everywhere.
- osmo_mi_name_buf() decodes the BCD digits from a raw MI every time, so we'd
need to pass around the raw message bytes. Also, osmo_mi_name_buf() has the
wrong signature, it should return a length like snprintf().
- osmo-bsc's extract_sub() first uses gsm48_mi_to_string() which encodes the
raw uint32_t TMSI to a string, and then calls strtoul() via
tmsi_from_string() to code those back to a raw uint32_t.
Each of the above implementations employ their own size overflow checks, each
invoke osmo_bcd2str() and implement their own TMSI osmo_load32be() handling.
Too much code dup, let's hope that each and every one is correct.
In osmo-bsc, I am now implementing MSC pooling, and need to extract NRI bits
from a TMSI Mobile Identity. Since none of the above functions are general
enough to be re-used, I found myself again copy-pasting Mobile Identity code:
locating the MI in a 24.008 message with proper size checks, decoding MI
octets.
This time I would like it to become a generally re-usable API.
Change-Id: Ic3f969e739654c1e8c387aedeeba5cce07fe2307
Diffstat (limited to 'src/gb')
-rw-r--r-- | src/gb/gprs_bssgp.c | 58 | ||||
-rw-r--r-- | src/gb/gprs_bssgp_bss.c | 30 |
2 files changed, 45 insertions, 43 deletions
diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c index 38794c28..2784d0a8 100644 --- a/src/gb/gprs_bssgp.c +++ b/src/gb/gprs_bssgp.c @@ -1170,19 +1170,20 @@ int bssgp_tx_dl_ud(struct msgb *msg, uint16_t pdu_lifetime, /* IMSI */ if (dup->imsi && strlen(dup->imsi)) { - uint8_t mi[GSM48_MID_MAX_SIZE]; -/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11, - * but somehow gcc (8.2) is not smart enough to figure this out and claims that - * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to - * mi[131], which is wrong */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" - int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi); - OSMO_ASSERT(imsi_len <= GSM48_MID_MAX_SIZE); - if (imsi_len > 2) - msgb_tvlv_push(msg, BSSGP_IE_IMSI, - imsi_len-2, mi+2); -#pragma GCC diagnostic pop + struct osmo_mobile_identity mi = { .type = GSM_MI_TYPE_IMSI, }; + OSMO_STRLCPY_ARRAY(mi.imsi, dup->imsi); + msgb_tvl_put(msg, BSSGP_IE_IMSI, osmo_mobile_identity_encoded_len(&mi, NULL)); + if (osmo_mobile_identity_encode_msgb(msg, &mi, false) <= 0) { + if (log_check_level(DBSSGP, LOGL_NOTICE)) { + char strbuf[64]; + osmo_mobile_identity_to_str_buf(strbuf, sizeof(strbuf), &mi); + LOGP(DBSSGP, LOGL_ERROR, + "NSEI=%u/BVCI=%u Cannot encode Mobile Identity %s\n", + nsei, bvci, strbuf); + } + msgb_free(msg); + return -EINVAL; + } } /* DRX parameters */ @@ -1227,12 +1228,8 @@ int bssgp_tx_paging(uint16_t nsei, uint16_t ns_bvci, struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph)); uint16_t drx_params = osmo_htons(pinfo->drx_params); - uint8_t mi[GSM48_MID_MAX_SIZE]; - int imsi_len = gsm48_generate_mid_from_imsi(mi, pinfo->imsi); struct gsm48_ra_id ra; - - if (imsi_len < 2) - return -EINVAL; + struct osmo_mobile_identity mi; msgb_nsei(msg) = nsei; msgb_bvci(msg) = ns_bvci; @@ -1241,16 +1238,23 @@ int bssgp_tx_paging(uint16_t nsei, uint16_t ns_bvci, bgph->pdu_type = BSSGP_PDUT_PAGING_PS; else bgph->pdu_type = BSSGP_PDUT_PAGING_CS; + /* IMSI */ -/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11, - * but somehow gcc (8.2) is not smart enough to figure this out and claims that - * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to - * mi[131], which is wrong */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" - OSMO_ASSERT(imsi_len <= GSM48_MID_MAX_SIZE); - msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2); -#pragma GCC diagnostic pop + mi = (struct osmo_mobile_identity){ .type = GSM_MI_TYPE_IMSI, }; + OSMO_STRLCPY_ARRAY(mi.imsi, pinfo->imsi); + msgb_tvl_put(msg, BSSGP_IE_IMSI, osmo_mobile_identity_encoded_len(&mi, NULL)); + if (osmo_mobile_identity_encode_msgb(msg, &mi, false) <= 0) { + if (log_check_level(DBSSGP, LOGL_NOTICE)) { + char strbuf[64]; + osmo_mobile_identity_to_str_buf(strbuf, sizeof(strbuf), &mi); + LOGP(DBSSGP, LOGL_ERROR, + "NSEI=%u/BVCI=%u Cannot encode Mobile Identity %s\n", + nsei, ns_bvci, strbuf); + } + msgb_free(msg); + return -EINVAL; + } + /* DRX Parameters */ msgb_tvlv_put(msg, BSSGP_IE_DRX_PARAMS, 2, (uint8_t *) &drx_params); diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c index 5c9d11cc..9e9cefc5 100644 --- a/src/gb/gprs_bssgp_bss.c +++ b/src/gb/gprs_bssgp_bss.c @@ -178,22 +178,17 @@ int bssgp_tx_radio_status_imsi(struct bssgp_bvc_ctx *bctx, uint8_t cause, const char *imsi) { struct msgb *msg = common_tx_radio_status(bctx); - uint8_t mi[GSM48_MID_MAX_SIZE]; - int imsi_len = gsm48_generate_mid_from_imsi(mi, imsi); + struct osmo_mobile_identity mi = { .type = GSM_MI_TYPE_IMSI, }; + OSMO_STRLCPY_ARRAY(mi.imsi, imsi); if (!msg) return -ENOMEM; -/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11, - * but somehow gcc (8.2) is not smart enough to figure this out and claims that - * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to - * mi[131], which is wrong */ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" - OSMO_ASSERT(imsi_len <= GSM48_MID_MAX_SIZE); - /* strip the MI type and length values (2 bytes) */ - if (imsi_len > 2) - msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2); -#pragma GCC diagnostic pop + + msgb_tvl_put(msg, BSSGP_IE_IMSI, osmo_mobile_identity_encoded_len(&mi, NULL)); + if (osmo_mobile_identity_encode_msgb(msg, &mi, false) <= 0) { + msgb_free(msg); + return -EINVAL; + } LOGPC(DBSSGP, LOGL_NOTICE, "IMSI=%s ", imsi); return common_tx_radio_status2(msg, cause); @@ -486,6 +481,7 @@ int bssgp_rx_paging(struct bssgp_paging_info *pinfo, struct tlv_parsed tp; uint8_t ra[6]; int rc, data_len; + struct osmo_mobile_identity mi; memset(ra, 0, sizeof(ra)); @@ -510,9 +506,11 @@ int bssgp_rx_paging(struct bssgp_paging_info *pinfo, goto err_mand_ie; if (!pinfo->imsi) pinfo->imsi = talloc_zero_size(pinfo, GSM_IMSI_LENGTH); - gsm48_mi_to_string(pinfo->imsi, GSM_IMSI_LENGTH, - TLVP_VAL(&tp, BSSGP_IE_IMSI), - TLVP_LEN(&tp, BSSGP_IE_IMSI)); + if (osmo_mobile_identity_decode(&mi, TLVP_VAL(&tp, BSSGP_IE_IMSI), TLVP_LEN(&tp, BSSGP_IE_IMSI), false)) + goto err_mand_ie; + if (mi.type != GSM_MI_TYPE_IMSI) + goto err_mand_ie; + osmo_talloc_replace_string(pinfo, &pinfo->imsi, mi.imsi); /* DRX Parameters */ if (!TLVP_PRESENT(&tp, BSSGP_IE_DRX_PARAMS)) |