August 2026 - gerrit-log - lists.osmocom.org

[S] Change in osmo-msc[master]: smpp: Fix potential write buffer overflow on sms->user_data
by pespin 14 Aug '26

14 Aug '26

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/43253?usp=email ) Change subject: smpp: Fix potential write buffer overflow on sms->user_data ...................................................................... smpp: Fix potential write buffer overflow on sms->user_data The writes to sms->user_data in submit_to_sms() were not being validated against the maximum size of the buffer, which could lead into writing past the buffer limits. In order to assure safe encdoing of septets into the buffer, the new libosmocore gsm_septet_pack2() is required. Related: OS#7059 Reported-By: Adam Bedard <adam.bedard(a)gmail.com> Change-Id: Iee701a3b033d78244fc10dd8563a094b8de385c0 --- M TODO-RELEASE M src/libsmpputil/smpp_msc.c 2 files changed, 25 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/53/43253/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index c8858d8..84a2985 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -9,3 +9,4 @@ #library what description / commit summary line libosmovty >=1.12.1 working optional-multi-choice libosmocore >1.12.0 log_get_context(), log_{get,set}_filter(_data)() +libosmogsm >1.14.1 gsm_septet_pack2() diff --git a/src/libsmpputil/smpp_msc.c b/src/libsmpputil/smpp_msc.c index d68dcc0..5fdba36 100644 --- a/src/libsmpputil/smpp_msc.c +++ b/src/libsmpputil/smpp_msc.c @@ -132,6 +132,7 @@ struct tlv_t *t; int mode; int can_store_sms = ((submit->esm_class & SMPP34_MSG_MODE_MASK) != 2); /* != forward mode */ + int rc; dest = subscr_by_dst(net, submit->dest_addr_npi, submit->dest_addr_ton, @@ -247,21 +248,39 @@ ud_len = *sms_msg + 1; if (ud_len > sms_msg_len) { sms_free(sms); - LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u > sms_msg_len=%u\n", ud_len, - sms_msg_len); + LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u > sms_msg_len=%u\n", + ud_len, sms_msg_len); + return ESME_RINVPARLEN; + } + if (ud_len > sizeof(sms->user_data)) { + sms_free(sms); + LOGP(DLSMS, LOGL_ERROR, "invalid sms_msg_len=%u > %zu\n", + sms_msg_len, sizeof(sms->user_data)); return ESME_RINVPARLEN; } printf("copying %u bytes user data...\n", ud_len); - memcpy(sms->user_data, sms_msg, - OSMO_MIN(ud_len, sizeof(sms->user_data))); + memcpy(sms->user_data, sms_msg, ud_len); sms_msg += ud_len; sms_msg_len -= ud_len; padbits = 7 - (ud_len % 7); } - gsm_septet_pack(sms->user_data+ud_len, sms_msg, sms_msg_len, padbits); + rc = gsm_septet_pack2(sms->user_data + ud_len, sizeof(sms->user_data) - ud_len, + sms_msg, sms_msg_len, padbits); + if (rc < 0) { + sms_free(sms); + LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u + sms_msg_len=%u > %zu\n", + ud_len, sms_msg_len, sizeof(sms->user_data)); + return ESME_RINVPARLEN; + } sms->user_data_len = (ud_len*8 + padbits)/7 + sms_msg_len;/* SEPTETS */ /* FIXME: sms->text */ } else { + if (sms_msg_len > sizeof(sms->user_data)) { + sms_free(sms); + LOGP(DLSMS, LOGL_ERROR, "invalid sms_msg_len=%u > %zu\n", + sms_msg_len, sizeof(sms->user_data)); + return ESME_RINVPARLEN; + } memcpy(sms->user_data, sms_msg, sms_msg_len); sms->user_data_len = sms_msg_len; } -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/43253?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Iee701a3b033d78244fc10dd8563a094b8de385c0 Gerrit-Change-Number: 43253 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack()
by pespin 14 Aug '26

14 Aug '26

pespin has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/libosmocore/+/43252?usp=email ) Change subject: gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack() ...................................................................... gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack() The new gsm_septet_pack2() comes with a new parameter containing the size of the output buffer, effectively protecting against write buffer overflows. Related: OS#7059 Reported-By: Adam Bedard <adam.bedard(a)gmail.com> Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 --- M TODO-RELEASE M include/osmocom/gsm/gsm_utils.h M src/gsm/gsm_utils.c M src/gsm/libosmogsm.map M tests/sms/sms_test.c 5 files changed, 27 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/43252/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43252?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 Gerrit-Change-Number: 43252 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder

1 0

[S] Change in libosmocore[master]: gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack()
by pespin 14 Aug '26

14 Aug '26

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/43252?usp=email ) Change subject: gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack() ...................................................................... gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack() The new gsm_septet_pack2() comes with a new parameter containing the size of the output buffer, effectively protecting against write buffer overflows. Related: OS#7059 Reported-By: Adam Bedard <adam.bedard(a)gmail.com> Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 --- M include/osmocom/gsm/gsm_utils.h M src/gsm/gsm_utils.c M src/gsm/libosmogsm.map M tests/sms/sms_test.c 4 files changed, 26 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/43252/1 diff --git a/include/osmocom/gsm/gsm_utils.h b/include/osmocom/gsm/gsm_utils.h index 72bd131..e31e559 100644 --- a/include/osmocom/gsm/gsm_utils.h +++ b/include/osmocom/gsm/gsm_utils.h @@ -109,8 +109,10 @@ /* the four functions below are helper functions and here for the unit test */ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len, uint8_t padding) OSMO_DEPRECATED("This function is unable to handle more than 255 septets, " - "use gsm_septet_pack() instead."); -int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding); + "use gsm_septet_pack2() instead."); +int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding) + OSMO_DEPRECATED("This function is not write-safe, use gsm_septet_pack2() instead."); +int gsm_septet_pack2(uint8_t *result, size_t result_size, const uint8_t *rdata, size_t septet_len, uint8_t padding); int gsm_septet_encode(uint8_t *result, const char *data); uint8_t gsm_get_octet_len(const uint8_t sept_len); int gsm_7bit_decode_n_hdr(char *decoded, size_t n, const uint8_t *user_data, uint8_t length, uint8_t ud_hdr_ind); diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index 3ca6812..27ecad2 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -90,6 +90,7 @@ #include <errno.h> #include <ctype.h> #include <inttypes.h> +#include <limits.h> #include <time.h> #include <unistd.h> @@ -317,11 +318,12 @@ /*! GSM Default Alphabet 7bit to octet packing * \param[out] result Caller-provided output buffer + * \param[in] result_size Caller-provided output buffer size * \param[in] rdata Input data septets * \param[in] septet_len Length of \a rdata * \param[in] padding padding bits at start - * \returns number of bytes used in \a result */ -int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding) + * \returns number of bytes used in \a result, negative on error */ +int gsm_septet_pack2(uint8_t *result, size_t result_size, const uint8_t *rdata, size_t septet_len, uint8_t padding) { int i = 0, z = 0; uint8_t cb, nb; @@ -358,6 +360,8 @@ cb = cb | nb; } + if (z == result_size) + return -ENOBUFS; result[z++] = cb; shift++; } @@ -367,10 +371,21 @@ return z; } +/*! GSM Default Alphabet 7bit to octet packing + * \param[out] result Caller-provided output buffer + * \param[in] rdata Input data septets + * \param[in] septet_len Length of \a rdata + * \param[in] padding padding bits at start + * \returns number of bytes used in \a result, negative on error */ +int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding) +{ + return gsm_septet_pack2(result, INT_MAX, rdata, septet_len, padding); +} + /*! Backwards compatibility wrapper for gsm_septets_pack(), deprecated. */ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len, uint8_t padding) { - return gsm_septet_pack(result, rdata, septet_len, padding); + return gsm_septet_pack2(result, INT_MAX, rdata, septet_len, padding); } /*! GSM 7-bit alphabet TS 03.38 6.2.1 Character packing @@ -397,7 +412,7 @@ y = max_septets; } - o = gsm_septet_pack(result, rdata, y, 0); + o = gsm_septet_pack2(result, n, rdata, y, 0); if (octets) *octets = o; diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index 78e0385..c57d286 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -566,6 +566,7 @@ gsm_milenage; gsm_septet_encode; gsm_septet_pack; +gsm_septet_pack2; gsm_septets2octets; lapd_dl_exit; diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c index 912c082..dda3c56 100644 --- a/tests/sms/sms_test.c +++ b/tests/sms/sms_test.c @@ -380,7 +380,7 @@ memcpy(tmp, septet_data, concatenated_part1_septet_length); /* In our case: test_multiple_decode[0].ud_hdr_ind equals number of padding bits*/ - octet_length = gsm_septet_pack(coded, tmp, concatenated_part1_septet_length, test_multiple_encode[0].ud_hdr_ind); + octet_length = gsm_septet_pack2(coded, sizeof(coded), tmp, concatenated_part1_septet_length, test_multiple_encode[0].ud_hdr_ind); /* copy header */ memset(tmp, 0x42, sizeof(tmp)); @@ -398,7 +398,7 @@ memcpy(tmp, septet_data + concatenated_part1_septet_length, concatenated_part2_septet_length); /* In our case: test_multiple_decode[1].ud_hdr_ind equals number of padding bits*/ - octet_length = gsm_septet_pack(coded, tmp, concatenated_part2_septet_length, test_multiple_encode[1].ud_hdr_ind); + octet_length = gsm_septet_pack2(coded, sizeof(coded), tmp, concatenated_part2_septet_length, test_multiple_encode[1].ud_hdr_ind); /* copy header */ memset(tmp, 0x42, sizeof(tmp)); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43252?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 Gerrit-Change-Number: 43252 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: gsm: Reject BSSMAP Encryption Information IE with no selected algo
by pespin 14 Aug '26

14 Aug '26

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43250?usp=email ) Change subject: gsm: Reject BSSMAP Encryption Information IE with no selected algo ...................................................................... gsm: Reject BSSMAP Encryption Information IE with no selected algo 3GPP TS 48.008 3.2.2.10 Encryption Information IE clearly states that case should not be used, so reject it. Change-Id: I48ace0b3590c993b86f1471e5b57c3afee72217f --- M src/gsm/gsm0808_utils.c M tests/gsm0808/gsm0808_test.c 2 files changed, 11 insertions(+), 3 deletions(-) Approvals: laforge: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c index c9f26d3..1abfbc8 100644 --- a/src/gsm/gsm0808_utils.c +++ b/src/gsm/gsm0808_utils.c @@ -820,7 +820,7 @@ return *tlv_len + 2; } -/*! Decode TS 08.08 Encryption Information IE +/*! Decode 3GPP TS 48.008 3.2.2.10 Encryption Information IE * \param[out] ei Caller-provided memory to store encryption information * \param[in] elem IE value to be decoded * \param[in] len Length of \a elem in bytes @@ -843,6 +843,10 @@ perm_algo = *elem; elem++; + /* "A permitted algorithms octet containing all bits encoded as 0 shall not be used." */ + if (perm_algo == 0x00) + return -EINVAL; + for (i = 0; i < ENCRY_INFO_PERM_ALGO_MAXLEN; i++) { if (perm_algo & (1 << i)) { ei->perm_algo[perm_algo_len] = i + 1; diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c index ed99245..497de6c 100644 --- a/tests/gsm0808/gsm0808_test.c +++ b/tests/gsm0808/gsm0808_test.c @@ -1281,10 +1281,14 @@ rc_dec = gsm0808_dec_encrypt_info(&dec_ei, msg->data + 2, msg->len - 2); OSMO_ASSERT(rc_dec == 9); - OSMO_ASSERT(memcmp(&enc_ei, &dec_ei, sizeof(enc_ei)) == 0); - msgb_free(msg); + + /* Test decoding of malformed IE with no algo selected: */ + uint8_t ei_enc_no_algo[] = { GSM0808_IE_ENCRYPTION_INFORMATION, 0x01, 0x00 }; + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_no_algo[2], sizeof(ei_enc_no_algo) - 2); + OSMO_ASSERT(rc_dec == -EINVAL); + } static void test_gsm0808_dec_cell_id_list_srvcc(void) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43250?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I48ace0b3590c993b86f1471e5b57c3afee72217f Gerrit-Change-Number: 43250 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: gsm: Reject BSSMAP Encryption Information IE with key length != 8
by pespin 14 Aug '26

14 Aug '26

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43251?usp=email ) Change subject: gsm: Reject BSSMAP Encryption Information IE with key length != 8 ...................................................................... gsm: Reject BSSMAP Encryption Information IE with key length != 8 From early TS 08.08 to current 3GPP TS 48.008 release 19, 3.2.2.10 Encryption Information IE clearly states that the key size should be 8 if present. This also in turn fixes a potential write buffer overflow if an IE with len>253 was passed to the decoding function. This patch fixes the problem by rewriting the function, since it actually was way too convoluted/complex for the kind of checks it had to do. This way also a call to memcpy is avoided if no key is present. Related: OS#7054 Reported-By: Adam Bedard <adam.bedard(a)gmail.com> Change-Id: I9fd70875f35cebb923278869e1260c5bf3047439 --- M src/gsm/gsm0808_utils.c M tests/gsm0808/gsm0808_test.c 2 files changed, 38 insertions(+), 7 deletions(-) Approvals: laforge: Looks good to me, approved osmith: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c index 1abfbc8..d7bc275 100644 --- a/src/gsm/gsm0808_utils.c +++ b/src/gsm/gsm0808_utils.c @@ -831,7 +831,6 @@ uint8_t perm_algo; unsigned int i; unsigned int perm_algo_len = 0; - const uint8_t *old_elem = elem; if (!elem) return -EINVAL; @@ -854,14 +853,18 @@ } } ei->perm_algo_len = perm_algo_len; - - /* FIXME: 48.008 3.2.2.10 Encryption Information says: - * "When present, the key shall be 8 octets long." */ ei->key_len = len - 1; - memcpy(ei->key, elem, ei->key_len); - elem+=ei->key_len; - return (int)(elem - old_elem); + /* No key present, done */ + if (ei->key_len == 0) + return len; + + /* "When present, the key shall be 8 octets long." */ + if (ei->key_len != 8) + return -EINVAL; + OSMO_ASSERT(sizeof(ei->key) >= ei->key_len); + memcpy(ei->key, elem, ei->key_len); + return len; } /*! Encode TS 48.008 Kc128 IE. diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c index 497de6c..8ebbd4b 100644 --- a/tests/gsm0808/gsm0808_test.c +++ b/tests/gsm0808/gsm0808_test.c @@ -1282,6 +1282,25 @@ rc_dec = gsm0808_dec_encrypt_info(&dec_ei, msg->data + 2, msg->len - 2); OSMO_ASSERT(rc_dec == 9); OSMO_ASSERT(memcmp(&enc_ei, &dec_ei, sizeof(enc_ei)) == 0); + + + /* Test decoding IE with No Encryption and hence with no Key */ + struct gsm0808_encrypt_info enc_ei_no_encryption = { + .perm_algo = { GSM0808_ALG_ID_A5_0 }, + .perm_algo_len = 1, + .key = { 0 }, + .key_len = 0, + }; + uint8_t ei_enc_no_encryption_expected[] = { GSM0808_IE_ENCRYPTION_INFORMATION, 0x01, 0x01 }; + + msg = msgb_alloc(1024, "output buffer"); + rc_enc = gsm0808_enc_encrypt_info(msg, &enc_ei_no_encryption); + OSMO_ASSERT(rc_enc == 3); + OSMO_ASSERT(memcmp(ei_enc_no_encryption_expected, msg->data, msg->len) == 0); + + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, msg->data + 2, msg->len - 2); + OSMO_ASSERT(rc_dec == 1); + OSMO_ASSERT(memcmp(&enc_ei_no_encryption, &dec_ei, sizeof(enc_ei_no_encryption)) == 0); msgb_free(msg); /* Test decoding of malformed IE with no algo selected: */ @@ -1289,6 +1308,15 @@ rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_no_algo[2], sizeof(ei_enc_no_algo) - 2); OSMO_ASSERT(rc_dec == -EINVAL); + /* Test decoding of malformed IE with wrong key length: */ + uint8_t ei_enc_wrong_key_len[256] = { GSM0808_IE_ENCRYPTION_INFORMATION, 0xfd, 0x03 }; + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_wrong_key_len[2], 0xfd); + OSMO_ASSERT(rc_dec == -EINVAL); + /* test with another invalid key length: */ + ei_enc_wrong_key_len[1] = 0x03; + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_wrong_key_len[2], 0x03); + OSMO_ASSERT(rc_dec == -EINVAL); + } static void test_gsm0808_dec_cell_id_list_srvcc(void) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43251?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9fd70875f35cebb923278869e1260c5bf3047439 Gerrit-Change-Number: 43251 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: gsm: Reject BSSMAP Encryption Information IE with key length != 8
by laforge 14 Aug '26

14 Aug '26

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/43251?usp=email ) Change subject: gsm: Reject BSSMAP Encryption Information IE with key length != 8 ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43251?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9fd70875f35cebb923278869e1260c5bf3047439 Gerrit-Change-Number: 43251 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 14 Aug 2026 13:58:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in libosmocore[master]: gsm: Reject BSSMAP Encryption Information IE with key length != 8
by osmith 14 Aug '26

14 Aug '26

Attention is currently required from: laforge, pespin. osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/43251?usp=email ) Change subject: gsm: Reject BSSMAP Encryption Information IE with key length != 8 ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43251?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9fd70875f35cebb923278869e1260c5bf3047439 Gerrit-Change-Number: 43251 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 14 Aug 2026 13:54:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in libosmocore[master]: iuup: test receival of IuUP Data with way too big payload
by pespin 14 Aug '26

14 Aug '26

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43248?usp=email ) Change subject: iuup: test receival of IuUP Data with way too big payload ...................................................................... iuup: test receival of IuUP Data with way too big payload Take the chance to improve logging in that case, otherwise it is confusing when stating CRC check against 0xffffffff fails. Related: OS#7052 Change-Id: Id7059e08aa78af4c18c2be1493fb780a31e3e862 --- M src/gsm/iuup.c M tests/iuup/iuup_test.c M tests/iuup/iuup_test.err M tests/iuup/iuup_test.ok 4 files changed, 55 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c index a233fb9..fb4734d 100644 --- a/src/gsm/iuup.c +++ b/src/gsm/iuup.c @@ -966,8 +966,13 @@ return 0; payload_crc_err: - LOGP(DLIUUP, LOGL_NOTICE, "Payload Checksum error (pdu type %u): rx 0x%04x vs exp 0x%04x\n", - pdu_type, payload_crc, payload_crc_computed); + if (payload_crc_computed < 0) { + LOGP(DLIUUP, LOGL_NOTICE, "Payload Checksum failed (pdu type %u): Packet too big? length %u\n", + pdu_type, len); + } else { + LOGP(DLIUUP, LOGL_NOTICE, "Payload Checksum error (pdu type %u): rx 0x%04x vs exp 0x%04x\n", + pdu_type, payload_crc, payload_crc_computed); + } return -EIO; } diff --git a/tests/iuup/iuup_test.c b/tests/iuup/iuup_test.c index 353c76a..ed97e22 100644 --- a/tests/iuup/iuup_test.c +++ b/tests/iuup/iuup_test.c @@ -912,6 +912,47 @@ _test_submit_iuup_initialization_trimmed(sizeof(iuup_initialization_no_iptis) - 1, __func__); } +/* Test IUT when a way too big IuUP data packet is received. It should be dropped. */ +void test_data_too_big(void) +{ + /* Here we check the passive INIT code path, aka receiving INIT and returning INIT_ACK/NACK */ + struct osmo_iuup_instance *iui; + struct osmo_iuup_rnl_prim *rnp; + struct osmo_iuup_tnl_prim *tnp; + struct iuup_pdutype0_hdr *hdr0; + uint16_t payload_crc; + int rc; + unsigned int pkt_len = sizeof(struct iuup_pdutype0_hdr) + 1600; + OSMO_ASSERT(pkt_len <= IUUP_MSGB_SIZE); + + iui = osmo_iuup_instance_alloc(iuup_test_ctx, __func__); + OSMO_ASSERT(iui); + + clock_override_set(0, 0); + + /* Tx CONFIG.req */ + rnp = osmo_iuup_rnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_RNL_CONFIG, PRIM_OP_REQUEST, IUUP_MSGB_SIZE); + rnp->u.config = def_configure_req; + rnp->u.config.active = false; + + rc = osmo_iuup_rnl_prim_down(iui, rnp); + OSMO_ASSERT(rc == 0); + + /* Send IuUP incoming data to the implementation: */ + tnp = osmo_iuup_tnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_INDICATION, IUUP_MSGB_SIZE); + tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, pkt_len); + hdr0 = (struct iuup_pdutype0_hdr *)msgb_l2(tnp->oph.msg); + memcpy(hdr0, iuup_data, sizeof(iuup_data)); + + payload_crc = osmo_iuup_compute_payload_crc(msgb_l2(tnp->oph.msg), msgb_l2len(tnp->oph.msg)); + hdr0->payload_crc_hi = (payload_crc >> 8) & 0x03; + hdr0->payload_crc_lo = payload_crc & 0xff; + + OSMO_ASSERT((rc = osmo_iuup_tnl_prim_up(iui, tnp)) == 0); + + osmo_iuup_instance_free(iui); +} + int main(int argc, char **argv) { iuup_test_ctx = talloc_named_const(NULL, 0, "iuup_test"); @@ -937,6 +978,7 @@ test_decode_passive_init_malformed_no_rfci(); test_decode_passive_init_malformed_rfci_too_short(); test_decode_passive_init_malformed_missing_last_byte(); + test_data_too_big(); printf("OK.\n"); } diff --git a/tests/iuup/iuup_test.err b/tests/iuup/iuup_test.err index 0298420..5a573b4 100644 --- a/tests/iuup/iuup_test.err +++ b/tests/iuup/iuup_test.err @@ -88,3 +88,8 @@ DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Initialization: Malformed packet, length 15 too short DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Tx Initialization NACK cause=8 orig_message=e0 00 df f7 06 01 51 67 3c 86 27 00 00 00 01 DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Deallocated +DLIUUP IuUP(test_data_too_big){NULL}: Allocated +DLIUUP IuUP(test_data_too_big){NULL}: Received Event IuUP-CONFIG-req +DLIUUP IuUP(test_data_too_big){NULL}: state_chg to Initialisation +DLIUUP Payload Checksum failed (pdu type 0): Packet too big? length 1604 +DLIUUP IuUP(test_data_too_big){Initialisation}: Discarding invalid IuUP PDU: 01 00 e3 ff 08 55 6d 94 4c 71 a1 a0 81 e7 ea d2 04 24 44 80 00 0e cd 82 b8 11 18 00 00 97 c4 79 4e 77 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0DLIUUP IuUP(test_data_too_big){Initialisation}: Deallocated diff --git a/tests/iuup/iuup_test.ok b/tests/iuup/iuup_test.ok index 204c290..08b011f 100644 --- a/tests/iuup/iuup_test.ok +++ b/tests/iuup/iuup_test.ok @@ -73,4 +73,5 @@ sys={0.000000}, clock_override_set _decode_passive_init_exp_nack_transport_prim_cb() Transport: DL len=5: e8 00 90 00 20 +sys={0.000000}, clock_override_set OK. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43248?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id7059e08aa78af4c18c2be1493fb780a31e3e862 Gerrit-Change-Number: 43248 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: iuup: Fix formatting of log line printing payload checksum error
by pespin 14 Aug '26

14 Aug '26

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43246?usp=email ) Change subject: iuup: Fix formatting of log line printing payload checksum error ...................................................................... iuup: Fix formatting of log line printing payload checksum error The payload CRC is 10bits long, hence 2 bytes are used. Printing 3 digits would be enough, but keep it as 4 digits since anyway we are storing it in uint16_t. Change-Id: Ife1de59a8257d6c857f724460b546f511db59ebd --- M src/gsm/iuup.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved osmith: Looks good to me, approved diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c index 622302a..b7aa5f9 100644 --- a/src/gsm/iuup.c +++ b/src/gsm/iuup.c @@ -923,7 +923,7 @@ return 0; payload_crc_err: - LOGP(DLIUUP, LOGL_NOTICE, "Payload Checksum error (pdu type %u): rx 0x%02x vs exp 0x%02x\n", + LOGP(DLIUUP, LOGL_NOTICE, "Payload Checksum error (pdu type %u): rx 0x%04x vs exp 0x%04x\n", pdu_type, payload_crc, payload_crc_computed); return -EIO; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43246?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ife1de59a8257d6c857f724460b546f511db59ebd Gerrit-Change-Number: 43246 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[L] Change in libosmocore[master]: iuup: Validate msgb input length in rx Initialization path
by pespin 14 Aug '26

14 Aug '26

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43247?usp=email ) Change subject: iuup: Validate msgb input length in rx Initialization path ...................................................................... iuup: Validate msgb input length in rx Initialization path Input length validation of msgb was left as a TODO in iuup_rx_initialization(), which could end up in OOB reads. Related: OS#7063 Reported-By: Adam Bedard <adam.bedard(a)gmail.com> Change-Id: Id0f7b0f63efc37376024359a3bf7fabbc40bc387 --- M src/gsm/iuup.c M tests/iuup/iuup_test.c M tests/iuup/iuup_test.err M tests/iuup/iuup_test.ok 4 files changed, 327 insertions(+), 59 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c index b7aa5f9..a233fb9 100644 --- a/src/gsm/iuup.c +++ b/src/gsm/iuup.c @@ -539,7 +539,8 @@ /* return: whether the last Init was Acked correctly and hence can transition to next state */ static bool iuup_rx_initialization(struct osmo_iuup_instance *iui, struct osmo_iuup_tnl_prim *itp) { - struct iuup_pdutype14_hdr *hdr; + struct msgb *msg = itp->oph.msg; + struct iuup_pdutype14_hdr *hdr = (struct iuup_pdutype14_hdr *)msgb_l2(msg); struct iuup_ctrl_init_hdr *ihdr; struct iuup_ctrl_init_rfci_hdr *ihdr_rfci; struct iuup_ctrl_init_tail *itail; @@ -551,9 +552,16 @@ struct osmo_iuup_rnl_prim *irp; struct osmo_iuup_tnl_prim *resp; - /* TODO: whenever we check message boundaries, length, etc. and we fail, send NACK */ + /* We expect at least the INIT header with at least 1 RFCI header: */ + if (msgb_l2len(msg) < sizeof(struct iuup_pdutype14_hdr) + + sizeof(struct iuup_ctrl_init_hdr) + + sizeof(struct iuup_ctrl_init_rfci_hdr)) { + LOGPFSML(iui->fi, LOGL_NOTICE, + "Initialization: Malformed packet, length %u too short\n", msgb_l2len(msg)); + err_cause = IUUP_ERR_CAUSE_FRAME_TOO_SHORT; + goto send_nack; + } - hdr = (struct iuup_pdutype14_hdr *)msgb_l2(itp->oph.msg); ihdr = (struct iuup_ctrl_init_hdr *)hdr->payload; if (ihdr->num_subflows_per_rfci == 0) { LOGPFSML(iui->fi, LOGL_NOTICE, "Initialization: Unexpected num_subflows=0 received\n"); @@ -562,16 +570,33 @@ } ihdr_rfci = (struct iuup_ctrl_init_rfci_hdr *)ihdr->rfci_data; + /* Iterate over RFCIs and parse and store its subflow lengths: */ do { struct osmo_iuup_rfci *rfci = &iui->config.rfci[num_rfci]; uint8_t l_size_bytes = ihdr_rfci->li + 1; + struct iuup_ctrl_init_rfci_hdr *next_ihdr_rfci = + (struct iuup_ctrl_init_rfci_hdr *)(&ihdr_rfci->subflow_length[0] + + (ihdr->num_subflows_per_rfci * l_size_bytes)); is_last = ihdr_rfci->lri; + if (num_rfci >= IUUP_MAX_RFCIS) { LOGPFSML(iui->fi, LOGL_NOTICE, "Initialization: Too many RFCIs received (%u)\n", - num_rfci); + num_rfci); err_cause = IUUP_ERR_CAUSE_UNEXPECTED_RFCI; goto send_nack; } + + /* Check contents of current RFCI are available in msgb, and if not last RFCI, + * also check for availability of next ihdr_rfci, all in one go: */ + if ((((uint8_t *)next_ihdr_rfci) + (is_last ? 0 : sizeof(struct iuup_ctrl_init_rfci_hdr))) > + msg->tail) { + LOGPFSML(iui->fi, LOGL_NOTICE, + "Initialization: Malformed packet, length %u too short\n", + msgb_l2len(msg)); + err_cause = IUUP_ERR_CAUSE_FRAME_TOO_SHORT; + goto send_nack; + } + rfci->used = 1; rfci->id = ihdr_rfci->rfci; if (l_size_bytes == 2) { @@ -588,13 +613,21 @@ } } num_rfci++; - ihdr_rfci++; - ihdr_rfci = (struct iuup_ctrl_init_rfci_hdr *)(((uint8_t *)ihdr_rfci) + ihdr->num_subflows_per_rfci * l_size_bytes); + ihdr_rfci = next_ihdr_rfci; } while (!is_last); if (ihdr->ti) { /* Timing information present */ uint8_t *buf = (uint8_t *)ihdr_rfci; uint8_t num_bytes = (num_rfci + 1) / 2; + + if (buf + num_bytes > msg->tail) { + LOGPFSML(iui->fi, LOGL_NOTICE, + "Initialization: Malformed packet, length %u too short\n", + msgb_l2len(msg)); + err_cause = IUUP_ERR_CAUSE_FRAME_TOO_SHORT; + goto send_nack; + } + iui->config.IPTIs_present = true; for (i = 0; i < num_bytes - 1; i++) { iui->config.rfci[i*2].IPTI = *buf >> 4; @@ -610,6 +643,15 @@ iui->config.IPTIs_present = false; itail = (struct iuup_ctrl_init_tail *)ihdr_rfci; } + + if (((uint8_t *)itail) + sizeof(*itail) > msg->tail) { + LOGPFSML(iui->fi, LOGL_NOTICE, + "Initialization: Malformed packet, length %u too short\n", + msgb_l2len(msg)); + err_cause = IUUP_ERR_CAUSE_FRAME_TOO_SHORT; + goto send_nack; + } + if (itail->data_pdu_type > 1) { LOGPFSML(iui->fi, LOGL_NOTICE, "Initialization: Unexpected Data PDU Type %u received\n", itail->data_pdu_type); err_cause = IUUP_ERR_CAUSE_UNEXPECTED_VALUE; @@ -643,9 +685,10 @@ resp = itp_ctrl_ack_alloc(iui, IUUP_PROC_INIT, hdr->frame_nr); iui->transport_prim_cb(&resp->oph, iui->transport_prim_priv); return ihdr->chain_ind == 0; + send_nack: LOGPFSML(iui->fi, LOGL_NOTICE, "Tx Initialization NACK cause=%u orig_message=%s\n", - err_cause, osmo_hexdump((const unsigned char *) msgb_l2(itp->oph.msg), msgb_l2len(itp->oph.msg))); + err_cause, osmo_hexdump((const unsigned char *) msgb_l2(msg), msgb_l2len(msg))); resp = tnp_ctrl_nack_alloc(iui, IUUP_PROC_INIT, err_cause, hdr->frame_nr); iui->transport_prim_cb(&resp->oph, iui->transport_prim_priv); return false; diff --git a/tests/iuup/iuup_test.c b/tests/iuup/iuup_test.c index 725a7d9..353c76a 100644 --- a/tests/iuup/iuup_test.c +++ b/tests/iuup/iuup_test.c @@ -31,6 +31,56 @@ .t_rc = { .t_ms = IUUP_TIMER_RC_T_DEFAULT, .n_max = IUUP_TIMER_RC_N_DEFAULT }, }; +/* Frame 46, "Initialization", SYS#5969 call4_Iu_Iuh.pcap + 1110 .... = PDU Type: Control Procedure (14) + .... 00.. = Ack/Nack: Procedure (0) + .... ..00 = Frame Number: 0 + 0000 .... = Mode Version: 0x0 + .... 0000 = Procedure: Initialization (0) + 1101 11.. = Header CRC: 0x37 [correct] + .... ..01 1011 0100 = Payload CRC: 0x1b4 + 000. .... = Spare: 0x0 + ...0 .... = TI: IPTIs not present (0) + .... 011. = Subflows: 3 + .... ...0 = Chain Indicator: this frame is the last frame for the procedure (0) + RFCI 1 Initialization + 0... .... = RFCI 0 LRI: Not last RFCI (0x0) + .0.. .... = RFCI 0 LI: one octet used (0x0) + ..00 0001 = RFCI 0: 1 + RFCI 0 Flow 0 Len: 81 + RFCI 0 Flow 1 Len: 103 + RFCI 0 Flow 2 Len: 60 + RFCI 6 Initialization + 1... .... = RFCI 1 LRI: Last RFCI in current frame (0x1) + .0.. .... = RFCI 1 LI: one octet used (0x0) + ..00 0110 = RFCI 1: 6 + RFCI 1 Flow 0 Len: 39 + RFCI 1 Flow 1 Len: 0 + RFCI 1 Flow 2 Len: 0 + Iu UP Mode Versions Supported: 0x0001 + 0... .... .... .... = Version 16: not supported (0x0) + .0.. .... .... .... = Version 15: not supported (0x0) + ..0. .... .... .... = Version 14: not supported (0x0) + ...0 .... .... .... = Version 13: not supported (0x0) + .... 0... .... .... = Version 12: not supported (0x0) + .... .0.. .... .... = Version 11: not supported (0x0) + .... ..0. .... .... = Version 10: not supported (0x0) + .... ...0 .... .... = Version 9: not supported (0x0) + .... .... 0... .... = Version 8: not supported (0x0) + .... .... .0.. .... = Version 7: not supported (0x0) + .... .... ..0. .... = Version 6: not supported (0x0) + .... .... ...0 .... = Version 5: not supported (0x0) + .... .... .... 0... = Version 4: not supported (0x0) + .... .... .... .0.. = Version 3: not supported (0x0) + .... .... .... ..0. = Version 2: not supported (0x0) + .... .... .... ...1 = Version 1: supported (0x1) + 0000 .... = RFCI Data Pdu Type: PDU type 0 (0x0) +*/ +const uint8_t iuup_initialization_no_iptis[] = { + 0xe0, 0x00, 0xdd, 0xb4, 0x06, 0x01, 0x51, 0x67, 0x3c, 0x86, 0x27, + 0x00, 0x00, 0x00, 0x01, 0x00 +}; + /* Frame 33, "Initialization", OS#4744 3g_call_23112021.pcapng IuUP 1110 .... = PDU Type: Control Procedure (14) @@ -665,56 +715,6 @@ struct iuup_pdutype14_hdr *hdr14; int rc; - /* Frame 46, "Initialization", SYS#5969 call4_Iu_Iuh.pcap - 1110 .... = PDU Type: Control Procedure (14) - .... 00.. = Ack/Nack: Procedure (0) - .... ..00 = Frame Number: 0 - 0000 .... = Mode Version: 0x0 - .... 0000 = Procedure: Initialization (0) - 1101 11.. = Header CRC: 0x37 [correct] - .... ..01 1011 0100 = Payload CRC: 0x1b4 - 000. .... = Spare: 0x0 - ...0 .... = TI: IPTIs not present (0) - .... 011. = Subflows: 3 - .... ...0 = Chain Indicator: this frame is the last frame for the procedure (0) - RFCI 1 Initialization - 0... .... = RFCI 0 LRI: Not last RFCI (0x0) - .0.. .... = RFCI 0 LI: one octet used (0x0) - ..00 0001 = RFCI 0: 1 - RFCI 0 Flow 0 Len: 81 - RFCI 0 Flow 1 Len: 103 - RFCI 0 Flow 2 Len: 60 - RFCI 6 Initialization - 1... .... = RFCI 1 LRI: Last RFCI in current frame (0x1) - .0.. .... = RFCI 1 LI: one octet used (0x0) - ..00 0110 = RFCI 1: 6 - RFCI 1 Flow 0 Len: 39 - RFCI 1 Flow 1 Len: 0 - RFCI 1 Flow 2 Len: 0 - Iu UP Mode Versions Supported: 0x0001 - 0... .... .... .... = Version 16: not supported (0x0) - .0.. .... .... .... = Version 15: not supported (0x0) - ..0. .... .... .... = Version 14: not supported (0x0) - ...0 .... .... .... = Version 13: not supported (0x0) - .... 0... .... .... = Version 12: not supported (0x0) - .... .0.. .... .... = Version 11: not supported (0x0) - .... ..0. .... .... = Version 10: not supported (0x0) - .... ...0 .... .... = Version 9: not supported (0x0) - .... .... 0... .... = Version 8: not supported (0x0) - .... .... .0.. .... = Version 7: not supported (0x0) - .... .... ..0. .... = Version 6: not supported (0x0) - .... .... ...0 .... = Version 5: not supported (0x0) - .... .... .... 0... = Version 4: not supported (0x0) - .... .... .... .0.. = Version 3: not supported (0x0) - .... .... .... ..0. = Version 2: not supported (0x0) - .... .... .... ...1 = Version 1: supported (0x1) - 0000 .... = RFCI Data Pdu Type: PDU type 0 (0x0) - */ - const uint8_t iuup_init[] = { - 0xe0, 0x00, 0xdd, 0xb4, 0x06, 0x01, 0x51, 0x67, 0x3c, 0x86, 0x27, - 0x00, 0x00, 0x00, 0x01, 0x00 - }; - iui = osmo_iuup_instance_alloc(iuup_test_ctx, __func__); OSMO_ASSERT(iui); osmo_iuup_instance_set_user_prim_cb(iui, _decode_passive_init_2_rfci_no_iptis_user_prim_cb, NULL); @@ -732,9 +732,9 @@ /* Send Init: */ tnp = osmo_iuup_tnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_INDICATION, IUUP_MSGB_SIZE); - tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, sizeof(iuup_init)); + tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, sizeof(iuup_initialization_no_iptis)); hdr14 = (struct iuup_pdutype14_hdr *)msgb_l2(tnp->oph.msg); - memcpy(hdr14, iuup_init, sizeof(iuup_init)); + memcpy(hdr14, iuup_initialization_no_iptis, sizeof(iuup_initialization_no_iptis)); rc = osmo_iuup_tnl_prim_up(iui, tnp); OSMO_ASSERT(rc == 0); @@ -742,6 +742,176 @@ osmo_iuup_instance_free(iui); } +static int _decode_passive_init_exp_nack_transport_prim_cb(struct osmo_prim_hdr *oph, void *ctx) +{ + struct osmo_iuup_tnl_prim *itp = (struct osmo_iuup_tnl_prim *)oph; + struct msgb *msg; + struct iuup_pdutype14_hdr *hdr; + + printf("%s()\n", __func__); + msg = oph->msg; + OSMO_ASSERT(OSMO_PRIM_HDR(&itp->oph) == OSMO_PRIM(OSMO_IUUP_TNL_UNITDATA, PRIM_OP_REQUEST)); + printf("Transport: DL len=%u: %s\n", msgb_l2len(msg), + osmo_hexdump((const unsigned char *) msgb_l2(msg), msgb_l2len(msg))); + hdr = msgb_l2(msg); + OSMO_ASSERT(hdr->pdu_type == IUUP_PDU_T_CONTROL); + OSMO_ASSERT(hdr->ack_nack == IUUP_AN_NACK); + msgb_free(msg); + return 0; +} +/* Send a malformed Initialization to UIT containing num_subflows_per_rfci = 0. + * It shall be rejected since num_subflows_per_rfci should be at least one accoridng to 3GPP TS 25.415 Figure 24 */ +void test_decode_passive_init_malformed_0subflows(void) +{ + /* Here we check the passive INIT code path, aka receiving INIT and returning INIT_ACK/NACK */ + struct osmo_iuup_instance *iui; + struct osmo_iuup_rnl_prim *rnp; + struct osmo_iuup_tnl_prim *tnp; + struct iuup_pdutype14_hdr *hdr14; + struct iuup_ctrl_init_hdr *ihdr; + uint16_t payload_crc; + int rc; + + iui = osmo_iuup_instance_alloc(iuup_test_ctx, __func__); + OSMO_ASSERT(iui); + osmo_iuup_instance_set_transport_prim_cb(iui, _decode_passive_init_exp_nack_transport_prim_cb, NULL); + + clock_override_set(0, 0); + + /* Tx CONFIG.req */ + rnp = osmo_iuup_rnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_RNL_CONFIG, PRIM_OP_REQUEST, IUUP_MSGB_SIZE); + rnp->u.config = def_configure_req; + rnp->u.config.active = false; + + rc = osmo_iuup_rnl_prim_down(iui, rnp); + OSMO_ASSERT(rc == 0); + + /* Prepare and send Init: copy iuup_initialization_no_iptis, modify setting TI=1 (becoming malformed) and dispatch it. */ + tnp = osmo_iuup_tnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_INDICATION, IUUP_MSGB_SIZE); + tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, sizeof(iuup_initialization_no_iptis)); + hdr14 = (struct iuup_pdutype14_hdr *)msgb_l2(tnp->oph.msg); + memcpy(hdr14, iuup_initialization_no_iptis, sizeof(iuup_initialization_no_iptis)); + + ihdr = (struct iuup_ctrl_init_hdr *)hdr14->payload; + ihdr->num_subflows_per_rfci = 0; + payload_crc = osmo_iuup_compute_payload_crc(msgb_l2(tnp->oph.msg), msgb_l2len(tnp->oph.msg)); + hdr14->payload_crc_hi = (payload_crc >> 8) & 0x03; + hdr14->payload_crc_lo = payload_crc & 0xff; + + rc = osmo_iuup_tnl_prim_up(iui, tnp); + OSMO_ASSERT(rc == 0); + + osmo_iuup_instance_free(iui); +} + +/* Send a malformed Initialization to UIT containing TI=1 and no IPTIs encoded. + * As a result, the msgb content is too short and should be rejected. */ +void test_decode_passive_init_malformed_ti1_no_iptis(void) +{ + /* Here we check the passive INIT code path, aka receiving INIT and returning INIT_ACK/NACK */ + struct osmo_iuup_instance *iui; + struct osmo_iuup_rnl_prim *rnp; + struct osmo_iuup_tnl_prim *tnp; + struct iuup_pdutype14_hdr *hdr14; + struct iuup_ctrl_init_hdr *ihdr; + uint16_t payload_crc; + int rc; + + iui = osmo_iuup_instance_alloc(iuup_test_ctx, __func__); + OSMO_ASSERT(iui); + osmo_iuup_instance_set_transport_prim_cb(iui, _decode_passive_init_exp_nack_transport_prim_cb, NULL); + + clock_override_set(0, 0); + + /* Tx CONFIG.req */ + rnp = osmo_iuup_rnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_RNL_CONFIG, PRIM_OP_REQUEST, IUUP_MSGB_SIZE); + rnp->u.config = def_configure_req; + rnp->u.config.active = false; + + rc = osmo_iuup_rnl_prim_down(iui, rnp); + OSMO_ASSERT(rc == 0); + + /* Prepare and send Init: copy iuup_initialization_no_iptis, modify setting TI=1 (becoming malformed) and dispatch it. */ + tnp = osmo_iuup_tnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_INDICATION, IUUP_MSGB_SIZE); + tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, sizeof(iuup_initialization_no_iptis)); + hdr14 = (struct iuup_pdutype14_hdr *)msgb_l2(tnp->oph.msg); + memcpy(hdr14, iuup_initialization_no_iptis, sizeof(iuup_initialization_no_iptis)); + + ihdr = (struct iuup_ctrl_init_hdr *)hdr14->payload; + ihdr->ti = 1; + payload_crc = osmo_iuup_compute_payload_crc(msgb_l2(tnp->oph.msg), msgb_l2len(tnp->oph.msg)); + hdr14->payload_crc_hi = (payload_crc >> 8) & 0x03; + hdr14->payload_crc_lo = payload_crc & 0xff; + + rc = osmo_iuup_tnl_prim_up(iui, tnp); + OSMO_ASSERT(rc == 0); + + osmo_iuup_instance_free(iui); +} + +/* Send a malformed Initialization to UIT containing no RFCIs. + * As a result, the msgb content is too short and should be rejected. */ +void _test_submit_iuup_initialization_trimmed(unsigned int pkt_len, const char *test_name) +{ + /* Here we check the passive INIT code path, aka receiving INIT and returning INIT_ACK/NACK */ + struct osmo_iuup_instance *iui; + struct osmo_iuup_rnl_prim *rnp; + struct osmo_iuup_tnl_prim *tnp; + struct iuup_pdutype14_hdr *hdr14; + uint16_t payload_crc; + int rc; + + OSMO_ASSERT(pkt_len <= sizeof(iuup_initialization_no_iptis)); + + iui = osmo_iuup_instance_alloc(iuup_test_ctx, test_name); + OSMO_ASSERT(iui); + osmo_iuup_instance_set_transport_prim_cb(iui, _decode_passive_init_exp_nack_transport_prim_cb, NULL); + + /* Tx CONFIG.req */ + rnp = osmo_iuup_rnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_RNL_CONFIG, PRIM_OP_REQUEST, IUUP_MSGB_SIZE); + rnp->u.config = def_configure_req; + rnp->u.config.active = false; + + rc = osmo_iuup_rnl_prim_down(iui, rnp); + OSMO_ASSERT(rc == 0); + + /* Prepare and send Init: copy iuup_initialization_no_iptis, modify setting TI=1 (becoming malformed) and dispatch it. */ + tnp = osmo_iuup_tnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_INDICATION, IUUP_MSGB_SIZE); + tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, pkt_len); + hdr14 = (struct iuup_pdutype14_hdr *)msgb_l2(tnp->oph.msg); + memcpy(hdr14, iuup_initialization_no_iptis, pkt_len); + + payload_crc = osmo_iuup_compute_payload_crc(msgb_l2(tnp->oph.msg), msgb_l2len(tnp->oph.msg)); + hdr14->payload_crc_hi = (payload_crc >> 8) & 0x03; + hdr14->payload_crc_lo = payload_crc & 0xff; + + rc = osmo_iuup_tnl_prim_up(iui, tnp); + OSMO_ASSERT(rc == 0); + + osmo_iuup_instance_free(iui); +} +void test_decode_passive_init_malformed_no_rfci(void) +{ + unsigned int pkt_len = sizeof(struct iuup_pdutype14_hdr) + sizeof(struct iuup_ctrl_init_hdr); + clock_override_set(0, 0); + _test_submit_iuup_initialization_trimmed(pkt_len, __func__); +} + +/* Send a malformed Initialization to UIT containing malformed RFCIs. + * As a result, the msgb content is too short and should be rejected. */ +void test_decode_passive_init_malformed_rfci_too_short(void) +{ + unsigned int pkt_len = sizeof(struct iuup_pdutype14_hdr) + sizeof(struct iuup_ctrl_init_hdr) + 2; + clock_override_set(0, 0); + _test_submit_iuup_initialization_trimmed(pkt_len, __func__); +} + +void test_decode_passive_init_malformed_missing_last_byte(void) +{ + clock_override_set(0, 0); + _test_submit_iuup_initialization_trimmed(sizeof(iuup_initialization_no_iptis) - 1, __func__); +} + int main(int argc, char **argv) { iuup_test_ctx = talloc_named_const(NULL, 0, "iuup_test"); @@ -762,6 +932,11 @@ test_passive_init(); test_passive_init_retrans(); test_decode_passive_init_2_rfci_no_iptis(); + test_decode_passive_init_malformed_0subflows(); + test_decode_passive_init_malformed_ti1_no_iptis(); + test_decode_passive_init_malformed_no_rfci(); + test_decode_passive_init_malformed_rfci_too_short(); + test_decode_passive_init_malformed_missing_last_byte(); printf("OK.\n"); } diff --git a/tests/iuup/iuup_test.err b/tests/iuup/iuup_test.err index fe1ed397..0298420 100644 --- a/tests/iuup/iuup_test.err +++ b/tests/iuup/iuup_test.err @@ -53,3 +53,38 @@ DLIUUP IuUP(test_decode_passive_init_2_rfci_no_iptis){Initialisation}: Tx Initialization ACK DLIUUP IuUP(test_decode_passive_init_2_rfci_no_iptis){Initialisation}: state_chg to SMpSDU_Data_Transfer_Ready DLIUUP IuUP(test_decode_passive_init_2_rfci_no_iptis){SMpSDU_Data_Transfer_Ready}: Deallocated +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){NULL}: Allocated +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){NULL}: Received Event IuUP-CONFIG-req +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){NULL}: state_chg to Initialisation +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){Initialisation}: Received Event INIT +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){Initialisation}: Initialization: Unexpected num_subflows=0 received +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){Initialisation}: Tx Initialization NACK cause=20 orig_message=e0 00 dc 62 00 01 51 67 3c 86 27 00 00 00 01 00 +DLIUUP IuUP(test_decode_passive_init_malformed_0subflows){Initialisation}: Deallocated +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){NULL}: Allocated +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){NULL}: Received Event IuUP-CONFIG-req +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){NULL}: state_chg to Initialisation +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){Initialisation}: Received Event INIT +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){Initialisation}: Initialization: Malformed packet, length 16 too short +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){Initialisation}: Tx Initialization NACK cause=8 orig_message=e0 00 dc 06 16 01 51 67 3c 86 27 00 00 00 01 00 +DLIUUP IuUP(test_decode_passive_init_malformed_ti1_no_iptis){Initialisation}: Deallocated +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){NULL}: Allocated +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){NULL}: Received Event IuUP-CONFIG-req +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){NULL}: state_chg to Initialisation +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){Initialisation}: Received Event INIT +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){Initialisation}: Initialization: Malformed packet, length 5 too short +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){Initialisation}: Tx Initialization NACK cause=8 orig_message=e0 00 dc cc 06 +DLIUUP IuUP(test_decode_passive_init_malformed_no_rfci){Initialisation}: Deallocated +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){NULL}: Allocated +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){NULL}: Received Event IuUP-CONFIG-req +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){NULL}: state_chg to Initialisation +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){Initialisation}: Received Event INIT +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){Initialisation}: Initialization: Malformed packet, length 7 too short +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){Initialisation}: Tx Initialization NACK cause=8 orig_message=e0 00 df 7d 06 01 51 +DLIUUP IuUP(test_decode_passive_init_malformed_rfci_too_short){Initialisation}: Deallocated +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){NULL}: Allocated +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){NULL}: Received Event IuUP-CONFIG-req +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){NULL}: state_chg to Initialisation +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Received Event INIT +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Initialization: Malformed packet, length 15 too short +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Tx Initialization NACK cause=8 orig_message=e0 00 df f7 06 01 51 67 3c 86 27 00 00 00 01 +DLIUUP IuUP(test_decode_passive_init_malformed_missing_last_byte){Initialisation}: Deallocated diff --git a/tests/iuup/iuup_test.ok b/tests/iuup/iuup_test.ok index 57baba9..204c290 100644 --- a/tests/iuup/iuup_test.ok +++ b/tests/iuup/iuup_test.ok @@ -58,4 +58,19 @@ _decode_passive_init_2_rfci_no_iptis_user_prim_cb(): Initialization decoded fine! _decode_passive_init_2_rfci_no_iptis_transport_prim_cb() Transport: DL len=4: e4 00 24 00 +sys={0.000000}, clock_override_set +_decode_passive_init_exp_nack_transport_prim_cb() +Transport: DL len=5: e8 00 90 00 50 +sys={0.000000}, clock_override_set +_decode_passive_init_exp_nack_transport_prim_cb() +Transport: DL len=5: e8 00 90 00 20 +sys={0.000000}, clock_override_set +_decode_passive_init_exp_nack_transport_prim_cb() +Transport: DL len=5: e8 00 90 00 20 +sys={0.000000}, clock_override_set +_decode_passive_init_exp_nack_transport_prim_cb() +Transport: DL len=5: e8 00 90 00 20 +sys={0.000000}, clock_override_set +_decode_passive_init_exp_nack_transport_prim_cb() +Transport: DL len=5: e8 00 90 00 20 OK. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43247?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id0f7b0f63efc37376024359a3bf7fabbc40bc387 Gerrit-Change-Number: 43247 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log August 2026