<p>Max has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/12227">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ACL: integrate sanitize check into sgsn_acl_* functions<br><br>Having this check in vty makes it hard to unit-test. Let's move this<br>into separate static function and use it directly from sgsn_acl_*<br>functions. Adjust test output accordingly.<br><br>Related: SYS#4300<br>Change-Id: Ic3dff108148683b107e9edac430a0475283580e9<br>---<br>M src/gprs/sgsn_auth.c<br>M src/gprs/sgsn_vty.c<br>M tests/sgsn/sgsn_test.ok<br>3 files changed, 27 insertions(+), 20 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/27/12227/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gprs/sgsn_auth.c b/src/gprs/sgsn_auth.c</span><br><span>index 5a62476..895cef3 100644</span><br><span>--- a/src/gprs/sgsn_auth.c</span><br><span>+++ b/src/gprs/sgsn_auth.c</span><br><span>@@ -43,11 +43,30 @@</span><br><span> INIT_LLIST_HEAD(&sgsn->cfg.imsi_acl);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static bool imsi_sanitize(char *dst, size_t dst_len, const char *imsi)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t len = strnlen(imsi, GSM23003_IMSI_MAX_DIGITS + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ memset(dst, '0', dst_len);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (len > GSM23003_IMSI_MAX_DIGITS)</span><br><span style="color: hsl(120, 100%, 40%);">+ return true;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_strlcpy(dst + GSM23003_IMSI_MAX_DIGITS - len, imsi, dst_len - (GSM23003_IMSI_MAX_DIGITS - len));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct imsi_acl_entry *sgsn_acl_lookup(const char *imsi, const struct sgsn_config *cfg)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ char imsi_sanitized[GSM23003_IMSI_MAX_DIGITS + 1];</span><br><span> struct imsi_acl_entry *acl;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (imsi_sanitize(imsi_sanitized, sizeof(imsi_sanitized), imsi))</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> llist_for_each_entry(acl, &cfg->imsi_acl, list) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!strcmp(imsi, acl->imsi))</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!strcmp(imsi_sanitized, acl->imsi))</span><br><span> return acl;</span><br><span> }</span><br><span> return NULL;</span><br><span>@@ -67,7 +86,8 @@</span><br><span> if (sgsn_acl_lookup(imsi, cfg))</span><br><span> return -EEXIST;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- osmo_strlcpy(acl->imsi, imsi, sizeof(acl->imsi));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (imsi_sanitize(acl->imsi, sizeof(acl->imsi), imsi))</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span> </span><br><span> llist_add(&acl->list, &cfg->imsi_acl);</span><br><span> </span><br><span>diff --git a/src/gprs/sgsn_vty.c b/src/gprs/sgsn_vty.c</span><br><span>index 601b3c5..7c972e1 100644</span><br><span>--- a/src/gprs/sgsn_vty.c</span><br><span>+++ b/src/gprs/sgsn_vty.c</span><br><span>@@ -634,29 +634,16 @@</span><br><span> "Remove IMSI from ACL\n"</span><br><span> "IMSI of subscriber\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- char imsi_sanitized[GSM23003_IMSI_MAX_DIGITS + 1] = { '0' };</span><br><span> const char *op = argv[0];</span><br><span style="color: hsl(0, 100%, 40%);">- const char *imsi = imsi_sanitized;</span><br><span style="color: hsl(0, 100%, 40%);">- size_t len = strnlen(argv[1], GSM23003_IMSI_MAX_DIGITS + 1);</span><br><span> int rc;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Sanitize IMSI */</span><br><span style="color: hsl(0, 100%, 40%);">- if (len > GSM23003_IMSI_MAX_DIGITS) {</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "%% IMSI (%s) too long (max %u digits) -- ignored!%s",</span><br><span style="color: hsl(0, 100%, 40%);">- argv[1], GSM23003_IMSI_MAX_DIGITS, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- return CMD_WARNING;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_strlcpy(imsi_sanitized + GSM23003_IMSI_MAX_DIGITS - len, argv[1],</span><br><span style="color: hsl(0, 100%, 40%);">- sizeof(imsi_sanitized) - (GSM23003_IMSI_MAX_DIGITS - len));</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (!strcmp(op, "add"))</span><br><span style="color: hsl(0, 100%, 40%);">- rc = sgsn_acl_add(imsi, g_cfg);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = sgsn_acl_add(argv[1], g_cfg);</span><br><span> else</span><br><span style="color: hsl(0, 100%, 40%);">- rc = sgsn_acl_del(imsi, g_cfg);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = sgsn_acl_del(argv[1], g_cfg);</span><br><span> </span><br><span> if (rc < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "%% unable to %s ACL%s", op, VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "%% unable to %s ACL%s: %s", op, strerror(-rc), VTY_NEWLINE);</span><br><span> return CMD_WARNING;</span><br><span> }</span><br><span> </span><br><span>diff --git a/tests/sgsn/sgsn_test.ok b/tests/sgsn/sgsn_test.ok</span><br><span>index 3d63a63..1a39e6e 100644</span><br><span>--- a/tests/sgsn/sgsn_test.ok</span><br><span>+++ b/tests/sgsn/sgsn_test.ok</span><br><span>@@ -25,7 +25,7 @@</span><br><span> Testing APN matching</span><br><span> Testing GGSN selection</span><br><span> Testing IMSI ACLs</span><br><span style="color: hsl(0, 100%, 40%);">-[0] Adding ACL 1010000000016 [13]... added as 1010000000016 [13], total entries 1</span><br><span style="color: hsl(120, 100%, 40%);">+[0] Adding ACL 1010000000016 [13]... added as 001010000000016 [15], total entries 1</span><br><span> [1] Adding ACL 001010000000011 [15]... added as 001010000000011 [15], total entries 2</span><br><span> [2] Adding ACL 001010000000012 [15]... added as 001010000000012 [15], total entries 3</span><br><span> [3] Adding ACL 001010000000013 [15]... added as 001010000000013 [15], total entries 4</span><br><span>@@ -35,5 +35,5 @@</span><br><span> [2] Removing ACL 001010000000011... OK, total entries 1</span><br><span> [1] Removing ACL 001010000000013... OK, total entries 0</span><br><span> [0] Removing ACL 001010000000013... failed to remove acl 001010000000013, total entries 0</span><br><span style="color: hsl(0, 100%, 40%);">-[0] Adding ACL 00101002222222222222222200000011 [32]... failed to obtain added 00101002222222222222222200000011 entry, total entries 1</span><br><span style="color: hsl(120, 100%, 40%);">+[0] Adding ACL 00101002222222222222222200000011 [32]... failed to add acl 00101002222222222222222200000011, total entries 0</span><br><span> Done</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12227">change 12227</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/12227"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ic3dff108148683b107e9edac430a0475283580e9 </div>
<div style="display:none"> Gerrit-Change-Number: 12227 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>