Attention is currently required from: Hoernchen.
dexter has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/pysim/+/43551?usp=email )
Change subject: transport: stop the T=0 layer from breaking GP 6310 ......................................................................
Patch Set 2: Code-Review+1
(6 comments)
Patchset:
PS2: From what I can see, the logic is ok, a few spec references and other minor issues might be checked back. Through the unit-tests I only looked through very briefly, but they look comprehensive.
Commit Message:
https://gerrit.osmocom.org/c/pysim/+/43551/comment/7b4556e9_7f09fd2c?usp=ema... : PS2, Line 15: That is not "redundant", as per GP CS v2.3 section 11.4.3.1 the I think we normally refer to the "GlobalPlatform Card Specification" as "GPC" (they haven't given themselves an official abbreviation - at least not that I heard of...)
File pySim/global_platform/__init__.py:
https://gerrit.osmocom.org/c/pysim/+/43551/comment/aadecb3a_ec057098?usp=ema... : PS2, Line 740: p2 = 0x02 # TLV format according to Table 11-36 maybe reference as GPC, section 11.4.3.1, table 11-36. I mention this since at the very beginning of that section they explain the meaning of the 0x02 (bit 2 set in P2)
https://gerrit.osmocom.org/c/pysim/+/43551/comment/1328ed5e_82b2533d?usp=ema... : PS2, Line 751: raise SwMatchError(sw, '9000/6310') I would recommend to check this back. When I look at the apidoc in exceptions.py:SwMatchError, the sw_expected is defined as a string with 4 hex digits.
I have also looked further. When using SwMatchError like this, nothing catastrophic happens. It just prints it out, but its clearly not how the inventors of SwMatchError originally thought.
I would recommend to revisit the sw_expected parameter. Maybe we can have a variable parameter here that can be a 4 digit hex-string or a list with multiple 4 digit hex-strings. Then you would call:
``` raise SwMatchError(sw, ['9000', '6310']) ```
I think this would be much cleaner and future-proof.
https://gerrit.osmocom.org/c/pysim/+/43551/comment/d5913610_e98cff9f?usp=ema... : PS2, Line 760: # GPC CardSpec v2.3, section 11.4.3.1, 6310 == additional matches. As it seems the spec ref is wrong. As far as I understand, the following is correct:
GPC, section 11.4.2.2, table 11-34
I wonder if we have to keep the state of bit 2 for the "Get next occurrence(s)"? Section 11.4.2.2 also mentions that bit 2 is deprecated. I would recommend to check this back.
I haven't found the spec version 2.3, I used 2.3.1 instead, but that should be close enough.
File pySim/transport/__init__.py:
https://gerrit.osmocom.org/c/pysim/+/43551/comment/9d1f91d5_bd743594?usp=ema... : PS2, Line 344: # makes the card answer 6982 and tears down the whole SCP session and following commands fail with 6985. I always had difficulties to understand what ETSI TS 102 221, section 7.3.1.1.4, step 4b describes, but here is what I understand now:
We come from step 3b, and are in 3b. We send the dummy GET RESPONSE as described and we continue with step 5. Never come back, so we never send a dummy GET RESPONSE again. (all the other if statements either break or set le_gr to a length value)
So I think that your fix is correct.