<p>Pau Espin Pedrol <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13433">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Replace broken ipa_ccm_idtag APIs with new ipa_ccm_id ones<br><br>ipa_ccm_idtag_parse_off is broken, and can only be used with<br>len_offset=1 on ID Request messages, otherwise won't work correctly.<br>Modify ipa_ccm_idtag_parse to at least parse those correctly, and<br>document the limitations.<br><br>Those two functions are already deprecated and only used in openbsc by 3<br>callers:<br>* ipa_ccm_idtag_parse in ussd_read_cb(): Broken, that function can only<br>work for Requests and it's used to parse a Response.<br>* ipa_ccm_idtag_parse_off in forward_sccp_to_msc (NAT): Broken, it can<br>only be used to parse Requests and it's used to parse a Response.<br>Furthermore, len_offset=2 is passed which makes no sense and most<br>probably it fails always, or can even make the program crash.<br>* ipa_ccm_idtag_parse_off in (answer_challenge): This one is fine and<br>could actually be replaced with ipa_ccm_id_get_parse after libosmocore<br>commit (see below) is merged.<br><br>See libosmocore I6efc852dfc041192f554e41a58290a0f63298021 for more information.<br><br>As a consequence of the fixes, osmo-bsc-nat now parses messages sent<br>from VTY test correctly and thus it goes into processing them instead of<br>silently dropping them. As a result, some VTY tests fail because they<br>are sending incorrect format (missing NULL char in unit id strings) and<br>osmo-bsc-nat closses its connection (due to bad auth).<br><br>Change-Id: I3b995f8ef0b48c0a5b3375e42926641934359cd2<br>---<br>M openbsc/src/osmo-bsc/osmo_bsc_msc.c<br>M openbsc/src/osmo-bsc_nat/bsc_nat.c<br>M openbsc/src/osmo-bsc_nat/bsc_ussd.c<br>M openbsc/tests/vty_test_runner.py<br>4 files changed, 10 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/openbsc/src/osmo-bsc/osmo_bsc_msc.c b/openbsc/src/osmo-bsc/osmo_bsc_msc.c</span><br><span>index b179ff1..badecc6 100644</span><br><span>--- a/openbsc/src/osmo-bsc/osmo_bsc_msc.c</span><br><span>+++ b/openbsc/src/osmo-bsc/osmo_bsc_msc.c</span><br><span>@@ -456,9 +456,7 @@</span><br><span> .algo = OSMO_AUTH_ALG_MILENAGE,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ret = ipa_ccm_idtag_parse_off(&tvp,</span><br><span style="color: hsl(0, 100%, 40%);">- inp->l2h + 1,</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l2len(inp) - 1, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = ipa_ccm_id_get_parse(&tvp, inp->l2h + 1, msgb_l2len(inp) - 1);</span><br><span> if (ret < 0) {</span><br><span> LOGP(DMSC, LOGL_ERROR, "ignoring IPA response "</span><br><span> "message with malformed TLVs: %s\n", osmo_hexdump(inp->l2h + 1,</span><br><span>diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c</span><br><span>index c8a9e74..670b0be 100644</span><br><span>--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c</span><br><span>+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c</span><br><span>@@ -1291,9 +1291,9 @@</span><br><span> if (msg->l2h[0] == IPAC_MSGT_ID_RESP && msgb_l2len(msg) > 2) {</span><br><span> struct tlv_parsed tvp;</span><br><span> int ret;</span><br><span style="color: hsl(0, 100%, 40%);">- ret = ipa_ccm_idtag_parse_off(&tvp,</span><br><span style="color: hsl(0, 100%, 40%);">- (unsigned char *) msg->l2h + 2,</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l2len(msg) - 2, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = ipa_ccm_id_resp_parse(&tvp,</span><br><span style="color: hsl(120, 100%, 40%);">+ (unsigned char *) msg->l2h + 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_l2len(msg) - 1);</span><br><span> if (ret < 0) {</span><br><span> LOGP(DNAT, LOGL_ERROR, "ignoring IPA response "</span><br><span> "message with malformed TLVs\n");</span><br><span>diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c</span><br><span>index ee0b085..dea1807 100644</span><br><span>--- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c</span><br><span>+++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c</span><br><span>@@ -141,9 +141,9 @@</span><br><span> if (msg->l2h[0] == IPAC_MSGT_ID_RESP) {</span><br><span> struct tlv_parsed tvp;</span><br><span> int ret;</span><br><span style="color: hsl(0, 100%, 40%);">- ret = ipa_ccm_idtag_parse(&tvp,</span><br><span style="color: hsl(0, 100%, 40%);">- (unsigned char *) msg->l2h + 2,</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l2len(msg) - 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = ipa_ccm_id_resp_parse(&tvp,</span><br><span style="color: hsl(120, 100%, 40%);">+ (unsigned char *) msg->l2h + 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_l2len(msg) - 1);</span><br><span> if (ret < 0) {</span><br><span> LOGP(DNAT, LOGL_ERROR, "ignoring IPA response "</span><br><span> "message with malformed TLVs\n");</span><br><span>diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py</span><br><span>index 5afcd2e..ca0b879 100644</span><br><span>--- a/openbsc/tests/vty_test_runner.py</span><br><span>+++ b/openbsc/tests/vty_test_runner.py</span><br><span>@@ -922,8 +922,8 @@</span><br><span> self.assertEqual(data, "\x00\x01\xfe\x04")</span><br><span> </span><br><span> print "Going to send ID_RESP response"</span><br><span style="color: hsl(0, 100%, 40%);">- res = ussdSocket.send(IPA().id_resp(IPA().tag_name('key')))</span><br><span style="color: hsl(0, 100%, 40%);">- self.assertEqual(res, 10)</span><br><span style="color: hsl(120, 100%, 40%);">+ res = ussdSocket.send(IPA().id_resp(IPA().tag_name('key'+'\0')))</span><br><span style="color: hsl(120, 100%, 40%);">+ self.assertEqual(res, 11)</span><br><span> </span><br><span> # initiating PING/PONG cycle to know, that the ID_RESP message has been processed</span><br><span> </span><br><span>@@ -1072,7 +1072,7 @@</span><br><span> while True:</span><br><span> print "\tsending IPA identity(%s) at %s" % (tk, time.strftime("%T"))</span><br><span> try:</span><br><span style="color: hsl(0, 100%, 40%);">- x.send(IPA().id_resp(IPA().identity(name = tk.encode('utf-8'))))</span><br><span style="color: hsl(120, 100%, 40%);">+ x.send(IPA().id_resp(IPA().identity(name = (tk+'\0').encode('utf-8'))))</span><br><span> print "\tdone sending IPA identity(%s) at %s" % (tk,</span><br><span> time.strftime("%T"))</span><br><span> break</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13433">change 13433</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13433"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: openbsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I3b995f8ef0b48c0a5b3375e42926641934359cd2 </div>
<div style="display:none"> Gerrit-Change-Number: 13433 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Assignee: Daniel Willmann <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Daniel Willmann <dwillmann@sysmocom.de> </div>