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 <[email protected]>
