gprs: Add replies for all GSUP requests

Currently, an incoming GSUP request message isn't answered at all if
it is not handled due to an error or missing implementation.

This patch adds GSUP error replies for these requests (and only for
requests). It also adds tests for these cases.

Note that several of these tests check for
GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL, which will have to be changed, when
the features are implemented.

Sponsored-by: On-Waves ehf

4 files changed, 103 insertions, 10 deletions

diff --git a/openbsc/include/openbsc/gprs_gsup_messages.h b/openbsc/include/openbsc/gprs_gsup_messages.h
index b63f74baa..9857b979d 100644
--- a/openbsc/include/openbsc/gprs_gsup_messages.h
+++ b/openbsc/include/openbsc/gprs_gsup_messages.h
@@ -74,6 +74,10 @@ enum gprs_gsup_message_type {
 	GPRS_GSUP_MSGT_LOCATION_CANCEL_RESULT	= 0b00011110,
 };
 
+#define GPRS_GSUP_IS_MSGT_REQUEST(msgt) (((msgt) & 0b00000011) == 0b00)
+#define GPRS_GSUP_IS_MSGT_ERROR(msgt)   (((msgt) & 0b00000011) == 0b01)
+#define GPRS_GSUP_TO_MSGT_ERROR(msgt)   (((msgt) & 0b11111100) | 0b01)
+
 enum gprs_gsup_cancel_type {
 	GPRS_GSUP_CANCEL_TYPE_UPDATE		= 1, /* on wire: 0 */
 	GPRS_GSUP_CANCEL_TYPE_WITHDRAW		= 2, /* on wire: 1 */
diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index d6a9bdada..25810ab3a 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -289,7 +289,8 @@ struct sgsn_subscriber_data {
 };
 
 #define LOGGSUBSCRP(level, subscr, fmt, args...) \
-	LOGP(DGPRS, level, "SUBSCR(%s) " fmt, (subscr)->imsi, \
+	LOGP(DGPRS, level, "SUBSCR(%s) " fmt, \
+	     (subscr) ? (subscr)->imsi : "---", \
 	     ## args)
 
 struct sgsn_config;
diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index 88e037e9d..e971210ef 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -231,12 +231,13 @@ static int gprs_subscr_tx_gsup_message(struct gsm_subscriber *subscr,
 {
 	struct msgb *msg = gprs_gsup_msgb_alloc();
 
-	strncpy(gsup_msg->imsi, subscr->imsi, sizeof(gsup_msg->imsi) - 1);
+	if (strlen(gsup_msg->imsi) == 0 && subscr)
+		strncpy(gsup_msg->imsi, subscr->imsi, sizeof(gsup_msg->imsi) - 1);
 
 	gprs_gsup_encode(msg, gsup_msg);
 
 	LOGGSUBSCRP(LOGL_INFO, subscr,
-		"Sending GSUP, will send: %s\n", msgb_hexdump(msg));
+		    "Sending GSUP, will send: %s\n", msgb_hexdump(msg));
 
 	if (!sgsn->gsup_client) {
 		msgb_free(msg);
@@ -246,6 +247,20 @@ static int gprs_subscr_tx_gsup_message(struct gsm_subscriber *subscr,
 	return gprs_gsup_client_send(sgsn->gsup_client, msg);
 }
 
+static int gprs_subscr_tx_gsup_error_reply(struct gsm_subscriber *subscr,
+					   struct gprs_gsup_message *gsup_orig,
+					   enum gsm48_gmm_cause cause)
+{
+	struct gprs_gsup_message gsup_reply = {0};
+
+	strncpy(gsup_reply.imsi, gsup_orig->imsi, sizeof(gsup_reply.imsi) - 1);
+	gsup_reply.cause = cause;
+	gsup_reply.message_type =
+		GPRS_GSUP_TO_MSGT_ERROR(gsup_orig->message_type);
+
+	return gprs_subscr_tx_gsup_message(subscr, &gsup_reply);
+}
+
 static int gprs_subscr_handle_gsup_auth_res(struct gsm_subscriber *subscr,
 					    struct gprs_gsup_message *gsup_msg)
 {
@@ -475,6 +490,32 @@ static int gprs_subscr_handle_gsup_purge_err(struct gsm_subscriber *subscr,
 	return -gsup_msg->cause;
 }
 
+static int gprs_subscr_handle_unknown_imsi(struct gprs_gsup_message *gsup_msg)
+{
+	if (GPRS_GSUP_IS_MSGT_REQUEST(gsup_msg->message_type)) {
+		gprs_subscr_tx_gsup_error_reply(NULL, gsup_msg,
+						GMM_CAUSE_IMSI_UNKNOWN);
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "Unknown IMSI %s, discarding GSUP request "
+		     "of type 0x%02x\n",
+		     gsup_msg->imsi, gsup_msg->message_type);
+	} else if (GPRS_GSUP_IS_MSGT_ERROR(gsup_msg->message_type)) {
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "Unknown IMSI %s, discarding GSUP error "
+		     "of type 0x%02x, cause '%s' (%d)\n",
+		     gsup_msg->imsi, gsup_msg->message_type,
+		     get_value_string(gsm48_gmm_cause_names, gsup_msg->cause),
+		     gsup_msg->cause);
+	} else {
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "Unknown IMSI %s, discarding GSUP response "
+		     "of type 0x%02x\n",
+		     gsup_msg->imsi, gsup_msg->message_type);
+	}
+
+	return -GMM_CAUSE_IMSI_UNKNOWN;
+}
+
 int gprs_subscr_rx_gsup_message(struct msgb *msg)
 {
 	uint8_t *data = msgb_l2(msg);
@@ -500,11 +541,8 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 	else
 		subscr = gprs_subscr_get_by_imsi(gsup_msg.imsi);
 
-	if (!subscr) {
-		LOGP(DGPRS, LOGL_NOTICE,
-		     "Unknown IMSI %s, discarding GSUP message\n", gsup_msg.imsi);
-		return -GMM_CAUSE_IMSI_UNKNOWN;
-	}
+	if (!subscr)
+		return gprs_subscr_handle_unknown_imsi(&gsup_msg);
 
 	LOGGSUBSCRP(LOGL_INFO, subscr,
 		"Received GSUP message of type 0x%02x\n", gsup_msg.message_type);
@@ -545,6 +583,8 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 		LOGGSUBSCRP(LOGL_ERROR, subscr,
 			"Rx GSUP message type %d not yet implemented\n",
 			gsup_msg.message_type);
+		gprs_subscr_tx_gsup_error_reply(subscr, &gsup_msg,
+						GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
 		rc = -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL;
 		break;
 
@@ -552,7 +592,10 @@ int gprs_subscr_rx_gsup_message(struct msgb *msg)
 		LOGGSUBSCRP(LOGL_ERROR, subscr,
 			"Rx GSUP message type %d not valid at SGSN\n",
 			gsup_msg.message_type);
-		rc = -GMM_CAUSE_MSGT_INCOMP_P_STATE;
+		if (GPRS_GSUP_IS_MSGT_REQUEST(gsup_msg.message_type))
+			gprs_subscr_tx_gsup_error_reply(
+				subscr, &gsup_msg, GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
+		rc = -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL;
 		break;
 	};
 
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index d419f0a92..7050a16ec 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -468,6 +468,21 @@ static void test_subscriber_gsup(void)
 		0x06, 0x01, 0x00,
 	};
 
+	static const uint8_t insert_data_req[] = {
+		0x10,
+		TEST_GSUP_IMSI1_IE,
+		0x05, 0x11,
+			0x10, 0x01, 0x03,
+			0x11, 0x02, 0xf1, 0x21, /* IPv4 */
+			0x12, 0x08, 0x03, 'b', 'a', 'r', 0x03, 'a', 'p', 'n',
+	};
+
+	static const uint8_t delete_data_req[] = {
+		0x14,
+		TEST_GSUP_IMSI1_IE,
+		0x10, 0x01, 0x03,
+	};
+
 	printf("Testing subcriber GSUP handling\n");
 
 	update_subscriber_data_cb = my_dummy_sgsn_update_subscriber_data;
@@ -527,7 +542,19 @@ static void test_subscriber_gsup(void)
 	/* Check authorization */
 	OSMO_ASSERT(s1->authorized == 0);
 
-	/* Inject UpdateLocReq GSUP message */
+	/* Inject InsertSubscrData GSUP message */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(insert_data_req, sizeof(insert_data_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject DeleteSubscrData GSUP message */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(delete_data_req, sizeof(delete_data_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_MSGT_NOTEXIST_NOTIMPL);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject LocCancelReq GSUP message */
 	rc = rx_gsup_message(location_cancellation_req,
 			     sizeof(location_cancellation_req));
 	OSMO_ASSERT(rc >= 0);
@@ -543,6 +570,24 @@ static void test_subscriber_gsup(void)
 	OSMO_ASSERT(s1found == NULL);
 	gprs_llgmm_assign(llme, local_tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
 
+	/* Inject InsertSubscrData GSUP message (unknown IMSI) */
+	last_updated_subscr = NULL;
+	rc = rx_gsup_message(insert_data_req, sizeof(insert_data_req));
+	/* TODO: Remove the comments when this is fixed */
+	/* OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN); */
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject DeleteSubscrData GSUP message (unknown IMSI) */
+	rc = rx_gsup_message(delete_data_req, sizeof(delete_data_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
+	/* Inject LocCancelReq GSUP message (unknown IMSI) */
+	rc = rx_gsup_message(location_cancellation_req,
+			     sizeof(location_cancellation_req));
+	OSMO_ASSERT(rc == -GMM_CAUSE_IMSI_UNKNOWN);
+	OSMO_ASSERT(last_updated_subscr == NULL);
+
 	update_subscriber_data_cb = __real_sgsn_update_subscriber_data;
 }