[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-24 Thread laforge
Attention is currently required from: dexter, fixeria.

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 3: Code-Review+2


--
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: 3
Gerrit-Owner: laforge 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Attention: fixeria 
Gerrit-Attention: dexter 
Gerrit-Comment-Date: Tue, 24 Oct 2023 13:26:50 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-24 Thread laforge
laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/pysim/+/34846?usp=email )

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

commands.py: Add support for multiple logical channels.

Historically we always only had one instance of SimCardCommands, but
with this patch we can now have multiple instances, one for each lchan.

The SimCardCommands class is aware of the logical channel it runs on
and will patch the CLA byte accordingly.

Change-Id: Ibe5650dedc0f7681acf82018a86f83377ba81d30
Related: OS#6230
---
M pySim/commands.py
1 file changed, 84 insertions(+), 9 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved




diff --git a/pySim/commands.py b/pySim/commands.py
index e072069..3edee98 100644
--- a/pySim/commands.py
+++ b/pySim/commands.py
@@ -26,7 +26,7 @@

 from construct import *
 from pySim.construct import LV
-from pySim.utils import rpad, lpad, b2h, h2b, sw_match, bertlv_encode_len, 
Hexstr, h2i, str_sanitize, expand_hex
+from pySim.utils import rpad, lpad, b2h, h2b, sw_match, bertlv_encode_len, 
Hexstr, h2i, i2h, str_sanitize, expand_hex
 from pySim.utils import Hexstr, SwHexstr, ResTuple
 from pySim.exceptions import SwMatchError
 from pySim.transport import LinkBase
@@ -34,11 +34,70 @@
 # A path can be either just a FID or a list of FID
 Path = typing.Union[Hexstr, List[Hexstr]]
 
+def lchan_nr_to_cla(cla: int, lchan_nr: int) -> int:
+"""Embed a logical channel number into the CLA byte."""
+# TS 102 221 10.1.1 Coding of Class Byte
+if lchan_nr < 4:
+# standard logical channel number
+if cla >> 4 in [0x0, 0xA, 0x8]:
+return (cla & 0xFC) | (lchan_nr & 3)
+else:
+raise ValueError('Undefined how to use CLA %2X with logical 
channel %u' % (cla, lchan_nr))
+elif lchan_nr < 16:
+# extended logical channel number
+if cla >> 6 in [1, 3]:
+return (cla & 0xF0) | ((lchan_nr - 4) & 0x0F)
+else:
+raise ValueError('Undefined how to use CLA %2X with logical 
channel %u' % (cla, lchan_nr))
+else:
+raise ValueError('logical channel outside of range 0 .. 15')
+
+def cla_with_lchan(cla_byte: Hexstr, lchan_nr: int) -> Hexstr:
+"""Embed a logical channel number into the hex-string encoded CLA value."""
+cla_int = h2i(cla_byte)[0]
+return i2h([lchan_nr_to_cla(cla_int, lchan_nr)])
+
 class SimCardCommands:
-def __init__(self, transport: LinkBase):
+"""Class providing methods for various card-specific commands such as 
SELECT, READ BINARY, etc.
+Historically one instance exists below CardBase, but with the introduction 
of multiple logical
+channels there can be multiple instances.  The lchan number will then be 
patched into the CLA
+byte by the respective instance. """
+def __init__(self, transport: LinkBase, lchan_nr: int = 0):
 self._tp = transport
-self.cla_byte = "a0"
+self._cla_byte = None
 self.sel_ctrl = ""
+self.lchan_nr = lchan_nr
+# invokes the setter below
+self.cla_byte = "a0"
+
+def fork_lchan(self, lchan_nr: int) -> 'SimCardCommands':
+"""Fork a per-lchan specific SimCardCommands instance off the current 
instance."""
+ret = SimCardCommands(transport = self._tp, lchan_nr = lchan_nr)
+ret.cla_byte = self._cla_byte
+return ret
+
+@property
+def cla_byte(self) -> Hexstr:
+"""Return the (cached) patched default CLA byte for this card."""
+return self._cla4lchan
+
+@cla_byte.setter
+def cla_byte(self, new_val: Hexstr):
+"""Set the (raw, without lchan) default CLA value for this card."""
+self._cla_byte = new_val
+# compute cached result
+self._cla4lchan = cla_with_lchan(self._cla_byte, self.lchan_nr)
+
+def cla4lchan(self, cla: Hexstr) -> Hexstr:
+"""Compute the lchan-patched value of the given CLA value. If no CLA
+value is provided as argument, the lchan-patched version of the 
SimCardCommands._cla_byte
+value is used. Most commands will use the latter, while some wish to 
override it and
+can pass it as argument here."""
+if not cla:
+# return cached result to avoid re-computing this over and over 
again
+return self._cla4lchan
+else:
+return cla_with_lchan(cla, self.lchan_nr)

 # Extract a single FCP item from TLV
 def __parse_fcp(self, fcp: Hexstr):
@@ -344,9 +403,9 @@
 # TS 102 221 Section 11.3.1 low-level helper
 def _retrieve_data(self, tag: int, first: bool = True) -> ResTuple:
 if first:
-pdu = '80cb008001%02x' % (tag)
+pdu = self.cla4lchan('80') + 'cb008001%02x' % (tag)
 else:
-pdu = '80cb00'
+pdu = self.cla4lchan('80') + 'cb00'
 return 

[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-24 Thread laforge
Attention is currently required from: dexter, fixeria.

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 3:

(1 comment)

File pySim/commands.py:

https://gerrit.osmocom.org/c/pysim/+/34846/comment/cae9299a_fc316f0b
PS2, Line 526: return self._tp.send_apdu_checksw(self.cla_byte + 
self.cla4lchan('e0') + '%02x%s' % (len(payload)//2, payload))
> this is complete bogus and a sign of lack of sleep at my end. […]
Done



--
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: 3
Gerrit-Owner: laforge 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Attention: fixeria 
Gerrit-Attention: dexter 
Gerrit-Comment-Date: Tue, 24 Oct 2023 13:10:58 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge 
Comment-In-Reply-To: dexter 
Gerrit-MessageType: comment


[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-24 Thread laforge
Attention is currently required from: dexter, fixeria.

Hello Jenkins Builder, dexter, fixeria,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/pysim/+/34846?usp=email

to look at the new patch set (#3).

The following approvals got outdated and were removed:
Code-Review+1 by fixeria, Verified+1 by Jenkins Builder


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

commands.py: Add support for multiple logical channels.

Historically we always only had one instance of SimCardCommands, but
with this patch we can now have multiple instances, one for each lchan.

The SimCardCommands class is aware of the logical channel it runs on
and will patch the CLA byte accordingly.

Change-Id: Ibe5650dedc0f7681acf82018a86f83377ba81d30
Related: OS#6230
---
M pySim/commands.py
1 file changed, 84 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/46/34846/3
-- 
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: 3
Gerrit-Owner: laforge 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Attention: fixeria 
Gerrit-Attention: dexter 
Gerrit-MessageType: newpatchset


[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-24 Thread laforge
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') + '%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 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Attention: dexter 
Gerrit-Comment-Date: Tue, 24 Oct 2023 13:10:22 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: dexter 
Gerrit-MessageType: comment


[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-24 Thread dexter
Attention is currently required from: laforge.

dexter 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/6e5fcbd2_098448ac
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. Also I am a bit lost here. The positions of the logical channel match 
ISO/IEC 7816, section 5.1.1 but 0xA0 and 0x80 are proprietary classes that are 
not encoded as per ISO/IEC 7816? Maybe a spec reference would help?


https://gerrit.osmocom.org/c/pysim/+/34846/comment/0701763c_40903ebc
PS2, Line 93: if not cla:
the caller must decide when to access the cache and when not?


https://gerrit.osmocom.org/c/pysim/+/34846/comment/c4f8e6be_6324b2ab
PS2, Line 526: return self._tp.send_apdu_checksw(self.cla_byte + 
self.cla4lchan('e0') + '%02x%s' % (len(payload)//2, payload))
Does this work? The class is E0 but E0 >> 4 is not in [0x0, 0xA, 0x8] (line 42)



--
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 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter 
Gerrit-Reviewer: fixeria 
Gerrit-Attention: laforge 
Gerrit-Comment-Date: Tue, 24 Oct 2023 10:29:15 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-22 Thread fixeria
Attention is currently required from: laforge.

fixeria 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 1: Code-Review+1


--
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: 1
Gerrit-Owner: laforge 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Attention: laforge 
Gerrit-Comment-Date: Sun, 22 Oct 2023 07:30:15 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in pysim[master]: commands.py: Add support for multiple logical channels.

2023-10-21 Thread laforge
laforge has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/pysim/+/34846?usp=email )


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

commands.py: Add support for multiple logical channels.

Historically we always only had one instance of SimCardCommands, but
with this patch we can now have multiple instances, one for each lchan.

The SimCardCommands class is aware of the logical channel it runs on
and will patch the CLA byte accordingly.

Change-Id: Ibe5650dedc0f7681acf82018a86f83377ba81d30
Related: OS#6230
---
M pySim/commands.py
1 file changed, 82 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/46/34846/1

diff --git a/pySim/commands.py b/pySim/commands.py
index e072069..e6a01e2 100644
--- a/pySim/commands.py
+++ b/pySim/commands.py
@@ -26,7 +26,7 @@

 from construct import *
 from pySim.construct import LV
-from pySim.utils import rpad, lpad, b2h, h2b, sw_match, bertlv_encode_len, 
Hexstr, h2i, str_sanitize, expand_hex
+from pySim.utils import rpad, lpad, b2h, h2b, sw_match, bertlv_encode_len, 
Hexstr, h2i, i2h, str_sanitize, expand_hex
 from pySim.utils import Hexstr, SwHexstr, ResTuple
 from pySim.exceptions import SwMatchError
 from pySim.transport import LinkBase
@@ -34,11 +34,67 @@
 # A path can be either just a FID or a list of FID
 Path = typing.Union[Hexstr, List[Hexstr]]

+def lchan_nr_to_cla(cla: int, lchan_nr: int) -> int:
+"""Embed a logical channel number into the CLA byte."""
+# TS 102 221 10.1.1 Coding of Class Byte
+if lchan_nr < 4:
+# standard logical channel number
+if cla >> 4 in [0x0, 0xA, 0x8]:
+return (cla & 0xFC) | (lchan_nr & 3)
+else:
+raise ValueError('Undefined how to use CLA %2X with logical 
channel %u' % (cla, lchan_nr))
+elif lchan_nr < 16:
+# extended logical channel number
+if cla >> 6 in [1, 3]:
+return (cla & 0xF0) | ((lchan_nr - 4) & 0x0F)
+else:
+raise ValueError('Undefined how to use CLA %2X with logical 
channel %u' % (cla, lchan_nr))
+else:
+raise ValueError('logical channel outside of range 0 .. 15')
+
+def cla_with_lchan(cla_byte: Hexstr, lchan_nr: int) -> Hexstr:
+"""Embed a logical channel number into the hex-string encoded CLA value."""
+cla_int = h2i(cla_byte)[0]
+return i2h([lchan_nr_to_cla(cla_int, lchan_nr)])
+
 class SimCardCommands:
-def __init__(self, transport: LinkBase):
+"""Class providing methods for various card-specific commands such as 
SELECT, READ BINARY, etc.
+Historically one instance exists below CardBase, but with the introduction 
of multiple logical
+channels there can be multiple instances.  The lchan number will then be 
patched into the CLA
+byte by the respective instance. """
+def __init__(self, transport: LinkBase, lchan_nr: int = 0):
 self._tp = transport
-self.cla_byte = "a0"
+self._cla_byte = None
 self.sel_ctrl = ""
+self.lchan_nr = lchan_nr
+# invokes the setter below
+self.cla_byte = "a0"
+
+def fork_lchan(self, lchan_nr: int) -> 'SimCardCommands':
+"""Fork a per-lchan specific SimCardCommands instance off the current 
instance."""
+ret = SimCardCommands(transport = self._tp, lchan_nr = lchan_nr)
+ret.cla_byte = self._cla_byte
+return ret
+
+@property
+def cla_byte(self) -> Hexstr:
+"""Return the (cached) patched default CLA byte for this card."""
+return self._cla4lchan
+
+@cla_byte.setter
+def cla_byte(self, new_val: Hexstr):
+"""Set the (raw, without lchan) default CLA value for this card."""
+self._cla_byte = new_val
+# compute cached result
+self._cla4lchan = cla_with_lchan(self._cla_byte, self.lchan_nr)
+
+def cla4lchan(self, cla: Hexstr) -> Hexstr:
+"""Compute the lchan-patched value of the given CLA value."""
+if not cla:
+# return cached result to avoid re-computing this over and over 
again
+return self._cla4lchan
+else:
+return cla_with_lchan(cla, self.lchan_nr)

 # Extract a single FCP item from TLV
 def __parse_fcp(self, fcp: Hexstr):
@@ -344,9 +400,9 @@
 # TS 102 221 Section 11.3.1 low-level helper
 def _retrieve_data(self, tag: int, first: bool = True) -> ResTuple:
 if first:
-pdu = '80cb008001%02x' % (tag)
+pdu = self.cla4lchan('80') + 'cb008001%02x' % (tag)
 else:
-pdu = '80cb00'
+pdu = self.cla4lchan('80') + 'cb00'
 return self._tp.send_apdu_checksw(pdu)

 def retrieve_data(self, ef: Path, tag: int) -> ResTuple:
@@ -376,7 +432,7 @@
 p1 = 0x00
 if isinstance(data, bytes) or isinstance(data, bytearray):
 data = b2h(data)
-pdu =