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 <[email protected]>
