laforge has submitted this change. (
https://gerrit.osmocom.org/c/osmo-sgsn/+/21591 )
Change subject: gb_proxy: Use osmo_tlv_prot_parse() to validate mandatory IEs
......................................................................
gb_proxy: Use osmo_tlv_prot_parse() to validate mandatory IEs
We recently introduced code to libosmocore which allows us to validate
the mandatory IE presence (and length) in a generic way. Let's use it.
Change-Id: I0ea3f5f9566d9bf5a8429c3ee748e3e90cda6cd7
Depends: libosmocore.git I7e4226463f3c935134b5c2c737696fbfd1dd5815
---
M src/gbproxy/gb_proxy.c
1 file changed, 45 insertions(+), 12 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 a90030e..8be67f7 100644
--- a/src/gbproxy/gb_proxy.c
+++ b/src/gbproxy/gb_proxy.c
@@ -104,6 +104,20 @@
return 1;
}
+/* generate BVC-STATUS message with cause value derived from TLV-parser error
*/
+static int tx_status_from_tlvp(enum osmo_tlv_parser_error tlv_p_err, struct
msgb *orig_msg)
+{
+ uint8_t bssgp_cause;
+ switch (tlv_p_err) {
+ case OSMO_TLVP_ERR_MAND_IE_MISSING:
+ bssgp_cause = BSSGP_CAUSE_MISSING_MAND_IE;
+ break;
+ default:
+ bssgp_cause = BSSGP_CAUSE_PROTO_ERR_UNSPEC;
+ }
+ return bssgp_tx_status(bssgp_cause, NULL, orig_msg);
+}
+
/* strip off the NS header */
static void strip_ns_hdr(struct msgb *msg)
{
@@ -423,26 +437,34 @@
int data_len = msgb_bssgp_len(msg) - sizeof(*bgph);
struct gbproxy_bvc *from_bvc = NULL;
struct gprs_ra_id raid;
+ char log_pfx[32];
int rc;
+ snprintf(log_pfx, sizeof(log_pfx), "NSE(%05u/BSS)", nsei);
+
if (ns_bvci != 0 && ns_bvci != 1) {
- LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/BSS) BVCI=%05u is not
signalling\n", nsei, ns_bvci);
+ LOGP(DGPRS, LOGL_NOTICE, "%s BVCI=%05u is not signalling\n",
log_pfx, ns_bvci);
return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg);
}
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));
+ LOGP(DGPRS, LOGL_NOTICE, "%s %s not allowed in signalling
BVC\n", log_pfx,
+ 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));
+ LOGP(DGPRS, LOGL_NOTICE, "%s %s not allowed in uplink
direction\n", log_pfx,
+ 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);
+ rc = osmo_tlv_prot_parse(&osmo_pdef_bssgp, &tp, 1, pdu_type,
bgph->data, data_len, 0, 0,
+ DGPRS, log_pfx);
+ if (rc < 0) {
+ rate_ctr_inc(&cfg->ctrg->ctr[GBPROX_GLOB_CTR_PROTO_ERR_BSS]);
+ return tx_status_from_tlvp(rc, msg);
+ }
switch (pdu_type) {
case BSSGP_PDUT_SUSPEND:
@@ -626,24 +648,27 @@
struct gbproxy_bvc *bvc;
uint16_t bvci;
struct msgb *msg;
+ char log_pfx[32];
int rc = 0;
int cause;
int i;
+ snprintf(log_pfx, sizeof(log_pfx), "NSE(%05u/SGSN)", nsei);
+
if (ns_bvci != 0 && ns_bvci != 1) {
- LOGP(DGPRS, LOGL_NOTICE, "NSE(%05u/SGSN) BVCI=%05u is not
signalling\n", nsei, ns_bvci);
+ LOGP(DGPRS, LOGL_NOTICE, "%s BVCI=%05u is not signalling\n",
log_pfx, 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));
+ LOGP(DGPRS, LOGL_NOTICE, "%s %s not allowed in signalling
BVC\n", log_pfx,
+ 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));
+ LOGP(DGPRS, LOGL_NOTICE, "%s %s not allowed in downlink
direction\n", log_pfx,
+ osmo_tlv_prot_msg_name(&osmo_pdef_bssgp, pdu_type));
return bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL,
orig_msg);
}
@@ -651,7 +676,15 @@
/* Update message info */
bgph = (struct bssgp_normal_hdr *) msgb_bssgph(msg);
data_len = msgb_bssgp_len(orig_msg) - sizeof(*bgph);
- rc = bssgp_tlv_parse(&tp, bgph->data, data_len);
+
+ rc = osmo_tlv_prot_parse(&osmo_pdef_bssgp, &tp, 1, pdu_type,
bgph->data, data_len, 0, 0,
+ DGPRS, log_pfx);
+ if (rc < 0) {
+ rc = tx_status_from_tlvp(rc, msg);
+ msgb_free(msg);
+ rate_ctr_inc(&cfg->ctrg->ctr[GBPROX_GLOB_CTR_PROTO_ERR_SGSN]);
+ return rc;
+ }
switch (pdu_type) {
case BSSGP_PDUT_BVC_RESET:
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/21591
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I0ea3f5f9566d9bf5a8429c3ee748e3e90cda6cd7
Gerrit-Change-Number: 21591
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-MessageType: merged
