Change in openbsc[master]: Replace broken ipa_ccm_idtag APIs with new ipa_ccm_id ones

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Fri Mar 29 15:32:12 UTC 2019


Pau Espin Pedrol has submitted this change and it was merged. ( https://gerrit.osmocom.org/13433 )

Change subject: Replace broken ipa_ccm_idtag APIs with new ipa_ccm_id ones
......................................................................

Replace broken ipa_ccm_idtag APIs with new ipa_ccm_id ones

ipa_ccm_idtag_parse_off is broken, and can only be used with
len_offset=1 on ID Request messages, otherwise won't work correctly.
Modify ipa_ccm_idtag_parse to at least parse those correctly, and
document the limitations.

Those two functions are already deprecated and only used in openbsc by 3
callers:
* ipa_ccm_idtag_parse in ussd_read_cb(): Broken, that function can only
work for Requests and it's used to parse a Response.
* ipa_ccm_idtag_parse_off in forward_sccp_to_msc (NAT): Broken, it can
only be used to parse Requests and it's used to parse a Response.
Furthermore, len_offset=2 is passed which makes no sense and most
probably it fails always, or can even make the program crash.
* ipa_ccm_idtag_parse_off in (answer_challenge): This one is fine and
could actually be replaced with ipa_ccm_id_get_parse after libosmocore
commit (see below) is merged.

See libosmocore I6efc852dfc041192f554e41a58290a0f63298021 for more information.

As a consequence of the fixes, osmo-bsc-nat now parses messages sent
from VTY test correctly and thus it goes into processing them instead of
silently dropping them. As a result, some VTY tests fail because they
are sending incorrect format (missing NULL char in unit id strings) and
osmo-bsc-nat closses its connection (due to bad auth).

Change-Id: I3b995f8ef0b48c0a5b3375e42926641934359cd2
---
M openbsc/src/osmo-bsc/osmo_bsc_msc.c
M openbsc/src/osmo-bsc_nat/bsc_nat.c
M openbsc/src/osmo-bsc_nat/bsc_ussd.c
M openbsc/tests/vty_test_runner.py
4 files changed, 10 insertions(+), 12 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/openbsc/src/osmo-bsc/osmo_bsc_msc.c b/openbsc/src/osmo-bsc/osmo_bsc_msc.c
index b179ff1..badecc6 100644
--- a/openbsc/src/osmo-bsc/osmo_bsc_msc.c
+++ b/openbsc/src/osmo-bsc/osmo_bsc_msc.c
@@ -456,9 +456,7 @@
 		.algo		= OSMO_AUTH_ALG_MILENAGE,
 	};
 
-	ret = ipa_ccm_idtag_parse_off(&tvp,
-				inp->l2h + 1,
-				msgb_l2len(inp) - 1, 1);
+	ret = ipa_ccm_id_get_parse(&tvp, inp->l2h + 1, msgb_l2len(inp) - 1);
 	if (ret < 0) {
 		LOGP(DMSC, LOGL_ERROR, "ignoring IPA response "
 			"message with malformed TLVs: %s\n", osmo_hexdump(inp->l2h + 1,
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index c8a9e74..670b0be 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -1291,9 +1291,9 @@
 		if (msg->l2h[0] == IPAC_MSGT_ID_RESP && msgb_l2len(msg) > 2) {
 			struct tlv_parsed tvp;
 			int ret;
-			ret = ipa_ccm_idtag_parse_off(&tvp,
-					     (unsigned char *) msg->l2h + 2,
-					     msgb_l2len(msg) - 2, 0);
+			ret = ipa_ccm_id_resp_parse(&tvp,
+					     (unsigned char *) msg->l2h + 1,
+					     msgb_l2len(msg) - 1);
 			if (ret < 0) {
 				LOGP(DNAT, LOGL_ERROR, "ignoring IPA response "
 					"message with malformed TLVs\n");
diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
index ee0b085..dea1807 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
@@ -141,9 +141,9 @@
 		if (msg->l2h[0] == IPAC_MSGT_ID_RESP) {
 			struct tlv_parsed tvp;
 			int ret;
-			ret = ipa_ccm_idtag_parse(&tvp,
-					     (unsigned char *) msg->l2h + 2,
-					     msgb_l2len(msg) - 2);
+			ret = ipa_ccm_id_resp_parse(&tvp,
+					     (unsigned char *) msg->l2h + 1,
+					     msgb_l2len(msg) - 1);
 			if (ret < 0) {
 				LOGP(DNAT, LOGL_ERROR, "ignoring IPA response "
 					"message with malformed TLVs\n");
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index 5afcd2e..ca0b879 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -922,8 +922,8 @@
         self.assertEqual(data, "\x00\x01\xfe\x04")
 
         print "Going to send ID_RESP response"
-        res = ussdSocket.send(IPA().id_resp(IPA().tag_name('key')))
-        self.assertEqual(res, 10)
+        res = ussdSocket.send(IPA().id_resp(IPA().tag_name('key'+'\0')))
+        self.assertEqual(res, 11)
 
         # initiating PING/PONG cycle to know, that the ID_RESP message has been processed
 
@@ -1072,7 +1072,7 @@
         while True:
             print "\tsending IPA identity(%s) at %s" % (tk, time.strftime("%T"))
             try:
-                x.send(IPA().id_resp(IPA().identity(name = tk.encode('utf-8'))))
+                x.send(IPA().id_resp(IPA().identity(name = (tk+'\0').encode('utf-8'))))
                 print "\tdone sending IPA identity(%s) at %s" % (tk,
                                                             time.strftime("%T"))
                 break

-- 
To view, visit https://gerrit.osmocom.org/13433
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3b995f8ef0b48c0a5b3375e42926641934359cd2
Gerrit-Change-Number: 13433
Gerrit-PatchSet: 3
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Assignee: Daniel Willmann <dwillmann at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-CC: Daniel Willmann <dwillmann at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190329/efea1c4c/attachment.html>


More information about the gerrit-log mailing list