[PATCH] osmo-hlr[master]: db_get_auth_data / db_get_auc: clarify return values

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Fri Nov 24 03:13:48 UTC 2017


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/4987

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

db_get_auth_data / db_get_auc: clarify return values

Differentiate between "IMSI unknown" and "IMSI has no auth data": in case of
known IMSI lacking auth data, return -ENOKEY instead of -ENOENT.

Fix API doc comments to reflect what the functions actually return, on top of
adding the -ENOKEY detail.

Adjust db_test expectations from -ENOENT to -ENOKEY where appropriate.

Adjust VTY and CTRL command rc evaluation.

A subsequent patch will use these return values to log details on each of these
situations.

Change-Id: Icf6304d23585f2ed45e050fa27c787f2d66fd3f7
---
M src/ctrl.c
M src/db_auc.c
M src/hlr.c
M src/hlr_vty_subscr.c
M tests/db/db_test.c
M tests/db/db_test.err
6 files changed, 53 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/87/4987/2

diff --git a/src/ctrl.c b/src/ctrl.c
index 3e81661..8ae9d7c 100644
--- a/src/ctrl.c
+++ b/src/ctrl.c
@@ -228,11 +228,16 @@
 
 	rc = db_get_auth_data(hlr->dbc, imsi, &aud2g, &aud3g, NULL);
 
-	if (rc == -ENOENT) {
+	switch (rc) {
+	case 0:
+		break;
+	case -ENOENT:
+	case -ENOKEY:
 		/* No auth data found, tell the print*() functions about it. */
 		aud2g.algo = OSMO_AUTH_ALG_NONE;
 		aud3g.algo = OSMO_AUTH_ALG_NONE;
-	} else if (rc) {
+		break;
+	default:
 		cmd->reply = "Error retrieving authentication data.";
 		return CTRL_CMD_ERROR;
 	}
@@ -258,11 +263,16 @@
 
 	rc = db_get_auth_data(hlr->dbc, subscr.imsi, &aud2g, &aud3g, NULL);
 
-	if (rc == -ENOENT) {
+	switch (rc) {
+	case 0:
+		break;
+	case -ENOENT:
+	case -ENOKEY:
 		/* No auth data found, tell the print*() functions about it. */
 		aud2g.algo = OSMO_AUTH_ALG_NONE;
 		aud3g.algo = OSMO_AUTH_ALG_NONE;
-	} else if (rc) {
+		break;
+	default:
 		cmd->reply = "Error retrieving authentication data.";
 		return CTRL_CMD_ERROR;
 	}
diff --git a/src/db_auc.c b/src/db_auc.c
index 7bbc93f..5fb5e3a 100644
--- a/src/db_auc.c
+++ b/src/db_auc.c
@@ -74,7 +74,9 @@
 }
 
 /* obtain the authentication data for a given imsi
- * returns -1 in case of error, 0 for unknown IMSI, 1 for success */
+ * returns 0 for success, negative value on error:
+ * -ENOENT if the IMSI is not known, -ENOKEY if the IMSI is known but has no auth data,
+ * -EIO on db failure */
 int db_get_auth_data(struct db_context *dbc, const char *imsi,
 		     struct osmo_sub_auth_data *aud2g,
 		     struct osmo_sub_auth_data *aud3g,
@@ -163,15 +165,16 @@
 		LOGAUC(imsi, LOGL_DEBUG, "No 3G Auth Data\n");
 
 	if (aud2g->type == 0 && aud3g->type == 0)
-		ret = -ENOENT;
+		ret = -ENOKEY;
 
 out:
 	db_remove_reset(stmt);
 	return ret;
 }
 
-/* return -1 in case of error, 0 for unknown imsi, positive for number
- * of vectors generated */
+/* return number of vectors generated, negative value on error:
+ * -ENOENT if the IMSI is not known, -ENOKEY if the IMSI is known but has no auth data,
+ * -EIO on db failure */
 int db_get_auc(struct db_context *dbc, const char *imsi,
 	       unsigned int auc_3g_ind, struct osmo_auth_vector *vec,
 	       unsigned int num_vec, const uint8_t *rand_auts,
diff --git a/src/hlr.c b/src/hlr.c
index bcae3b5..58f94f3 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -71,6 +71,9 @@
 		gsup_out.message_type = OSMO_GSUP_MSGT_SEND_AUTH_INFO_ERROR;
 		switch (rc) {
 		case 0:
+			/* 0 means "0 tuples generated", which shouldn't happen.
+			 * Treat the same as "no auth data". */
+		case -ENOKEY:
 		case -ENOENT:
 			gsup_out.cause = GMM_CAUSE_IMSI_UNKNOWN;
 			break;
diff --git a/src/hlr_vty_subscr.c b/src/hlr_vty_subscr.c
index 0a9ba76..5a300a7 100644
--- a/src/hlr_vty_subscr.c
+++ b/src/hlr_vty_subscr.c
@@ -72,14 +72,17 @@
 	OSMO_ASSERT(g_hlr);
 	rc = db_get_auth_data(g_hlr->dbc, subscr->imsi, &aud2g, &aud3g, NULL);
 
-	if (rc) {
-		if (rc == -ENOENT) {
-			aud2g.algo = OSMO_AUTH_ALG_NONE;
-			aud3g.algo = OSMO_AUTH_ALG_NONE;
-		} else {
-			vty_out(vty, "%% Error retrieving data from database (%d)%s", rc, VTY_NEWLINE);
-			return;
-		}
+	switch (rc) {
+	case 0:
+		break;
+	case -ENOENT:
+	case -ENOKEY:
+		aud2g.algo = OSMO_AUTH_ALG_NONE;
+		aud3g.algo = OSMO_AUTH_ALG_NONE;
+		break;
+	default:
+		vty_out(vty, "%% Error retrieving data from database (%d)%s", rc, VTY_NEWLINE);
+		return;
 	}
 
 	if (aud2g.type != OSMO_AUTH_TYPE_NONE && aud2g.type != OSMO_AUTH_TYPE_GSM) {
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index 23b84cc..1a5d7e0 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -464,7 +464,7 @@
 	ASSERT_SEL(imsi, imsi0, 0);
 
 	id = g_subscr.id;
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 
 	comment("Set auth data, 2G only");
@@ -500,7 +500,7 @@
 	ASSERT_RC(db_subscr_update_aud_by_id(dbc, id,
 		mk_aud_2g(OSMO_AUTH_ALG_NONE, NULL)),
 		0);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 	/* Removing nothing results in -ENOENT */
 	ASSERT_RC(db_subscr_update_aud_by_id(dbc, id,
@@ -515,7 +515,7 @@
 	ASSERT_RC(db_subscr_update_aud_by_id(dbc, id,
 		mk_aud_2g(OSMO_AUTH_ALG_NONE, "f000000000000f00000000000f000000")),
 		0);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 
 	comment("Set auth data, 3G only");
@@ -562,7 +562,7 @@
 	ASSERT_RC(db_subscr_update_aud_by_id(dbc, id,
 		mk_aud_3g(OSMO_AUTH_ALG_NONE, NULL, false, NULL, 0)),
 		0);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 	/* Removing nothing results in -ENOENT */
 	ASSERT_RC(db_subscr_update_aud_by_id(dbc, id,
@@ -581,7 +581,7 @@
 			  "asdfasdfasd", false,
 			  "asdfasdfasdf", 99999)),
 		0);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 
 	comment("Set auth data, 2G and 3G");
@@ -669,7 +669,7 @@
 	/* For this test to work, we want to get the same subscriber ID back,
 	 * and make sure there are no auth data leftovers for this ID. */
 	OSMO_ASSERT(id == g_subscr.id);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 	ASSERT_RC(db_subscr_delete_by_id(dbc, id), 0);
 	ASSERT_SEL(imsi, imsi0, -ENOENT);
@@ -697,15 +697,15 @@
 	ASSERT_SEL(imsi, imsi0, 0);
 
 	id = g_subscr.id;
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 	comment("Set SQN, but no 3G auth data present");
 
 	ASSERT_RC(db_update_sqn(dbc, id, 123), -ENOENT);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 	ASSERT_RC(db_update_sqn(dbc, id, 543), -ENOENT);
-	ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+	ASSERT_SEL_AUD(imsi0, -ENOKEY, id);
 
 	comment("Set auth 3G data");
 
diff --git a/tests/db/db_test.err b/tests/db/db_test.err
index f7acfec..c5e5bac 100644
--- a/tests/db/db_test.err
+++ b/tests/db/db_test.err
@@ -726,7 +726,7 @@
   .imsi = '123456789000000',
 }
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -799,7 +799,7 @@
 
 db_subscr_update_aud_by_id(dbc, id, mk_aud_2g(OSMO_AUTH_ALG_NONE, NULL)) --> 0
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -820,7 +820,7 @@
 
 db_subscr_update_aud_by_id(dbc, id, mk_aud_2g(OSMO_AUTH_ALG_NONE, "f000000000000f00000000000f000000")) --> 0
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -912,7 +912,7 @@
 
 db_subscr_update_aud_by_id(dbc, id, mk_aud_3g(OSMO_AUTH_ALG_NONE, NULL, false, NULL, 0)) --> 0
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -937,7 +937,7 @@
 
 db_subscr_update_aud_by_id(dbc, id, mk_aud_3g(OSMO_AUTH_ALG_NONE, "asdfasdfasd", false, "asdfasdfasdf", 99999)) --> 0
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -1174,7 +1174,7 @@
   .imsi = '123456789000000',
 }
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -1214,7 +1214,7 @@
   .imsi = '123456789000000',
 }
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -1225,7 +1225,7 @@
 db_update_sqn(dbc, id, 123) --> -ENOENT
 DAUC Cannot update SQN for subscriber ID=1: no auc_3g entry for such subscriber
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 
@@ -1233,7 +1233,7 @@
 db_update_sqn(dbc, id, 543) --> -ENOENT
 DAUC Cannot update SQN for subscriber ID=1: no auc_3g entry for such subscriber
 
-db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126
 DAUC IMSI='123456789000000': No 2G Auth Data
 DAUC IMSI='123456789000000': No 3G Auth Data
 

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf6304d23585f2ed45e050fa27c787f2d66fd3f7
Gerrit-PatchSet: 2
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list