From: Tobias Brunner Date: Tue, 16 Dec 2008 17:21:28 +0000 (-0000) Subject: improved IKE_SA uniqueness check X-Git-Tag: 4.2.10~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=58464dd737b3050c1d7dd577a015945fb4f7e333;p=thirdparty%2Fstrongswan.git improved IKE_SA uniqueness check --- diff --git a/src/charon/sa/ike_sa.h b/src/charon/sa/ike_sa.h index c792ed2e1e..f9c264749f 100644 --- a/src/charon/sa/ike_sa.h +++ b/src/charon/sa/ike_sa.h @@ -657,9 +657,9 @@ struct ike_sa_t { * * @return * - SUCCESS if deletion is initialized - * - INVALID_STATE, if the IKE_SA is not in + * - DESTROY_ME, if the IKE_SA is not in * an established state and can not be - * delete (but destroyed). + * deleted (but destroyed). */ status_t (*delete) (ike_sa_t *this); diff --git a/src/charon/sa/ike_sa_manager.c b/src/charon/sa/ike_sa_manager.c index fc23bdd21f..826a637458 100644 --- a/src/charon/sa/ike_sa_manager.c +++ b/src/charon/sa/ike_sa_manager.c @@ -1234,18 +1234,23 @@ static ike_sa_t* checkout_by_name(private_ike_sa_manager_t *this, char *name, } /** - * Implementation of ike_sa_manager_t.checkout_duplicate. + * Implementation of ike_sa_manager_t.check_uniqueness. */ -static ike_sa_t* checkout_duplicate(private_ike_sa_manager_t *this, - ike_sa_t *ike_sa) +static bool check_uniqueness(private_ike_sa_manager_t *this, ike_sa_t *ike_sa) { - linked_list_t *list; - entry_t *entry; + bool cancel = FALSE; + peer_cfg_t *peer_cfg = ike_sa->get_peer_cfg(ike_sa); + unique_policy_t policy = peer_cfg->get_unique_policy(peer_cfg); + linked_list_t *list, *duplicate_ids = NULL; ike_sa_id_t *duplicate_id = NULL; - ike_sa_t *duplicate = NULL; identification_t *me, *other; u_int row, segment; - rwlock_t *lock; + rwlock_t *lock; + + if (policy == UNIQUE_NO) + { + return FALSE; + } me = ike_sa->get_my_id(ike_sa); other = ike_sa->get_other_id(ike_sa); @@ -1262,25 +1267,70 @@ static ike_sa_t* checkout_duplicate(private_ike_sa_manager_t *this, if (list->find_first(list, (linked_list_match_t)connected_peers_match, (void**)¤t, me, other) == SUCCESS) { - /* we just return the first ike_sa_id we have cached for these ids */ - current->sas->get_first(current->sas, (void**)&duplicate_id); + /* clone the list, so we can release the lock */ + duplicate_ids = current->sas->clone_offset(current->sas, + offsetof(ike_sa_id_t, clone)); } } lock->unlock(lock); - if (duplicate_id) + if (!duplicate_ids) + { + return FALSE; + } + + enumerator_t *enumerator = duplicate_ids->create_enumerator(duplicate_ids); + while (enumerator->enumerate(enumerator, (void**)&duplicate_id)) { - if (get_entry_by_id(this, duplicate_id, &entry, &segment) == SUCCESS) + status_t status = SUCCESS; + ike_sa_t *duplicate = checkout(this, duplicate_id); + if (!duplicate) + { + continue; + } + peer_cfg = duplicate->get_peer_cfg(duplicate); + if (peer_cfg && peer_cfg->equals(peer_cfg, ike_sa->get_peer_cfg(ike_sa))) { - if (wait_for_entry(this, entry, segment)) + switch (duplicate->get_state(duplicate)) { - duplicate = entry->ike_sa; - entry->checked_out = TRUE; + case IKE_ESTABLISHED: + case IKE_REKEYING: + switch (policy) + { + case UNIQUE_REPLACE: + DBG1(DBG_IKE, "deleting duplicate IKE_SA due" + " uniqueness policy"); + status = duplicate->delete(duplicate); + break; + case UNIQUE_KEEP: + cancel = TRUE; + /* we keep the first IKE_SA and delete all + * other duplicates that might exist */ + policy = UNIQUE_REPLACE; + break; + default: + break; + } + break; + default: + break; } - unlock_single_segment(this, segment); } + if (status == DESTROY_ME) + { + charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, + duplicate); + } + else + { + charon->ike_sa_manager->checkin(charon->ike_sa_manager, duplicate); + } + /* reset thread's current IKE_SA after checkin */ + charon->bus->set_sa(charon->bus, ike_sa); } - return duplicate; + enumerator->destroy(enumerator); + duplicate_ids->destroy_offset(duplicate_ids, offsetof(ike_sa_id_t, destroy)); + return cancel; } /** @@ -1645,7 +1695,7 @@ ike_sa_manager_t *ike_sa_manager_create() this->public.checkout_by_config = (ike_sa_t*(*)(ike_sa_manager_t*,peer_cfg_t*))checkout_by_config; this->public.checkout_by_id = (ike_sa_t*(*)(ike_sa_manager_t*,u_int32_t,bool))checkout_by_id; this->public.checkout_by_name = (ike_sa_t*(*)(ike_sa_manager_t*,char*,bool))checkout_by_name; - this->public.checkout_duplicate = (ike_sa_t*(*)(ike_sa_manager_t*, ike_sa_t *ike_sa))checkout_duplicate; + this->public.check_uniqueness = (bool(*)(ike_sa_manager_t*, ike_sa_t *ike_sa))check_uniqueness; this->public.create_enumerator = (enumerator_t*(*)(ike_sa_manager_t*))create_enumerator; this->public.checkin = (void(*)(ike_sa_manager_t*,ike_sa_t*))checkin; this->public.checkin_and_destroy = (void(*)(ike_sa_manager_t*,ike_sa_t*))checkin_and_destroy; diff --git a/src/charon/sa/ike_sa_manager.h b/src/charon/sa/ike_sa_manager.h index 4edf8018c4..b5c479e409 100644 --- a/src/charon/sa/ike_sa_manager.h +++ b/src/charon/sa/ike_sa_manager.h @@ -1,4 +1,5 @@ /* + * Copyright (C) 2008 Tobias Brunner * Copyright (C) 2005-2006 Martin Willi * Copyright (C) 2005 Jan Hutter * Hochschule fuer Technik Rapperswil @@ -103,12 +104,17 @@ struct ike_sa_manager_t { peer_cfg_t *peer_cfg); /** - * Check out a duplicate if ike_sa to do uniqueness tests. - * - * @param ike_sa ike_sa to get a duplicate from - * @return checked out duplicate + * Check for duplicates of the given IKE_SA. + * + * Measures are taken according to the uniqueness policy of the IKE_SA. + * The return value indicates whether duplicates have been found and if + * further measures should be taken (e.g. cancelling an IKE_AUTH exchange). + * + * @param ike_sa ike_sa to check + * @return TRUE, if the given IKE_SA has duplicates and + * should be deleted */ - ike_sa_t* (*checkout_duplicate)(ike_sa_manager_t *this, ike_sa_t *ike_sa); + bool (*check_uniqueness)(ike_sa_manager_t *this, ike_sa_t *ike_sa); /** * Check out an IKE_SA a unique ID. diff --git a/src/charon/sa/tasks/ike_auth.c b/src/charon/sa/tasks/ike_auth.c index 16fcb4ae43..2e405348c6 100644 --- a/src/charon/sa/tasks/ike_auth.c +++ b/src/charon/sa/tasks/ike_auth.c @@ -87,70 +87,6 @@ struct private_ike_auth_t { bool peer_authenticated; }; -/** - * check uniqueness and delete duplicates - */ -static bool check_uniqueness(private_ike_auth_t *this) -{ - ike_sa_t *duplicate; - unique_policy_t policy; - status_t status = SUCCESS; - peer_cfg_t *peer_cfg; - bool cancel = FALSE; - - peer_cfg = this->ike_sa->get_peer_cfg(this->ike_sa); - policy = peer_cfg->get_unique_policy(peer_cfg); - if (policy == UNIQUE_NO) - { - return FALSE; - } - duplicate = charon->ike_sa_manager->checkout_duplicate( - charon->ike_sa_manager, this->ike_sa); - if (duplicate) - { - peer_cfg = duplicate->get_peer_cfg(duplicate); - if (peer_cfg && - peer_cfg->equals(peer_cfg, this->ike_sa->get_peer_cfg(this->ike_sa))) - { - switch (duplicate->get_state(duplicate)) - { - case IKE_ESTABLISHED: - case IKE_REKEYING: - switch (policy) - { - case UNIQUE_REPLACE: - DBG1(DBG_IKE, "deleting duplicate IKE_SA due " - "uniqueness policy"); - status = duplicate->delete(duplicate); - break; - case UNIQUE_KEEP: - DBG1(DBG_IKE, "cancelling IKE_SA setup due " - "uniqueness policy"); - cancel = TRUE; - break; - default: - break; - } - break; - default: - break; - } - } - if (status == DESTROY_ME) - { - charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, - duplicate); - } - else - { - charon->ike_sa_manager->checkin(charon->ike_sa_manager, duplicate); - } - } - /* set threads active IKE_SA after checkin */ - charon->bus->set_sa(charon->bus, this->ike_sa); - return cancel; -} - /** * get the authentication class of a config */ @@ -681,8 +617,10 @@ static status_t build_r(private_ike_auth_t *this, message_t *message) return FAILED; } - if (check_uniqueness(this)) + if (charon->ike_sa_manager->check_uniqueness(charon->ike_sa_manager, + this->ike_sa)) { + DBG1(DBG_IKE, "cancelling IKE_SA setup due uniqueness policy"); message->add_notify(message, TRUE, AUTHENTICATION_FAILED, chunk_empty); return FAILED; }