[PATCH] osmo-msc[master]: Implement checks for duplicate uplink UL L3 message

Harald Welte gerrit-no-reply at lists.osmocom.org
Sat Feb 3 20:19:11 UTC 2018


Review at  https://gerrit.osmocom.org/6267

Implement checks for duplicate uplink UL L3 message

According to TS 24.007 Section 11.2.3.2.3, it is possible that uplink L3
messages are duplicated in some scenarios, particularly during
assignment/handover procedure.

To avoid L3 entities from seeing duplicated messages, there's a modulo-2
or modulo-4 message sequence counter, based on which the MSC can detect
and suppress such duplicate messages.

It appears that even our unit tests were wrong in that regard so far.
Rather than manually adjusting each and every message, let's make sure
that the sequence number generation always increments as expected, and
that during matching of incoming messages, sequence numbers are masked
out.

Note: the tests will only pass from libosmocore Change-Id
Iec875a77f5458322dfbef174f5abfc0e8c09d464 onwards, due to
gsm48_hdr_msg_type() being broken in earlier versions.

Change-Id: Id15e399ab7e1b05dcd426b292886fa19d36082b1
Closes: #2908
---
M include/osmocom/msc/gsm_data.h
M src/libmsc/gsm_04_08.c
M tests/msc_vlr/msc_vlr_tests.c
3 files changed, 137 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/67/6267/1

diff --git a/include/osmocom/msc/gsm_data.h b/include/osmocom/msc/gsm_data.h
index 27324d7..16e83f3 100644
--- a/include/osmocom/msc/gsm_data.h
+++ b/include/osmocom/msc/gsm_data.h
@@ -184,6 +184,9 @@
 	uint16_t lac;
 	struct gsm_encr encr;
 
+	/* N(SD) expected in the received frame, per flow (TS 24.007 11.2.3.2.3.2.2) */
+	uint8_t n_sd_next[4];
+
 	struct {
 		unsigned int mgcp_rtp_endpoint;
 		uint16_t port_subscr;
diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index c9ea2c8..5acc4e4 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -3302,6 +3302,94 @@
 	conn->received_cm_service_request = false;
 }
 
+/* TS 24.007 11.2.3.2.3 Message Type Octet / Duplicate Detection */
+int gsm0407_pdisc_ctr_bin(uint8_t pdisc)
+{
+	switch (pdisc) {
+	case GSM48_PDISC_MM:
+	case GSM48_PDISC_CC:
+	case GSM48_PDISC_NC_SS:
+		return 0;
+	case GSM48_PDISC_GROUP_CC:
+		return 1;
+	case GSM48_PDISC_BCAST_CC:
+		return 2;
+	case GSM48_PDISC_LOC:
+		return 3;
+	default:
+		return -1;
+	}
+}
+
+/* extract the N(SD) and return the modulo value for a R98 message */
+static uint8_t gsm0407_determine_nsd_ret_modulo_r99(uint8_t pdisc, uint8_t msg_type, uint8_t *n_sd)
+{
+	switch (pdisc) {
+	case GSM48_PDISC_MM:
+	case GSM48_PDISC_CC:
+	case GSM48_PDISC_NC_SS:
+		*n_sd = (msg_type >> 6) & 0x3;
+		return 4;
+	case GSM48_PDISC_GROUP_CC:
+	case GSM48_PDISC_BCAST_CC:
+	case GSM48_PDISC_LOC:
+		*n_sd = (msg_type >> 6) & 0x1;
+		return 2;
+	default:
+		/* no sequence number, we cannot detect dups */
+		return 0;
+	}
+}
+
+/* extract the N(SD) and return the modulo value for a R99 message */
+static uint8_t gsm0407_determine_nsd_ret_modulo_r98(uint8_t pdisc, uint8_t msg_type, uint8_t *n_sd)
+{
+	switch (pdisc) {
+	case GSM48_PDISC_MM:
+	case GSM48_PDISC_CC:
+	case GSM48_PDISC_NC_SS:
+	case GSM48_PDISC_GROUP_CC:
+	case GSM48_PDISC_BCAST_CC:
+	case GSM48_PDISC_LOC:
+		*n_sd = (msg_type >> 6) & 0x1;
+		return 2;
+	default:
+		/* no sequence number, we cannot detect dups */
+		return 0;
+	}
+}
+
+/* TS 24.007 11.2.3.2 Message Type Octet / Duplicate Detection */
+static bool gsm0407_is_duplicate(struct gsm_subscriber_connection *conn, struct msgb *msg)
+{
+	struct gsm48_hdr *gh;
+	uint8_t pdisc;
+	uint8_t n_sd, modulo, bin;
+
+	gh = msgb_l3(msg);
+	pdisc = gsm48_hdr_pdisc(gh);
+
+	if (conn->classmark.classmark1_set && conn->classmark.classmark1.rev_lev < 2) {
+		modulo = gsm0407_determine_nsd_ret_modulo_r98(pdisc, gh->msg_type, &n_sd);
+	} else { /* R99 */
+		modulo = gsm0407_determine_nsd_ret_modulo_r99(pdisc, gh->msg_type, &n_sd);
+	}
+	if (modulo == 0)
+		return false;
+	bin = gsm0407_pdisc_ctr_bin(pdisc);
+	if (bin < 0)
+		return false;
+
+	OSMO_ASSERT(bin < ARRAY_SIZE(conn->n_sd_next));
+	if (n_sd != conn->n_sd_next[bin]) {
+		/* not what we expected: duplicate */
+		return true;
+	} else {
+		/* as expected: no dup; update expected counter for next message */
+		conn->n_sd_next[bin] = (n_sd + 1) % modulo;
+		return false;
+	}
+}
 
 /* Main entry point for GSM 04.08/44.008 Layer 3 data (e.g. from the BSC). */
 int gsm0408_dispatch(struct gsm_subscriber_connection *conn, struct msgb *msg)
@@ -3317,6 +3405,12 @@
 	gh = msgb_l3(msg);
 	pdisc = gsm48_hdr_pdisc(gh);
 
+	if (gsm0407_is_duplicate(conn, msg)) {
+		LOGP(DRLL, LOGL_NOTICE, "%s: Discarding duplicate L3 message\n",
+			(conn && conn->vsub) ? vlr_subscr_name(conn->vsub) : "UNKNOWN");
+		return 0;
+	}
+
 	LOGP(DRLL, LOGL_DEBUG, "Dispatching 04.08 message %s (0x%x:0x%x)\n",
 	     gsm48_pdisc_msgtype_name(pdisc, gsm48_hdr_msg_type(gh)),
 	     pdisc, gsm48_hdr_msg_type(gh));
diff --git a/tests/msc_vlr/msc_vlr_tests.c b/tests/msc_vlr/msc_vlr_tests.c
index e4adfe0..e0e9d83 100644
--- a/tests/msc_vlr/msc_vlr_tests.c
+++ b/tests/msc_vlr/msc_vlr_tests.c
@@ -73,6 +73,36 @@
 bool cc_to_mncc_tx_confirmed = false;
 uint32_t cc_to_mncc_tx_got_callref = 0;
 
+extern int gsm0407_pdisc_ctr_bin(uint8_t pdisc);
+
+/* static state variables for the L3 send sequence numbers */
+static uint8_t n_sd[4];
+
+/* patch a correct send sequence number into the given message */
+static void patch_l3_seq_nr(struct msgb *msg)
+{
+	struct gsm48_hdr *gh = msgb_l3(msg);
+	uint8_t pdisc = gsm48_hdr_pdisc(gh);
+	uint8_t *msg_type_oct = &msg->l3h[1];
+	int bin = gsm0407_pdisc_ctr_bin(pdisc);
+
+	if (bin >= 0 && bin < ARRAY_SIZE(n_sd)) {
+		/* patch in n_sd into the msg_type octet */
+		*msg_type_oct = (*msg_type_oct & 0x3f) | ((n_sd[bin] & 0x3) << 6);
+		//fprintf(stderr, "pdisc=0x%02x bin=%d, patched n_sd=%u\n\n", pdisc, bin, n_sd[bin] & 3);
+		/* increment N(SD) */
+		n_sd[bin] = (n_sd[bin] + 1) % 4;
+	} else {
+		//fprintf(stderr, "pdisc=0x%02x NO SEQ\n\n", pdisc);
+	}
+}
+
+/* reset L3 sequence numbers (e.g. new RR connection) */
+static void reset_l3_seq_nr()
+{
+	memset(n_sd, 0, sizeof(n_sd));
+}
+
 struct msgb *msgb_from_hex(const char *label, uint16_t size, const char *hex)
 {
 	struct msgb *msg = msgb_alloc(size, label);
@@ -101,6 +131,9 @@
 	if (!hex)
 		return;
 	dtap_tx_expected = msgb_from_hex("dtap_tx_expected", 1024, hex);
+	/* Mask the sequence number out */
+	if (msgb_length(dtap_tx_expected) >= 2)
+		dtap_tx_expected->data[1] &= 0x3f;
 	dtap_tx_confirmed = false;
 }
 
@@ -188,12 +221,15 @@
 	if (!g_conn) {
 		log("new conn");
 		g_conn = conn_new();
+		reset_l3_seq_nr();
+		patch_l3_seq_nr(msg);
 		rc = msc_compl_l3(g_conn, msg, 23);
 		if (rc == BSC_API_CONN_POL_REJECT) {
 			msc_subscr_con_free(g_conn);
 			g_conn = NULL;
 		}
 	} else {
+		patch_l3_seq_nr(msg);
 		if ((gsm48_hdr_pdisc(gh) == GSM48_PDISC_RR)
 		    && (gsm48_hdr_msg_type(gh) == GSM48_MT_RR_CIPH_M_COMPL))
 			msc_cipher_mode_compl(g_conn, msg, 0);
@@ -230,6 +266,7 @@
 	/* some amount of data, whatever */
 	msgb_put(msg, 123);
 
+	patch_l3_seq_nr(msg);
 	rc = gsm0408_dispatch(g_conn, msg);
 
 	talloc_free(msg);
@@ -492,6 +529,9 @@
 	    osmo_hexdump_nospc(msg->data, msg->len));
 
 	OSMO_ASSERT(dtap_tx_expected);
+
+	/* Mask the sequence number out before comparing */
+	msg->data[1] &= 0x3f;
 	if (msg->len != dtap_tx_expected->len
 	    || memcmp(msg->data, dtap_tx_expected->data, msg->len)) {
 		fprintf(stderr, "Mismatch! Expected:\n%s\n",

-- 
To view, visit https://gerrit.osmocom.org/6267
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id15e399ab7e1b05dcd426b292886fa19d36082b1
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>


More information about the gerrit-log mailing list