Change in osmo-sgsn[master]: ACL: integrate sanitize check into sgsn_acl_* functions

[Max]

Max has uploaded this change for review. ( https://gerrit.osmocom.org/12227


Change subject: ACL: integrate sanitize check into sgsn_acl_* functions
......................................................................

ACL: integrate sanitize check into sgsn_acl_* functions

Having this check in vty makes it hard to unit-test. Let's move this
into separate static function and use it directly from sgsn_acl_*
functions. Adjust test output accordingly.

Related: SYS#4300
Change-Id: Ic3dff108148683b107e9edac430a0475283580e9
---
M src/gprs/sgsn_auth.c
M src/gprs/sgsn_vty.c
M tests/sgsn/sgsn_test.ok
3 files changed, 27 insertions(+), 20 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/27/12227/1

diff --git a/src/gprs/sgsn_auth.c b/src/gprs/sgsn_auth.c
index 5a62476..895cef3 100644
--- a/src/gprs/sgsn_auth.c
+++ b/src/gprs/sgsn_auth.c
@@ -43,11 +43,30 @@
 	INIT_LLIST_HEAD(&sgsn->cfg.imsi_acl);
 }
 
+static bool imsi_sanitize(char *dst, size_t dst_len, const char *imsi)
+{
+	size_t len = strnlen(imsi, GSM23003_IMSI_MAX_DIGITS + 1);
+
+	memset(dst, '0', dst_len);
+
+	if (len > GSM23003_IMSI_MAX_DIGITS)
+		return true;
+
+	osmo_strlcpy(dst + GSM23003_IMSI_MAX_DIGITS - len, imsi, dst_len - (GSM23003_IMSI_MAX_DIGITS - len));
+
+	return false;
+}
+
 struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, const struct sgsn_config *cfg)
 {
+	char imsi_sanitized[GSM23003_IMSI_MAX_DIGITS + 1];
 	struct imsi_acl_entry *acl;
+
+	if (imsi_sanitize(imsi_sanitized, sizeof(imsi_sanitized), imsi))
+		return NULL;
+
 	llist_for_each_entry(acl, &cfg->imsi_acl, list) {
-		if (!strcmp(imsi, acl->imsi))
+		if (!strcmp(imsi_sanitized, acl->imsi))
 			return acl;
 	}
 	return NULL;
@@ -67,7 +86,8 @@
 	if (sgsn_acl_lookup(imsi, cfg))
 		return -EEXIST;
 
-	osmo_strlcpy(acl->imsi, imsi, sizeof(acl->imsi));
+	if (imsi_sanitize(acl->imsi, sizeof(acl->imsi), imsi))
+		return -EINVAL;
 
 	llist_add(&acl->list, &cfg->imsi_acl);
 
diff --git a/src/gprs/sgsn_vty.c b/src/gprs/sgsn_vty.c
index 601b3c5..7c972e1 100644
--- a/src/gprs/sgsn_vty.c
+++ b/src/gprs/sgsn_vty.c
@@ -634,29 +634,16 @@
 	"Remove IMSI from ACL\n"
 	"IMSI of subscriber\n")
 {
-	char imsi_sanitized[GSM23003_IMSI_MAX_DIGITS + 1] = { '0' };
 	const char *op = argv[0];
-	const char *imsi = imsi_sanitized;
-	size_t len = strnlen(argv[1], GSM23003_IMSI_MAX_DIGITS + 1);
 	int rc;
 
-	/* Sanitize IMSI */
-	if (len > GSM23003_IMSI_MAX_DIGITS) {
-		vty_out(vty, "%% IMSI (%s) too long (max %u digits) -- ignored!%s",
-			argv[1], GSM23003_IMSI_MAX_DIGITS, VTY_NEWLINE);
-		return CMD_WARNING;
-	}
-
-	osmo_strlcpy(imsi_sanitized + GSM23003_IMSI_MAX_DIGITS - len, argv[1],
-		     sizeof(imsi_sanitized) - (GSM23003_IMSI_MAX_DIGITS - len));
-
 	if (!strcmp(op, "add"))
-		rc = sgsn_acl_add(imsi, g_cfg);
+		rc = sgsn_acl_add(argv[1], g_cfg);
 	else
-		rc = sgsn_acl_del(imsi, g_cfg);
+		rc = sgsn_acl_del(argv[1], g_cfg);
 
 	if (rc < 0) {
-		vty_out(vty, "%% unable to %s ACL%s", op, VTY_NEWLINE);
+		vty_out(vty, "%% unable to %s ACL%s: %s", op, strerror(-rc), VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
diff --git a/tests/sgsn/sgsn_test.ok b/tests/sgsn/sgsn_test.ok
index 3d63a63..1a39e6e 100644
--- a/tests/sgsn/sgsn_test.ok
+++ b/tests/sgsn/sgsn_test.ok
@@ -25,7 +25,7 @@
 Testing APN matching
 Testing GGSN selection
 Testing IMSI ACLs
-[0] Adding ACL 1010000000016 [13]... added as 1010000000016 [13], total entries 1
+[0] Adding ACL 1010000000016 [13]... added as 001010000000016 [15], total entries 1
 [1] Adding ACL 001010000000011 [15]... added as 001010000000011 [15], total entries 2
 [2] Adding ACL 001010000000012 [15]... added as 001010000000012 [15], total entries 3
 [3] Adding ACL 001010000000013 [15]... added as 001010000000013 [15], total entries 4
@@ -35,5 +35,5 @@
 [2] Removing ACL 001010000000011... OK, total entries 1
 [1] Removing ACL 001010000000013... OK, total entries 0
 [0] Removing ACL 001010000000013... failed to remove acl 001010000000013, total entries 0
-[0] Adding ACL 00101002222222222222222200000011 [32]... failed to obtain added 00101002222222222222222200000011 entry, total entries 1
+[0] Adding ACL 00101002222222222222222200000011 [32]... failed to add acl 00101002222222222222222200000011, total entries 0
 Done

-- 
To view, visit https://gerrit.osmocom.org/12227
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic3dff108148683b107e9edac430a0475283580e9
Gerrit-Change-Number: 12227
Gerrit-PatchSet: 1
Gerrit-Owner: Max <msuraev at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181210/89dc268f/attachment.htm>