[PATCH] openbsc[master]: Make random MSISDN assignment optional

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/.

Max gerrit-no-reply at lists.osmocom.org
Wed Jun 15 14:41:28 UTC 2016


Hello Harald Welte, Jenkins Builder, Holger Freyther,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/201

to look at the new patch set (#3).

Make random MSISDN assignment optional

Previously if subscriber was automatically created it got assigned
random MSISDN number. Make it optional (defaulting to previous behavior)
by adding following:

* new subscriber-create-no-extension vty command
* db unit tests
* vty test

Note: using the db made with new code might result in subscribers with
empty extension. Such subscribers cannot be deleted using old
code. Make sure not to mix db versions or manually fix it by editing
sqlite with external program.

Change-Id: Ibbc2e88e4722b08854ebc631485f19ed56443cbb
Fixes: OS#1658
---
M openbsc/include/openbsc/db.h
M openbsc/include/openbsc/gsm_data.h
M openbsc/include/openbsc/gsm_subscriber.h
M openbsc/src/libbsc/net_init.c
M openbsc/src/libmsc/ctrl_commands.c
M openbsc/src/libmsc/db.c
M openbsc/src/libmsc/gsm_04_08.c
M openbsc/src/libmsc/gsm_subscriber.c
M openbsc/src/libmsc/vty_interface_layer3.c
M openbsc/tests/db/db_test.c
M openbsc/tests/vty_test_runner.py
11 files changed, 139 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/01/201/3

diff --git a/openbsc/include/openbsc/db.h b/openbsc/include/openbsc/db.h
index 6ffe1ad..bb90705 100644
--- a/openbsc/include/openbsc/db.h
+++ b/openbsc/include/openbsc/db.h
@@ -20,6 +20,8 @@
 #ifndef _DB_H
 #define _DB_H
 
+#include <stdbool.h>
+
 #include "gsm_subscriber.h"
 
 struct gsm_equipment;
@@ -36,7 +38,7 @@
 
 /* subscriber management */
 struct gsm_subscriber *db_create_subscriber(const char *imsi, uint64_t smin,
-					    uint64_t smax);
+					    uint64_t smax, bool alloc_exten);
 struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field field,
 					 const char *subscr);
 int db_sync_subscriber(struct gsm_subscriber *subscriber);
diff --git a/openbsc/include/openbsc/gsm_data.h b/openbsc/include/openbsc/gsm_data.h
index e7cd520..f61ed4e 100644
--- a/openbsc/include/openbsc/gsm_data.h
+++ b/openbsc/include/openbsc/gsm_data.h
@@ -4,6 +4,7 @@
 #include <stdint.h>
 #include <regex.h>
 #include <sys/types.h>
+#include <stdbool.h>
 
 #include <osmocom/core/timer.h>
 #include <osmocom/core/select.h>
@@ -22,7 +23,6 @@
 #define OBSC_LINKID_CB(__msgb)	(__msgb)->cb[3]
 
 enum gsm_subscr_creation_mode {
-	GSM_SUBSCR_DONT_CREATE = 0,
 	GSM_SUBSCR_CREAT_W_RAND_EXT = 1,
 	GSM_SUBSCR_CREAT_W_REGEXP = 2,
 };
@@ -290,6 +290,8 @@
 
 	/* subscriber related features */
 	int subscr_creation_mode;
+	bool auto_create_subscr;
+	bool auto_assign_exten;
 	uint64_t ext_min;
 	uint64_t ext_max;
 	struct gsm_subscriber_group *subscr_group;
diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h
index 3cba5d1..45e5816 100644
--- a/openbsc/include/openbsc/gsm_subscriber.h
+++ b/openbsc/include/openbsc/gsm_subscriber.h
@@ -5,6 +5,8 @@
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/gsm/protocol/gsm_23_003.h>
 
+#include <stdbool.h>
+
 #define GSM_NAME_LENGTH 160
 
 #define GSM_EXTENSION_LENGTH 15 /* MSISDN can only be 15 digits length */
@@ -91,7 +93,8 @@
 struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr);
 struct gsm_subscriber *subscr_create_subscriber(struct gsm_subscriber_group *sgrp,
 						const char *imsi, uint64_t smin,
-						uint64_t smax);
+						uint64_t smax,
+						bool alloc_exten);
 struct gsm_subscriber *subscr_get_by_tmsi(struct gsm_subscriber_group *sgrp,
 					  uint32_t tmsi);
 struct gsm_subscriber *subscr_get_by_imsi(struct gsm_subscriber_group *sgrp,
diff --git a/openbsc/src/libbsc/net_init.c b/openbsc/src/libbsc/net_init.c
index 4636d57..c3095c4 100644
--- a/openbsc/src/libbsc/net_init.c
+++ b/openbsc/src/libbsc/net_init.c
@@ -21,6 +21,8 @@
 #include <openbsc/osmo_msc_data.h>
 #include <openbsc/gsm_subscriber.h>
 
+#include <stdbool.h>
+
 struct gsm_network *gsm_network_init(uint16_t country_code, uint16_t network_code,
 				     int (*mncc_recv)(struct gsm_network *, struct msgb *))
 {
@@ -49,6 +51,8 @@
 
 	net->subscr_group->net = net;
 	net->subscr_creation_mode = GSM_SUBSCR_CREAT_W_RAND_EXT;
+	net->auto_create_subscr = true;
+	net->auto_assign_exten = true;
 
 	net->country_code = country_code;
 	net->network_code = network_code;
diff --git a/openbsc/src/libmsc/ctrl_commands.c b/openbsc/src/libmsc/ctrl_commands.c
index a02db36..a2a1511 100644
--- a/openbsc/src/libmsc/ctrl_commands.c
+++ b/openbsc/src/libmsc/ctrl_commands.c
@@ -25,6 +25,8 @@
 #include <openbsc/db.h>
 #include <openbsc/debug.h>
 
+#include <stdbool.h>
+
 static bool alg_supported(const char *alg)
 {
 	/*
@@ -98,7 +100,8 @@
 	if (!subscr)
 		subscr = subscr_create_subscriber(net->subscr_group, imsi,
 						  net->ext_min,
-						  net->ext_max);
+						  net->ext_max,
+						  net->auto_assign_exten);
 	if (!subscr)
 		goto fail;
 
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index b367139..68eba3e 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -23,6 +23,7 @@
 #include <inttypes.h>
 #include <libgen.h>
 #include <stdio.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
@@ -519,7 +520,7 @@
 }
 
 struct gsm_subscriber *db_create_subscriber(const char *imsi, uint64_t smin,
-					    uint64_t smax)
+					    uint64_t smax, bool alloc_exten)
 {
 	dbi_result result;
 	struct gsm_subscriber *subscr;
@@ -551,7 +552,8 @@
 	strncpy(subscr->imsi, imsi, sizeof(subscr->imsi)-1);
 	dbi_result_free(result);
 	LOGP(DDB, LOGL_INFO, "New Subscriber: ID %llu, IMSI %s\n", subscr->id, subscr->imsi);
-	db_subscriber_alloc_exten(subscr, smin, smax);
+	if (alloc_exten)
+		db_subscriber_alloc_exten(subscr, smin, smax);
 	return subscr;
 }
 
@@ -956,8 +958,11 @@
 
 	dbi_conn_quote_string_copy(conn, 
 				   subscriber->name, &q_name);
-	dbi_conn_quote_string_copy(conn, 
-				   subscriber->extension, &q_extension);
+	if (subscriber->extension[0] != '\0')
+		dbi_conn_quote_string_copy(conn,
+					   subscriber->extension, &q_extension);
+	else
+		q_extension = strdup("NULL");
 	
 	if (subscriber->tmsi != GSM_RESERVED_TMSI) {
 		sprintf(tmsi, "%u", subscriber->tmsi);
@@ -1062,15 +1067,17 @@
 	}
 	dbi_result_free(result);
 
-	result = dbi_conn_queryf(conn,
-			"DELETE FROM SMS WHERE src_addr=%s OR dest_addr=%s",
-			subscr->extension, subscr->extension);
-	if (!result) {
-		LOGP(DDB, LOGL_ERROR,
-			"Failed to delete SMS for %llu\n", subscr->id);
-		return -1;
+	if (subscr->extension[0] != '\0') {
+		result = dbi_conn_queryf(conn,
+			    "DELETE FROM SMS WHERE src_addr=%s OR dest_addr=%s",
+					 subscr->extension, subscr->extension);
+		if (!result) {
+			LOGP(DDB, LOGL_ERROR,
+			     "Failed to delete SMS for %llu\n", subscr->id);
+			return -1;
+		}
+		dbi_result_free(result);
 	}
-	dbi_result_free(result);
 
 	result = dbi_conn_queryf(conn,
 			"DELETE FROM VLR WHERE subscriber_id=%llu",
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index 6704497..e182c48 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -530,15 +530,15 @@
 static struct gsm_subscriber *subscr_create(const struct gsm_network *net,
 					    const char *imsi)
 {
-	if (net->subscr_creation_mode == GSM_SUBSCR_DONT_CREATE)
+	if (!net->auto_create_subscr)
 		return NULL;
 
-	if (net->subscr_creation_mode & GSM_SUBSCR_CREAT_W_REGEXP)
+	if (net->subscr_creation_mode == GSM_SUBSCR_CREAT_W_REGEXP)
 		if (!subscr_regexp_check(net, imsi))
 			return NULL;
 
 	return subscr_create_subscriber(net->subscr_group, imsi, net->ext_min,
-					net->ext_max);
+					net->ext_max, net->auto_assign_exten);
 }
 
 /* Parse Chapter 9.2.11 Identity Response */
diff --git a/openbsc/src/libmsc/gsm_subscriber.c b/openbsc/src/libmsc/gsm_subscriber.c
index 1dc2cc2..147524b 100644
--- a/openbsc/src/libmsc/gsm_subscriber.c
+++ b/openbsc/src/libmsc/gsm_subscriber.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <assert.h>
 #include <time.h>
+#include <stdbool.h>
 
 #include <osmocom/core/talloc.h>
 
@@ -204,9 +205,11 @@
 
 struct gsm_subscriber *subscr_create_subscriber(struct gsm_subscriber_group *sgrp,
 						const char *imsi, uint64_t smin,
-						uint64_t smax)
+						uint64_t smax,
+						bool alloc_exten)
 {
-	struct gsm_subscriber *subscr = db_create_subscriber(imsi, smin, smax);
+	struct gsm_subscriber *subscr = db_create_subscriber(imsi, smin, smax,
+							     alloc_exten);
 	if (subscr)
 		subscr->group = sgrp;
 	return subscr;
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c
index a035bf9..a009e95 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -244,7 +244,8 @@
 	else {
 		subscr = subscr_create_subscriber(gsmnet->subscr_group, argv[0],
 						  gsmnet->ext_min,
-						  gsmnet->ext_max);
+						  gsmnet->ext_max,
+						  gsmnet->auto_assign_exten);
 
 		if (!subscr) {
 			vty_out(vty, "%% No subscriber created for IMSI %s%s",
@@ -1044,6 +1045,8 @@
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
 	uint64_t mi = atoi(argv[0]), ma = atoi(argv[1]);
+	gsmnet->auto_create_subscr = true;
+	gsmnet->auto_assign_exten = true;
 	if (mi >= ma) {
 		vty_out(vty, "Incorrect range: %s >= %s, expected MIN < MAX%s",
 			argv[0], argv[1], VTY_NEWLINE);
@@ -1054,6 +1057,15 @@
         return CMD_SUCCESS;
 }
 
+DEFUN(cfg_nitb_subscr_noext, cfg_nitb_subscr_noext_cmd,
+      "subscriber-create-no-extension",
+      "Do not assign extension when creating subscriber on demand.\n")
+{
+       struct gsm_network *gsmnet = gsmnet_from_vty(vty);
+       gsmnet->auto_assign_exten = false;
+       return CMD_SUCCESS;
+}
+
 DEFUN(cfg_nitb_subscr_create, cfg_nitb_subscr_create_cmd,
       "subscriber-create-on-demand [regexp]",
       "Make a new record when a subscriber is first seen.\n"
@@ -1061,9 +1073,12 @@
       "authorized-regexp command\n")
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
-	gsmnet->subscr_creation_mode = GSM_SUBSCR_CREAT_W_RAND_EXT;
+	gsmnet->auto_create_subscr = true;
+	gsmnet->auto_assign_exten = true;
 	if (argc)
-		gsmnet->subscr_creation_mode |= GSM_SUBSCR_CREAT_W_REGEXP;
+		gsmnet->subscr_creation_mode = GSM_SUBSCR_CREAT_W_REGEXP;
+	else
+		gsmnet->subscr_creation_mode = GSM_SUBSCR_CREAT_W_RAND_EXT;
 	return CMD_SUCCESS;
 }
 
@@ -1072,7 +1087,7 @@
       NO_STR "Make a new record when a subscriber is first seen.\n")
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
-	gsmnet->subscr_creation_mode = GSM_SUBSCR_DONT_CREATE;
+	gsmnet->auto_create_subscr = false;
 	return CMD_SUCCESS;
 }
 
@@ -1098,11 +1113,13 @@
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
 	enum gsm_subscr_creation_mode scm = gsmnet->subscr_creation_mode;
-	const char *reg = (scm & GSM_SUBSCR_CREAT_W_REGEXP) ? " regexp" : "",
-		*pref = scm ? "" : "no ";
+	const char *reg = (scm == GSM_SUBSCR_CREAT_W_REGEXP) ? " regexp" : "",
+		*pref = gsmnet->auto_create_subscr ? "" : "no ";
 	vty_out(vty, "nitb%s", VTY_NEWLINE);
 	vty_out(vty, " %ssubscriber-create-on-demand%s%s",
 		pref, reg, VTY_NEWLINE);
+	if(!gsmnet->auto_assign_exten)
+		vty_out(vty, " subscriber-create-no-extension%s", VTY_NEWLINE);
 	if (gsmnet->ext_min != GSM_MIN_EXTEN || gsmnet->ext_max != GSM_MAX_EXTEN)
 		vty_out(vty, " subscriber-create-on-demand random %"PRIu64" %"
 			PRIu64"%s", gsmnet->ext_min, gsmnet->ext_max,
@@ -1162,6 +1179,7 @@
 	install_node(&nitb_node, config_write_nitb);
 	install_element(NITB_NODE, &cfg_nitb_subscr_create_cmd);
 	install_element(NITB_NODE, &cfg_nitb_subscr_random_cmd);
+	install_element(NITB_NODE, &cfg_nitb_subscr_noext_cmd);
 	install_element(NITB_NODE, &cfg_nitb_no_subscr_create_cmd);
 	install_element(NITB_NODE, &cfg_nitb_assign_tmsi_cmd);
 	install_element(NITB_NODE, &cfg_nitb_no_assign_tmsi_cmd);
diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c
index dc81481..755a6e9 100644
--- a/openbsc/tests/db/db_test.c
+++ b/openbsc/tests/db/db_test.c
@@ -28,6 +28,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <inttypes.h>
 
 static struct gsm_network dummy_net;
@@ -159,12 +160,13 @@
 	subscr_put(rcv_subscr);
 }
 
-static void test_subs(const char *alice_imsi, char *imei1, char *imei2)
+static void test_subs(const char *imsi, char *imei1, char *imei2, bool make_ext)
 {
 	struct gsm_subscriber *alice = NULL, *alice_db;
 	char scratch_str[256];
 
-	alice = db_create_subscriber(alice_imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN);
+	alice = db_create_subscriber(imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN,
+				     make_ext);
 	db_subscriber_assoc_imei(alice, imei1);
 	if (imei2)
 		db_subscriber_assoc_imei(alice, imei2);
@@ -177,7 +179,7 @@
 	COMPARE(alice, alice_db);
 	SUBSCR_PUT(alice_db);
 	/* Get by IMSI */
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice_imsi);
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, imsi);
 	COMPARE(alice, alice_db);
 	SUBSCR_PUT(alice_db);
 	/* Get by id */
@@ -187,8 +189,14 @@
 	SUBSCR_PUT(alice_db);
 	/* Get by extension */
 	alice_db = db_get_subscriber(GSM_SUBSCRIBER_EXTENSION, alice->extension);
-	COMPARE(alice, alice_db);
-	SUBSCR_PUT(alice_db);
+	if (alice_db) {
+		if (!make_ext)
+			printf("FAIL: bogus extension created for IMSI %s\n",
+			       imsi);
+		COMPARE(alice, alice_db);
+		SUBSCR_PUT(alice_db);
+	} else if (make_ext)
+		printf("FAIL: no subscriber extension for IMSI %s\n", imsi);
 	SUBSCR_PUT(alice);
 }
 
@@ -217,18 +225,22 @@
 	struct gsm_subscriber *alice_db;
 
 	char *alice_imsi = "3243245432345";
-	alice = db_create_subscriber(alice_imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN);
+	alice = db_create_subscriber(alice_imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN,
+				     true);
 	db_sync_subscriber(alice);
 	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice->imsi);
 	COMPARE(alice, alice_db);
 	SUBSCR_PUT(alice_db);
 	SUBSCR_PUT(alice);
 
-	test_subs("3693245423445", "1234567890", NULL);
-	test_subs("9993245423445", "1234567890", "6543560920");
+	test_subs("3693245423445", "1234567890", NULL, true);
+	test_subs("9993245423445", "1234567890", "6543560920", true);
+	test_subs("3123122223445", "1234567890", NULL, false);
+	test_subs("9123121223445", "1234567890", "6543560920", false);
 
 	/* create it again and see it fails */
-	alice = db_create_subscriber(alice_imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN);
+	alice = db_create_subscriber(alice_imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN,
+				     true);
 	OSMO_ASSERT(!alice);
 
 	test_sms();
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index 9ea988d..da1a8fe 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -400,6 +400,9 @@
         self.vty.enable()
 
         imsi = "204300854013739"
+        imsi2 = "204301824913762"
+        imsi3 = "100500854113763"
+        imsi4 = "100583744053764"
 
         # Initially we don't have this subscriber
         self.vty.verify('show subscriber imsi '+imsi, ['% No subscriber found for imsi '+imsi])
@@ -407,14 +410,60 @@
         # Lets create one
         res = self.vty.command('subscriber create imsi '+imsi)
         self.assert_(res.find("    IMSI: "+imsi) > 0)
+        self.assert_(res.find("Extension") > 0)
 
         # Now we have it
         res = self.vty.command('show subscriber imsi '+imsi)
         self.assert_(res.find("    IMSI: "+imsi) > 0)
 
+        # With narrow random interval
+        self.vty.command("configure terminal")
+        self.vty.command("nitb")
+        self.assertTrue(self.vty.verify("subscriber-create-on-demand", ['']))
+        # wrong interval
+        res = self.vty.command("subscriber-create-on-demand random 221 122")
+        self.assert_(res.find("122") > 0)
+        self.assert_(res.find("221") > 0)
+        # correct interval
+        self.assertTrue(self.vty.verify("subscriber-create-on-demand random 221 222", ['']))
+        self.vty.command("end")
+
+        res = self.vty.command('subscriber create imsi ' + imsi2)
+        self.assert_(res.find("    IMSI: " + imsi2) > 0)
+        self.assert_(res.find("221") > 0 or res.find("222") > 0)
+        self.assert_(res.find("    Extension: ") > 0)
+
+        # Without extension
+        self.vty.command("configure terminal")
+        self.vty.command("nitb")
+        self.assertTrue(self.vty.verify("subscriber-create-no-extension", ['']))
+        self.vty.command("end")
+
+        res = self.vty.command('subscriber create imsi ' + imsi3)
+        self.assert_(res.find("    IMSI: " + imsi3) > 0)
+        self.assertEquals(res.find("Extension"), -1)
+
+        # With extension again
+        self.vty.command("configure terminal")
+        self.vty.command("nitb")
+        self.assertTrue(self.vty.verify("no subscriber-create-on-demand", ['']))
+        self.assertTrue(self.vty.verify("subscriber-create-on-demand", ['']))
+        self.assertTrue(self.vty.verify("subscriber-create-on-demand random 221 666", ['']))
+        self.vty.command("end")
+
+        res = self.vty.command('subscriber create imsi ' + imsi4)
+        self.assert_(res.find("    IMSI: " + imsi4) > 0)
+        self.assert_(res.find("    Extension: ") > 0)
+
         # Delete it
         res = self.vty.command('subscriber delete imsi '+imsi)
         self.assert_(res != "")
+        res = self.vty.command('subscriber delete imsi ' + imsi2)
+        self.assert_(res != "")
+        res = self.vty.command('subscriber delete imsi ' + imsi3)
+        self.assert_(res != "")
+        res = self.vty.command('subscriber delete imsi ' + imsi4)
+        self.assert_(res != "")
 
         # Now it should not be there anymore
         res = self.vty.command('show subscriber imsi '+imsi)

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibbc2e88e4722b08854ebc631485f19ed56443cbb
Gerrit-PatchSet: 3
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list