added IKE_SA_INIT retransmission detection
authorMartin Willi <martin@strongswan.org>
Thu, 29 Mar 2007 14:20:10 +0000 (14:20 -0000)
committerMartin Willi <martin@strongswan.org>
Thu, 29 Mar 2007 14:20:10 +0000 (14:20 -0000)
fixed thread exhaustion when IKE_SA is blocked for a longer time

src/charon/queues/jobs/process_message_job.c
src/charon/sa/ike_sa.c
src/charon/sa/ike_sa_manager.c
src/charon/sa/ike_sa_manager.h
src/charon/sa/tasks/ike_rekey.c

index 8d2a97e..6541a1e 100644 (file)
@@ -57,12 +57,9 @@ static job_type_t get_type(private_process_message_job_t *this)
 static status_t execute(private_process_message_job_t *this)
 {
        ike_sa_t *ike_sa;
-       ike_sa_id_t *ike_sa_id;
        
-       ike_sa_id = this->message->get_ike_sa_id(this->message);
-       ike_sa_id = ike_sa_id->clone(ike_sa_id);
-       ike_sa_id->switch_initiator(ike_sa_id);
-       ike_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, ike_sa_id);
+       ike_sa = charon->ike_sa_manager->checkout_by_message(charon->ike_sa_manager,
+                                                                                                                this->message);
        if (ike_sa)
        {
                DBG1(DBG_NET, "received packet: from %#H to %#H",
@@ -80,10 +77,10 @@ static status_t execute(private_process_message_job_t *this)
        }
        else
        {
-               DBG1(DBG_NET, "received packet from %#H for IKE_SA: %J, but no such "
-                        "IKE_SA", this->message->get_source(this->message), ike_sa_id);
+               DBG1(DBG_NET, "unable to handle message from %#H for IKE_SA: %J",
+                        this->message->get_source(this->message),
+                        this->message->get_ike_sa_id(this->message));
        }
-       ike_sa_id->destroy(ike_sa_id);
        return DESTROY_ME;
 }
 
index 39a28c1..68aba30 100644 (file)
@@ -1088,14 +1088,11 @@ static status_t retransmit(private_ike_sa_t *this, u_int32_t message_id)
                /* create a new IKE_SA if we have to route or to restart */
                if (to_route->get_count(to_route) || to_restart->get_count(to_restart))
                {
-                       ike_sa_id_t *other_id;
                        private_ike_sa_t *new;
                        task_t *task;
                        
-                       other_id =  ike_sa_id_create(0, 0, TRUE);
-                       new = (private_ike_sa_t*)charon->ike_sa_manager->checkout(
-                                                                                       charon->ike_sa_manager, other_id);
-                       other_id->destroy(other_id);
+                       new = (private_ike_sa_t*)charon->ike_sa_manager->checkout_new(
+                                                                                               charon->ike_sa_manager, TRUE);
                        
                        apply_config(new, this->connection, this->policy);
                        /* use actual used host, not the wildcarded one in connection */
@@ -1542,7 +1539,6 @@ static status_t rekey(private_ike_sa_t *this)
  */
 static void reestablish(private_ike_sa_t *this)
 {
-       ike_sa_id_t *other_id;
        private_ike_sa_t *other;
        iterator_t *iterator;
        child_sa_t *child_sa;
@@ -1550,10 +1546,8 @@ static void reestablish(private_ike_sa_t *this)
        task_t *task;
        job_t *job;
        
-       other_id =  ike_sa_id_create(0, 0, TRUE);
-       other = (private_ike_sa_t*)charon->ike_sa_manager->checkout(
-                                                                                       charon->ike_sa_manager, other_id);
-       other_id->destroy(other_id);
+       other = (private_ike_sa_t*)charon->ike_sa_manager->checkout_new(
+                                                                                       charon->ike_sa_manager, TRUE);
        
        apply_config(other, this->connection, this->policy);
        other->other_host->destroy(other->other_host);
index b4a7324..bbd7335 100644 (file)
@@ -72,6 +72,16 @@ struct entry_t {
         * The contained ike_sa_t object.
         */
        ike_sa_t *ike_sa;
+       
+       /**
+        * hash of the IKE_SA_INIT message, used to detect retransmissions
+        */
+       chunk_t init_hash;
+       
+       /**
+        * message ID currently processing, if any
+        */
+       u_int32_t message_id;
 };
 
 /**
@@ -82,6 +92,7 @@ static status_t entry_destroy(entry_t *this)
        /* also destroy IKE SA */
        this->ike_sa->destroy(this->ike_sa);
        this->ike_sa_id->destroy(this->ike_sa_id);
+       chunk_free(&this->init_hash);
        free(this);
        return SUCCESS;
 }
@@ -94,12 +105,14 @@ static entry_t *entry_create(ike_sa_id_t *ike_sa_id)
        entry_t *this = malloc_thing(entry_t);
        
        this->waiting_threads = 0;
-       pthread_cond_init(&(this->condvar), NULL);
+       pthread_cond_init(&this->condvar, NULL);
        
        /* we set checkout flag when we really give it out */
        this->checked_out = FALSE;
        this->driveout_new_threads = FALSE;
        this->driveout_waiting_threads = FALSE;
+       this->message_id = -1;
+       this->init_hash = chunk_empty;
        
        /* ike_sa_id is always cloned */
        this->ike_sa_id = ike_sa_id->clone(ike_sa_id);
@@ -136,6 +149,11 @@ struct private_ike_sa_manager_t {
          * A randomizer, to get random SPIs for our side
          */
         randomizer_t *randomizer;
+        
+        /**
+         * SHA1 hasher for IKE_SA_INIT retransmit detection
+         */
+        hasher_t *hasher;
 };
 
 /**
@@ -269,7 +287,7 @@ static bool wait_for_entry(private_ike_sa_manager_t *this, entry_t *entry)
        while (entry->checked_out && !entry->driveout_waiting_threads)  
        {
                /* so wait until we can get it for us.
-               * we register us as waiting. */
+                * we register us as waiting. */
                entry->waiting_threads++;
                pthread_cond_wait(&(entry->condvar), &(this->mutex));
                entry->waiting_threads--;
@@ -291,8 +309,8 @@ static u_int64_t get_next_spi(private_ike_sa_manager_t *this)
 {
        u_int64_t spi;
        
-       this->randomizer->get_pseudo_random_bytes(this->randomizer, 8, (u_int8_t*)&spi);
-       
+       this->randomizer->get_pseudo_random_bytes(this->randomizer, sizeof(spi),
+                                                                                         (u_int8_t*)&spi);
        return spi;
 }
 
@@ -301,119 +319,156 @@ static u_int64_t get_next_spi(private_ike_sa_manager_t *this)
  */
 static ike_sa_t* checkout(private_ike_sa_manager_t *this, ike_sa_id_t *ike_sa_id)
 {
-       bool responder_spi_set;
-       bool initiator_spi_set;
-       bool original_initiator;
        ike_sa_t *ike_sa = NULL;
+       entry_t *entry;
        
-       DBG2(DBG_MGR, "checkout IKE_SA: %J", ike_sa_id);
-       
-       DBG2(DBG_MGR,  "%d IKE_SAs in manager",
-                this->ike_sa_list->get_count(this->ike_sa_list));
+       DBG2(DBG_MGR, "checkout IKE_SA: %J, %d IKE_SAs in manager",
+                ike_sa_id, this->ike_sa_list->get_count(this->ike_sa_list));
        
-       /* each access is locked */
        pthread_mutex_lock(&(this->mutex));
+       if (get_entry_by_id(this, ike_sa_id, &entry) == SUCCESS)
+       {
+               if (wait_for_entry(this, entry))
+               {
+                       DBG2(DBG_MGR, "IKE_SA successfully checked out");
+                       entry->checked_out = TRUE;
+                       ike_sa = entry->ike_sa;
+               }
+       }
+       pthread_mutex_unlock(&this->mutex);
+       charon->bus->set_sa(charon->bus, ike_sa);
+       return ike_sa;
+}
+
+/**
+ * Implementation of of ike_sa_manager.checkout_new.
+ */
+static ike_sa_t *checkout_new(private_ike_sa_manager_t* this, bool initiator)
+{
+       entry_t *entry;
+       ike_sa_id_t *id;
        
-       responder_spi_set = ike_sa_id->get_responder_spi(ike_sa_id);
-       initiator_spi_set = ike_sa_id->get_initiator_spi(ike_sa_id);
-       original_initiator = ike_sa_id->is_initiator(ike_sa_id);
+       if (initiator)
+       {
+               id = ike_sa_id_create(get_next_spi(this), 0, TRUE);
+       }
+       else
+       {
+               id = ike_sa_id_create(0, get_next_spi(this), FALSE);
+       }
+       entry = entry_create(id);       
+       pthread_mutex_lock(&this->mutex);       
+       this->ike_sa_list->insert_last(this->ike_sa_list, entry);
+       entry->checked_out = TRUE;
+       pthread_mutex_unlock(&this->mutex);     
+       DBG2(DBG_MGR, "created IKE_SA: %J, %d IKE_SAs in manager",
+                id, this->ike_sa_list->get_count(this->ike_sa_list));
+       return entry->ike_sa;
+}
+
+/**
+ * Implementation of of ike_sa_manager.checkout_by_id.
+ */
+static ike_sa_t* checkout_by_message(private_ike_sa_manager_t* this,
+                                                                        message_t *message)
+{
+       entry_t *entry;
+       ike_sa_t *ike_sa = NULL;
+       ike_sa_id_t *id = message->get_ike_sa_id(message);
+       id = id->clone(id);
+       id->switch_initiator(id);
+       
+       DBG2(DBG_MGR, "checkout IKE_SA: %J by message, %d IKE_SAs in manager",
+                id, this->ike_sa_list->get_count(this->ike_sa_list));
        
-       if ((initiator_spi_set && responder_spi_set) ||
-               ((initiator_spi_set && !responder_spi_set) && (original_initiator)))
+       if (message->get_request(message) &&
+               message->get_exchange_type(message) == IKE_SA_INIT)
        {
-               /* we SHOULD have an IKE_SA for these SPIs in the list,
-                * if not, we can't handle the request...
-                */
-               entry_t *entry;
-               /* look for the entry */
-               if (get_entry_by_id(this, ike_sa_id, &entry) == SUCCESS)
+               /* IKE_SA_INIT request. Check for an IKE_SA with such a message hash. */
+               iterator_t *iterator;
+               chunk_t data, hash;
+               bool occupied = FALSE;
+               
+               data = message->get_packet_data(message);
+               this->hasher->allocate_hash(this->hasher, data, &hash);
+               chunk_free(&data);
+               
+               pthread_mutex_lock(&this->mutex);
+               iterator = this->ike_sa_list->create_iterator(this->ike_sa_list, TRUE);
+               while (iterator->iterate(iterator, (void**)&entry))
                {
-                       if (wait_for_entry(this, entry))
+                       if (chunk_equals(hash, entry->init_hash))
                        {
-                               DBG2(DBG_MGR, "IKE_SA successfully checked out");
-                               /* ok, this IKE_SA is finally ours */
-                               entry->checked_out = TRUE;
-                               ike_sa = entry->ike_sa;
-                               /* update responder SPI when it's not set */
-                               if (entry->ike_sa_id->get_responder_spi(entry->ike_sa_id) == 0)
+                               if (entry->message_id == 0)
                                {
-                                       ike_sa_id_t *ike_sa_ike_sa_id = ike_sa->get_id(ike_sa);
-                                       u_int64_t spi = ike_sa_id->get_responder_spi(ike_sa_id);
-                                       
-                                       ike_sa_ike_sa_id->set_responder_spi(ike_sa_ike_sa_id, spi);
-                                       entry->ike_sa_id->set_responder_spi(entry->ike_sa_id, spi);
+                                       occupied = TRUE;
                                }
-                       }
-                       else
-                       {
-                               DBG2(DBG_MGR, "IKE_SA found, but not allowed to check it out");
+                               else if (wait_for_entry(this, entry))
+                               {
+                                       DBG2(DBG_MGR, "IKE_SA checked out by hash");
+                                       entry->checked_out = TRUE;
+                                       entry->message_id = message->get_message_id(message);
+                                       ike_sa = entry->ike_sa;
+                               }
+                               break;
                        }
                }
-               else
+               iterator->destroy(iterator);
+               pthread_mutex_unlock(&this->mutex);
+               if (occupied)
                {
-                       DBG2(DBG_MGR, "IKE_SA not stored in list");
-                       /* looks like there is no such IKE_SA, better luck next time... */
+                       /* already processing this message ID, discard */
+                       chunk_free(&hash);
+                       id->destroy(id);
+                       return NULL;
                }
-       }
-       else if ((initiator_spi_set && !responder_spi_set) && (!original_initiator))
-       {
-               /* an IKE_SA_INIT from an another endpoint,
-                * he is the initiator.
-                * For simplicity, we do NOT check for retransmitted
-                * IKE_SA_INIT-Requests here, so EVERY single IKE_SA_INIT-
-                * Request (even a retransmitted one) will result in a
-                * IKE_SA. This could be improved...
-                */
-               u_int64_t responder_spi;
-               entry_t *new_entry;
-               
-               /* set SPIs, we are the responder */
-               responder_spi = get_next_spi(this);
-               
-               /* we also set arguments spi, so its still valid */
-               ike_sa_id->set_responder_spi(ike_sa_id, responder_spi);
-               
-               /* create entry */
-               new_entry = entry_create(ike_sa_id);
-               
-               this->ike_sa_list->insert_last(this->ike_sa_list, new_entry);
-               
-               /* check ike_sa out */
-               DBG2(DBG_MGR,  "IKE_SA added to list of known IKE_SAs");
-               new_entry->checked_out = TRUE;
-               ike_sa = new_entry->ike_sa;
-       }
-       else if (!initiator_spi_set && !responder_spi_set)
-       {
-               /* checkout of a new and unused IKE_SA, used for rekeying */
-               entry_t *new_entry;
-               
-               if (original_initiator)
+               if (ike_sa == NULL)
                {
-                       ike_sa_id->set_initiator_spi(ike_sa_id, get_next_spi(this));
+                       /* no IKE_SA found, create a new one */
+                       id->set_responder_spi(id, get_next_spi(this));
+                       entry = entry_create(id);
+                       
+                       pthread_mutex_lock(&this->mutex);
+                       this->ike_sa_list->insert_last(this->ike_sa_list, entry);
+                       entry->checked_out = TRUE;
+                       entry->message_id = message->get_message_id(message);
+                       pthread_mutex_unlock(&this->mutex);
+                       entry->init_hash = hash;
+                       ike_sa = entry->ike_sa;
                }
                else
                {
-                       ike_sa_id->set_responder_spi(ike_sa_id, get_next_spi(this));
+                       chunk_free(&hash);
                }
-               /* create entry */
-               new_entry = entry_create(ike_sa_id);
-               DBG2(DBG_MGR, "created IKE_SA: %J", ike_sa_id);
-                       
-               this->ike_sa_list->insert_last(this->ike_sa_list, new_entry);
-               
-               /* check ike_sa out */
-               new_entry->checked_out = TRUE;
-               ike_sa = new_entry->ike_sa;
+               id->destroy(id);
+               charon->bus->set_sa(charon->bus, ike_sa);
+               return ike_sa;
        }
-       else
+       
+       pthread_mutex_lock(&(this->mutex));
+       if (get_entry_by_id(this, id, &entry) == SUCCESS)
        {
-               /* responder set, initiator not: here is something seriously wrong! */
-               DBG2(DBG_MGR, "invalid IKE_SA SPIs");
+               /* only check out if we are not processing this request */
+               if (message->get_request(message) &&
+                       message->get_message_id(message) == entry->message_id)
+               {
+                       DBG2(DBG_MGR, "not checking out, message already processing");
+               }
+               else if (wait_for_entry(this, entry))
+               {
+                       ike_sa_id_t *ike_id = entry->ike_sa->get_id(entry->ike_sa);
+                       DBG2(DBG_MGR, "IKE_SA successfully checked out");
+                       entry->checked_out = TRUE;
+                       entry->message_id = message->get_message_id(message);
+                       if (ike_id->get_responder_spi(ike_id) == 0)
+                       {
+                               ike_id->set_responder_spi(ike_id, id->get_responder_spi(id));
+                       }
+                       ike_sa = entry->ike_sa;
+               }
        }
-       
-       pthread_mutex_unlock(&(this->mutex));
-       
+       pthread_mutex_unlock(&this->mutex);
+       id->destroy(id);
        charon->bus->set_sa(charon->bus, ike_sa);
        return ike_sa;
 }
@@ -669,6 +724,7 @@ static status_t checkin(private_ike_sa_manager_t *this, ike_sa_t *ike_sa)
                entry->ike_sa_id->replace_values(entry->ike_sa_id, ike_sa->get_id(ike_sa));
                /* signal waiting threads */
                entry->checked_out = FALSE;
+               entry->message_id = -1;
                DBG2(DBG_MGR, "check-in of IKE_SA successful.");
                pthread_cond_signal(&(entry->condvar));
                retval = SUCCESS;
@@ -816,6 +872,7 @@ static void destroy(private_ike_sa_manager_t *this)
        pthread_mutex_unlock(&(this->mutex));
        
        this->randomizer->destroy(this->randomizer);
+       this->hasher->destroy(this->hasher);
        
        free(this);
 }
@@ -830,6 +887,8 @@ ike_sa_manager_t *ike_sa_manager_create()
        /* assign public functions */
        this->public.destroy = (void(*)(ike_sa_manager_t*))destroy;
        this->public.checkout = (ike_sa_t*(*)(ike_sa_manager_t*, ike_sa_id_t*))checkout;
+       this->public.checkout_new = (ike_sa_t*(*)(ike_sa_manager_t*,bool))checkout_new;
+       this->public.checkout_by_message = (ike_sa_t*(*)(ike_sa_manager_t*,message_t*))checkout_by_message;
        this->public.checkout_by_peer = (ike_sa_t*(*)(ike_sa_manager_t*,host_t*,host_t*,identification_t*,identification_t*))checkout_by_peer;
        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;
@@ -842,6 +901,7 @@ ike_sa_manager_t *ike_sa_manager_create()
        this->ike_sa_list = linked_list_create();
        pthread_mutex_init(&this->mutex, NULL);
        this->randomizer = randomizer_create();
+       this->hasher = hasher_create(HASH_SHA1);
        
        return &this->public;
 }
index 5fcead4..1125e5d 100644 (file)
@@ -28,6 +28,7 @@ typedef struct ike_sa_manager_t ike_sa_manager_t;
 
 #include <library.h>
 #include <sa/ike_sa.h>
+#include <encoding/message.h>
 
 /**
  * @brief The IKE_SA-Manager is responsible for managing all initiated and responded IKE_SA's.
@@ -47,26 +48,52 @@ typedef struct ike_sa_manager_t ike_sa_manager_t;
  * @ingroup sa
  */
 struct ike_sa_manager_t {
+       
        /**
-        * @brief Checkout an IKE_SA, create it when necesarry.
-        * 
-        * Checks out a SA by its ID. An SA will be created, when the responder
-        * SPI is not set (when received an IKE_SA_INIT from initiator).
-        * Management of SPIs is the managers job, he will set it.
-        * This function blocks until SA is available for checkout.
-        * 
-        * @warning checking out two times without checking in will
-        * result in a deadlock!
+        * @brief Checkout an existing IKE_SA.
         * 
         * @param this                          the manager object
         * @param ike_sa_id                     the SA identifier, will be updated
         * @returns                                     
         *                                                      - checked out IKE_SA if found
-        *                                                      - NULL, if no such IKE_SA available
+        *                                                      - NULL, if specified IKE_SA is not found.
         */
        ike_sa_t* (*checkout) (ike_sa_manager_t* this, ike_sa_id_t *sa_id);
        
        /**
+        * @brief Create and check out a new IKE_SA.
+        * 
+        * @param this                          the manager object
+        * @param initiator                     TRUE for initiator, FALSE otherwise
+        * @returns                             created andchecked out IKE_SA
+        */
+       ike_sa_t* (*checkout_new) (ike_sa_manager_t* this, bool initiator);
+       
+       /**
+        * @brief Checkout an IKE_SA by a message.
+        * 
+        * In some situations, it is necessary that the manager knows the
+        * message to use for the checkout. This has the folloing reasons:
+        * 
+        * 1. If the targeted IKE_SA is already processing a message, we do not
+        *    check it out if the message ID is the same.
+        * 2. If it is an IKE_SA_INIT request, we have to check if it is a 
+        *    retransmission. If so, we have to drop the message, we would
+        *    create another unneded IKE_SA for each retransmitted packet.
+        *
+        * A call to checkout_by_message() returns a (maybe new created) IKE_SA.
+        * If processing the message does not make sense (for the reasons above),
+        * NULL is returned.
+        * 
+        * @param this                          the manager object
+        * @param ike_sa_id                     the SA identifier, will be updated
+        * @returns                                     
+        *                                                      - checked out/created IKE_SA
+        *                                                      - NULL to not process message further
+        */
+       ike_sa_t* (*checkout_by_message) (ike_sa_manager_t* this, message_t *message);
+       
+       /**
         * @brief Checkout an existing IKE_SA by hosts and identifications.
         *
         * Allows the lookup of an IKE_SA by user IDs and hosts. It returns the
index 7a6b353..a33e7ee 100644 (file)
@@ -75,11 +75,9 @@ static status_t build_i(private_ike_rekey_t *this, message_t *message)
 {
        connection_t *connection;
        policy_t *policy;
-       ike_sa_id_t *id;
        
-       id = ike_sa_id_create(0, 0, TRUE);
-       this->new_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, id);
-       id->destroy(id);
+       this->new_sa = charon->ike_sa_manager->checkout_new(charon->ike_sa_manager,
+                                                                                                               TRUE);
        
        connection = this->ike_sa->get_connection(this->ike_sa);
        policy = this->ike_sa->get_policy(this->ike_sa);
@@ -101,7 +99,6 @@ static status_t process_r(private_ike_rekey_t *this, message_t *message)
 {
        connection_t *connection;
        policy_t *policy;
-       ike_sa_id_t *id;
        iterator_t *iterator;
        child_sa_t *child_sa;
        
@@ -129,9 +126,8 @@ static status_t process_r(private_ike_rekey_t *this, message_t *message)
        }
        iterator->destroy(iterator);
        
-       id = ike_sa_id_create(0, 0, FALSE);
-       this->new_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, id);
-       id->destroy(id);
+       this->new_sa = charon->ike_sa_manager->checkout_new(charon->ike_sa_manager,
+                                                                                                               FALSE);
        
        connection = this->ike_sa->get_connection(this->ike_sa);
        policy = this->ike_sa->get_policy(this->ike_sa);