Remove unused auth code and add comment
diff mbox

Message ID 1457368606-18297-1-git-send-email-nhofmeyr@sysmocom.de
State New
Headers show

Commit Message

Neels Hofmeyr March 7, 2016, 4:36 p.m. UTC
As commented in the code, the GSM_SECURITY_AUTH_FAILED path is never invoked by
the gsm48_secure_channel() function as it is today.

Note that the upcoming Iu auth will probably add a GSM_SECURITY_AUTH_FAILED
status. In that case, sending a LU Reject immediately may be desirable, but
arguably a bit of timeout could make life harder for auth attackers.

The code removed by this patch doesn't send out a LU Reject ever, since a call
to release_loc_updating_req() only releases the connection. To reject, a call
to gsm0408_loc_upd_rej() would be necessary, as seen in loc_upd_rej_cb().

And finally, if _gsm0408_authorize_sec_cb() doesn't do anything about anything,
the same loc_upd_rej_cb() will be run by a timeout and send a LU Reject
properly (as commented in the code).
---
 openbsc/src/libmsc/gsm_04_08.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Patch
diff mbox

diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index d9d7390..47f3fa7 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -340,10 +340,6 @@  static int _gsm0408_authorize_sec_cb(unsigned int hooknum, unsigned int event,
 	int rc = 0;
 
 	switch (event) {
-		case GSM_SECURITY_AUTH_FAILED:
-			release_loc_updating_req(conn, 1);
-			break;
-
 		case GSM_SECURITY_ALREADY:
 			LOGP(DMM, LOGL_ERROR, "We don't expect LOCATION "
 				"UPDATING after CM SERVICE REQUEST\n");
@@ -354,6 +350,19 @@  static int _gsm0408_authorize_sec_cb(unsigned int hooknum, unsigned int event,
 			rc = finish_lu(conn);
 			break;
 
+		case GSM_SECURITY_AUTH_FAILED:
+			/*
+			 * gsm48_secure_channel() will pass only
+			 * GSM_SECURITY_NOAVAIL in case of failure. If future
+			 * code should add a GSM_SECURITY_AUTH_FAILED status in
+			 * this code path, letting the Location Update time out
+			 * will do all necessary error messaging and logging,
+			 * see loc_upd_rej_cb().
+			 */
+			LOGP(DMM, LOGL_ERROR,
+			     "Authorization failed for subscriber %s\n",
+			     subscr_name(conn->subscr));
+			/* fall through */
 		default:
 			rc = -EINVAL;
 	};