diff mbox

Make random MSISDN assignment optional

Message ID 1462209366-12110-1-git-send-email-msuraev@sysmocom.de
State New
Headers show

Commit Message

Max May 2, 2016, 5:16 p.m. UTC 
From: Max <msuraev@sysmocom.de>

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

* optional "no-extension" argument to subscriber-create-on-demand
* db unit tests
* vty test

The range for random extension can now be specified with
"subscriber-create-on-demand random" command.

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.

Fixes: OS#1658
---
 openbsc/include/openbsc/db.h                |  8 ++-
 openbsc/include/openbsc/gsm_data.h          |  8 +++
 openbsc/include/openbsc/gsm_subscriber.h    |  5 +-
 openbsc/src/libbsc/net_init.c               |  5 +-
 openbsc/src/libcommon/gsm_subscriber_base.c |  1 +
 openbsc/src/libmsc/ctrl_commands.c          |  9 ++-
 openbsc/src/libmsc/db.c                     | 37 +++++++----
 openbsc/src/libmsc/gsm_04_08.c              | 12 +++-
 openbsc/src/libmsc/gsm_subscriber.c         |  6 +-
 openbsc/src/libmsc/vty_interface_layer3.c   | 51 +++++++++++++--
 openbsc/tests/db/db_test.c                  | 99 +++++++++++++----------------
 openbsc/tests/vty_test_runner.py            | 14 ++++
 12 files changed, 169 insertions(+), 86 deletions(-)

Comments

Harald Welte May 2, 2016, 5:39 p.m. UTC | #1 
Hi Max,

thanks for your patch.

On Mon, May 02, 2016 at 07:16:06PM +0200, msuraev@sysmocom.de wrote:
> -struct gsm_subscriber *db_create_subscriber(const char *imsi);
> +struct gsm_subscriber *db_create_subscriber(const char *imsi, uint64_t smin,
> +					    uint64_t smax, bool ext);

I think 'ext' is not very expressive, let's call it 'alloc_ext',
'rand_ext' or something like that, indicating that the argument is about
allocation of a random extension number or not.

> +enum gsm_subscr_ext {

Also here, please use 'gsm_subscr_ext_alloc_policy' to make it more
explicit about what it is.

> +	uint64_t ext_min;
> +	uint64_t ext_max;

pretty large numbers (uint64_t), considering that your VTY code
restricts them to five-digit numbers.  That's not a problem, it can stay
that way, but just something that struck my mind.  Maybe it makes sense
to allow longer/larger ranges in the VTY?  Or is there something in the
code restricting it to 5 digits or a low numeric range?

Regards,
	Harald
diff mbox

Patch

diff --git a/openbsc/include/openbsc/db.h b/openbsc/include/openbsc/db.h
index 6699a86..4da9634 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;
@@ -35,13 +37,15 @@  int db_prepare(void);
 int db_fini(void);
 
 /* subscriber management */
-struct gsm_subscriber *db_create_subscriber(const char *imsi);
+struct gsm_subscriber *db_create_subscriber(const char *imsi, uint64_t smin,
+					    uint64_t smax, bool ext);
 struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field field,
 					 const char *subscr);
 int db_sync_subscriber(struct gsm_subscriber *subscriber);
 int db_subscriber_expire(void *priv, void (*callback)(void *priv, long long unsigned int id));
 int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber);
-int db_subscriber_alloc_exten(struct gsm_subscriber *subscriber);
+int db_subscriber_alloc_exten(struct gsm_subscriber *subscriber, uint64_t smin,
+			      uint64_t smax);
 int db_subscriber_alloc_token(struct gsm_subscriber *subscriber, uint32_t* token);
 int db_subscriber_assoc_imei(struct gsm_subscriber *subscriber, char *imei);
 int db_subscriber_delete(struct gsm_subscriber *subscriber);
diff --git a/openbsc/include/openbsc/gsm_data.h b/openbsc/include/openbsc/gsm_data.h
index 6d7aba3..14b002a 100644
--- a/openbsc/include/openbsc/gsm_data.h
+++ b/openbsc/include/openbsc/gsm_data.h
@@ -18,6 +18,12 @@  struct gsm_subscriber_group;
 
 #define OBSC_LINKID_CB(__msgb)	(__msgb)->cb[3]
 
+enum gsm_subscr_ext {
+	GSM_SUBSCR_DONT_CREATE = 0,
+	GSM_SUBSCR_RANDOM_EXT = 1,
+	GSM_SUBSCR_NO_EXT = 2
+};
+
 enum gsm_security_event {
 	GSM_SECURITY_NOAVAIL,
 	GSM_SECURITY_AUTH_FAILED,
@@ -283,6 +289,8 @@  struct gsm_network {
 
 	/* subscriber related features */
 	int create_subscriber;
+	uint64_t ext_min;
+	uint64_t ext_max;
 	struct gsm_subscriber_group *subscr_group;
 	struct gsm_sms_queue *sms_queue;
 
diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h
index 7d6c776..5030d65 100644
--- a/openbsc/include/openbsc/gsm_subscriber.h
+++ b/openbsc/include/openbsc/gsm_subscriber.h
@@ -1,6 +1,8 @@ 
 #ifndef _GSM_SUBSCR_H
 #define _GSM_SUBSCR_H
 
+#include <stdbool.h>
+
 #include "gsm_data.h"
 #include <osmocom/core/linuxlist.h>
 
@@ -91,7 +93,8 @@  enum gsm_subscriber_update_reason {
 struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr);
 struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr);
 struct gsm_subscriber *subscr_create_subscriber(struct gsm_subscriber_group *sgrp,
-						const char *imsi);
+						const char *imsi, uint64_t smin,
+						uint64_t smax, bool ext);
 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 568a0b8..a524ead 100644
--- a/openbsc/src/libbsc/net_init.c
+++ b/openbsc/src/libbsc/net_init.c
@@ -48,7 +48,7 @@  struct gsm_network *gsm_network_init(uint16_t country_code, uint16_t network_cod
 	INIT_LLIST_HEAD(&net->bsc_data->mscs);
 
 	net->subscr_group->net = net;
-	net->create_subscriber = 1;
+	net->create_subscriber = GSM_SUBSCR_RANDOM_EXT;
 
 	net->country_code = country_code;
 	net->network_code = network_code;
@@ -104,7 +104,8 @@  struct gsm_network *gsm_network_init(uint16_t country_code, uint16_t network_cod
 	net->stats.bts.rsl_fail = osmo_counter_alloc("net.bts.rsl_fail");
 
 	net->mncc_recv = mncc_recv;
-
+	net->ext_min = GSM_MIN_EXTEN;
+	net->ext_max = GSM_MAX_EXTEN;
 	gsm_net_update_ctype(net);
 
 	return net;
diff --git a/openbsc/src/libcommon/gsm_subscriber_base.c b/openbsc/src/libcommon/gsm_subscriber_base.c
index a455824..5f0a1be 100644
--- a/openbsc/src/libcommon/gsm_subscriber_base.c
+++ b/openbsc/src/libcommon/gsm_subscriber_base.c
@@ -60,6 +60,7 @@  struct gsm_subscriber *subscr_alloc(void)
 	llist_add_tail(&s->entry, &active_subscribers);
 	s->use_count = 1;
 	s->tmsi = GSM_RESERVED_TMSI;
+	s->extension[0] = '\0';
 
 	INIT_LLIST_HEAD(&s->requests);
 
diff --git a/openbsc/src/libmsc/ctrl_commands.c b/openbsc/src/libmsc/ctrl_commands.c
index 9ac39de..44b1ebd 100644
--- a/openbsc/src/libmsc/ctrl_commands.c
+++ b/openbsc/src/libmsc/ctrl_commands.c
@@ -18,6 +18,9 @@ 
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
+
+#include <stdbool.h>
+
 #include <osmocom/ctrl/control_cmd.h>
 #include <openbsc/gsm_data.h>
 #include <openbsc/gsm_subscriber.h>
@@ -83,6 +86,8 @@  static int set_subscriber_modify(struct ctrl_cmd *cmd, void *data)
 	char *tmp, *imsi, *msisdn, *alg, *ki, *saveptr = NULL;
 	struct gsm_subscriber* subscr;
 	int rc;
+	bool ext = (GSM_SUBSCR_RANDOM_EXT == net->create_subscriber) ? true :
+		false;
 
 	tmp = talloc_strdup(cmd, cmd->value);
 	if (!tmp)
@@ -95,7 +100,9 @@  static int set_subscriber_modify(struct ctrl_cmd *cmd, void *data)
 
 	subscr = subscr_get_by_imsi(net->subscr_group, imsi);
 	if (!subscr)
-		subscr = subscr_create_subscriber(net->subscr_group, imsi);
+		subscr = subscr_create_subscriber(net->subscr_group, imsi,
+						  net->ext_min,
+						  net->ext_max, ext);
 	if (!subscr)
 		goto fail;
 
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 267b5ef..1e112cb 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>
@@ -500,7 +501,8 @@  int db_fini(void)
 	return 0;
 }
 
-struct gsm_subscriber *db_create_subscriber(const char *imsi)
+struct gsm_subscriber *db_create_subscriber(const char *imsi, uint64_t smin,
+					    uint64_t smax, bool ext)
 {
 	dbi_result result;
 	struct gsm_subscriber *subscr;
@@ -532,7 +534,8 @@  struct gsm_subscriber *db_create_subscriber(const char *imsi)
 	strncpy(subscr->imsi, imsi, GSM_IMSI_LENGTH-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);
+	if (ext)
+		db_subscriber_alloc_exten(subscr, smin, smax);
 	return subscr;
 }
 
@@ -937,8 +940,11 @@  int db_sync_subscriber(struct gsm_subscriber *subscriber)
 
 	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);
@@ -1043,15 +1049,17 @@  int db_subscriber_delete(struct gsm_subscriber *subscr)
 	}
 	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",
@@ -1231,13 +1239,14 @@  int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber)
 	return 0;
 }
 
-int db_subscriber_alloc_exten(struct gsm_subscriber *subscriber)
+int db_subscriber_alloc_exten(struct gsm_subscriber *subscriber, uint64_t smin,
+			      uint64_t smax)
 {
 	dbi_result result = NULL;
 	uint32_t try;
 
 	for (;;) {
-		try = (rand()%(GSM_MAX_EXTEN-GSM_MIN_EXTEN+1)+GSM_MIN_EXTEN);
+		try = (rand() % (smax - smin + 1) + smin);
 		result = dbi_conn_queryf(conn,
 			"SELECT * FROM Subscriber "
 			"WHERE extension = %i",
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index 05cb886..d22db73 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -25,6 +25,7 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdbool.h>
 #include <errno.h>
 #include <time.h>
 #include <netinet/in.h>
@@ -517,6 +518,8 @@  static int mm_rx_id_resp(struct gsm_subscriber_connection *conn, struct msgb *ms
 	struct gsm_network *net = bts->network;
 	uint8_t mi_type = gh->data[1] & GSM_MI_TYPE_MASK;
 	char mi_string[GSM48_MI_SIZE];
+	bool ext = (GSM_SUBSCR_RANDOM_EXT == net->create_subscriber) ? true :
+		false;
 
 	gsm48_mi_to_string(mi_string, sizeof(mi_string), &gh->data[1], gh->data[0]);
 	DEBUGP(DMM, "IDENTITY RESPONSE: MI(%s)=%s\n",
@@ -532,7 +535,8 @@  static int mm_rx_id_resp(struct gsm_subscriber_connection *conn, struct msgb *ms
 							  mi_string);
 			if (!conn->subscr && net->create_subscriber)
 				conn->subscr = subscr_create_subscriber(
-					net->subscr_group, mi_string);
+					net->subscr_group, mi_string,
+					net->ext_min, net->ext_max, ext);
 		}
 		if (!conn->subscr && conn->loc_operation) {
 			gsm0408_loc_upd_rej(conn, bts->network->reject_cause);
@@ -593,6 +597,8 @@  static int mm_rx_loc_upd_req(struct gsm_subscriber_connection *conn, struct msgb
 	struct gsm_bts *bts = conn->bts;
 	uint8_t mi_type;
 	char mi_string[GSM48_MI_SIZE];
+	bool ext = (GSM_SUBSCR_RANDOM_EXT == bts->network->create_subscriber) ?
+		true : false;
 
  	lu = (struct gsm48_loc_upd_req *) gh->data;
 
@@ -643,7 +649,9 @@  static int mm_rx_loc_upd_req(struct gsm_subscriber_connection *conn, struct msgb
 		subscr = subscr_get_by_imsi(bts->network->subscr_group, mi_string);
 		if (!subscr && bts->network->create_subscriber) {
 			subscr = subscr_create_subscriber(
-				bts->network->subscr_group, mi_string);
+				bts->network->subscr_group, mi_string,
+				bts->network->ext_min, bts->network->ext_max,
+				ext);
 		}
 		if (!subscr) {
 			gsm0408_loc_upd_rej(conn, bts->network->reject_cause);
diff --git a/openbsc/src/libmsc/gsm_subscriber.c b/openbsc/src/libmsc/gsm_subscriber.c
index 57c10cf..7d62888 100644
--- a/openbsc/src/libmsc/gsm_subscriber.c
+++ b/openbsc/src/libmsc/gsm_subscriber.c
@@ -203,9 +203,11 @@  void subscr_remove_request(struct subscr_request *request)
 }
 
 struct gsm_subscriber *subscr_create_subscriber(struct gsm_subscriber_group *sgrp,
-					const char *imsi)
+						const char *imsi, uint64_t smin,
+						uint64_t smax, bool ext)
 {
-	struct gsm_subscriber *subscr = db_create_subscriber(imsi);
+	struct gsm_subscriber *subscr =
+		db_create_subscriber(imsi, smin, smax, ext);
 	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 790fedf..51b7258 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -21,6 +21,8 @@ 
 #include <stdlib.h>
 #include <limits.h>
 #include <unistd.h>
+#include <stdbool.h>
+#include <inttypes.h>
 #include <time.h>
 
 #include <osmocom/vty/command.h>
@@ -235,12 +237,16 @@  DEFUN(subscriber_create,
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
 	struct gsm_subscriber *subscr;
+	bool ext = (GSM_SUBSCR_RANDOM_EXT == gsmnet->create_subscriber) ? true :
+		false;
 
 	subscr = subscr_get_by_imsi(gsmnet->subscr_group, argv[0]);
 	if (subscr)
 		db_sync_subscriber(subscr);
 	else {
-		subscr = subscr_create_subscriber(gsmnet->subscr_group, argv[0]);
+		subscr = subscr_create_subscriber(gsmnet->subscr_group, argv[0],
+						  gsmnet->ext_min,
+						  gsmnet->ext_max, ext);
 
 		if (!subscr) {
 			vty_out(vty, "%% No subscriber created for IMSI %s%s",
@@ -1032,11 +1038,33 @@  DEFUN(cfg_nitb, cfg_nitb_cmd,
 }
 
 DEFUN(cfg_nitb_subscr_create, cfg_nitb_subscr_create_cmd,
-      "subscriber-create-on-demand",
-      "Make a new record when a subscriber is first seen.\n")
+      "subscriber-create-on-demand [no-extension]",
+      "Make a new record when a subscriber is first seen.\n"
+      "Do not assign random extension when creating subscriber\n")
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
-	gsmnet->create_subscriber = 1;
+	if (argc)
+		gsmnet->create_subscriber = GSM_SUBSCR_NO_EXT;
+	else
+		gsmnet->create_subscriber = GSM_SUBSCR_RANDOM_EXT;
+	return CMD_SUCCESS;
+}
+
+DEFUN(cfg_nitb_subscr_random, cfg_nitb_subscr_random_cmd,
+      "subscriber-create-on-demand random <1-99999> <2-99999>",
+      "Set random parameters for a new record when a subscriber is first seen.\n"
+      "Set random parameters for a new record when a subscriber is first seen.\n"
+      "Minimum for subscriber extension\n""Maximum for subscriber extension\n")
+{
+	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
+	uint64_t mi = atoi(argv[0]), ma = atoi(argv[1]);
+	if (mi >= ma) {
+		vty_out(vty, "Incorrect range: %s >= %s, expected MIN < MAX%s",
+			argv[0], argv[1], VTY_NEWLINE);
+		return CMD_WARNING;
+	}
+	gsmnet->ext_min = mi;
+	gsmnet->ext_max = ma;
 	return CMD_SUCCESS;
 }
 
@@ -1045,7 +1073,7 @@  DEFUN(cfg_nitb_no_subscr_create, cfg_nitb_no_subscr_create_cmd,
       NO_STR "Make a new record when a subscriber is first seen.\n")
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
-	gsmnet->create_subscriber = 0;
+	gsmnet->create_subscriber = GSM_SUBSCR_DONT_CREATE;
 	return CMD_SUCCESS;
 }
 
@@ -1070,9 +1098,17 @@  DEFUN(cfg_nitb_no_assign_tmsi, cfg_nitb_no_assign_tmsi_cmd,
 static int config_write_nitb(struct vty *vty)
 {
 	struct gsm_network *gsmnet = gsmnet_from_vty(vty);
+	const char *pref = gsmnet->create_subscriber ? "" : "no ", *post =
+		(GSM_SUBSCR_NO_EXT == gsmnet->create_subscriber) ?
+		" no-extension" : "";
+
 	vty_out(vty, "nitb%s", VTY_NEWLINE);
-	vty_out(vty, " %ssubscriber-create-on-demand%s",
-		gsmnet->create_subscriber ? "" : "no ", VTY_NEWLINE);
+	vty_out(vty, " %ssubscriber-create-on-demand%s%s", pref, post,
+		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,
+			VTY_NEWLINE);
 	vty_out(vty, " %sassign-tmsi%s",
 		gsmnet->avoid_tmsi ? "no " : "", VTY_NEWLINE);
 	return CMD_SUCCESS;
@@ -1127,6 +1163,7 @@  int bsc_vty_init_extra(void)
 	install_element(CONFIG_NODE, &cfg_nitb_cmd);
 	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_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 fb159a5..a689789 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,10 +160,44 @@  static void test_sms_migrate(void)
 	subscr_put(rcv_subscr);
 }
 
-int main()
+static void test_subs(const char *alice_imsi, char *imei1, char *imei2, bool ext)
 {
+	struct gsm_subscriber *alice = NULL, *alice_db;
 	char scratch_str[256];
 
+	alice = db_create_subscriber(alice_imsi, GSM_MIN_EXTEN, GSM_MAX_EXTEN,
+				     ext);
+	db_subscriber_assoc_imei(alice, imei1);
+	if (imei2)
+		db_subscriber_assoc_imei(alice, imei2);
+	db_subscriber_alloc_tmsi(alice);
+	alice->lac=42;
+	db_sync_subscriber(alice);
+	/* Get by TMSI */
+	snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
+	COMPARE(alice, alice_db);
+	SUBSCR_PUT(alice_db);
+	/* Get by IMSI */
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice_imsi);
+	COMPARE(alice, alice_db);
+	SUBSCR_PUT(alice_db);
+	/* Get by id */
+	snprintf(scratch_str, sizeof(scratch_str), "%llu", alice->id);
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_ID, scratch_str);
+	COMPARE(alice, alice_db);
+	SUBSCR_PUT(alice_db);
+	/* Get by extension */
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_EXTENSION, alice->extension);
+	if (alice_db) {
+		COMPARE(alice, alice_db);
+		SUBSCR_PUT(alice_db);
+	}
+	SUBSCR_PUT(alice);
+}
+
+int main()
+{
 	printf("Testing subscriber database code.\n");
 	osmo_init_logging(&log_info);
 	log_set_print_filename(osmo_stderr_target, 0);
@@ -186,68 +221,22 @@  int main()
 	struct gsm_subscriber *alice_db;
 
 	char *alice_imsi = "3243245432345";
-	alice = db_create_subscriber(alice_imsi);
+	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);
 
-	alice_imsi = "3693245423445";
-	alice = db_create_subscriber(alice_imsi);
-	db_subscriber_assoc_imei(alice, "1234567890");
-	db_subscriber_alloc_tmsi(alice);
-	alice->lac=42;
-	db_sync_subscriber(alice);
-	/* Get by TMSI */
-	snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
-	COMPARE(alice, alice_db);
-	SUBSCR_PUT(alice_db);
-	/* Get by IMSI */
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice_imsi);
-	COMPARE(alice, alice_db);
-	SUBSCR_PUT(alice_db);
-	/* Get by id */
-	snprintf(scratch_str, sizeof(scratch_str), "%llu", alice->id);
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_ID, scratch_str);
-	COMPARE(alice, alice_db);
-	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);
-	SUBSCR_PUT(alice);
-
-	alice_imsi = "9993245423445";
-	alice = db_create_subscriber(alice_imsi);
-	db_subscriber_alloc_tmsi(alice);
-	alice->lac=42;
-	db_sync_subscriber(alice);
-	db_subscriber_assoc_imei(alice, "1234567890");
-	db_subscriber_assoc_imei(alice, "6543560920");
-	/* Get by TMSI */
-	snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
-	COMPARE(alice, alice_db);
-	SUBSCR_PUT(alice_db);
-	/* Get by IMSI */
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice_imsi);
-	COMPARE(alice, alice_db);
-	SUBSCR_PUT(alice_db);
-	/* Get by id */
-	snprintf(scratch_str, sizeof(scratch_str), "%llu", alice->id);
-	alice_db = db_get_subscriber(GSM_SUBSCRIBER_ID, scratch_str);
-	COMPARE(alice, alice_db);
-	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);
-	SUBSCR_PUT(alice);
+	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);
+	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 c088855..6c49a03 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -408,11 +408,13 @@  class TestVTYNITB(TestVTYGenericBSC):
         self.vty.enable()
 
         imsi = "204300854013739"
+        imsi2 = "204300854013769"
         wrong_imsi = "204300999999999"
 
         # Lets create one
         res = self.vty.command('subscriber create imsi '+imsi)
         self.assert_(res.find("    IMSI: "+imsi) > 0)
+        self.assert_(res.find("Extension") > 0)
 
         self.vty.verify('subscriber imsi '+wrong_imsi+' name wrong', ['% No subscriber found for imsi '+wrong_imsi])
         res = self.vty.command('subscriber imsi '+imsi+' name '+('X' * 160))
@@ -426,9 +428,21 @@  class TestVTYNITB(TestVTYGenericBSC):
 
         self.vty.verify('subscriber imsi '+imsi+' extension '+('1' * 14), [''])
 
+        # Another one - without extension
+        self.vty.command("configure terminal")
+        self.vty.command("nitb")
+        self.vty.command("subscriber-create-on-demand no-extension")
+        self.vty.command("end")
+
+        res = self.vty.command('subscriber create imsi ' + imsi2)
+        self.assert_(res.find("    IMSI: " + imsi2) > 0)
+        self.assertEquals(res.find("Extension"), -1)
+
         # Delete it
         res = self.vty.command('subscriber delete imsi '+imsi)
         self.assert_(res != "")
+        res = self.vty.command('subscriber delete imsi ' + imsi2)
+        self.assert_(res != "")
 
     def testShowPagingGroup(self):
         res = self.vty.command("show paging-group 255 1234567")