From d651edcce06c5ec13efab7de885b9dd7fc7ff306 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Sun, 6 Dec 2020 13:35:24 +0100 Subject: 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 --- src/gbproxy/gb_proxy.c | 84 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 62 insertions(+), 22 deletions(-) diff --git a/src/gbproxy/gb_proxy.c b/src/gbproxy/gb_proxy.c index 98fa92834..a30f5ad16 100644 --- a/src/gbproxy/gb_proxy.c +++ b/src/gbproxy/gb_proxy.c @@ -249,8 +249,26 @@ static int gbprox_rx_ptp_from_bss(struct gbproxy_config *cfg, 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 @@ static int gbprox_rx_ptp_from_sgsn(struct gbproxy_config *cfg, 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; - bvc = gbproxy_bvc_by_bvci(cfg, ns_bvci); + 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); + } - /* Send status messages before patching */ + 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); if (!bvc) { LOGP(DGPRS, LOGL_INFO, "BVC(%05u/??) Didn't find bvc for " "for message from NSE(%05u/SGSN)\n", @@ -393,18 +426,20 @@ static int gbprox_rx_sig_from_bss(struct gbproxy_config *cfg, 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 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg, 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); + } + + 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); } - /* 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_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 gbprox_rcvmsg(void *ctx, struct msgb *msg) 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) -- cgit v1.2.3