From patchwork Wed Mar 30 09:22:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Neels Hofmeyr X-Patchwork-Id: 603296 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.osmocom.org (lists.osmocom.org [IPv6:2a01:4f8:191:444b::2:7]) by ozlabs.org (Postfix) with ESMTP id 3qZhzn4hpdz9snm for ; Wed, 30 Mar 2016 20:24:21 +1100 (AEDT) Received: from lists.osmocom.org (lists.osmocom.org [144.76.43.76]) by lists.osmocom.org (Postfix) with ESMTP id B8DEF1CDF7; Wed, 30 Mar 2016 09:24:19 +0000 (UTC) X-Original-To: openbsc@lists.osmocom.org Delivered-To: openbsc@lists.osmocom.org Received: from einhorn.in-berlin.de (einhorn.in-berlin.de [IPv6:2001:bf0:c000::1:8]) by lists.osmocom.org (Postfix) with ESMTP id 5B8811CDBE for ; Wed, 30 Mar 2016 09:24:05 +0000 (UTC) X-Envelope-From: nhofmeyr@sysmocom.de X-Envelope-To: Received: from localhost (gw-01.freifunk.isp.faust2k.net [87.128.109.145]) (authenticated bits=0) by einhorn.in-berlin.de (8.14.4/8.14.4/Debian-4) with ESMTP id u2U9O4ik008206 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 30 Mar 2016 11:24:05 +0200 From: Neels Hofmeyr To: openbsc@lists.osmocom.org Subject: [PATCH 7/7] Fix MM Auth: zero-initialize auth tuple before first use Date: Wed, 30 Mar 2016 11:22:30 +0200 Message-Id: <1459329750-2318-8-git-send-email-nhofmeyr@sysmocom.de> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1459329750-2318-1-git-send-email-nhofmeyr@sysmocom.de> References: <1459329750-2318-1-git-send-email-nhofmeyr@sysmocom.de> X-BeenThere: openbsc@lists.osmocom.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "Development of OpenBSC, OsmoBSC, OsmoNITB, OsmoCSCN" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openbsc-bounces@lists.osmocom.org Sender: "OpenBSC" Make sure a new auth tuple is initialized after db_get_lastauthtuple_for_subscr() returns an error, i.e. if no tuple is present for the subscriber yet. Before this patch, the first key_seq depended on the typically uninitialized value that was present in auth tuple's key_seq upon calling auth_get_tuple_for_subscr(). The very first key_seq used for a new subscriber will now always be 0. Before, it used to be mostly 1 ("(0 + 1) % 7"), but depended on whether the key_seq was indeed initialized with 0, actually by random. --- openbsc/src/libmsc/auth.c | 11 ++++++++++- openbsc/tests/mm_auth/mm_auth_test.c | 24 +++++++++++++++++++++++- openbsc/tests/mm_auth/mm_auth_test.ok | 4 ++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index ca39d01..f30d56d 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -110,8 +110,17 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, } /* Generate a new one */ + if (rc != 0) { + /* If db_get_lastauthtuple_for_subscr() returned nothing, make + * sure the atuple memory is initialized to zero and thus start + * off with key_seq = 0. */ + memset(atuple, 0, sizeof(*atuple)); + } else { + /* If db_get_lastauthtuple_for_subscr() returned a previous + * tuple, use the next key_seq. */ + atuple->key_seq = (atuple->key_seq + 1) % 7; + } atuple->use_count = 1; - atuple->key_seq = (atuple->key_seq + 1) % 7; if (RAND_bytes(atuple->rand, sizeof(atuple->rand)) != 1) { LOGP(DMM, LOGL_NOTICE, "RAND_bytes failed, can't generate new auth tuple\n"); diff --git a/openbsc/tests/mm_auth/mm_auth_test.c b/openbsc/tests/mm_auth/mm_auth_test.c index 2b45861..34d96f1 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.c +++ b/openbsc/tests/mm_auth/mm_auth_test.c @@ -183,7 +183,29 @@ static void test_auth_then_ciph1() OSMO_ASSERT(auth_tuple_is(&atuple, "gsm_auth_tuple {\n" " .use_count = 1\n" - " .key_seq = 1\n" + " .key_seq = 0\n" + " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" + " .sres = a1 ab c6 90 \n" + " .kc = 0f 27 ed f3 ac 97 ac 00 \n" + "}\n" + )); + + /* With a different last saved key_seq stored in the out-arg of + * db_get_lastauthtuple_for_subscr() by coincidence, expect absolutely + * the same as above. */ + test_auth_info = default_auth_info; + test_last_auth_tuple = default_auth_tuple; + test_last_auth_tuple.key_seq = 3; + test_get_authinfo_rc = 0; + test_get_lastauthtuple_rc = -ENOENT; + key_seq = 0; + auth_action = auth_get_tuple_for_subscr_verbose(&atuple, &subscr, + key_seq); + OSMO_ASSERT(auth_action == AUTH_DO_AUTH_THEN_CIPH); + OSMO_ASSERT(auth_tuple_is(&atuple, + "gsm_auth_tuple {\n" + " .use_count = 1\n" + " .key_seq = 0\n" " .rand = 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 \n" " .sres = a1 ab c6 90 \n" " .kc = 0f 27 ed f3 ac 97 ac 00 \n" diff --git a/openbsc/tests/mm_auth/mm_auth_test.ok b/openbsc/tests/mm_auth/mm_auth_test.ok index 9d89bfb..6c49f97 100644 --- a/openbsc/tests/mm_auth/mm_auth_test.ok +++ b/openbsc/tests/mm_auth/mm_auth_test.ok @@ -12,6 +12,10 @@ wrapped: db_get_authinfo_for_subscr(): rc = 0 wrapped: db_get_lastauthtuple_for_subscr(): rc = -2 wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_DO_AUTH_THEN_CIPH +wrapped: db_get_authinfo_for_subscr(): rc = 0 +wrapped: db_get_lastauthtuple_for_subscr(): rc = -2 +wrapped: db_sync_lastauthtuple_for_subscr(): rc = 0 +auth_get_tuple_for_subscr(key_seq=0) --> auth_action == AUTH_DO_AUTH_THEN_CIPH * test_auth_then_ciph2() wrapped: db_get_authinfo_for_subscr(): rc = 0