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.orgNeels Hofmeyr has submitted this change and it was merged.
Change subject: fix db_update_sqn(): reset stmt in all error cases
......................................................................
fix db_update_sqn(): reset stmt in all error cases
Use the common db_bind_int64() so that the stmt bindings are cleared for any
errors and to get error logging for free.
On error with sqlite3_step(), log the SQL error message, and make sure the stmt
is cleared of bindings and reset.
After sqlite3_step(), verify that exactly one row was modifed, log and return
errors otherwise.
After this patch, the DB interaction closely matches the other (refactored) DB
functions.
Change-Id: I0d870d405e2e0a830360d9ad19f0a3f9e09d8cf2
---
M src/db.c
M src/db_auc.c
M tests/db/db_test.c
M tests/db/db_test.err
4 files changed, 317 insertions(+), 17 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/db.c b/src/db.c
index 4136e39..f1c14c9 100644
--- a/src/db.c
+++ b/src/db.c
@@ -53,7 +53,7 @@
" LEFT JOIN auc_2g ON auc_2g.subscriber_id = subscriber.id"
" LEFT JOIN auc_3g ON auc_3g.subscriber_id = subscriber.id"
" WHERE imsi = ?",
- [DB_STMT_AUC_UPD_SQN] = "UPDATE auc_3g SET sqn = ? WHERE subscriber_id = ?",
+ [DB_STMT_AUC_UPD_SQN] = "UPDATE auc_3g SET sqn = $sqn WHERE subscriber_id = $subscriber_id",
[DB_STMT_UPD_PURGE_CS_BY_IMSI] = "UPDATE subscriber SET ms_purged_cs = $val WHERE imsi = $imsi",
[DB_STMT_UPD_PURGE_PS_BY_IMSI] = "UPDATE subscriber SET ms_purged_ps = $val WHERE imsi = $imsi",
[DB_STMT_UPD_NAM_CS_BY_IMSI] = "UPDATE subscriber SET nam_cs = $val WHERE imsi = $imsi",
diff --git a/src/db_auc.c b/src/db_auc.c
index d469920..eae5070 100644
--- a/src/db_auc.c
+++ b/src/db_auc.c
@@ -34,34 +34,43 @@
#define LOGAUC(imsi, level, fmt, args ...) LOGP(DAUC, level, "IMSI='%s': " fmt, imsi, ## args)
/* update the SQN for a given subscriber ID */
-int db_update_sqn(struct db_context *dbc, int64_t id,
- uint64_t new_sqn)
+int db_update_sqn(struct db_context *dbc, int64_t subscr_id, uint64_t new_sqn)
{
sqlite3_stmt *stmt = dbc->stmt[DB_STMT_AUC_UPD_SQN];
int rc;
+ int ret = 0;
- /* bind new SQN and subscriber ID */
- rc = sqlite3_bind_int64(stmt, 1, new_sqn);
- if (rc != SQLITE_OK) {
- LOGP(DAUC, LOGL_ERROR, "Error binding SQN: %d\n", rc);
- return -1;
- }
+ if (!db_bind_int64(stmt, "$sqn", new_sqn))
+ return -EIO;
- rc = sqlite3_bind_int64(stmt, 2, id);
- if (rc != SQLITE_OK) {
- LOGP(DAUC, LOGL_ERROR, "Error binding Subscrber ID: %d\n", rc);
- return -1;
- }
+ if (!db_bind_int64(stmt, "$subscriber_id", subscr_id))
+ return -EIO;
/* execute the statement */
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
- LOGP(DAUC, LOGL_ERROR, "Error updating SQN: %d\n", rc);
- return -2;
+ LOGP(DAUC, LOGL_ERROR, "Cannot update SQN for subscriber ID=%"PRId64
+ ": SQL error: (%d) %s\n",
+ subscr_id, rc, sqlite3_errmsg(dbc->db));
+ ret = -EIO;
+ goto out;
}
+ /* verify execution result */
+ rc = sqlite3_changes(dbc->db);
+ if (!rc) {
+ LOGP(DAUC, LOGL_ERROR, "Cannot update SQN for subscriber ID=%"PRId64
+ ": no auc_3g entry for such subscriber\n", subscr_id);
+ ret = -ENOENT;
+ } else if (rc != 1) {
+ LOGP(DAUC, LOGL_ERROR, "Update SQN for subscriber ID=%"PRId64
+ ": SQL modified %d rows (expected 1)\n", subscr_id, rc);
+ ret = -EIO;
+ }
+
+out:
db_remove_reset(stmt);
- return 0;
+ return ret;
}
/* obtain the authentication data for a given imsi
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index 1bdabdb..db3318c 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -664,6 +664,79 @@
comment_end();
}
+static void test_subscr_sqn()
+{
+ int64_t id;
+
+ comment_start();
+
+ comment("Set SQN for unknown subscriber");
+
+ ASSERT_RC(db_update_sqn(dbc, 99, 999), -ENOENT);
+ ASSERT_SEL(id, 99, -ENOENT);
+
+ ASSERT_RC(db_update_sqn(dbc, 9999, 99), -ENOENT);
+ ASSERT_SEL(id, 9999, -ENOENT);
+
+ comment("Create subscriber");
+
+ ASSERT_RC(db_subscr_create(dbc, imsi0), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+
+ id = g_subscr.id;
+ ASSERT_SEL_AUD(imsi0, -ENOENT, 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_RC(db_update_sqn(dbc, id, 543), -ENOENT);
+ ASSERT_SEL_AUD(imsi0, -ENOENT, id);
+
+ comment("Set auth 3G data");
+
+ ASSERT_RC(db_subscr_update_aud_by_id(dbc, id,
+ mk_aud_3g(OSMO_AUTH_ALG_MILENAGE,
+ "BeefedCafeFaceAcedAddedDecadeFee", true,
+ "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)),
+ 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ comment("Set SQN");
+
+ ASSERT_RC(db_update_sqn(dbc, id, 23315), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ ASSERT_RC(db_update_sqn(dbc, id, 23315), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ ASSERT_RC(db_update_sqn(dbc, id, 423), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ comment("Set SQN: thru uint64_t range, using the int64_t SQLite bind");
+
+ ASSERT_RC(db_update_sqn(dbc, id, 0), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ ASSERT_RC(db_update_sqn(dbc, id, INT64_MAX), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ ASSERT_RC(db_update_sqn(dbc, id, INT64_MIN), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ ASSERT_RC(db_update_sqn(dbc, id, UINT64_MAX), 0);
+ ASSERT_SEL_AUD(imsi0, 0, id);
+
+ comment("Delete subscriber");
+
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_delete_by_id(dbc, id), 0);
+ ASSERT_SEL(imsi, imsi0, -ENOENT);
+
+ comment_end();
+}
+
static struct {
bool verbose;
} cmdline_opts = {
@@ -739,6 +812,7 @@
test_subscr_create_update_sel_delete();
test_subscr_aud();
+ test_subscr_sqn();
printf("Done\n");
return 0;
diff --git a/tests/db/db_test.err b/tests/db/db_test.err
index 4813ea6..0b09583 100644
--- a/tests/db/db_test.err
+++ b/tests/db/db_test.err
@@ -1191,3 +1191,220 @@
===== test_subscr_aud: SUCCESS
+
+===== test_subscr_sqn
+
+--- Set SQN for unknown subscriber
+
+db_update_sqn(dbc, 99, 999) --> -ENOENT
+DAUC Cannot update SQN for subscriber ID=99: no auc_3g entry for such subscriber
+
+db_subscr_get_by_id(dbc, 99, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: ID=99: No such subscriber
+
+db_update_sqn(dbc, 9999, 99) --> -ENOENT
+DAUC Cannot update SQN for subscriber ID=9999: no auc_3g entry for such subscriber
+
+db_subscr_get_by_id(dbc, 9999, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: ID=9999: No such subscriber
+
+
+--- Create subscriber
+
+db_subscr_create(dbc, imsi0) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+}
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -2
+DAUC IMSI='123456789000000': No 2G Auth Data
+DAUC IMSI='123456789000000': No 3G Auth Data
+
+
+
+--- Set SQN, but no 3G auth data present
+
+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
+DAUC IMSI='123456789000000': No 2G Auth Data
+DAUC IMSI='123456789000000': No 3G Auth Data
+
+
+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
+DAUC IMSI='123456789000000': No 2G Auth Data
+DAUC IMSI='123456789000000': No 3G Auth Data
+
+
+
+--- Set auth 3G data
+
+db_subscr_update_aud_by_id(dbc, id, mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, "BeefedCafeFaceAcedAddedDecadeFee", true, "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.ind_bitlen = 5,
+}
+
+
+--- Set SQN
+
+db_update_sqn(dbc, id, 23315) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.sqn = 23315,
+ .u.umts.sqn = 0x5b13,
+ .u.umts.ind_bitlen = 5,
+}
+
+db_update_sqn(dbc, id, 23315) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.sqn = 23315,
+ .u.umts.sqn = 0x5b13,
+ .u.umts.ind_bitlen = 5,
+}
+
+db_update_sqn(dbc, id, 423) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.sqn = 423,
+ .u.umts.sqn = 0x1a7,
+ .u.umts.ind_bitlen = 5,
+}
+
+
+--- Set SQN: thru uint64_t range, using the int64_t SQLite bind
+
+db_update_sqn(dbc, id, 0) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.ind_bitlen = 5,
+}
+
+db_update_sqn(dbc, id, INT64_MAX) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.sqn = 9223372036854775807,
+ .u.umts.sqn = 0x7fffffffffffffff,
+ .u.umts.ind_bitlen = 5,
+}
+
+db_update_sqn(dbc, id, INT64_MIN) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.sqn = 9223372036854775808,
+ .u.umts.sqn = 0x8000000000000000,
+ .u.umts.ind_bitlen = 5,
+}
+
+db_update_sqn(dbc, id, UINT64_MAX) --> 0
+
+db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> 0
+DAUC IMSI='123456789000000': No 2G Auth Data
+
+2G: none
+3G: struct osmo_sub_auth_data {
+ .type = UMTS,
+ .algo = MILENAGE,
+ .u.umts.opc = 'beefedcafefaceacedaddeddecadefee',
+ .u.umts.opc_is_op = 1,
+ .u.umts.k = 'c01ffedc1cadaeac1d1f1edacac1ab0a',
+ .u.umts.amf = '0000',
+ .u.umts.sqn = 18446744073709551615,
+ .u.umts.sqn = 0xffffffffffffffff,
+ .u.umts.ind_bitlen = 5,
+}
+
+
+--- Delete subscriber
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+}
+
+db_subscr_delete_by_id(dbc, id) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456789000000': No such subscriber
+
+===== test_subscr_sqn: SUCCESS
+
--
To view, visit https://gerrit.osmocom.org/4188
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0d870d405e2e0a830360d9ad19f0a3f9e09d8cf2
Gerrit-PatchSet: 3
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>