Attention is currently required from: dexter.

laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/pysim/+/34846?usp=email )

Change subject: commands.py: Add support for multiple logical channels.
......................................................................


Patch Set 2:

(3 comments)

File pySim/commands.py:

https://gerrit.osmocom.org/c/pysim/+/34846/comment/ee66be8f_3a9b5721
PS2, Line 42:         if cla >> 4 in [0x0, 0xA, 0x8]:
> I find this a bit difficult to read. Maybe cla & 0xF0 in [0x00, 0x0A, 0x80] 
> is better

this is the same syntax as used in pySim.runtime.lchan_nr_from_cla for exactly 
the same operation.  I think it's better to stay consistent.

>  but 0xA0 and 0x80 are proprietary classes that are not encoded as per 
> ISO/IEC 7816? Maybe a spec reference would help?

The usual spec to look at in terms of UICCs should always be ETSI TS 102 221.  
Only if that spec refers to its "parent class" ISO7816, we should look a the 
latter.

Also, the spec reference is given 3 lines above: "TS 102 221 10.1.1 Coding of 
Class Byte"


https://gerrit.osmocom.org/c/pysim/+/34846/comment/ce549bcb_525c8de2
PS2, Line 93:         if not cla:
> the caller must decide when to access the cache and when not?
as you know, not all commands (command bytes) use the same CLA value.  See for 
example TS 102 222 section 6.1 which means CLA=00 for most commands, but CLA=80 
for RESIZE FILE.  So even if SimCardCommands._cla_byte is e.g. 0x00, there are 
some specific commands that deviate from that standard.  So far, the respective 
code had hard-coded CLA values.  We now need to patch-in the lchan number into 
those.

The cla4lchan function now by default uses the SimCardCommands._cla_byte and 
adds the lchan number to it.  However, the caller can pass in an override CLA 
byte, which will then also be patched. I'll add a larger docstring about this 
behavirur.


https://gerrit.osmocom.org/c/pysim/+/34846/comment/e8e2d1ee_770cf5e1
PS2, Line 526:         return self._tp.send_apdu_checksw(self.cla_byte + 
self.cla4lchan('e0') + '0000%02x%s' % (len(payload)//2, payload))
> Does this work? The class is E0 but E0 >> 4 is not in [0x0, 0xA, 0x8] (line 
> 42)
this is complete bogus and a sign of lack of sleep at my end. E0 is INS, it 
should not be patched at all. Thanks!



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34846?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ibe5650dedc0f7681acf82018a86f83377ba81d30
Gerrit-Change-Number: 34846
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Attention: dexter <pma...@sysmocom.de>
Gerrit-Comment-Date: Tue, 24 Oct 2023 13:10:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: dexter <pma...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to