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

9 files changed, 189 insertions, 9 deletions

diff --git a/configure.ac b/configure.ac
index ee80900ff..b5cd5943e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -255,6 +255,7 @@ AC_OUTPUT(
     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 4414a8db2..cf7d7ce9b 100644
--- a/include/osmocom/msc/mncc.h
+++ b/include/osmocom/msc/mncc.h
@@ -159,6 +159,9 @@ struct gsm_mncc {
 
 	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 @@ struct gsm_data_frame {
 	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 @@ struct gsm_mncc_rtp {
 	uint16_t	port;
 	uint32_t	payload_type;
 	uint32_t	payload_msg_type;
+	char		sdp[1024];
 };
 
 struct gsm_mncc_bridge {
@@ -226,6 +230,7 @@ int mncc_sock_init(struct gsm_network *net, const char *sock_path);
 		|| 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 d0b2ff263..0a38997bc 100644
--- a/src/libmsc/mncc.c
+++ b/src/libmsc/mncc.c
@@ -235,6 +235,34 @@ static int mncc_prim_check_sign(const struct gsm_mncc *mncc_prim)
 	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 @@ int mncc_prim_check(const struct gsm_mncc *mncc_prim, unsigned int len)
 	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 @@ int mncc_prim_check(const struct gsm_mncc *mncc_prim, unsigned int len)
 		}
 		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 864ac7ccb..5af80a464 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -3,6 +3,7 @@ SUBDIRS = \
 	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 000000000..ae859ab21
--- /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 000000000..580b47922
--- /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 000000000..5ee7e2cc3
--- /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 000000000..807904d9b
--- /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 946d0db11..58855f817 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -114,3 +114,10 @@ cat $abs_srcdir/sdp_msg/sdp_msg_test.ok > expout
 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