[PATCH] openbsc[master]: sgsn: Fix broken ACL based authentication

[dexter]

Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/1939

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

sgsn: Fix broken ACL based authentication

The function sgsn_auth_state() in sgsn_auth.c checks if a subscriber
is allowed to enter the network or not. Depending on the auth policy
that is set via the VTY config, different checks apply:

SGSN_AUTH_POLICY_CLOSED: requires checking the net (MCC/MNC must
match) and also requires to check if the IMSI is inside the ACL
list. In this case check_net and check_acl are set to one.

SGSN_AUTH_POLICY_ACL_ONLY: only requires the ACL to be correct.
Here only check_acl is set to one.

In the code at the end of the function we can see that if checking
the network is required (check_acl=1) The authentication is granted
if MCC/MNC are correct. The function returns at that point, meaning,
that an evenually required ACL check is completely ignored.

This commit corrects the check logic.

Change-Id: I463afa5cc407f5c56d29fb5a501185cd3e7ea5be
---
M openbsc/src/gprs/sgsn_auth.c
M openbsc/tests/sgsn/sgsn_test.c
2 files changed, 9 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/39/1939/2

diff --git a/openbsc/src/gprs/sgsn_auth.c b/openbsc/src/gprs/sgsn_auth.c
index a64339c..c0954c0 100644
--- a/openbsc/src/gprs/sgsn_auth.c
+++ b/openbsc/src/gprs/sgsn_auth.c
@@ -51,6 +51,7 @@
 		if (!strcmp(imsi, acl->imsi))
 			return acl;
 	}
+
 	return NULL;
 }
 
@@ -135,14 +136,16 @@
 		 * of 'our' network */
 		snprintf(mccmnc, sizeof(mccmnc), "%03d%02d",
 			 mmctx->ra.mcc, mmctx->ra.mnc);
-		if (strncmp(mccmnc, mmctx->imsi, 5) == 0)
-			return SGSN_AUTH_ACCEPTED;
+		if (strncmp(mccmnc, mmctx->imsi, 5) != 0)
+			return SGSN_AUTH_REJECTED;
 	}
 
-	if (check_acl && sgsn_acl_lookup(mmctx->imsi, &sgsn->cfg))
-		return SGSN_AUTH_ACCEPTED;
+	if (check_acl) {
+		if (sgsn_acl_lookup(mmctx->imsi, &sgsn->cfg) == NULL)
+			return SGSN_AUTH_REJECTED;
+	}
 
-	return SGSN_AUTH_REJECTED;
+	return SGSN_AUTH_ACCEPTED;
 }
 
 /*
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index 2f1513a..3394584 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -930,7 +930,7 @@
  */
 static void test_gmm_attach(int retry)
 {
-	struct gprs_ra_id raid = { 0, };
+	struct gprs_ra_id raid = { 45, 123};
 	struct sgsn_mm_ctx *ctx = NULL;
 	struct sgsn_mm_ctx *ictx;
 	uint32_t ptmsi1;

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I463afa5cc407f5c56d29fb5a501185cd3e7ea5be
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder