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

[M] Change in pysim[master]: GP: LOAD/STORE DATA chunk size from SCP overhead
by laforge 07 Aug '26

07 Aug '26

Attention is currently required from: Hoernchen. laforge has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/pysim/+/43172?usp=email ) Change subject: GP: LOAD/STORE DATA chunk size from SCP overhead ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/43172?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ic208f3959a38896f64fb6ccefb24cc360a3ac3a2 Gerrit-Change-Number: 43172 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Comment-Date: Fri, 07 Aug 2026 07:50:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in pysim[master]: GP: mixed PSK TLS PUT KEY (Amendment B Table 3-13)
by laforge 07 Aug '26

07 Aug '26

Attention is currently required from: Hoernchen, dexter, lynxis lazus. laforge has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/pysim/+/43136?usp=email ) Change subject: GP: mixed PSK TLS PUT KEY (Amendment B Table 3-13) ...................................................................... Patch Set 1: (2 comments) File pySim/global_platform/__init__.py: https://gerrit.osmocom.org/c/pysim/+/43136/comment/f9c966a0_e5a90a56?usp=em… : PS1, Line 655: (b'\x85' + bertlv_encode_len(len(block)) + block + : bytes([len(kcv)]) + kcv) I would have preferred the use of our object-oriented BER_TLV_IE encoder for the 0x85 as well as some new Transcodable-derived class for the LV KCV section. That would feel more "pysim-onic" to me ,than the low-level byte-concatenation here in a rather high-level method. Or alternatively some kind of costruct. https://gerrit.osmocom.org/c/pysim/+/43136/comment/fb5dd59c_19f11f10?usp=em… : PS1, Line 1113: wouldn't this return more than the three bytes that the comment above indicates? But then, I see, the existing functions above do the same and the truncation happens in the shared compute_kcv() below. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/43136?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I480a9d049a052aa5ae54fe6e2771dba44e89434d Gerrit-Change-Number: 43136 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 07 Aug 2026 07:48:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in ...pyosmocom[master]: tlv: preserve the comprehension bit
by Hoernchen 07 Aug '26

07 Aug '26

Attention is currently required from: laforge. Hoernchen has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email ) Change subject: tlv: preserve the comprehension bit ...................................................................... Patch Set 1: (1 comment) File src/osmocom/tlv.py: https://gerrit.osmocom.org/c/python/pyosmocom/+/43171/comment/97067343_d68b… : PS1, Line 638: if x.tag > 0xff: : # two byte tag: the comprehension bit is 0x8000! : if x.tag & 0x8000 == 0: : print("Fixing up COMPR_TLV_IE class %s: tag=0x%x has no comprehension bit" % (name, x.tag)) : x.tag = x.tag | 0x8000 > what exactly is the difference here? isn't this change semantically identical? No, because the original condition was AND, so if either part didnt match it would fall through to the elif and that is what clobbered the two byte tag. As the comment says two byte is 0x8000 - precisely the part in the previous AND that would fail. -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: Ifae37785e2a586d9130d154bc7244f35fa6f2a55 Gerrit-Change-Number: 43171 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 06 Aug 2026 21:31:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

1 0

[M] Change in pysim[master]: GP: LOAD/STORE DATA chunk size from SCP overhead
by Hoernchen 07 Aug '26

07 Aug '26

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/43172?usp=email ) Change subject: GP: LOAD/STORE DATA chunk size from SCP overhead ...................................................................... GP: LOAD/STORE DATA chunk size from SCP overhead SCP.overhead was so far set at construction time (SCP02: 8, SCP03: s_mode), so the C-MAC length only. Unfortunately sec lvl >= 3 pads the data field to the cipher block size before encryption, so the real worst-case overhead is larger, scc.max_cmd_len (255 - overhead) was too big, and ADF_SD.load() used a hardcoded chunk_len=240. Real world issue with a 286 byte CAP + SCP02 + sec lvl 3: - 240-byte LOAD block is padded to 248, - encrypted - gets 8 byte C-MAC appended -> Lc = 256 That dies with a weird "ValueError: bytes must be in range(0, 256)". The only "fix" for that was to downgrade the seclevel. STORE DATA has the same overflow with large max_cmd_len (247 + padding + MAC = 256 as well). Therefore the overhead must be properly calculated from the sec level. While at it adjust the error in case I missed something to get a more useful ValueError. Change-Id: Ic208f3959a38896f64fb6ccefb24cc360a3ac3a2 --- M pySim/global_platform/__init__.py M pySim/global_platform/scp.py M tests/unittests/test_globalplatform.py 3 files changed, 220 insertions(+), 12 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/72/43172/1 diff --git a/pySim/global_platform/__init__.py b/pySim/global_platform/__init__.py index 31f3907..d14c6ff 100644 --- a/pySim/global_platform/__init__.py +++ b/pySim/global_platform/__init__.py @@ -868,23 +868,32 @@ load_parser_from_grp.add_argument('--from-hex', type=is_hexstr, help='load from hex string') load_parser_from_grp.add_argument('--from-file', type=argparse.FileType('rb', 0), help='load from binary file') load_parser_from_grp.add_argument('--from-cap-file', type=argparse.FileType('rb', 0), help='load from JAVA-card CAP file') + load_parser.add_argument('--chunk-len', type=auto_uint8, default=None, + help='Block size for the LOAD command; default: as large as the current secure channel overhead permits, at most 240') @cmd2.with_argparser(load_parser) def do_load(self, opts): """Perform a GlobalPlatform LOAD command. (We currently only support loading without DAP and without ciphering.)""" if opts.from_hex is not None: - self.load(h2b(opts.from_hex)) + self.load(h2b(opts.from_hex), opts.chunk_len) elif opts.from_file is not None: - self.load(opts.from_file.read()) + self.load(opts.from_file.read(), opts.chunk_len) elif opts.from_cap_file is not None: cap = CapFile(opts.from_cap_file) - self.load(cap.get_loadfile()) + self.load(cap.get_loadfile(), opts.chunk_len) else: raise ValueError('load source not specified!') - def load(self, contents:bytes, chunk_len:int = 240): - # TODO:tune chunk_len based on the overhead of the used SCP? + def load(self, contents:bytes, chunk_len:Optional[int] = None): + # scc.max_cmd_len knows the overhead the currently active SCP + # 240 is the old default, keep it for now. + max_chunk_len = self._cmd.lchan.scc.max_cmd_len + if chunk_len is None: + chunk_len = min(240, max_chunk_len) + elif not 1 <= chunk_len <= max_chunk_len: + raise ValueError('chunk_len must be in range 1..%u (limited by the overhead of the current secure channel)' % + max_chunk_len) # build TLV according to GPC_SPE_034 section 11.6.2.3 / Table 11-58 for unencrypted case remainder = b'\xC4' + bertlv_encode_len(len(contents)) + contents # transfer this in various chunks to the card @@ -923,6 +932,8 @@ install_cap_parser_inst_prm_grp.add_argument('--install-parameters-stk', type=is_hexstr, default=None, help='Load Parameters (ETSI TS 102 226, section 8.2.1.3.2.1)') + install_cap_parser.add_argument('--chunk-len', type=auto_uint8, default=None, + help='Block size for the LOAD command; default: as large as the current secure channel overhead permits, at most 240') @cmd2.with_argparser(install_cap_parser) def do_install_cap(self, opts): @@ -961,7 +972,7 @@ self._cmd.poutput("step #1: install for load...") self.do_install_for_load("--load-file-aid %s --security-domain-aid %s" % (load_file_aid, security_domain_aid)) self._cmd.poutput("step #2: load...") - self.load(load_file) + self.load(load_file, opts.chunk_len) self._cmd.poutput("step #3: install_for_install (and make selectable)...") self.do_install_for_install("--load-file-aid %s --module-aid %s --application-aid %s --install-parameters %s --make-selectable" % (load_file_aid, module_aid, application_aid, install_parameters)) diff --git a/pySim/global_platform/scp.py b/pySim/global_platform/scp.py index a5fcf51..7573b47 100644 --- a/pySim/global_platform/scp.py +++ b/pySim/global_platform/scp.py @@ -182,6 +182,20 @@ """Should we perform R-ENC?""" return self.security_level & 0x20 + @property + def overhead(self) -> int: + """Worst-case len that wrapping a command APDU adds to its data field at the + current sec level is (255 - overhead), C-MAC + C-DECRYPTION encryption padding.""" + if not self.do_cmac: + return 0 + if not self.do_cenc: + return self.mac_len + # C-DECRYPTION pads with ('80'+['00'...] at least 1 byte) up to + # the cipher block size + C-MAC on top -> largest usable data field + # is one byte less than the largest block-size multiple within 255 - mac_len. + bs = self.sk.blocksize + return 255 - ((255 - self.mac_len) // bs * bs - 1) + def __str__(self) -> str: return "%s[%02x]" % (self.__class__.__name__, self.security_level) @@ -260,10 +274,8 @@ # Key Version Number 0x70 is a non-spec special-case of sysmoISIM-SJA2/SJA5 and possibly more sysmocom products # Key Version Number 0x01 is a non-spec special-case of sysmoUSIM-SJS1 kvn_ranges = [[0x01, 0x01], [0x20, 0x2f], [0x70, 0x70]] - - def __init__(self, *args, **kwargs): - self.overhead = 8 - super().__init__(*args, **kwargs) + # C-MAC (Single DES + final 3DES, B.1.2.2) is always one full DES block + mac_len = 8 def dek_encrypt(self, plaintext:bytes) -> bytes: # See also GPC section B.1.1.2, E.4.7, and E.4.1 @@ -338,10 +350,16 @@ # CMAC on modified APDU mlc = lc + 8 clac = cla | CLA_SM + if mlc >= 256: + raise ValueError('Modified Lc (%u) would exceed maximum when appending 8 bytes of mac' % mlc) mac = self.sk.calc_mac_1des(bytes([clac]) + apdu[1:4] + bytes([mlc]) + data) if self.do_cenc: + padded_data = pad80(data, 8) + if len(padded_data) + 8 >= 256: + raise ValueError('Modified Lc (%u) would exceed maximum when appending padding and mac' % + (len(padded_data) + 8)) k = DES3.new(self.sk.enc, DES.MODE_CBC, b'\x00'*8) - data = k.encrypt(pad80(data, 8)) + data = k.encrypt(padded_data) lc = len(data) lc += 8 @@ -477,9 +495,13 @@ def __init__(self, *args, **kwargs): self.s_mode = kwargs.pop('s_mode', 8) - self.overhead = self.s_mode super().__init__(*args, **kwargs) + @property + def mac_len(self) -> int: + # C-MAC truncated to 8 in S8 or 16 bytes in S16 mode + return self.s_mode + def dek_encrypt(self, plaintext:bytes) -> bytes: cipher = AES.new(self.card_keys.dek, AES.MODE_CBC, b'\x00'*16) return cipher.encrypt(plaintext) diff --git a/tests/unittests/test_globalplatform.py b/tests/unittests/test_globalplatform.py index d016d7d..98ff02e 100644 --- a/tests/unittests/test_globalplatform.py +++ b/tests/unittests/test_globalplatform.py @@ -424,5 +424,180 @@ load_parameters = gen_install_parameters() self.assertEqual(load_parameters, 'c900') +class SCP_Overhead_Test(unittest.TestCase): + """SCP.overhead varies according to the current security level: + C-MAC + at level >= 3 the worst-case padding! + """ + + def _scp02(self, security_level): + scp = SCP02(card_keys=ck_3des_70) + scp.sk = Scp02SessionKeys(0x0001, ck_3des_70) + scp.security_level = security_level + return scp + + def _scp03(self, security_level, s_mode=8): + scp = SCP03(card_keys=KEYSET_AES128, s_mode=s_mode) + scp.sk = Scp03SessionKeys(KEYSET_AES128, b'\x00' * s_mode, b'\x11' * s_mode) + scp.security_level = security_level + return scp + + def test_scp02(self): + self.assertEqual(self._scp02(0x00).overhead, 0) # no wrapping at all + self.assertEqual(self._scp02(0x01).overhead, 8) # C-MAC + self.assertEqual(self._scp02(0x03).overhead, 16) # C-MAC + C-DEC: pad80 to 8, largest fit 239 + + def test_scp03_s8(self): + self.assertEqual(self._scp03(0x00).overhead, 0) + self.assertEqual(self._scp03(0x01).overhead, 8) + self.assertEqual(self._scp03(0x03).overhead, 16) # pad80 to 16 within 247 -> 240, minus pad byte + self.assertEqual(self._scp03(0x33).overhead, 16) # R-MAC/R-ENC add no *command* overhead + + def test_scp03_s16(self): + self.assertEqual(self._scp03(0x01, s_mode=16).overhead, 16) + self.assertEqual(self._scp03(0x03, s_mode=16).overhead, 32) # pad80 to 16 within 239 -> 224, minus pad byte + + +class SCP_Lc_Limit_Test_Base: + """Test wrap_cmd_apdu() boundary handling: data of (255 - overhead) must produce Lc <= 255 else ValueError""" + + def _load_apdu(self, data_len): + return h2b('80E80000') + bytes([data_len]) + b'\xa5' * data_len + + def _check_boundary(self, scp): + fits = 255 - scp.overhead + wrapped = scp.wrap_cmd_apdu(self._load_apdu(fits)) + self.assertLessEqual(wrapped[4], 255) + self.assertEqual(len(wrapped), 5 + wrapped[4]) # case #3: header + Lc bytes, no Le + with self.assertRaises(ValueError) as ctx: + scp.wrap_cmd_apdu(self._load_apdu(fits + 1)) + self.assertIn('Lc', str(ctx.exception)) + + +class SCP02_Lc_Limit_Test(SCP_Lc_Limit_Test_Base, unittest.TestCase): + """Same session vectors as SCP02_Auth_Test""" + + def setUp(self): + self.scp02 = SCP02(card_keys=ck_3des_70) + self.scp02.gen_init_update_apdu(host_challenge=h2b('40A62C37FA6304F8')) + self.scp02.parse_init_update_resp(h2b('00000000000000000000700200016B4524ABEE7CF32EA3838BC148F3')) + self.scp02.gen_ext_auth_apdu() + + def test_cmac_only(self): + self.scp02.security_level = 0x01 + self._check_boundary(self.scp02) # 247 fits, 248 raises + + def test_cmac_cdec(self): + self.scp02.security_level = 0x03 + self._check_boundary(self.scp02) # 239 fits (-> Lc 248), 240 raises (would be 256) + + def test_cmac_cdec_wrapped_lc(self): + # my actual failing case: 240 bytes at level 3 + self.scp02.security_level = 0x03 + wrapped = self.scp02.wrap_cmd_apdu(self._load_apdu(239)) + self.assertEqual(wrapped[4], 248) # 239 -> pad80 -> 240 ciphertext + 8 mac + + +class SCP03_Lc_Limit_Test(SCP_Lc_Limit_Test_Base, unittest.TestCase): + """Session keys derived directly""" + + def _scp03(self, security_level, s_mode): + scp = SCP03(card_keys=KEYSET_AES128, s_mode=s_mode) + scp.sk = Scp03SessionKeys(KEYSET_AES128, b'\x00' * s_mode, b'\x11' * s_mode) + scp.security_level = security_level + return scp + + def test_s8_cmac_only(self): + self._check_boundary(self._scp03(0x01, 8)) # 247 fits, 248 raises + + def test_s8_cmac_cdec(self): + self._check_boundary(self._scp03(0x03, 8)) # 239 fits, 240 raises + + def test_s16_cmac_only(self): + self._check_boundary(self._scp03(0x01, 16)) # 239 fits, 240 raises + + def test_s16_cmac_cdec(self): + self._check_boundary(self._scp03(0x03, 16)) # 223 fits, 224 raises + + +class _FakeSccForLoad: + """mock lchan.scc: records LOAD APDUs, optionally wrapping them through a real SCP + instance first where the Lc overflow used to blow up""" + + def __init__(self, max_cmd_len=255, scp=None): + self.max_cmd_len = max_cmd_len + self.scp = scp + self.sent = [] + self.wrapped = [] + + def send_apdu_checksw(self, apdu, sw='9000'): + self.sent.append(apdu.lower()) + if self.scp: + self.wrapped.append(self.scp.wrap_cmd_apdu(h2b(apdu))) + return ('', '9000') + + +class Load_ChunkLen_Test(unittest.TestCase): + """ADF_SD.load() chunking: block size must use scc.max_cmd_len""" + + payload = b'\xaa' * 500 # actual real world case LOAD TLV: C4 + 8201f4 + 500 = 504 total + + def _sd(self, scc): + cmd = type('_Cmd', (), {'lchan': type('_Lchan', (), {'scc': scc})(), + 'poutput': lambda self, *args: None})() + # cmd2 CommandSet has a r/o _cmd property -> shadow it + _SD = type('_SD', (ADF_SD.AddlShellCommands,), {'_cmd': cmd}) + return _SD.__new__(_SD) + + def _blocks(self, scc): + """Get (p1, p2, lc) from LOAD APDU""" + for apdu in scc.sent: + self.assertEqual(apdu[0:4], '80e8') + yield int(apdu[4:6], 16), int(apdu[6:8], 16), int(apdu[8:10], 16) + + def test_default_no_scp(self): + """Without SCP the old 240 byte block size is kept, no idea what else might rely on this number""" + scc = _FakeSccForLoad(max_cmd_len=255) + self._sd(scc).load(self.payload) + blocks = list(self._blocks(scc)) + self.assertEqual([b[2] for b in blocks], [240, 240, 24]) + self.assertEqual([b[0] for b in blocks], [0x00, 0x00, 0x80]) # P1: last block flagged + self.assertEqual([b[1] for b in blocks], [0, 1, 2]) # P2: block num + + def test_default_scp02_level3(self): + """max_cmd_len 239 (SCP02 lvl 3) squeezes the blocks""" + scc = _FakeSccForLoad(max_cmd_len=239) + self._sd(scc).load(self.payload) + self.assertEqual([b[2] for b in list(self._blocks(scc))], [239, 239, 26]) + + def test_explicit_chunk_len(self): + scc = _FakeSccForLoad(max_cmd_len=255) + self._sd(scc).load(self.payload, chunk_len=100) + self.assertEqual([b[2] for b in list(self._blocks(scc))], [100] * 5 + [4]) + + def test_explicit_chunk_len_too_large(self): + scc = _FakeSccForLoad(max_cmd_len=239) + with self.assertRaises(ValueError): + self._sd(scc).load(self.payload, chunk_len=240) + self.assertEqual(scc.sent, []) # nothing sent! + + def test_explicit_chunk_len_zero(self): + scc = _FakeSccForLoad(max_cmd_len=255) + with self.assertRaises(ValueError): + self._sd(scc).load(self.payload, chunk_len=0) + + def test_end_to_end_scp02_level3(self): + """original failure: 286 byte CAP + SCP02 lvl 3""" + scp02 = SCP02(card_keys=ck_3des_70) + scp02.gen_init_update_apdu(host_challenge=h2b('40A62C37FA6304F8')) + scp02.parse_init_update_resp(h2b('00000000000000000000700200016B4524ABEE7CF32EA3838BC148F3')) + scp02.gen_ext_auth_apdu() + scp02.security_level = 0x03 + scc = _FakeSccForLoad(max_cmd_len=255 - scp02.overhead, scp=scp02) + self._sd(scc).load(b'\x5a' * 286) + self.assertEqual(len(scc.sent), 2) # 289 byte TLV in blocks of 239 + for wrapped in scc.wrapped: + self.assertLessEqual(wrapped[4], 255) + + if __name__ == "__main__": unittest.main() -- To view, visit https://gerrit.osmocom.org/c/pysim/+/43172?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ic208f3959a38896f64fb6ccefb24cc360a3ac3a2 Gerrit-Change-Number: 43172 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>

1 0

[M] Change in ...pyosmocom[master]: tlv: preserve the comprehension bit
by laforge 07 Aug '26

07 Aug '26

Attention is currently required from: Hoernchen. laforge has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email ) Change subject: tlv: preserve the comprehension bit ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/osmocom/tlv.py: https://gerrit.osmocom.org/c/python/pyosmocom/+/43171/comment/1567ed43_a4ce… : PS1, Line 638: if x.tag > 0xff: : # two byte tag: the comprehension bit is 0x8000! : if x.tag & 0x8000 == 0: : print("Fixing up COMPR_TLV_IE class %s: tag=0x%x has no comprehension bit" % (name, x.tag)) : x.tag = x.tag | 0x8000 what exactly is the difference here? isn't this change semantically identical? -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: Ifae37785e2a586d9130d154bc7244f35fa6f2a55 Gerrit-Change-Number: 43171 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Comment-Date: Thu, 06 Aug 2026 16:33:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in onomondo-eim[master]: esipa_rest_utils: add support for PSMOs added in SGP.32 V.1.2
by dexter 07 Aug '26

07 Aug '26

Attention is currently required from: fixeria, jolly. dexter has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/onomondo-eim/+/42881?usp=email ) Change subject: esipa_rest_utils: add support for PSMOs added in SGP.32 V.1.2 ...................................................................... Patch Set 9: (1 comment) File contrib/rest_api_resource_schema.json: https://gerrit.osmocom.org/c/onomondo-eim/+/42881/comment/452d1efe_6e49b3cd… : PS9, Line 169: , > This would allow 0-9 and A-F and ','. I see it also in other definitions above and below. Are you sure? I always thought the ',' is a delimiter. I used SGP.32, section 6.4.1. as an example. They also use a ',' in their definitions. -- To view, visit https://gerrit.osmocom.org/c/onomondo-eim/+/42881?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: onomondo-eim Gerrit-Branch: master Gerrit-Change-Id: I03cdd70065a83dfc611d614cf32d817c13fad347 Gerrit-Change-Number: 42881 Gerrit-PatchSet: 9 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: jolly <andreas(a)eversberg.eu> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 06 Aug 2026 15:57:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>

1 0

[M] Change in ...pyosmocom[master]: tlv: preserve the comprehension bit
by Hoernchen 07 Aug '26

07 Aug '26

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email ) Change subject: tlv: preserve the comprehension bit ...................................................................... tlv: preserve the comprehension bit The comprehension bit is data, not part of the tag: TS 101 220 7.1.1 defines it as an instruction to the receiver for treating IEs it does not understand, which is why is_tag_compatible() masks it when matching -> _encode_tag() re-emits the class tag, but ComprTlvMeta forces CR, so decode(x)/encode() did not round trip properly for IEs without the bit set That is not hypothetical! GP Amendment B administration session parameters stored by UICCs in an EF carry the CAT TLVs inside tag 0x84 with the bit mixed: - set on Command details and Device identities - clear on the optional ones That means reading and writing the same data led to not writing the same data at all! The cleanest fix here is to tri-state COMPR_TLV_IE.comprehension: - None for an IE built from scratch which encode as before - True/False for one parsed from a file which will now properly roundtrip. _encode_tag() properly passes the flag to comprehensiontlv_encode_tag() explicitly instead of deriving it from the int: The int can't express "CR clear" for a one-byte tag at all, and silently drops the flag for a two-byte one.... While at it fix ComprTlvMeta which would destroy two-byte tags instead of setting the flag, currently tag=0x8123 -> 0x81a3. Change-Id: Ifae37785e2a586d9130d154bc7244f35fa6f2a55 --- M src/osmocom/tlv.py M tests/test_tlv.py 2 files changed, 63 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/python/pyosmocom refs/changes/71/43171/1 diff --git a/src/osmocom/tlv.py b/src/osmocom/tlv.py index ada7380..9ca8728 100644 --- a/src/osmocom/tlv.py +++ b/src/osmocom/tlv.py @@ -635,20 +635,28 @@ if x.tag: # we currently assume that the tag values always have the comprehension bit set; # let's fix it up if a derived class has forgotten about that - if x.tag > 0xff and x.tag & 0x8000 == 0: - print("Fixing up COMPR_TLV_IE class %s: tag=0x%x has no comprehension bit" % (name, x.tag)) - x.tag = x.tag | 0x8000 + if x.tag > 0xff: + # two byte tag: the comprehension bit is 0x8000! + if x.tag & 0x8000 == 0: + print("Fixing up COMPR_TLV_IE class %s: tag=0x%x has no comprehension bit" % (name, x.tag)) + x.tag = x.tag | 0x8000 elif x.tag & 0x80 == 0: print("Fixing up COMPR_TLV_IE class %s: tag=0x%x has no comprehension bit" % (name, x.tag)) x.tag = x.tag | 0x80 return x class COMPR_TLV_IE(TLV_IE, metaclass=ComprTlvMeta): - """TLV_IE formated as COMPREHENSION-TLV as described in ETSI TS 101 220.""" + """TLV_IE formated as COMPREHENSION-TLV as described in ETSI TS 101 220. + - parsed IEs remember the bit and are emitted unchanged + - constructed IEs leave self.comprehension == None + and encode from class tag with forced CR by ComprTlvMeta with default "always CR" assumption + """ def __init__(self, **kwargs): super().__init__(**kwargs) - self.comprehension = False + # None: not parsed from file etc, so encode from the class tag + # True/False: as received + self.comprehension = None @classmethod def _decode_tag(cls, do: bytes) -> Tuple[dict, bytes]: @@ -662,6 +670,19 @@ def _parse_len(cls, do: bytes) -> Tuple[int, bytes]: return bertlv_parse_len(do) + @staticmethod + def _cr_mask(tag: int) -> int: + # The raw tag is always 0x7f_xx_xx for the 3 byte variant + return 0x8000 if tag > 0xffff else 0x80 + + def from_tlv(self, do: bytes, context: dict = {}): + """Record the comprehension bit and decode as usual.""" + if len(do): + rawtag, _remainder = self._parse_tag_raw(do) + if rawtag: + self.comprehension = bool(rawtag & self._cr_mask(rawtag)) + return super().from_tlv(do, context=context) + def is_tag_compatible(self, rawtag: int) -> bool: """Override is_tag_compatible as we need to mask out the comprehension bit when doing compares.""" @@ -672,7 +693,14 @@ return ctag & 0x7f == rawtag & 0x7f def _encode_tag(self) -> bytes: - return comprehensiontlv_encode_tag(self._compute_tag()) + # The class tag has a forced CR bit by ComprTlvMeta, so comprehensiontlv_encode_tag() + # gets bare tag + our explicit flag. + # The tag here is _just_ the class tag like 0x8123, not the raw tag like 0x7f8123 + # so we check for 0xff and NOT 0xffff as above. + tag = self._compute_tag() + bit = 0x8000 if tag > 0xff else 0x80 + compr = self.comprehension if self.comprehension is not None else bool(tag & bit) + return comprehensiontlv_encode_tag({'tag': tag & ~bit, 'comprehension': compr}) def _encode_len(self, val: bytes) -> bytes: return bertlv_encode_len(len(val)) diff --git a/tests/test_tlv.py b/tests/test_tlv.py index 6d33492..4d06d1d 100755 --- a/tests/test_tlv.py +++ b/tests/test_tlv.py @@ -18,7 +18,7 @@ import unittest from construct import Int8ub, GreedyBytes -from osmocom.tlv import COMPACT_TLV_IE, IE, TLV_IE_Collection, Transcodable, flatten_dict_lists, camel_to_snake +from osmocom.tlv import COMPACT_TLV_IE, COMPR_TLV_IE, IE, TLV_IE_Collection, Transcodable, flatten_dict_lists, camel_to_snake from osmocom.tlv import bertlv_encode_len, bertlv_parse_len, bertlv_parse_one, bertlv_parse_tag from osmocom.tlv import comprehensiontlv_encode_tag, comprehensiontlv_parse_tag from osmocom.tlv import dgi_encode_len, dgi_parse_len @@ -74,6 +74,34 @@ res = comprehensiontlv_encode_tag({'tag': 0x1234, 'comprehension':True}) self.assertEqual(res, b'\x7f\x92\x34') + def test_ComprTlvIeCrRoundTrip(self): + """comprehension bit is data (TS 101 220 7.1.1) and has to survive decode/encode round trips. + It is clear in stored files, e.g. GP Amd B administration session parameters in a card EF + """ + class MyIE(COMPR_TLV_IE, tag=0xbe): + _construct = GreedyBytes + + for encoded in (b'\xbe\x02\xca\xfe', b'\x3e\x02\xca\xfe'): + ie = MyIE() + ie.from_tlv(encoded) + self.assertEqual(ie.to_tlv(), encoded) + + # ...but an IE we build from scratch still encodes from the class tag (CR set) + ie = MyIE() + ie.from_bytes(b'\xca\xfe') + self.assertIsNone(ie.comprehension) + self.assertEqual(ie.to_tlv(), b'\xbe\x02\xca\xfe') + + def test_ComprTlvIeCrRoundTripTwoByteTag(self): + """check that we do not mangle two byte tags""" + class MyLongIE(COMPR_TLV_IE, tag=0x8123): + _construct = GreedyBytes + + for encoded in (b'\x7f\x81\x23\x01\xff', b'\x7f\x01\x23\x01\xff'): + ie = MyLongIE() + ie.from_tlv(encoded) + self.assertEqual(ie.to_tlv(), encoded) + class TestDgiTlv(unittest.TestCase): def test_DgiTlvLenEnc(self): self.assertEqual(dgi_encode_len(10), b'\x0a') -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: Ifae37785e2a586d9130d154bc7244f35fa6f2a55 Gerrit-Change-Number: 43171 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>

1 0

[XL] Change in osmo-trx[master]: libosmo-trx/client: add TRXC client (command queue) API
by fixeria 06 Aug '26

06 Aug '26

Attention is currently required from: pespin. fixeria has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/osmo-trx/+/43108?usp=email ) Change subject: libosmo-trx/client: add TRXC client (command queue) API ...................................................................... Patch Set 3: (1 comment) File libosmo-trx/src/trxc_client.c: https://gerrit.osmocom.org/c/osmo-trx/+/43108/comment/005a5f69_0cfa9b1d?usp… : PS3, Line 474: sf->cb(client, 0, sf->cb_data); > By freeing "sf" after the cb, you may end up in a double free if the user callback frees the "client […] Many things can go wrong if the callback free()s the client - this is currently not supported and calling `osmo_trxc_client_free()` would result in an assertion failure `OSMO_ASSERT(!client->in_rx)`. -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/43108?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I817e394f74a10e3adae4a0b58342c82acdf0794e Gerrit-Change-Number: 43108 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 06 Aug 2026 13:53:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmo-sigtran[master]: Add VTY test for "listen" node of osmo-stp VTY config
by pespin 06 Aug '26

06 Aug '26

Attention is currently required from: jolly. pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/42817?usp=email ) Change subject: Add VTY test for "listen" node of osmo-stp VTY config ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/42817?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ia1ceb5f0374f47ff269b557be30fc4d59550d1a6 Gerrit-Change-Number: 42817 Gerrit-PatchSet: 4 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 06 Aug 2026 08:31:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in libosmo-sigtran[master]: Add test case to verify proper DSCP settings
by pespin 06 Aug '26

06 Aug '26

Attention is currently required from: jolly. pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/42809?usp=email ) Change subject: Add test case to verify proper DSCP settings ...................................................................... Patch Set 11: Code-Review+1 (1 comment) File tests/vty/vty_test_runner.py: https://gerrit.osmocom.org/c/libosmo-sigtran/+/42809/comment/492b60f4_5432a… : PS11, Line 239: time.sleep(1.0) why is this sleep needed? -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/42809?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I6ac965998433b4d8213cce30fc3fcf8fe485a092 Gerrit-Change-Number: 42809 Gerrit-PatchSet: 11 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 06 Aug 2026 08:30:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

2026

2025

2024

2023

2022

gerrit-log August 2026