improved IKE_SA uniqueness check
authorTobias Brunner <tobias@strongswan.org>
Tue, 16 Dec 2008 17:21:28 +0000 (17:21 -0000)
committerTobias Brunner <tobias@strongswan.org>
Tue, 16 Dec 2008 17:21:28 +0000 (17:21 -0000)
src/charon/sa/ike_sa.h
src/charon/sa/ike_sa_manager.c
src/charon/sa/ike_sa_manager.h
src/charon/sa/tasks/ike_auth.c

index c792ed2..f9c2647 100644 (file)
@@ -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);
        
index fc23bdd..826a637 100644 (file)
@@ -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**)&current, 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;
index 4edf801..b5c479e 100644 (file)
@@ -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.
index 16fcb4a..2e40534 100644 (file)
@@ -88,70 +88,6 @@ struct private_ike_auth_t {
 };
 
 /**
- * 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
  */
 auth_class_t get_auth_class(peer_cfg_t *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;
        }