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