<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/21560">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gb_proxy: Introduce more validation / constraint checks<br><br>* ensure the BSSGP PDU header length before reading pdu_type field<br>* ensure we never process uplink PDUs in downlink and vice-versa<br>* ensure we never proceses PTP PDUs on SIGNALING BVCI and vice-versa<br><br>Change-Id: I6e40aed0283f1a0860ab273606605f7fb28717cf<br>Depends: libosmocore.git I7e4226463f3c935134b5c2c737696fbfd1dd5815<br>---<br>M src/gbproxy/gb_proxy.c<br>1 file changed, 63 insertions(+), 23 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gbproxy/gb_proxy.c b/src/gbproxy/gb_proxy.c</span><br><span>index 98fa928..a30f5ad 100644</span><br><span>--- a/src/gbproxy/gb_proxy.c</span><br><span>+++ b/src/gbproxy/gb_proxy.c</span><br><span>@@ -249,8 +249,26 @@</span><br><span> struct msgb *msg, uint16_t nsei,</span><br><span> uint16_t ns_bvci)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_bssgph(msg);</span><br><span> struct gbproxy_bvc *bvc;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (ns_bvci == 0 && ns_bvci == 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/BSS) BVCI=%05u is not PTP\n", nsei, ns_bvci);</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(bgph->pdu_type) & BSSGP_PDUF_PTP)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/%05u) %s not allowed in PTP BVC\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, ns_bvci, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, bgph->pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(bgph->pdu_type) & BSSGP_PDUF_UL)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/%05u) %s not allowed in uplink direction\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, ns_bvci, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, bgph->pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bvc = gbproxy_bvc_by_bvci(cfg, ns_bvci);</span><br><span> if (!bvc) {</span><br><span> LOGP(DGPRS, LOGL_NOTICE, "BVC(%05u/??) Didn't find bvc "</span><br><span>@@ -272,12 +290,27 @@</span><br><span> struct msgb *msg, uint16_t nsei,</span><br><span> uint16_t ns_bvci)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct bssgp_normal_hdr *bgph = (struct bssgp_normal_hdr *) msgb_bssgph(msg);</span><br><span> struct gbproxy_bvc *bvc;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (ns_bvci == 0 && ns_bvci == 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/BSS) BVCI=%05u is not PTP\n", nsei, ns_bvci);</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(bgph->pdu_type) & BSSGP_PDUF_PTP)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/%05u) %s not allowed in PTP BVC\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, ns_bvci, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, bgph->pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(bgph->pdu_type) & BSSGP_PDUF_DL)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/%05u) %s not allowed in downlink direction\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, ns_bvci, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, bgph->pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bvc = gbproxy_bvc_by_bvci(cfg, ns_bvci);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Send status messages before patching */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (!bvc) {</span><br><span> LOGP(DGPRS, LOGL_INFO, "BVC(%05u/??) Didn't find bvc for "</span><br><span> "for message from NSE(%05u/SGSN)\n",</span><br><span>@@ -393,18 +426,20 @@</span><br><span> int rc;</span><br><span> </span><br><span> if (ns_bvci != 0 && ns_bvci != 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u) BVCI=%05u is not signalling\n",</span><br><span style="color: hsl(0, 100%, 40%);">- nsei, ns_bvci);</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/BSS) BVCI=%05u is not signalling\n", nsei, ns_bvci);</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* we actually should never see those two for BVCI == 0, but double-check</span><br><span style="color: hsl(0, 100%, 40%);">- * just to make sure */</span><br><span style="color: hsl(0, 100%, 40%);">- if (pdu_type == BSSGP_PDUT_UL_UNITDATA ||</span><br><span style="color: hsl(0, 100%, 40%);">- pdu_type == BSSGP_PDUT_DL_UNITDATA) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u) UNITDATA not allowed in "</span><br><span style="color: hsl(0, 100%, 40%);">- "signalling\n", nsei);</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(pdu_type) & BSSGP_PDUF_SIG)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/BSS) %s not allowed in signalling BVC\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(pdu_type) & BSSGP_PDUF_UL)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/BSS) %s not allowed in uplink direction\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);</span><br><span> }</span><br><span> </span><br><span> bssgp_tlv_parse(&tp, bgph->data, data_len);</span><br><span>@@ -596,18 +631,19 @@</span><br><span> int i;</span><br><span> </span><br><span> if (ns_bvci != 0 && ns_bvci != 1) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/SGSN) BVCI=%05u is not "</span><br><span style="color: hsl(0, 100%, 40%);">- "signalling\n", nsei, ns_bvci);</span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: Send proper error message */</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/SGSN) BVCI=%05u is not signalling\n", nsei, ns_bvci);</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, orig_msg);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* we actually should never see those two for BVCI == 0, but double-check</span><br><span style="color: hsl(0, 100%, 40%);">- * just to make sure */</span><br><span style="color: hsl(0, 100%, 40%);">- if (pdu_type == BSSGP_PDUT_UL_UNITDATA ||</span><br><span style="color: hsl(0, 100%, 40%);">- pdu_type == BSSGP_PDUT_DL_UNITDATA) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/SGSN) UNITDATA not allowed in "</span><br><span style="color: hsl(0, 100%, 40%);">- "signalling\n", nsei);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(pdu_type) & BSSGP_PDUF_SIG)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/SGSN) %s not allowed in signalling BVC\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, pdu_type));</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, orig_msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!(bssgp_pdu_type_flags(pdu_type) & BSSGP_PDUF_DL)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/SGSN) %s not allowed in downlink direction\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ nsei, osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, pdu_type));</span><br><span> return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, orig_msg);</span><br><span> }</span><br><span> </span><br><span>@@ -758,6 +794,10 @@</span><br><span> </span><br><span> int remote_end_is_sgsn = gbproxy_is_sgsn_nsei(cfg, nsei);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* ensure minimum length to decode PCU type */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_bssgp_len(msg) < sizeof(struct bssgp_normal_hdr))</span><br><span style="color: hsl(120, 100%, 40%);">+ return bssgp_tx_status(BSSGP_CAUSE_SEM_INCORR_PDU, NULL, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Only BVCI=0 messages need special treatment */</span><br><span> if (ns_bvci == 0 || ns_bvci == 1) {</span><br><span> if (remote_end_is_sgsn)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/21560">change 21560</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/c/osmo-sgsn/+/21560"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I6e40aed0283f1a0860ab273606605f7fb28717cf </div>
<div style="display:none"> Gerrit-Change-Number: 21560 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>