laforge has submitted 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
......................................................................
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(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
neels: Looks good to me, but someone else must approve
diff --git a/src/iu_helpers.c b/src/iu_helpers.c
index 718c30a..5c7ff31 100644
--- a/src/iu_helpers.c
+++ b/src/iu_helpers.c
@@ -140,14 +140,21 @@
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) {
+ /* ITU-T Rec. X.213 A.5.2.1.2.7, RFC 1888 section 6
+ * 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:
+ /* "RFC 1888 provides guidance on how to embed an IPv6 address within the DSP of an NSAP
+ * address. The IPv6 address is carried in the first 16 octets of the DSP.
+ * Octet 17 of the DSP is set to zero, but has no significance for IPv6." */
+ 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: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: neels, pespin.
laforge 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+2
(1 comment)
File src/iu_helpers.c:
https://gerrit.osmocom.org/c/osmo-iuh/+/34963/comment/4bbf01cf_a36b1c9b
PS3, Line 143: if ((len == 7 || len == 20) && buf[0] == 0x35) {
> What do you think about allowing anything between 7.. […]
it gives you a feeling for how alien IP was for CCITT/ITU at the time.
--
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: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Nov 2023 19:36:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
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/+/34961?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: transport: Move printing of reader number/name to generic code
......................................................................
transport: Move printing of reader number/name to generic code
Let's avoid copy+pasting print statements everywhere. The instances
do already have a __str__ method for the purpose of printing their name in a
generic way.
Change-Id: I663a9ea69bf7e7aaa6502896b6a71ef692f8d844
---
M docs/suci-tutorial.rst
M pySim/transport/__init__.py
M pySim/transport/calypso.py
M pySim/transport/modem_atcmd.py
M pySim/transport/pcsc.py
M pySim/transport/serial.py
M pysim-testdata/Fairwaves-SIM.ok
M pysim-testdata/Wavemobile-SIM.ok
M pysim-testdata/fakemagicsim.ok
M pysim-testdata/sysmoISIM-SJA2.ok
M pysim-testdata/sysmoUSIM-SJS1.ok
M pysim-testdata/sysmosim-gr1.ok
12 files changed, 27 insertions(+), 26 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/61/34961/3
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34961?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: I663a9ea69bf7e7aaa6502896b6a71ef692f8d844
Gerrit-Change-Number: 34961
Gerrit-PatchSet: 3
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email )
Change subject: vty and log: also show local port for RTP conns
......................................................................
vty and log: also show local port for RTP conns
Before:
CONN: (1226/rtp, id:0xD94316AD, ip:127.0.0.2, rtp:2344 rtcp:2345)
After:
CONN: (1226/rtp C:D94316AD r=127.0.0.2:2344<->l=127.0.0.1:4002)
While changing that string, also include these changes for consistency
and readability:
- use the same r:...<->l:... format as osmo_sock_get_name().
- Instead of 'id:0x' use the actual MGCP format 'C: 9B686BE3'.
- drop the commas
- drop RTCP port: it is always RTP+1 and always an odd number.
Rationale:
The CONN pairs associated with each endpoint show remote RTP
ports. When osmo-mgw is being used by both BSC and MSC, one
side of the pair is showing the internal loop connection inside
osmo-mgw, while my intuition suggested this connection pair
is showing me the RTP port tuple of a single RTP stream. Adding
the local port to the display makes it more clear, IMHO.
Seeing the local port can also help to correlate the MGW vty
dump with a capture of RTP.
Implementation:
I first tried directly using osmo_sock_get_name_buf() on
conn->u.rtp.end.rtp.fd, but that might hide already known information
when the fd is not actively used yet (before SDP): the local address and
port would then be shown from the fd, not from
conn->u.rtp.end.local_addr/_port == hidden before the fd is set up.
Patch-By: whytek, nhofmeyr
Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132
---
M src/libosmo-mgcp/mgcp_conn.c
1 file changed, 59 insertions(+), 32 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
keith: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index afc9e4e..283a213 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -358,53 +358,37 @@
{
static char str[sizeof(conn->name)+sizeof(conn->id)+256];
char ipbuf[INET6_ADDRSTRLEN];
+ struct osmo_strbuf sb = { .buf = str, .len = sizeof(str) };
- if (!conn) {
- snprintf(str, sizeof(str), "(null connection)");
- return str;
- }
+ if (!conn)
+ return "NULL";
switch (conn->type) {
case MGCP_CONN_TYPE_RTP:
+ OSMO_STRBUF_PRINTF(sb, "(%s/%s C:%s r=%s:%u<->l=%s:%u",
+ conn->name,
+ mgcp_conn_rtp_type_name(conn->type),
+ conn->id,
+ osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf) ? : "NULL",
+ osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa),
+ conn->u.rtp.end.local_addr ? : "NULL",
+ conn->u.rtp.end.local_port);
+
switch (conn->u.rtp.type) {
- case MGCP_RTP_DEFAULT:
- /* Dump RTP connection */
- snprintf(str, sizeof(str), "(%s/rtp, id:0x%s, ip:%s, "
- "rtp:%u rtcp:%u)",
- conn->name, conn->id,
- osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf),
- osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa),
- ntohs(conn->u.rtp.end.rtcp_port));
- break;
case MGCP_RTP_OSMUX:
- snprintf(str, sizeof(str), "(%s/osmux, id:0x%s, ip:%s, "
- "port:%u CID:%u)",
- conn->name, conn->id,
- osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf),
- osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa),
- conn->u.rtp.osmux.local_cid);
- break;
- case MGCP_RTP_IUUP:
- snprintf(str, sizeof(str), "(%s/iuup, id:0x%s, ip:%s, "
- "port:%u)",
- conn->name, conn->id,
- osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf),
- osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa));
+ OSMO_STRBUF_PRINTF(sb, " CID=%u", conn->u.rtp.osmux.local_cid);
break;
default:
- /* Should not happen, we should be able to dump
- * every possible connection type. */
- snprintf(str, sizeof(str), "(unknown conn_rtp connection type %u)",
- conn->u.rtp.type);
break;
}
+
+ OSMO_STRBUF_PRINTF(sb, ")");
break;
default:
/* Should not happen, we should be able to dump
* every possible connection type. */
- snprintf(str, sizeof(str), "(unknown connection type)");
- break;
+ return "(unknown connection type)";
}
return str;
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132
Gerrit-Change-Number: 34150
Gerrit-PatchSet: 4
Gerrit-Owner: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged