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')