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

pySim/global_platform/__init__.py

@@ -707,6 +707,14 @@ class ADF_SD(CardADF):
             """Perform the GlobalPlatform PUT KEY command in order to store a new key on the card.
             See GlobalPlatform CardSpecification v2.3 Section 11.8 for details."""
             key_data = self.build_put_key_data(kvn, keys, self._cmd.lchan.scc.scp)
+            # Lc of Table 11-64 is a single byte, while LOAD or STORE DATA splits we can't:
+            # 11.8.2.3.3 splits a key at component boundaries -> not helping here
+            max_cmd_len = self._cmd.lchan.scc.max_cmd_len
+            if len(key_data) > max_cmd_len:
+                raise ValueError('key data field of %u bytes exceeds the maximum command length of %u '
+                                 '(limited by the overhead of the current secure channel); use fewer '
+                                 'keys per command, a single key component that large needs STORE DATA' %
+                                 (len(key_data), max_cmd_len))
             hdr = "80D8%02x%02x%02x" % (old_kvn, kid, len(key_data))
             data, _sw = self._cmd.lchan.scc.send_apdu_checksw(hdr + b2h(key_data) + "00")
             return data
@@ -886,23 +894,32 @@ class ADF_SD(CardADF):
         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):
+        def load(self, contents:bytes, chunk_len:Optional[int] = None):
-            # TODO:tune chunk_len based on the overhead of the used SCP?
+            # 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
@@ -941,6 +958,8 @@ class ADF_SD(CardADF):
         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):
@@ -979,7 +998,7 @@ class ADF_SD(CardADF):
             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))

pySim/global_platform/scp.py

@@ -182,6 +182,29 @@ class SCP(SecureChannel, abc.ABC):
         """Should we perform R-ENC?"""
         return self.security_level & 0x20
 
+    @property
+    @abc.abstractmethod
+    def mac_len(self) -> int:
+        """Length of the appended C-MAC, to be provided by derived class."""
+
+    @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
+        # see Secure Channel Protocol '03' Card Specification v2.3 - Amendment D v1.1.2
+        # which defers to GPCS v2.3 Section B.2 which then defers to
+        # NIST SP 800-38B for encryption and points out that
+        # the padding is, as expected, just the usual padding from NIST SP 800-38A
+        # 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)
 
@@ -268,10 +291,8 @@ class SCP02(SCP):
     # 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]]
-
+    # C-MAC (Single DES + final 3DES, B.1.2.2) is always one full DES block
-    def __init__(self, *args, **kwargs):
+    mac_len = 8
-        self.overhead = 8
-        super().__init__(*args, **kwargs)
 
     def dek_encrypt(self, plaintext:bytes) -> bytes:
         # See also GPC section B.1.1.2, E.4.7, and E.4.1
@@ -346,10 +367,16 @@ class SCP02(SCP):
             # 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
@@ -485,9 +512,13 @@ class SCP03(SCP):
 
     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)

tests/unittests/test_globalplatform.py

@@ -18,6 +18,7 @@
 import unittest
 import logging
 import hashlib
+from types import SimpleNamespace
 from osmocom.utils import b2h, h2b
 from osmocom.tlv import bertlv_encode_len
 
@@ -477,6 +478,58 @@ class PutKey_PSK_Test(unittest.TestCase):
         self.assertEqual(b2h(field), '8511' '10' + b2h(self.PSK_CLEAR) + '03' + b2h(self.PSK_KCV))
 
 
+class PutKey_Length_Test(unittest.TestCase):
+    """Tests for the length of the PUT KEY command APDU.  Lc of GP CardSpec v2.3 Table 11-64 is a
+    single byte, so an oversized key data field cannot be sent."""
+
+    class PutKeyOnly(ADF_SD.AddlShellCommands):
+        """ADF_SD.AddlShellCommands with a canned scc to drive put_key()"""
+        def __init__(self, scp=None, max_cmd_len=255):
+            super().__init__()
+            self.sent = []
+            self.scc = SimpleNamespace(scp=scp, max_cmd_len=max_cmd_len,
+                                       send_apdu_checksw=lambda pdu: (self.sent.append(pdu), ('', '9000'))[1])
+
+        @property
+        def _cmd(self):
+            return SimpleNamespace(lchan=SimpleNamespace(scc=self.scc))
+
+    # KVN, key type, two byte BER length of the key component block, KCV length; KCV suppressed
+    FRAMING = 1 + 1 + 2 + 1
+
+    @staticmethod
+    def key(nbytes: int):
+        return [{'key_type': 'rsa_modulus_n', 'clear_key': bytes(nbytes), 'kcv': b''}]
+
+    def test_lc_matches_data_field(self):
+        # largest key component block that still fits without a secure channel
+        sd = self.PutKeyOnly()
+        sd.put_key(0, 0x40, 1, self.key(255 - self.FRAMING))
+        apdu = sd.sent[0]
+        self.assertEqual(apdu[:8], '80D80001')
+        lc = int(apdu[8:10], 16)
+        self.assertEqual(lc, 255)                       # Lc ...
+        self.assertEqual(len(apdu[10:-2]) // 2, lc)     # ... and it matches the actual data field
+
+    def test_oversized_key_data_raises(self):
+        # real world fat example: RSA-2048 modulus does not fit, led to 3 nibble Lc 106,
+        # which silently shifted and broke the whole APDU by half a byte.
+        sd = self.PutKeyOnly()
+        with self.assertRaises(ValueError) as ctx:
+            sd.put_key(0, 0x40, 1, self.key(256))
+        self.assertIn('262', str(ctx.exception))
+        self.assertIn('255', str(ctx.exception))
+        self.assertEqual(sd.sent, [])                   # nothing was sent to the card
+
+    def test_secure_channel_overhead_lowers_the_limit(self):
+        # scc.max_cmd_len shrinks by the C-MAC + encryption padding of active SCP
+        sd = self.PutKeyOnly(max_cmd_len=239)
+        sd.put_key(0, 0x40, 1, self.key(239 - self.FRAMING))
+        self.assertEqual(int(sd.sent[0][8:10], 16), 239)
+        with self.assertRaises(ValueError):
+            sd.put_key(0, 0x40, 1, self.key(239 - self.FRAMING + 1))
+
+
 class Install_param_Test(unittest.TestCase):
     def test_gen_install_parameters(self):
         load_parameters = gen_install_parameters(256, 256, '010001001505000000000000000000000000')
@@ -485,5 +538,180 @@ class Install_param_Test(unittest.TestCase):
         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(unittest.TestCase):
+    """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):
+    """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):
+    """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()