Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email )
Change subject: ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/iu_helpers.c:
https://gerrit.osmocom.org/c/osmo-iuh/+/34963/comment/8d3e4ab7_b7337e68
PS3, Line 143: if ((len == 7 || len == 20) && buf[0] == 0x35) {
What do you think about allowing anything between 7..20? For an IPv4, there are just zero padding bytes, so we could allow any number of padding...
The X.213 does indicate IP version explicitly, so we don't need to derive the address family from the nr of bytes. We can just check that there are enough bytes available. Not sure if it makes sense to include the entire range, just asking what you think...
Just after writing this, I noticed that we use the length to detect X.213 vs other formats, particularly headerless IPv6 == 16 bytes. So better to just allow 7, right.
(This here is a contender for the weirdest convoluted spaghetti way of conveying an IP address on the wire, can you believe it.)
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I507fb1605d976bd8573162e4fa81721245330184
Gerrit-Change-Number: 34963
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Nov 2023 19:25:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
......................................................................
ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
It was found in the field that some peers sends an X.213 IP address
consisting of 7 bytes (1byte IDP/AFI, 2byte ICP, 4 byte IPv4 address) insetad
of 20 bytes.
This is indeed possible when reading ITU Rec X.213 A.5.2.3, where it
states that Table A5 defining the 17 bytes DSP len for IANA ICP
"gives the maximum length of the DSP". So smaller values are still
possible/acceptable.
Related: SYS#6623
Change-Id: I507fb1605d976bd8573162e4fa81721245330184
---
M src/iu_helpers.c
1 file changed, 30 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/63/34963/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I507fb1605d976bd8573162e4fa81721245330184
Gerrit-Change-Number: 34963
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
......................................................................
ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
It was found in the field that some peers sends an X.213 IP address
consisting of 7 bytes (1byte IDP/AFI, 2byte ICP, 4 byte IPv4 address) insetad
of 20 bytes.
This is indeed possible when reading ITU Rec X.213 A.5.2.3, where it
states that Table A5 defining the 17 bytes DSP len for IANA ICP
"gives the maximum length of the DSP". So smaller values are still
possible/acceptable.
Related: SYS#6623
Change-Id: I507fb1605d976bd8573162e4fa81721245330184
---
M src/iu_helpers.c
1 file changed, 30 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/63/34963/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I507fb1605d976bd8573162e4fa81721245330184
Gerrit-Change-Number: 34963
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email )
Change subject: ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
......................................................................
ranap_transp_layer_addr_decode2(): Fix decoding X.213 IPv4 address len=7
It was found in the field that some peers sends an X.213 IP address
consisting of 7 bytes (1byte IDP/AFI, 2byte ICP, 4 byte IPv4 address) insetad
of 20 bytes.
Related: SYS#6623
Change-Id: I507fb1605d976bd8573162e4fa81721245330184
---
M src/iu_helpers.c
1 file changed, 21 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-iuh refs/changes/63/34963/1
diff --git a/src/iu_helpers.c b/src/iu_helpers.c
index 718c30a..04f6904 100644
--- a/src/iu_helpers.c
+++ b/src/iu_helpers.c
@@ -140,14 +140,17 @@
memset(addr, 0, sizeof(*addr));
- if (len == 20 && buf[0] == 0x35) {
- /* For an X.213 NSAP encoded address we expect a buffer of exactly 20 bytes (3 bytes IDP + 17 bytes
- * DSP). we also expect AFI = 0x35, which means that two byte IDI and an IP address follows. (see also
- * comments in function ranap_new_transp_layer_addr below) */
+ if ((len == 7 || len == 20) && buf[0] == 0x35) {
+ /* For an X.213 NSAP encoded address we expect:
+ * 3 bytes IDP (first byte AFI = 0x35, which means that two byte IDI and an IP address follows)
+ * Either 4 or 17 bytes of DSP containing the IP address.
+ * (see also comments in function ranap_new_transp_layer_addr below) */
x213_nsap = true;
icp = osmo_load16be(&buf[1]);
switch (icp) {
case 0x0000:
+ if (len != 20)
+ return -EINVAL;
addr->u.sa.sa_family = AF_INET6;
memcpy(addr->u.sin6.sin6_addr.s6_addr, buf + 3, sizeof(addr->u.sin6.sin6_addr.s6_addr));
break;
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/34963?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I507fb1605d976bd8573162e4fa81721245330184
Gerrit-Change-Number: 34963
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/34960?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: transport/pcsc: Allow opening PC/SC readers by a regex of their name
......................................................................
transport/pcsc: Allow opening PC/SC readers by a regex of their name
Opening PC/SC readers by index/number is very error-prone as the order
is never deterministic in any system with multiple (hot-plugged, USB)
readers. Instead, let's offer the alternative of specifying a regular
expression to match the reader name (similar to remsim-bankd).
Change-Id: I983f19c6741904c1adf27749c9801b44a03a5d78
---
M pySim/transport/pcsc.py
1 file changed, 51 insertions(+), 13 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/60/34960/2
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34960?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: I983f19c6741904c1adf27749c9801b44a03a5d78
Gerrit-Change-Number: 34960
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/34962?usp=email )
Change subject: transport: Extend the documentation for each transport driver
......................................................................
transport: Extend the documentation for each transport driver
This driver description we add to the code is automatically added to the
respective user manual sections.
Change-Id: I8807bfb11f43b167f1321d556e09ec5234fff629
---
M pySim/transport/calypso.py
M pySim/transport/modem_atcmd.py
M pySim/transport/pcsc.py
M pySim/transport/serial.py
4 files changed, 28 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/62/34962/1
diff --git a/pySim/transport/calypso.py b/pySim/transport/calypso.py
index 64e9d0c..d9d9033 100644
--- a/pySim/transport/calypso.py
+++ b/pySim/transport/calypso.py
@@ -165,6 +165,9 @@
@staticmethod
def argparse_add_reader_args(arg_parser: argparse.ArgumentParser):
- osmobb_group = arg_parser.add_argument_group('OsmocomBB Reader')
+ osmobb_group = arg_parser.add_argument_group('OsmocomBB Reader', """Use an OsmocomBB compatible phone
+to access the SIM inserted to the phone SIM slot. This will require you to run the OsmocomBB firmware inside
+the phone (can be ram-loaded). It also requires that you run the ``osmocon`` program, which provides a unix
+domain socket to which this reader driver can attach.""")
osmobb_group.add_argument('--osmocon', dest='osmocon_sock', metavar='PATH', default=None,
help='Socket path for Calypso (e.g. Motorola C1XX) based reader (via OsmocomBB)')
diff --git a/pySim/transport/modem_atcmd.py b/pySim/transport/modem_atcmd.py
index 28c30e3..ff4d960 100644
--- a/pySim/transport/modem_atcmd.py
+++ b/pySim/transport/modem_atcmd.py
@@ -176,7 +176,9 @@
@staticmethod
def argparse_add_reader_args(arg_parser: argparse.ArgumentParser):
- modem_group = arg_parser.add_argument_group('AT Command Modem Reader')
+ modem_group = arg_parser.add_argument_group('AT Command Modem Reader', """Talk to a SIM Card inside a
+mobile phone or cellular modem which is attached to this computer and offers an AT command interface including
+the AT+CSIM interface for Generic SIM access as specified in 3GPP TS 27.007.""")
modem_group.add_argument('--modem-device', dest='modem_dev', metavar='DEV', default=None,
help='Serial port of modem for Generic SIM Access (3GPP TS 27.007)')
modem_group.add_argument('--modem-baud', type=int, metavar='BAUD', default=115200,
diff --git a/pySim/transport/pcsc.py b/pySim/transport/pcsc.py
index 74c167e..4c7698b 100644
--- a/pySim/transport/pcsc.py
+++ b/pySim/transport/pcsc.py
@@ -122,6 +122,11 @@
@staticmethod
def argparse_add_reader_args(arg_parser: argparse.ArgumentParser):
- pcsc_group = arg_parser.add_argument_group('PC/SC Reader')
+ pcsc_group = arg_parser.add_argument_group('PC/SC Reader',
+ """Use a PC/SC card reader to talk to the SIM card. PC/SC is a standard API for how applications
+access smart card readers, and is available on a variety of operating systems, such as Microsoft
+Windows, MacOS X and Linux. Most vendors of smart card readers provide drivers that offer a PC/SC
+interface, if not even a generic USB CCID driver is used. You can use a tool like ``pcsc_scan -r``
+to obtain a list of readers available on your system. """)
pcsc_group.add_argument('-p', '--pcsc-device', type=str, dest='pcsc_dev', metavar='PCSC', default=None,
help='Number of (or regex matching) PC/SC reader to use for SIM access')
diff --git a/pySim/transport/serial.py b/pySim/transport/serial.py
index d5dc1db..29c0bc5 100644
--- a/pySim/transport/serial.py
+++ b/pySim/transport/serial.py
@@ -243,7 +243,9 @@
@staticmethod
def argparse_add_reader_args(arg_parser: argparse.ArgumentParser):
- serial_group = arg_parser.add_argument_group('Serial Reader')
+ serial_group = arg_parser.add_argument_group('Serial Reader', """Use a simple/ultra-low-cost serial reader
+attached to a (physical or USB/virtual) RS232 port. This doesn't work with all RS232-attached smart card
+readers, only with the very primitive readers following the ancient `Phoenix` or `Smart Mouse` design.""")
serial_group.add_argument('-d', '--device', metavar='DEV', default='/dev/ttyUSB0',
help='Serial Device for SIM access')
serial_group.add_argument('-b', '--baud', dest='baudrate', type=int, metavar='BAUD', default=9600,
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34962?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: I8807bfb11f43b167f1321d556e09ec5234fff629
Gerrit-Change-Number: 34962
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newchange
Attention is currently required from: fixeria, laforge.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/34957?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: Use construct 'Flag' instead of 'Bit' for type descriptions
......................................................................
Use construct 'Flag' instead of 'Bit' for type descriptions
It's better for the human reader (and more obvious that it's a boolean
value) if we decode single Bits as True/False instead of 1/0.
Change-Id: Ib025f9c4551af7cf57090a0678ab0f66a6684fa4
---
M pySim/sysmocom_sja2.py
M pySim/ts_31_102.py
2 files changed, 31 insertions(+), 19 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/57/34957/3
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34957?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: Ib025f9c4551af7cf57090a0678ab0f66a6684fa4
Gerrit-Change-Number: 34957
Gerrit-PatchSet: 3
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset