Change in osmo-msc[master]: MNCC v6: add optional SDP to the socket protocol

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Thu Nov 28 15:24:28 UTC 2019


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15948 )

Change subject: MNCC v6: add optional SDP to the socket protocol
......................................................................

MNCC v6: add optional SDP to the socket protocol

Add a char buffer of 1024 characters length as space for SDP to pass to /
receive from MNCC.

Actually support receiving MNCC without such an SDP tail. The main reason for
this is to avoid the need to adjust the ttcn3 implementation of MNCC: it would
stop working for older osmo-msc.

Older or non-SIP MNCC peers could operate the previous MNCC protocol unchanged
(save the protocol number bump) without having to implement SDP.

The SDP part in the MNCC protocol will be used in upcoming patch
I8c3b2de53ffae4ec3a66b9dabf308c290a2c999f.

This patch must be merged at the same time as osmo-sip-connector patch
Iaca9ed6611fc5ca8ca749bbbefc31f54bea5e925, so that both sides have a matching
MNCC protocol version number.

Change-Id: Ie16f0804c4d99760cd4a0c544d0889b6313eebb7
---
M configure.ac
M include/osmocom/msc/mncc.h
M src/libmsc/mncc.c
M tests/Makefile.am
A tests/mncc/Makefile.am
A tests/mncc/mncc_test.c
A tests/mncc/mncc_test.err
A tests/mncc/mncc_test.ok
M tests/testsuite.at
9 files changed, 189 insertions(+), 9 deletions(-)

Approvals:
  Jenkins Builder: Verified
  neels: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  laforge: Looks good to me, but someone else must approve



diff --git a/configure.ac b/configure.ac
index ee80900..b5cd594 100644
--- a/configure.ac
+++ b/configure.ac
@@ -255,6 +255,7 @@
     tests/sms_queue/Makefile
     tests/msc_vlr/Makefile
     tests/sdp_msg/Makefile
+    tests/mncc/Makefile
     doc/Makefile
     doc/examples/Makefile
     doc/manuals/Makefile
diff --git a/include/osmocom/msc/mncc.h b/include/osmocom/msc/mncc.h
index 4414a8d..cf7d7ce 100644
--- a/include/osmocom/msc/mncc.h
+++ b/include/osmocom/msc/mncc.h
@@ -159,6 +159,9 @@
 
 	unsigned char	lchan_type;
 	unsigned char	lchan_mode;
+
+	/* A buffer to contain SDP ('\0' terminated) */
+	char		sdp[1024];
 };
 
 struct gsm_data_frame {
@@ -167,7 +170,7 @@
 	unsigned char	data[0];
 };
 
-#define MNCC_SOCK_VERSION	5
+#define MNCC_SOCK_VERSION	6
 struct gsm_mncc_hello {
 	uint32_t	msg_type;
 	uint32_t	version;
@@ -190,6 +193,7 @@
 	uint16_t	port;
 	uint32_t	payload_type;
 	uint32_t	payload_msg_type;
+	char		sdp[1024];
 };
 
 struct gsm_mncc_bridge {
@@ -226,6 +230,7 @@
 		|| msg_type == GSM_BAD_FRAME)
 
 int mncc_prim_check(const struct gsm_mncc *mncc_prim, unsigned int len);
+int mncc_check_sdp_termination(const char *label, const struct gsm_mncc *mncc, unsigned int len, const char *sdp);
 
 int mncc_bearer_cap_to_channel_type(struct gsm0808_channel_type *ct, const struct gsm_mncc_bearer_cap *bc);
 
diff --git a/src/libmsc/mncc.c b/src/libmsc/mncc.c
index d0b2ff2..0a38997 100644
--- a/src/libmsc/mncc.c
+++ b/src/libmsc/mncc.c
@@ -235,6 +235,34 @@
 	return 0;
 }
 
+/* Make sure that the SDP section has a terminating \0. The MNCC message may end after that \0, and if SDP is omitted it
+ * must contain at least one \0 byte. */
+int mncc_check_sdp_termination(const char *label, const struct gsm_mncc *mncc, unsigned int len, const char *sdp)
+{
+	size_t sdp_offset;
+	size_t sdp_data_len;
+	size_t sdp_str_len;
+
+	OSMO_ASSERT(((char*)mncc) < sdp);
+
+	sdp_offset = sdp - (char*)mncc;
+	if (len < sdp_offset)
+		goto too_short;
+
+	sdp_data_len = len - sdp_offset;
+	if (sdp_data_len < 1)
+		goto too_short;
+
+	sdp_str_len = strnlen(sdp, sdp_data_len);
+	/* There must be a \0, so sdp_str_len must be at most sdp_data_len - 1 */
+	if (sdp_str_len >= sdp_data_len)
+		goto too_short;
+	return 0;
+too_short:
+	LOGP(DMNCC, LOGL_ERROR, "Short %s\n", label);
+	return -EINVAL;
+}
+
 int mncc_prim_check(const struct gsm_mncc *mncc_prim, unsigned int len)
 {
 	if (len < sizeof(mncc_prim->msg_type)) {
@@ -262,11 +290,7 @@
 	case MNCC_RTP_FREE:
 	case MNCC_RTP_CONNECT:
 	case MNCC_RTP_CREATE:
-		if (len < sizeof(struct gsm_mncc_rtp)) {
-			LOGP(DMNCC, LOGL_ERROR, "Short MNCC RTP\n");
-			return -EINVAL;
-		}
-		break;
+		return mncc_check_sdp_termination("MNCC RTP", mncc_prim, len, ((struct gsm_mncc_rtp*)mncc_prim)->sdp);
 	case MNCC_LCHAN_MODIFY:
 	case MNCC_FRAME_DROP:
 	case MNCC_FRAME_RECV:
@@ -279,10 +303,8 @@
 		}
 		break;
 	default:
-		if (len < sizeof(struct gsm_mncc)) {
-			LOGP(DMNCC, LOGL_ERROR, "Short MNCC Signalling\n");
+		if (mncc_check_sdp_termination("MNCC Signalling", mncc_prim, len, mncc_prim->sdp))
 			return -EINVAL;
-		}
 		return mncc_prim_check_sign(mncc_prim);
 	}
 	return 0;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 864ac7c..5af80a4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -3,6 +3,7 @@
 	msc_vlr \
 	db_sms \
 	sdp_msg \
+	mncc \
 	$(NULL)
 
 if BUILD_SMPP
diff --git a/tests/mncc/Makefile.am b/tests/mncc/Makefile.am
new file mode 100644
index 0000000..ae859ab
--- /dev/null
+++ b/tests/mncc/Makefile.am
@@ -0,0 +1,32 @@
+AM_CPPFLAGS = \
+	$(all_includes) \
+	-I$(top_srcdir)/include \
+	$(NULL)
+
+AM_CFLAGS = \
+	-Wall \
+	-ggdb3 \
+	$(LIBOSMOCORE_CFLAGS) \
+	$(NULL)
+
+LDADD = \
+	$(top_builddir)/src/libmsc/libmsc.a \
+	$(LIBOSMOCORE_LIBS) \
+	$(NULL)
+
+EXTRA_DIST = \
+	mncc_test.ok \
+	mncc_test.err \
+	$(NULL)
+
+noinst_PROGRAMS = \
+	mncc_test \
+	$(NULL)
+
+mncc_test_SOURCES = \
+	mncc_test.c \
+	$(NULL)
+
+.PHONY: update_exp
+update_exp:
+	$(builddir)/mncc_test >$(srcdir)/mncc_test.ok 2>$(srcdir)/mncc_test.err
diff --git a/tests/mncc/mncc_test.c b/tests/mncc/mncc_test.c
new file mode 100644
index 0000000..580b479
--- /dev/null
+++ b/tests/mncc/mncc_test.c
@@ -0,0 +1,79 @@
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <osmocom/core/application.h>
+#include <osmocom/core/logging.h>
+#include <osmocom/msc/debug.h>
+#include <osmocom/msc/mncc.h>
+
+#define _test_sdp_termination(LABEL, MNCC, MNCC_MSG_LEN, RC) do { \
+		int sdp_len = ((int)(MNCC_MSG_LEN)) - ((MNCC)->sdp - (char*)MNCC); \
+		size_t sdp_strlen = strnlen(MNCC->sdp, sizeof(MNCC->sdp)); \
+		int rc = mncc_check_sdp_termination("<" LABEL ">", (struct gsm_mncc*)MNCC, MNCC_MSG_LEN, MNCC->sdp); \
+		printf("%s: len=%d sdplen=%d sdp=%s rc=%d\n", \
+		       LABEL, (int)(MNCC_MSG_LEN), sdp_len, \
+		       sdp_len > 0? osmo_quote_str((MNCC)->sdp, OSMO_MIN(sdp_len, sdp_strlen+1)) : "-", rc); \
+		if (RC != rc) \
+			printf("ERROR!\n"); \
+	} while (0)
+
+#define test_sdp_termination_cases(MNCC) \
+	_test_sdp_termination("empty SDP", MNCC, sizeof(*MNCC), 0); \
+	_test_sdp_termination("empty SDP, shortest possible", MNCC, MNCC->sdp - ((char*)MNCC) + 1, 0); \
+	_test_sdp_termination("empty SDP, zero len", MNCC, MNCC->sdp - ((char*)MNCC), -EINVAL); \
+	OSMO_STRLCPY_ARRAY(MNCC->sdp, "Privacy is a desirable marketing option"); \
+	_test_sdp_termination("terminated SDP str", MNCC, sizeof(*MNCC), 0); \
+	_test_sdp_termination("terminated SDP str, shortest possible", MNCC, \
+			      MNCC->sdp - ((char*)MNCC) + strlen(MNCC->sdp) + 1, 0); \
+	_test_sdp_termination("terminated SDP str, but len excludes nul", MNCC, \
+			      MNCC->sdp - ((char*)MNCC) + strlen(MNCC->sdp), -EINVAL); \
+	_test_sdp_termination("terminated SDP str, but len too short", MNCC, \
+			      MNCC->sdp - ((char*)MNCC) + 23, -EINVAL); \
+	_test_sdp_termination("len way too short", MNCC, 10, -EINVAL); \
+	_test_sdp_termination("len zero", MNCC, 0, -EINVAL);
+
+
+void test_sdp_termination(void)
+{
+	struct gsm_mncc _mncc = {};
+	struct gsm_mncc_rtp _mncc_rtp = {};
+
+	struct gsm_mncc *mncc = &_mncc;
+	struct gsm_mncc_rtp *mncc_rtp = &_mncc_rtp;
+
+	printf("%s()\n", __func__);
+	printf("\nstruct gsm_mncc:\n");
+	test_sdp_termination_cases(mncc);
+
+	_mncc = (struct gsm_mncc){};
+	_mncc_rtp = (struct gsm_mncc_rtp){};
+	printf("\nstruct gsm_mncc_rtp:\n");
+	test_sdp_termination_cases(mncc_rtp);
+}
+
+static const struct log_info_cat default_categories[] = {
+	[DMNCC] = {
+		.name = "DMNCC",
+		.description = "MNCC API for Call Control application",
+		.color = "\033[1;39m",
+		.enabled = 1, .loglevel = LOGL_NOTICE,
+	},
+};
+
+const struct log_info log_info = {
+	.cat = default_categories,
+	.num_cat = ARRAY_SIZE(default_categories),
+};
+
+int main(void)
+{
+	void *ctx = talloc_named_const(NULL, 0, "smpp_test");
+	osmo_init_logging2(ctx, &log_info);
+	log_set_use_color(osmo_stderr_target, 0);
+	log_set_print_filename(osmo_stderr_target, 0);
+	log_set_print_category(osmo_stderr_target, 1);
+	log_set_print_category_hex(osmo_stderr_target, 0);
+
+	test_sdp_termination();
+	return 0;
+}
diff --git a/tests/mncc/mncc_test.err b/tests/mncc/mncc_test.err
new file mode 100644
index 0000000..5ee7e2c
--- /dev/null
+++ b/tests/mncc/mncc_test.err
@@ -0,0 +1,10 @@
+DMNCC Short <empty SDP, zero len>
+DMNCC Short <terminated SDP str, but len excludes nul>
+DMNCC Short <terminated SDP str, but len too short>
+DMNCC Short <len way too short>
+DMNCC Short <len zero>
+DMNCC Short <empty SDP, zero len>
+DMNCC Short <terminated SDP str, but len excludes nul>
+DMNCC Short <terminated SDP str, but len too short>
+DMNCC Short <len way too short>
+DMNCC Short <len zero>
diff --git a/tests/mncc/mncc_test.ok b/tests/mncc/mncc_test.ok
new file mode 100644
index 0000000..807904d
--- /dev/null
+++ b/tests/mncc/mncc_test.ok
@@ -0,0 +1,23 @@
+test_sdp_termination()
+
+struct gsm_mncc:
+empty SDP: len=1860 sdplen=1026 sdp="\0" rc=0
+empty SDP, shortest possible: len=835 sdplen=1 sdp="\0" rc=0
+empty SDP, zero len: len=834 sdplen=0 sdp=- rc=-22
+terminated SDP str: len=1860 sdplen=1026 sdp="Privacy is a desirable marketing option\0" rc=0
+terminated SDP str, shortest possible: len=874 sdplen=40 sdp="Privacy is a desirable marketing option\0" rc=0
+terminated SDP str, but len excludes nul: len=873 sdplen=39 sdp="Privacy is a desirable marketing option" rc=-22
+terminated SDP str, but len too short: len=857 sdplen=23 sdp="Privacy is a desirable " rc=-22
+len way too short: len=10 sdplen=-824 sdp=- rc=-22
+len zero: len=0 sdplen=-834 sdp=- rc=-22
+
+struct gsm_mncc_rtp:
+empty SDP: len=1048 sdplen=1024 sdp="\0" rc=0
+empty SDP, shortest possible: len=25 sdplen=1 sdp="\0" rc=0
+empty SDP, zero len: len=24 sdplen=0 sdp=- rc=-22
+terminated SDP str: len=1048 sdplen=1024 sdp="Privacy is a desirable marketing option\0" rc=0
+terminated SDP str, shortest possible: len=64 sdplen=40 sdp="Privacy is a desirable marketing option\0" rc=0
+terminated SDP str, but len excludes nul: len=63 sdplen=39 sdp="Privacy is a desirable marketing option" rc=-22
+terminated SDP str, but len too short: len=47 sdplen=23 sdp="Privacy is a desirable " rc=-22
+len way too short: len=10 sdplen=-14 sdp=- rc=-22
+len zero: len=0 sdplen=-24 sdp=- rc=-22
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 946d0db..58855f8 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -114,3 +114,10 @@
 cat $abs_srcdir/sdp_msg/sdp_msg_test.err > experr
 AT_CHECK([$abs_top_builddir/tests/sdp_msg/sdp_msg_test], [], [expout], [experr])
 AT_CLEANUP
+
+AT_SETUP([mncc_test])
+AT_KEYWORDS([mncc_test])
+cat $abs_srcdir/mncc/mncc_test.ok > expout
+cat $abs_srcdir/mncc/mncc_test.err > experr
+AT_CHECK([$abs_top_builddir/tests/mncc/mncc_test], [], [expout], [experr])
+AT_CLEANUP

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15948
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie16f0804c4d99760cd4a0c544d0889b6313eebb7
Gerrit-Change-Number: 15948
Gerrit-PatchSet: 6
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191128/2ef149d1/attachment.htm>


More information about the gerrit-log mailing list