<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13778">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
dexter: Looks good to me, approved
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">osmo-bsc_nat: Parse MGCP Connection ID as hex<br><br>Our ttcn3-bscnat-tests would randomly fail. After the CRCX ACK returns<br>from the MSC the bsc-nat reports it could not find a CI it it and<br>deletes the connection on the BSC-side.<br><br>This happens because the field is parsed as a decimal value instead of<br>hexadecimal. So a value of 00FED122 is parsed as '0' which is a reserved<br>value in our program.<br><br>This fix parses the field as hexadecimal value and also logs an error if<br>the value happens to be 0.<br><br>make check will now test if a hexadecimal CI is parsed correctly.<br><br>Fixes: OS#3951<br>Change-Id: I49b8b61644bf706162102dce268cae2265536fc5<br>---<br>M openbsc/src/libmgcp/mgcp_protocol.c<br>M openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c<br>M openbsc/tests/bsc-nat/bsc_data.c<br>M openbsc/tests/bsc-nat/bsc_nat_test.c<br>4 files changed, 11 insertions(+), 8 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c</span><br><span>index 84dbc1f..689c91f 100644</span><br><span>--- a/openbsc/src/libmgcp/mgcp_protocol.c</span><br><span>+++ b/openbsc/src/libmgcp/mgcp_protocol.c</span><br><span>@@ -208,7 +208,7 @@</span><br><span> </span><br><span> len = snprintf(sdp_record, size,</span><br><span> "v=0\r\n"</span><br><span style="color: hsl(0, 100%, 40%);">- "o=- %u 23 IN IP4 %s\r\n"</span><br><span style="color: hsl(120, 100%, 40%);">+ "o=- %x 23 IN IP4 %s\r\n"</span><br><span> "s=-\r\n"</span><br><span> "c=IN IP4 %s\r\n"</span><br><span> "t=0 0\r\n",</span><br><span>@@ -285,7 +285,7 @@</span><br><span> }</span><br><span> </span><br><span> len = snprintf(sdp_record, sizeof(sdp_record),</span><br><span style="color: hsl(0, 100%, 40%);">- "I: %u%s\n\n", endp->ci, osmux_extension);</span><br><span style="color: hsl(120, 100%, 40%);">+ "I: %x%s\n\n", endp->ci, osmux_extension);</span><br><span> if (len < 0)</span><br><span> return NULL;</span><br><span> </span><br><span>@@ -512,7 +512,7 @@</span><br><span> uint32_t ci = strtoul(_ci, NULL, 10);</span><br><span> </span><br><span> if (ci != endp->ci) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMGCP, LOGL_ERROR, "ConnectionIdentifiers do not match on 0x%x. %u != %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMGCP, LOGL_ERROR, "ConnectionIdentifiers do not match on 0x%x. %x != %x\n",</span><br><span> ENDPOINT_NUMBER(endp), endp->ci, _ci);</span><br><span> return -1;</span><br><span> }</span><br><span>@@ -891,7 +891,7 @@</span><br><span> osmo_jibuf_set_dequeue_cb(endp->bts_jb, mgcp_dejitter_udp_send, &endp->net_end);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMGCP, LOGL_DEBUG, "Creating endpoint on: 0x%x CI: %u port: %u/%u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMGCP, LOGL_DEBUG, "Creating endpoint on: 0x%x CI: %x port: %u/%u\n",</span><br><span> ENDPOINT_NUMBER(endp), endp->ci,</span><br><span> endp->net_end.local_port, endp->bts_end.local_port);</span><br><span> if (p->cfg->change_cb)</span><br><span>diff --git a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c</span><br><span>index 311ab94..17dc659 100644</span><br><span>--- a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c</span><br><span>+++ b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c</span><br><span>@@ -812,11 +812,14 @@</span><br><span> return CI_UNUSED;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (sscanf(res, "I: %u", &ci) != 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sscanf(res, "I: %x", &ci) != 1) {</span><br><span> LOGP(DMGCP, LOGL_ERROR, "Failed to parse CI in msg '%s'\n", str);</span><br><span> return CI_UNUSED;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (ci == CI_UNUSED)</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMGCP, LOGL_ERROR, "CI field '%s' parsed as reserved value CI_UNUSED\n", str);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> return ci;</span><br><span> }</span><br><span> </span><br><span>diff --git a/openbsc/tests/bsc-nat/bsc_data.c b/openbsc/tests/bsc-nat/bsc_data.c</span><br><span>index 71d5391..d29caef 100644</span><br><span>--- a/openbsc/tests/bsc-nat/bsc_data.c</span><br><span>+++ b/openbsc/tests/bsc-nat/bsc_data.c</span><br><span>@@ -157,8 +157,8 @@</span><br><span> </span><br><span> </span><br><span> /* patch the ip and port */</span><br><span style="color: hsl(0, 100%, 40%);">-static const char crcx_resp[] = "200 23265295\r\nI: 1\r\n\r\nv=0\r\nc=IN IP4 172.16.18.2\r\nm=audio 4002 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\n";</span><br><span style="color: hsl(0, 100%, 40%);">-static const char crcx_resp_patched[] = "200 23265295\r\nI: 1\r\n\r\nv=0\r\nc=IN IP4 10.0.0.1\r\nm=audio 999 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\na=fmtp:98 mode-set=2 octet-align=1\r\n";</span><br><span style="color: hsl(120, 100%, 40%);">+static const char crcx_resp[] = "200 23265295\r\nI: 0F\r\n\r\nv=0\r\nc=IN IP4 172.16.18.2\r\nm=audio 4002 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\n";</span><br><span style="color: hsl(120, 100%, 40%);">+static const char crcx_resp_patched[] = "200 23265295\r\nI: 0F\r\n\r\nv=0\r\nc=IN IP4 10.0.0.1\r\nm=audio 999 RTP/AVP 98 3\r\na=rtpmap:98 AMR/8000\r\na=fmtp:98 mode-set=2 octet-align=1\r\n";</span><br><span> </span><br><span> /* patch the ip and port */</span><br><span> static const char mdcx[] = "MDCX 23330829 8@mgw MGCP 1.0\r\nC: 394b0439fb\r\nI: 1\r\nL: p:20, a:AMR, nt:IN\r\nM: recvonly\r\n\r\nv=0\r\no=- 1049380491 0 IN IP4 172.16.18.2\r\ns=-\r\nc=IN IP4 172.16.18.2\r\nt=0 0\r\nm=audio 4410 RTP/AVP 126\r\na=rtpmap:126 AMR/8000/1\r\na=fmtp:126 mode-set=2 octet-align=1;start-mode=0\r\na=ptime:20\r\na=recvonly\r\nm=image 4412 udptl t38\r\na=T38FaxVersion:0\r\na=T38MaxBitRate:14400\r\n";</span><br><span>diff --git a/openbsc/tests/bsc-nat/bsc_nat_test.c b/openbsc/tests/bsc-nat/bsc_nat_test.c</span><br><span>index 2914a01..e0d0051 100644</span><br><span>--- a/openbsc/tests/bsc-nat/bsc_nat_test.c</span><br><span>+++ b/openbsc/tests/bsc-nat/bsc_nat_test.c</span><br><span>@@ -683,7 +683,7 @@</span><br><span> }</span><br><span> </span><br><span> ci = bsc_mgcp_extract_ci(crcx_resp);</span><br><span style="color: hsl(0, 100%, 40%);">- if (ci != 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ci != 0x0F) {</span><br><span> printf("Failed to parse the CI. Got: %d\n", ci);</span><br><span> abort();</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13778">change 13778</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/13778"/><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: I49b8b61644bf706162102dce268cae2265536fc5 </div>
<div style="display:none"> Gerrit-Change-Number: 13778 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Daniel Willmann <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: 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: dexter <pmaier@sysmocom.de> </div>