diff options
author | Oliver Smith <osmith@sysmocom.de> | 2019-07-24 11:32:45 +0200 |
---|---|---|
committer | Oliver Smith <osmith@sysmocom.de> | 2019-07-25 14:52:20 +0200 |
commit | 6401b905743f1e47c7225c35c0852cefb00f673d (patch) | |
tree | c56bebe7ba073d8a4674028b5507973e41958606 | |
parent | 5b5cac7e94d8af269003728daa7cd9ee2439f959 (diff) |
db_auc.c: verify hex key sizes read from DB
Replace commented out size check for Ki with a real check, and use it
consistently for Ki, K, OP and OPC. Add a test that sets all keys to the
wrong size and tries to read them.
Related: OS#2565
Change-Id: Ib8e8e9394fb65c6e7932ce9f8bebc321b99f7696
-rw-r--r-- | src/db_auc.c | 69 | ||||
-rw-r--r-- | tests/db/db_test.c | 76 | ||||
-rw-r--r-- | tests/db/db_test.err | 85 |
3 files changed, 201 insertions, 29 deletions
diff --git a/src/db_auc.c b/src/db_auc.c index e29b44b..2cf7143 100644 --- a/src/db_auc.c +++ b/src/db_auc.c @@ -73,6 +73,32 @@ out: return ret; } +/* hexparse a specific column of a sqlite prepared statement into dst (with length check) + * returns 0 for success, -EIO on error */ +static int hexparse_stmt(uint8_t *dst, size_t dst_len, sqlite3_stmt *stmt, int col, const char *col_name, + const char *imsi) +{ + const uint8_t *text; + size_t col_len; + + /* Bytes are stored as hex strings in database, hence divide length by two */ + col_len = sqlite3_column_bytes(stmt, col) / 2; + + if (col_len != dst_len) { + LOGAUC(imsi, LOGL_ERROR, "Error reading %s, expected length %lu but has length %lu\n", col_name, + dst_len, col_len); + return -EIO; + } + + text = sqlite3_column_text(stmt, col); + if (!text) { + LOGAUC(imsi, LOGL_ERROR, "Error reading %s\n", col_name); + return -EIO; + } + osmo_hexparse((void *)text, dst, dst_len); + return 0; +} + /* obtain the authentication data for a given imsi * 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, @@ -113,49 +139,34 @@ int db_get_auth_data(struct db_context *dbc, const char *imsi, /* obtain result values using sqlite3_column_*() */ if (sqlite3_column_type(stmt, 1) == SQLITE_INTEGER) { /* we do have some 2G authentication data */ - const uint8_t *ki; - - aud2g->algo = sqlite3_column_int(stmt, 1); - ki = sqlite3_column_text(stmt, 2); -#if 0 - if (sqlite3_column_bytes(stmt, 2) != sizeof(aud2g->u.gsm.ki)) { - LOGAUC(imsi, LOGL_ERROR, "Error reading Ki: %d\n", rc); + if (hexparse_stmt(aud2g->u.gsm.ki, sizeof(aud2g->u.gsm.ki), stmt, 2, "Ki", imsi)) goto end_2g; - } -#endif - osmo_hexparse((void*)ki, (void*)&aud2g->u.gsm.ki, sizeof(aud2g->u.gsm.ki)); + aud2g->algo = sqlite3_column_int(stmt, 1); aud2g->type = OSMO_AUTH_TYPE_GSM; } else LOGAUC(imsi, LOGL_DEBUG, "No 2G Auth Data\n"); -//end_2g: +end_2g: if (sqlite3_column_type(stmt, 3) == SQLITE_INTEGER) { /* we do have some 3G authentication data */ - const uint8_t *k, *op, *opc; - - aud3g->algo = sqlite3_column_int(stmt, 3); - k = sqlite3_column_text(stmt, 4); - if (!k) { - LOGAUC(imsi, LOGL_ERROR, "Error reading K: %d\n", rc); + if (hexparse_stmt(aud3g->u.umts.k, sizeof(aud3g->u.umts.k), stmt, 4, "K", imsi)) { ret = -EIO; goto out; } - osmo_hexparse((void*)k, (void*)&aud3g->u.umts.k, sizeof(aud3g->u.umts.k)); + aud3g->algo = sqlite3_column_int(stmt, 3); + /* UMTS Subscribers can have either OP or OPC */ - op = sqlite3_column_text(stmt, 5); - if (!op) { - opc = sqlite3_column_text(stmt, 6); - if (!opc) { - LOGAUC(imsi, LOGL_ERROR, "Error reading OPC: %d\n", rc); + if (sqlite3_column_text(stmt, 5)) { + if (hexparse_stmt(aud3g->u.umts.opc, sizeof(aud3g->u.umts.opc), stmt, 5, "OP", imsi)) { ret = -EIO; goto out; } - osmo_hexparse((void*)opc, (void*)&aud3g->u.umts.opc, - sizeof(aud3g->u.umts.opc)); - aud3g->u.umts.opc_is_op = 0; - } else { - osmo_hexparse((void*)op, (void*)&aud3g->u.umts.opc, - sizeof(aud3g->u.umts.opc)); aud3g->u.umts.opc_is_op = 1; + } else { + if (hexparse_stmt(aud3g->u.umts.opc, sizeof(aud3g->u.umts.opc), stmt, 6, "OPC", imsi)) { + ret = -EIO; + goto out; + } + aud3g->u.umts.opc_is_op = 0; } aud3g->u.umts.sqn = sqlite3_column_int64(stmt, 7); aud3g->u.umts.ind_bitlen = sqlite3_column_int(stmt, 8); diff --git a/tests/db/db_test.c b/tests/db/db_test.c index fdd62c5..cc299bf 100644 --- a/tests/db/db_test.c +++ b/tests/db/db_test.c @@ -208,6 +208,17 @@ void dump_aud(const char *label, struct osmo_sub_auth_data *aud) #undef Phex } +void db_raw_sql(struct db_context *dbc, const char *sql) +{ + sqlite3_stmt *stmt; + + fprintf(stderr, "raw SQL: %s\n", sql); + ASSERT_RC(sqlite3_prepare_v2(dbc->db, sql, -1, &stmt, NULL), SQLITE_OK); + ASSERT_RC(sqlite3_step(stmt), SQLITE_DONE); + db_remove_reset(stmt); + sqlite3_finalize(stmt); +} + static const char *imsi0 = "123456789000000"; static const char *imsi1 = "123456789000001"; static const char *imsi2 = "123456789000002"; @@ -749,6 +760,70 @@ static void test_subscr_aud() comment_end(); } +/* Make each key too short in this test. Note that we can't set them longer than the allowed size without changing the + * table structure. */ +static void test_subscr_aud_invalid_len() +{ + int64_t id; + + comment_start(); + comment("Create subscriber"); + ASSERT_RC(db_subscr_create(dbc, imsi0, DB_SUBSCR_FLAG_NAM_CS | DB_SUBSCR_FLAG_NAM_PS), 0); + ASSERT_SEL(imsi, imsi0, 0); + id = g_subscr.id; + + + /* Invalid Ki length */ + comment("Set auth data, 2G only, with invalid Ki length"); + ASSERT_RC(db_subscr_update_aud_by_id(dbc, id, + mk_aud_2g(OSMO_AUTH_ALG_COMP128v1, "0123456789abcdef0123456789abcdef")), + 0); + /* Use raw SQL to avoid length check in db_subscr_update_aud_by_id(). This changes all rows in the table, which + * is fine for this test (implicit WHERE 1). */ + db_raw_sql(dbc, "UPDATE auc_2g SET ki = '0123456789abcdef0123456789abcde'"); + ASSERT_SEL_AUD(imsi0, -ENOKEY, id); + + comment("Remove 2G auth data"); + ASSERT_RC(db_subscr_update_aud_by_id(dbc, id, + mk_aud_2g(OSMO_AUTH_ALG_NONE, NULL)), + 0); + + /* Invalid K length */ + comment("Set auth data, 3G only, with invalid K length"); + ASSERT_RC(db_subscr_update_aud_by_id(dbc, id, + mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, + "BeefedCafeFaceAcedAddedDecadeFee", true, + "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)), + 0); + db_raw_sql(dbc, "UPDATE auc_3g SET k = 'C01ffedC1cadaeAc1d1f1edAcac1aB0'"); + ASSERT_SEL_AUD(imsi0, -EIO, id); + + /* Invalid OP length */ + comment("Set auth data, 3G only, with invalid OP length"); + ASSERT_RC(db_subscr_update_aud_by_id(dbc, id, + mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, + "BeefedCafeFaceAcedAddedDecadeFee", true, + "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)), + 0); + db_raw_sql(dbc, "UPDATE auc_3g SET op = 'BeefedCafeFaceAcedAddedDecadeFe'"); + ASSERT_SEL_AUD(imsi0, -EIO, id); + + /* Invalid OPC length */ + comment("Set auth data, 3G only, with invalid OPC length"); + ASSERT_RC(db_subscr_update_aud_by_id(dbc, id, + mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, + "BeefedCafeFaceAcedAddedDecadeFee", false, + "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)), + 0); + db_raw_sql(dbc, "UPDATE auc_3g SET opc = 'BeefedCafeFaceAcedAddedDecadeFe'"); + ASSERT_SEL_AUD(imsi0, -EIO, id); + + + comment("Delete subscriber"); + ASSERT_RC(db_subscr_delete_by_id(dbc, id), 0); + comment_end(); +} + static void test_subscr_sqn() { int64_t id; @@ -900,6 +975,7 @@ int main(int argc, char **argv) test_subscr_create_update_sel_delete(); test_subscr_aud(); + test_subscr_aud_invalid_len(); test_subscr_sqn(); printf("Done\n"); diff --git a/tests/db/db_test.err b/tests/db/db_test.err index 4dc77e8..a7c4cf1 100644 --- a/tests/db/db_test.err +++ b/tests/db/db_test.err @@ -1338,6 +1338,91 @@ DAUC IMSI='123456789000000': No such subscriber ===== test_subscr_aud: SUCCESS +===== test_subscr_aud_invalid_len + +--- Create subscriber + +db_subscr_create(dbc, imsi0, DB_SUBSCR_FLAG_NAM_CS | DB_SUBSCR_FLAG_NAM_PS) --> 0 + +db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', +} + + +--- Set auth data, 2G only, with invalid Ki length + +db_subscr_update_aud_by_id(dbc, id, mk_aud_2g(OSMO_AUTH_ALG_COMP128v1, "0123456789abcdef0123456789abcdef")) --> 0 + +raw SQL: UPDATE auc_2g SET ki = '0123456789abcdef0123456789abcde' +sqlite3_prepare_v2(dbc->db, sql, -1, &stmt, NULL) --> SQLITE_OK + +sqlite3_step(stmt) --> SQLITE_DONE + +db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -126 +DAUC IMSI='123456789000000': Error reading Ki, expected length 16 but has length 15 +DAUC IMSI='123456789000000': No 3G Auth Data + + + +--- Remove 2G auth data + +db_subscr_update_aud_by_id(dbc, id, mk_aud_2g(OSMO_AUTH_ALG_NONE, NULL)) --> 0 + + +--- Set auth data, 3G only, with invalid K length + +db_subscr_update_aud_by_id(dbc, id, mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, "BeefedCafeFaceAcedAddedDecadeFee", true, "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)) --> 0 + +raw SQL: UPDATE auc_3g SET k = 'C01ffedC1cadaeAc1d1f1edAcac1aB0' +sqlite3_prepare_v2(dbc->db, sql, -1, &stmt, NULL) --> SQLITE_OK + +sqlite3_step(stmt) --> SQLITE_DONE + +db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -5 +DAUC IMSI='123456789000000': No 2G Auth Data +DAUC IMSI='123456789000000': Error reading K, expected length 16 but has length 15 + + + +--- Set auth data, 3G only, with invalid OP length + +db_subscr_update_aud_by_id(dbc, id, mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, "BeefedCafeFaceAcedAddedDecadeFee", true, "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)) --> 0 + +raw SQL: UPDATE auc_3g SET op = 'BeefedCafeFaceAcedAddedDecadeFe' +sqlite3_prepare_v2(dbc->db, sql, -1, &stmt, NULL) --> SQLITE_OK + +sqlite3_step(stmt) --> SQLITE_DONE + +db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -5 +DAUC IMSI='123456789000000': No 2G Auth Data +DAUC IMSI='123456789000000': Error reading OP, expected length 16 but has length 15 + + + +--- Set auth data, 3G only, with invalid OPC length + +db_subscr_update_aud_by_id(dbc, id, mk_aud_3g(OSMO_AUTH_ALG_MILENAGE, "BeefedCafeFaceAcedAddedDecadeFee", false, "C01ffedC1cadaeAc1d1f1edAcac1aB0a", 5)) --> 0 + +raw SQL: UPDATE auc_3g SET opc = 'BeefedCafeFaceAcedAddedDecadeFe' +sqlite3_prepare_v2(dbc->db, sql, -1, &stmt, NULL) --> SQLITE_OK + +sqlite3_step(stmt) --> SQLITE_DONE + +db_get_auth_data(dbc, imsi0, &g_aud2g, &g_aud3g, &g_id) --> -5 +DAUC IMSI='123456789000000': No 2G Auth Data +DAUC IMSI='123456789000000': Error reading OPC, expected length 16 but has length 15 + + + +--- Delete subscriber + +db_subscr_delete_by_id(dbc, id) --> 0 + +===== test_subscr_aud_invalid_len: SUCCESS + + ===== test_subscr_sqn --- Set SQN for unknown subscriber |