From 2826df56b2af5a6a0f20e5a9bcf1d50a1130f0ba Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Fri, 1 Apr 2016 20:21:03 +0200 Subject: subscr: Make db_create_subscriber fail on duplicates The issue of db_create_subscriber updating an already existing subscr is that the same subscriber will then have two entries in the active subscribers list. In general this will break assumptions that a subscr can be compared by comparing the pointer. In the case of the VTY this was not an issue as the created subscr was immediately destroyed again but it is better to avoid this problem. Change the VTY command to find the subscriber and then call sync to have the updated time set. The side-effect is we will now have two queries for the subscriber. Once through subscr_get_by_imsi and once through db_create_subscriber. Change the db_create_subscriber to fail if a subscriber already exists, and add a testcase for this behavior and do not updated the 'updated' timestamp of an already existing subscriber. Add a testcase for this behavior. Related: OS Issue #1657 --- openbsc/src/libmsc/db.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'openbsc/src/libmsc/db.c') diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 0935fc54d..17bea2470 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -508,14 +508,8 @@ struct gsm_subscriber *db_create_subscriber(const char *imsi) /* Is this subscriber known in the db? */ subscr = db_get_subscriber(GSM_SUBSCRIBER_IMSI, imsi); if (subscr) { - result = dbi_conn_queryf(conn, - "UPDATE Subscriber set updated = datetime('now') " - "WHERE imsi = %s " , imsi); - if (!result) - LOGP(DDB, LOGL_ERROR, "failed to update timestamp\n"); - else - dbi_result_free(result); - return subscr; + subscr_put(subscr); + return NULL; } subscr = subscr_alloc(); -- cgit v1.2.3 From adb86759daa80a484eef4b5a24bc0ce0de0a0763 Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Fri, 1 Apr 2016 20:31:11 +0200 Subject: db: If creating a subscriber in the db fails, return NULL We should not return a subscriber in case it was not written to the database. Instead free the memory allocated and return NULL. Callers in gsm_04_08.c are prepared to have the creation fail. Related: OS Issue #1657 --- openbsc/src/libmsc/db.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'openbsc/src/libmsc/db.c') diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 17bea2470..267b5ef41 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -523,8 +523,11 @@ struct gsm_subscriber *db_create_subscriber(const char *imsi) "(%s, datetime('now'), datetime('now')) ", imsi ); - if (!result) + if (!result) { LOGP(DDB, LOGL_ERROR, "Failed to create Subscriber by IMSI.\n"); + subscr_put(subscr); + return NULL; + } subscr->id = dbi_conn_sequence_last(conn, NULL); strncpy(subscr->imsi, imsi, GSM_IMSI_LENGTH-1); dbi_result_free(result); -- cgit v1.2.3 From 121e9a4164e65dfb68b2bf09297a8537a2f659c5 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Wed, 20 Apr 2016 13:13:19 +0200 Subject: Start to use struct osmo_auth_vector from gsm_auth_tuple Rather than having a 'private' structure for kc, sres and rand, we now finally (with 4 years delay) use osmo_auth_vector from libosmogsm, which encapsulates authentication vectors that can be either GSM triplets or UMTS quintuples or a combination of both. gsm_auth_tuple becomes a wrapper around osmo_auth_vector, adding use_count and key_seq to it. key_seq is no longer initialized inside gprs_gsup_messages.c, as there is no CKSN / key_seq inside the message anyway. If a usre of the code needs key_seq, they need to manage it themselves. --- openbsc/src/libmsc/db.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'openbsc/src/libmsc/db.c') diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 267b5ef41..a23ec89ae 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -700,25 +700,25 @@ int db_get_lastauthtuple_for_subscr(struct gsm_auth_tuple *atuple, atuple->key_seq = dbi_result_get_ulonglong(result, "key_seq"); len = dbi_result_get_field_length(result, "rand"); - if (len != sizeof(atuple->rand)) + if (len != sizeof(atuple->vec.rand)) goto err_size; blob = dbi_result_get_binary(result, "rand"); - memcpy(atuple->rand, blob, len); + memcpy(atuple->vec.rand, blob, len); len = dbi_result_get_field_length(result, "sres"); - if (len != sizeof(atuple->sres)) + if (len != sizeof(atuple->vec.sres)) goto err_size; blob = dbi_result_get_binary(result, "sres"); - memcpy(atuple->sres, blob, len); + memcpy(atuple->vec.sres, blob, len); len = dbi_result_get_field_length(result, "kc"); - if (len != sizeof(atuple->kc)) + if (len != sizeof(atuple->vec.kc)) goto err_size; blob = dbi_result_get_binary(result, "kc"); - memcpy(atuple->kc, blob, len); + memcpy(atuple->vec.kc, blob, len); dbi_result_free(result); @@ -759,11 +759,11 @@ int db_sync_lastauthtuple_for_subscr(struct gsm_auth_tuple *atuple, /* Update / Insert */ dbi_conn_quote_binary_copy(conn, - atuple->rand, sizeof(atuple->rand), &rand_str); + atuple->vec.rand, sizeof(atuple->vec.rand), &rand_str); dbi_conn_quote_binary_copy(conn, - atuple->sres, sizeof(atuple->sres), &sres_str); + atuple->vec.sres, sizeof(atuple->vec.sres), &sres_str); dbi_conn_quote_binary_copy(conn, - atuple->kc, sizeof(atuple->kc), &kc_str); + atuple->vec.kc, sizeof(atuple->vec.kc), &kc_str); if (!upd) { result = dbi_conn_queryf(conn, -- cgit v1.2.3 From d3fa84dbba3b67cdbe2d8c789b2833b5ddf42068 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Wed, 20 Apr 2016 17:50:17 +0200 Subject: use new libosmocore gsm_23_003.h for IMEI/IMSI length ... rather than our private definitions everwhere. As an added benefit, gprs_gsup_messages.h is now free of any header dependencies within openbsc. --- openbsc/src/libmsc/db.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'openbsc/src/libmsc/db.c') diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index a23ec89ae..e5017ae7b 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -529,7 +530,7 @@ struct gsm_subscriber *db_create_subscriber(const char *imsi) return NULL; } subscr->id = dbi_conn_sequence_last(conn, NULL); - strncpy(subscr->imsi, imsi, GSM_IMSI_LENGTH-1); + strncpy(subscr->imsi, imsi, sizeof(subscr->imsi)-1); dbi_result_free(result); LOGP(DDB, LOGL_INFO, "New Subscriber: ID %llu, IMSI %s\n", subscr->id, subscr->imsi); db_subscriber_alloc_exten(subscr); @@ -803,7 +804,7 @@ static void db_set_from_query(struct gsm_subscriber *subscr, dbi_conn result) const char *string; string = dbi_result_get_string(result, "imsi"); if (string) - strncpy(subscr->imsi, string, GSM_IMSI_LENGTH-1); + strncpy(subscr->imsi, string, sizeof(subscr->imsi)-1); string = dbi_result_get_string(result, "tmsi"); if (string) @@ -1317,7 +1318,7 @@ int db_subscriber_alloc_token(struct gsm_subscriber *subscriber, uint32_t *token return 0; } -int db_subscriber_assoc_imei(struct gsm_subscriber *subscriber, char imei[GSM_IMEI_LENGTH]) +int db_subscriber_assoc_imei(struct gsm_subscriber *subscriber, char imei[GSM23003_IMEISV_NUM_DIGITS]) { unsigned long long equipment_id, watch_id; dbi_result result; -- cgit v1.2.3