Change in libosmocore[master]: gprs_ns: Don't use initial IP/port for anything but SNS

Harald Welte gerrit-no-reply at lists.osmocom.org
Sat Mar 16 13:43:19 UTC 2019


Harald Welte has uploaded this change for review. ( https://gerrit.osmocom.org/13291


Change subject: gprs_ns: Don't use initial IP/port for anything but SNS
......................................................................

gprs_ns: Don't use initial IP/port for anything but SNS

Section 6.2.1 of 3GPP TS 48.016 states:
> A pre-configured endpoint shall not be used for NSE data or signalling
> traffic (with the exception of Size and Configuration procedures) unless
> it is configured by the SGSN using the auto-configuration procedures.

However, in the current SNS implementation, the initial IP/Port over
which we perform the SNS-SIZE + SNS-CONFIG are treated as one of the
normal NS-VCs. Specifically, we also perform the NS-ALIVE procedure on
it, which is clearly wrong.

Let's explicitly create the "initial" NS-VC with data and signalling
weight of 0, and ensure we never start the alive timer or send any
non-SNS PDUs on this connection as long as SNS was not used to change
either of the two weights to non-zero.

While at it, also safeguard against processing any incoming non-SNS
messages on such a all-zero-weight connection.

Change-Id: I16a91a07e5914d123b2ea2f8413b94e7cd518628
Closes: OS#3844
---
M src/gb/gprs_ns.c
1 file changed, 55 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/91/13291/1

diff --git a/src/gb/gprs_ns.c b/src/gb/gprs_ns.c
index 93c219c..6508aa7 100644
--- a/src/gb/gprs_ns.c
+++ b/src/gb/gprs_ns.c
@@ -195,6 +195,14 @@
 			LOGP(DNS, LOGL_ERROR, "TX failed (%d) to peer %s\n",	\
 				rc, gprs_ns_ll_str(nsvc));
 
+static bool nsvc_is_not_used(const struct gprs_nsvc *nsvc)
+{
+	if (nsvc->data_weight == 0 && nsvc->sig_weight == 0)
+		return true;
+	else
+		return false;
+}
+
 struct msgb *gprs_ns_msgb_alloc(void)
 {
 	struct msgb *msg = msgb_alloc_headroom(NS_ALLOC_SIZE, NS_ALLOC_HEADROOM,
@@ -444,12 +452,40 @@
 static int nsip_sendmsg(struct gprs_nsvc *nsvc, struct msgb *msg);
 extern int grps_ns_frgre_sendmsg(struct gprs_nsvc *nsvc, struct msgb *msg);
 
+static bool ns_is_sns(uint8_t pdu_type)
+{
+	switch (pdu_type) {
+	case SNS_PDUT_CONFIG:
+	case SNS_PDUT_ACK:
+	case SNS_PDUT_ADD:
+	case SNS_PDUT_CHANGE_WEIGHT:
+	case SNS_PDUT_DELETE:
+	case SNS_PDUT_CONFIG_ACK:
+	case SNS_PDUT_SIZE:
+	case SNS_PDUT_SIZE_ACK:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int gprs_ns_tx(struct gprs_nsvc *nsvc, struct msgb *msg)
 {
+	struct gprs_ns_hdr *nsh = (struct gprs_ns_hdr *) msg->l2h;
 	int ret;
 
 	log_set_context(LOG_CTX_GB_NSVC, nsvc);
 
+	/* A pre-configured endpoint shall not be used for NSE data or signalling
+	 * traffic (with the exception of Size and Configuration procedures) unless it
+	 * is configured by the SGSN using the auto-configuration procedures. */
+	if (nsvc_is_not_used(nsvc) && !ns_is_sns(nsh->pdu_type) && nsh->pdu_type != NS_PDUT_STATUS) {
+		LOGP(DNS, LOGL_NOTICE, "Not transmitting %s on unused/pre-configured eindpoint\n",
+			get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
+		msgb_free(msg);
+		return -EINVAL;
+	}
+
 	/* Increment number of Uplink bytes */
 	rate_ctr_inc(&nsvc->ctrg->ctr[NS_CTR_PKTS_OUT]);
 	rate_ctr_add(&nsvc->ctrg->ctr[NS_CTR_BYTES_OUT], msgb_l2len(msg));
@@ -1690,6 +1726,13 @@
 	rate_ctr_inc(&(*nsvc)->ctrg->ctr[NS_CTR_PKTS_IN]);
 	rate_ctr_add(&(*nsvc)->ctrg->ctr[NS_CTR_BYTES_IN], msgb_l2len(msg));
 
+	if (nsvc_is_not_used(*nsvc) && !ns_is_sns(nsh->pdu_type) && nsh->pdu_type != NS_PDUT_STATUS) {
+		LOGP(DNS, LOGL_NOTICE, "NSEI=%u Rx %s on unused/pre-configured endpoint, discarding\n",
+			(*nsvc)->nsei, get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
+		gprs_ns_tx_status(*nsvc, NS_CAUSE_PROTO_ERR_UNSPEC, 0, msg);
+		return 0;
+	}
+
 	switch (nsh->pdu_type) {
 	case NS_PDUT_ALIVE:
 		/* If we're dead and blocked and suddenly receive a
@@ -2116,8 +2159,14 @@
 	 * require some massive code and API changes compared to existing libosmogb,
 	 * so let's keep the old logic. */
 	nsvc = gprs_nsvc_by_rem_addr(nsi, dest);
-	if (!nsvc)
-		nsvc = gprs_nsvc_create(nsi, nsvci);
+	if (!nsvc) {
+		/* create NSVC with 0 data + signalling weight. This is illegal in SNS
+		 * and can hence only be created locally and serves as indication that
+		 * this NS-VC shall not be used for anything except SNS _unless_ it is
+		 * modified via SNS-{CONFIG,CHANGEWEIGHT,ADD} to become part of the
+		 * active NS-VCs */
+		nsvc = gprs_nsvc_create2(nsi, nsvci, 0, 0);
+	}
 	nsvc->ip.bts_addr = *dest;
 	nsvc->nsei = nsei;
 	nsvc->remote_end_is_sgsn = 1;
@@ -2161,6 +2210,10 @@
 /*! Start the ALIVE timer procedure in all NS-VCs part of this NS Instance */
 void gprs_nsvc_start_test(struct gprs_nsvc *nsvc)
 {
+	/* skip the initial NS-VC unless it has explicitly been configured
+	 * via SNS-CONFIG from the SGSN */
+	if (nsvc_is_not_used(nsvc))
+		return;
 	gprs_ns_tx_alive(nsvc);
 	nsvc_start_timer(nsvc, NSVC_TIMER_TNS_TEST);
 }

-- 
To view, visit https://gerrit.osmocom.org/13291
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I16a91a07e5914d123b2ea2f8413b94e7cd518628
Gerrit-Change-Number: 13291
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190316/42b29e7e/attachment.html>


More information about the gerrit-log mailing list