Change in ...osmo-sgsn[master]: sgsn: Have authentication required on by default

pespin gerrit-no-reply at lists.osmocom.org
Thu Jun 13 17:19:14 UTC 2019


pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/14445


Change subject: sgsn: Have authentication required on by default
......................................................................

sgsn: Have authentication required on by default

Previous commit introduced command "authentication (optional|required)",
which is only meaningful if auth-policy is remote. Upon adding the cmd,
it changed the default logic for remote policy to not require
authentication, which broke TTCN3 tests because sgsn no longer tries to
authenticate the users.

Since it's actually good to enable authentication by default where
possible, let's enable it by default when on auth-policy remote.

In order to do so, let's simply not care about the value of variable
require_authentication if auth_policy is not REMOTE. As a result, we
drop parts of the previous patch and remove unneeded checks (which are
only partially useful based on order of commands during VTY read).

Fixes: 794f446a284ed1ac6d31eb79a8f4c874d66fc34e
Change-Id: Ic707a95af178b44f08809df3d3bc8354bf34273c
---
M doc/manuals/vty/sgsn_vty_reference.xml
M include/osmocom/sgsn/sgsn.h
M src/gprs/gprs_sgsn.c
M src/gprs/sgsn_vty.c
4 files changed, 11 insertions(+), 20 deletions(-)



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

diff --git a/doc/manuals/vty/sgsn_vty_reference.xml b/doc/manuals/vty/sgsn_vty_reference.xml
index ed11777..e9cd322 100644
--- a/doc/manuals/vty/sgsn_vty_reference.xml
+++ b/doc/manuals/vty/sgsn_vty_reference.xml
@@ -2232,9 +2232,9 @@
     </command>
     <command id='authentication (optional|required)'>
       <params>
-        <param name='authentication' doc='Whether to enforce MS authentication in GERAN' />
-        <param name='optional' doc='Allow MS to attach via GERAN without authentication' />
-        <param name='required' doc='Always require authentication' />
+        <param name='authentication' doc='Whether to enforce MS authentication in GERAN (only with auth-policy remote)' />
+        <param name='optional' doc='Allow MS to attach via GERAN without authentication (default and only possible value for non-remote auth-policy)' />
+        <param name='required' doc='Always require authentication (only available for auth-policy remote, default with that auth-policy)' />
       </params>
     </command>
     <command id='encryption (GEA0|GEA1|GEA2|GEA3|GEA4)'>
diff --git a/include/osmocom/sgsn/sgsn.h b/include/osmocom/sgsn/sgsn.h
index c80355d..0a6ea29 100644
--- a/include/osmocom/sgsn/sgsn.h
+++ b/include/osmocom/sgsn/sgsn.h
@@ -78,7 +78,9 @@
 	struct sockaddr_in gsup_server_addr;
 	int gsup_server_port;
 
+	/* Only meaningful if auth_policy is SGSN_AUTH_POLICY_REMOTE */
 	int require_authentication;
+
 	int require_update_location;
 
 	/* CDR configuration */
diff --git a/src/gprs/gprs_sgsn.c b/src/gprs/gprs_sgsn.c
index f725811..5801107 100644
--- a/src/gprs/gprs_sgsn.c
+++ b/src/gprs/gprs_sgsn.c
@@ -987,6 +987,7 @@
 	inst = talloc_zero(talloc_ctx, struct sgsn_instance);
 	inst->cfg.gtp_statedir = talloc_strdup(inst, "./");
 	inst->cfg.auth_policy = SGSN_AUTH_POLICY_CLOSED;
+	inst->cfg.require_authentication = true; /* only applies if auth_policy is REMOTE */
 	inst->cfg.gsup_server_port = OSMO_GSUP_PORT;
 	return inst;
 }
diff --git a/src/gprs/sgsn_vty.c b/src/gprs/sgsn_vty.c
index 29c9771..c7ec48b 100644
--- a/src/gprs/sgsn_vty.c
+++ b/src/gprs/sgsn_vty.c
@@ -211,8 +211,8 @@
 	if (g_cfg->gsup_server_port)
 		vty_out(vty, " gsup remote-port %d%s",
 			g_cfg->gsup_server_port, VTY_NEWLINE);
-	vty_out(vty, " authentication %s%s",
-		g_cfg->require_authentication ? "required" : "optional", VTY_NEWLINE);
+	if (cfg->auth_policy == SGSN_AUTH_POLICY_REMOTE && !g_cfg->require_authentication)
+		vty_out(vty, " authentication optional%s", VTY_NEWLINE);
 	vty_out(vty, " auth-policy %s%s",
 		get_value_string(sgsn_auth_pol_strs, g_cfg->auth_policy),
 		VTY_NEWLINE);
@@ -697,9 +697,9 @@
 
 DEFUN(cfg_authentication, cfg_authentication_cmd,
       "authentication (optional|required)",
-      "Whether to enforce MS authentication in GERAN\n"
-      "Allow MS to attach via GERAN without authentication\n"
-      "Always require authentication\n")
+      "Whether to enforce MS authentication in GERAN (only with auth-policy remote)\n"
+      "Allow MS to attach via GERAN without authentication (default and only possible value for non-remote auth-policy)\n"
+      "Always require authentication (only available for auth-policy remote, default with that auth-policy)\n")
 {
 	int required = (argv[0][0] == 'r');
 
@@ -730,10 +730,6 @@
 	g_cfg->auth_policy = val;
 	g_cfg->require_update_location = (val == SGSN_AUTH_POLICY_REMOTE);
 
-	/* Authentication is not possible without HLR */
-	if (val != SGSN_AUTH_POLICY_REMOTE)
-		g_cfg->require_authentication = 0;
-
 	return CMD_SUCCESS;
 }
 
@@ -1489,14 +1485,6 @@
 		return rc;
 	}
 
-	if (g_cfg->auth_policy != SGSN_AUTH_POLICY_REMOTE
-	    && g_cfg->require_authentication) {
-		fprintf(stderr, "Configuration error:"
-			" authentication is not possible without HLR."
-			" Consider setting 'auth-policy' to 'remote'\n");
-		return -EINVAL;
-	}
-
 	if (g_cfg->auth_policy == SGSN_AUTH_POLICY_REMOTE
 	    && !(g_cfg->gsup_server_addr.sin_addr.s_addr
 		 && g_cfg->gsup_server_port)) {

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/14445
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: Ic707a95af178b44f08809df3d3bc8354bf34273c
Gerrit-Change-Number: 14445
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190613/c12fef8d/attachment.html>


More information about the gerrit-log mailing list