fixed child rekey collision

implemented ike rekey collision

diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c
index 9073bed7a5c5f83de6f5094dbf5000892a1f4616..d63c614c0ce858ed66b45744034efc5445ca45dc 100644 (file)
--- a/src/charon/sa/task_manager.c
+++ b/src/charon/sa/task_manager.c
@@ -459,10 +459,10 @@ static void handle_collisions(private_task_manager_t *this, task_t *task)
        switch (active->get_type(active))
        {
                case IKE_REKEY:
-                       if (type == IKE_REKEY)
+                       if (type == IKE_REKEY || type == IKE_DELETE)
                        {
-                               //ike_rekey_t *rekey = (ike_rekey_t*)active;
-                               //rekey->collide(rekey, (ike_rekey_t*)task);
+                               ike_rekey_t *rekey = (ike_rekey_t*)active;
+                               rekey->collide(rekey, task);
                                break;
                        }
                        continue;

diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c
index 7fa3c6081505f845194dcc7c0ba5e0e3a1f3e682..60172cf202bbb12ea0e8ea3a981f18d9ced8029e 100644 (file)
--- a/src/charon/sa/tasks/child_create.c
+++ b/src/charon/sa/tasks/child_create.c
@@ -687,6 +687,22 @@ static child_sa_t* get_child(private_child_create_t *this)
        return NULL;
 }
 
+/**
+ * Implementation of child_create_t.get_lower_nonce
+ */
+static chunk_t get_lower_nonce(private_child_create_t *this)
+{      
+       if (memcmp(this->my_nonce.ptr, this->other_nonce.ptr,
+                          min(this->my_nonce.len, this->other_nonce.len)) < 0)
+       {
+               return this->my_nonce;
+       }
+       else
+       {
+               return this->other_nonce;
+       }
+}
+
 /**
  * Implementation of task_t.migrate
  */
@@ -756,6 +772,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa, policy_t *policy)
        private_child_create_t *this = malloc_thing(private_child_create_t);
 
        this->public.get_child = (child_sa_t*(*)(child_create_t*))get_child;
+       this->public.get_lower_nonce = (chunk_t(*)(child_create_t*))get_lower_nonce;
        this->public.use_reqid = (void(*)(child_create_t*,u_int32_t))use_reqid;
        this->public.task.get_type = (task_type_t(*)(task_t*))get_type;
        this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate;

diff --git a/src/charon/sa/tasks/child_create.h b/src/charon/sa/tasks/child_create.h
index f3f1a633f00a56fe360ffa040596a572c55a970b..7ff2fb22d7c1f148ba0808efde77aa34e6463a27 100644 (file)
--- a/src/charon/sa/tasks/child_create.h
+++ b/src/charon/sa/tasks/child_create.h
@@ -59,6 +59,14 @@ struct child_create_t {
         */
        void (*use_reqid) (child_create_t *this, u_int32_t reqid);
        
+       /**
+        * @brief Get the lower of the two nonces, used for rekey collisions.
+        *
+        * @param this          calling object
+        * @return                      lower nonce
+        */
+       chunk_t (*get_lower_nonce) (child_create_t *this);
+       
        /**
         * @brief Get the CHILD_SA established by this task.
         *

diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c
index fddfa598acb0d5b85de56960ea176136e73ade4d..3fb4a54f6ce87f28fe3334f576f5255a8d4a6ca8 100644 (file)
--- a/src/charon/sa/tasks/child_rekey.c
+++ b/src/charon/sa/tasks/child_rekey.c
@@ -24,9 +24,7 @@
 #include "child_rekey.h"
 
 #include <daemon.h>
-#include <crypto/diffie_hellman.h>
 #include <encoding/payloads/notify_payload.h>
-#include <encoding/payloads/nonce_payload.h>
 #include <sa/tasks/child_create.h>
 
 
@@ -66,39 +64,8 @@ struct private_child_rekey_t {
         * colliding task, may be delete or rekey
         */
        task_t *collision;
-       
-       /**
-        * the lowest nonce compared so far
-        */
-       chunk_t nonce;
 };
 
-/**
- * get the nonce from a message, IF it is lower than a previous one
- */
-static void get_nonce(private_child_rekey_t *this, message_t *message)
-{
-       nonce_payload_t *payload;
-       chunk_t nonce;
-       
-       payload = (nonce_payload_t*)message->get_payload(message, NONCE);
-       if (payload)
-       {       
-               nonce = payload->get_nonce(payload);
-       
-               if (this->nonce.ptr && memcmp(nonce.ptr, this->nonce.ptr,
-                                                                         min(nonce.len, this->nonce.len)) > 0)
-               {
-                       chunk_free(&nonce);
-               }
-               else
-               {               
-                       chunk_free(&this->nonce);
-                       this->nonce = nonce;
-               }
-       }
-}
-
 /**
  * find a child using the REKEY_SA notify
  */
@@ -155,7 +122,6 @@ static status_t build_i(private_child_rekey_t *this, message_t *message)
        reqid = this->child_sa->get_reqid(this->child_sa);
        this->child_create->use_reqid(this->child_create, reqid);
        this->child_create->task.build(&this->child_create->task, message);
-       get_nonce(this, message);
        
        this->child_sa->set_state(this->child_sa, CHILD_REKEYING);
 
@@ -169,7 +135,6 @@ static status_t process_r(private_child_rekey_t *this, message_t *message)
 {
        /* let the CHILD_CREATE task process the message */
        this->child_create->task.process(&this->child_create->task, message);
-       get_nonce(this, message);
 
        find_child(this, message);
        
@@ -204,8 +169,6 @@ static status_t build_r(private_child_rekey_t *this, message_t *message)
                return SUCCESS;
        }
        
-       get_nonce(this, message);
-       
        this->child_sa->set_state(this->child_sa, CHILD_REKEYING);
        
        return SUCCESS;
@@ -234,20 +197,22 @@ static status_t process_i(private_child_rekey_t *this, message_t *message)
                return SUCCESS;
        }
        
-       get_nonce(this, message);
-       
        to_delete = this->child_sa;
        
        /* check for rekey collisions */
        if (this->collision &&
                this->collision->get_type(this->collision) == CHILD_REKEY)
        {
+               chunk_t this_nonce, other_nonce;
                private_child_rekey_t *other = (private_child_rekey_t*)this->collision;
                
+               this_nonce = this->child_create->get_lower_nonce(this->child_create);
+               other_nonce = other->child_create->get_lower_nonce(other->child_create);
+               
                /* if we have the lower nonce, delete rekeyed SA. If not, delete
                 * the redundant. */
-               if (memcmp(this->nonce.ptr, other->nonce.ptr, 
-                                  min(this->nonce.len, other->nonce.len)) < 0)
+               if (memcmp(this_nonce.ptr, other_nonce.ptr, 
+                                  min(this_nonce.len, other_nonce.len)) < 0)
                {
                        DBG1(DBG_IKE, "CHILD_SA rekey collision won, deleting rekeyed child");
                }
@@ -295,7 +260,7 @@ static void collide(private_child_rekey_t *this, task_t *other)
 static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa)
 {      
        this->child_create->task.migrate(&this->child_create->task, ike_sa);
-       chunk_free(&this->nonce);
+       DESTROY_IF(this->collision);
 
        this->ike_sa = ike_sa;
        this->collision = NULL;
@@ -307,7 +272,6 @@ static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa)
 static void destroy(private_child_rekey_t *this)
 {
        this->child_create->task.destroy(&this->child_create->task);
-       chunk_free(&this->nonce);
        DESTROY_IF(this->collision);
        free(this);
 }
@@ -342,7 +306,6 @@ child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
        
        this->ike_sa = ike_sa;
        this->child_sa = child_sa;
-       this->nonce = chunk_empty;
        this->collision = NULL;
        
        return &this->public;

diff --git a/src/charon/sa/tasks/ike_init.c b/src/charon/sa/tasks/ike_init.c
index d49209aaa2aa10f62f880eeb7144b1f1caf15204..5b5ef35126a98c34de7636ea04907a8ae29197a1 100644 (file)
--- a/src/charon/sa/tasks/ike_init.c
+++ b/src/charon/sa/tasks/ike_init.c
@@ -503,6 +503,22 @@ static task_type_t get_type(private_ike_init_t *this)
        return IKE_INIT;
 }
 
+/**
+ * Implementation of task_t.get_type
+ */
+static chunk_t get_lower_nonce(private_ike_init_t *this)
+{
+       if (memcmp(this->my_nonce.ptr, this->other_nonce.ptr,
+                          min(this->my_nonce.len, this->other_nonce.len)) < 0)
+       {
+               return this->my_nonce;
+       }
+       else
+       {
+               return this->other_nonce;
+       }
+}
+
 /**
  * Implementation of task_t.migrate
  */
@@ -537,6 +553,7 @@ ike_init_t *ike_init_create(ike_sa_t *ike_sa, bool initiator, ike_sa_t *old_sa)
 {
        private_ike_init_t *this = malloc_thing(private_ike_init_t);
 
+       this->public.get_lower_nonce = (chunk_t(*)(ike_init_t*))get_lower_nonce;
        this->public.task.get_type = (task_type_t(*)(task_t*))get_type;
        this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate;
        this->public.task.destroy = (void(*)(task_t*))destroy;

diff --git a/src/charon/sa/tasks/ike_init.h b/src/charon/sa/tasks/ike_init.h
index 290cc46e8d45233dbbe6528291390a3301250399..f60c096e8a2a568e39ead88067d10d1865aa387d 100644 (file)
--- a/src/charon/sa/tasks/ike_init.h
+++ b/src/charon/sa/tasks/ike_init.h
@@ -32,7 +32,7 @@ typedef struct ike_init_t ike_init_t;
 /**
  * @brief Task of type IKE_INIT, creates an IKE_SA without authentication.
  *
- * The authentication of is handle in the ike_auth and/or ike_auth_eap task.
+ * The authentication of is handle in the ike_auth task.
  *
  * @b Constructors:
  *  - ike_init_create()
@@ -45,6 +45,14 @@ struct ike_init_t {
         * Implements the task_t interface
         */
        task_t task;
+       
+       /**
+        * @brief Get the lower of the two nonces, used for rekey collisions.
+        *
+        * @param this          calling object
+        * @return                      lower nonce
+        */
+       chunk_t (*get_lower_nonce) (ike_init_t *this);
 };
 
 /**

diff --git a/src/charon/sa/tasks/ike_rekey.c b/src/charon/sa/tasks/ike_rekey.c
index 01698b11ead8f7887c0f3158f5a0e496e9f36dcd..879a3ee62e1bba1d55ac6f3e6a0fe9c2cbb9dc90 100644 (file)
--- a/src/charon/sa/tasks/ike_rekey.c
+++ b/src/charon/sa/tasks/ike_rekey.c
@@ -24,9 +24,7 @@
 #include "ike_rekey.h"
 
 #include <daemon.h>
-#include <crypto/diffie_hellman.h>
 #include <encoding/payloads/notify_payload.h>
-#include <encoding/payloads/nonce_payload.h>
 #include <sa/tasks/ike_init.h>
 #include <queues/jobs/delete_ike_sa_job.h>
 
@@ -62,6 +60,11 @@ struct private_ike_rekey_t {
         * the IKE_INIT task which is reused to simplify rekeying
         */
        ike_init_t *ike_init;
+       
+       /**
+        * colliding task detected by the task manager
+        */
+       task_t *collision;
 };
 
 /**
@@ -158,10 +161,7 @@ static status_t build_r(private_ike_rekey_t *this, message_t *message)
        }
        
        this->ike_sa->set_state(this->ike_sa, IKE_REKEYING);
-       this->new_sa->inherit(this->new_sa, this->ike_sa);
        this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
-       charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
-       this->new_sa = NULL;
        
        return SUCCESS;
 }
@@ -172,23 +172,62 @@ static status_t build_r(private_ike_rekey_t *this, message_t *message)
 static status_t process_i(private_ike_rekey_t *this, message_t *message)
 {
        job_t *job;
+       ike_sa_id_t *to_delete;
 
        if (this->ike_init->task.process(&this->ike_init->task, message) == FAILED)
        {
                /* rekeying failed, fallback to old SA */
-               this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
-               /* TODO: reschedule rekeying */
+               if (!(this->collision &&
+                       this->collision->get_type(this->collision) == IKE_DELETE))
+               {
+                       this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
+                       /* TODO: reschedule rekeying */
+               }
                return SUCCESS;
        }
-       
+
        this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
-       this->new_sa->inherit(this->new_sa, this->ike_sa);
-       charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
-       this->new_sa = NULL;
+       to_delete = this->ike_sa->get_id(this->ike_sa);
+       
+       /* check for collisions */
+       if (this->collision &&
+               this->collision->get_type(this->collision) == IKE_REKEY)
+       {
+               chunk_t this_nonce, other_nonce;
+               host_t *host;
+               private_ike_rekey_t *other = (private_ike_rekey_t*)this->collision;
+               
+               this_nonce = this->ike_init->get_lower_nonce(this->ike_init);
+               other_nonce = other->ike_init->get_lower_nonce(other->ike_init);
+               
+               /* if we have the lower nonce, delete rekeyed SA. If not, delete
+                * the redundant. */
+               if (memcmp(this_nonce.ptr, other_nonce.ptr, 
+                                  min(this_nonce.len, other_nonce.len)) < 0)
+               {
+                       DBG1(DBG_IKE, "IKE_SA rekey collision won, deleting rekeyed IKE_SA");
+                       charon->ike_sa_manager->checkin(charon->ike_sa_manager, other->new_sa);
+               }
+               else
+               {
+                       DBG1(DBG_IKE, "IKE_SA rekey collision lost, deleting redundant IKE_SA");
+                       /* apply host for a proper delete */
+                       host = this->ike_sa->get_my_host(this->ike_sa);
+                       this->new_sa->set_my_host(this->new_sa, host->clone(host));
+                       host = this->ike_sa->get_other_host(this->ike_sa);
+                       this->new_sa->set_other_host(this->new_sa, host->clone(host));
+                       this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
+                       to_delete = this->new_sa->get_id(this->new_sa);
+                       charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
+                       /* inherit to other->new_sa in destroy() */
+                       this->new_sa = other->new_sa;
+                       other->new_sa = NULL;
+               }
+       }
+       
+       job = (job_t*)delete_ike_sa_job_create(to_delete, TRUE);
+       charon->job_queue->add(charon->job_queue, job); 
        
-       job = (job_t*)delete_ike_sa_job_create(this->ike_sa->get_id(this->ike_sa),
-                                                                                  TRUE);
-       charon->job_queue->add(charon->job_queue, job);
        return SUCCESS;
 }
 
@@ -200,6 +239,12 @@ static task_type_t get_type(private_ike_rekey_t *this)
        return IKE_REKEY;
 }
 
+static void collide(private_ike_rekey_t* this, task_t *other)
+{
+       DESTROY_IF(this->collision);
+       this->collision = other;
+}
+
 /**
  * Implementation of task_t.migrate
  */
@@ -214,7 +259,9 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
                charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
                                                                                                        this->new_sa);
        }
+       DESTROY_IF(this->collision);
 
+       this->collision = NULL;
        this->ike_sa = ike_sa;
        this->new_sa = NULL;
        this->ike_init = NULL;
@@ -225,15 +272,24 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
  */
 static void destroy(private_ike_rekey_t *this)
 {
-       if (this->ike_init)
+       if (this->new_sa)
        {
-               this->ike_init->task.destroy(&this->ike_init->task);
+               if (this->new_sa->get_state(this->new_sa) == IKE_ESTABLISHED)
+               {
+                       this->new_sa->inherit(this->new_sa, this->ike_sa);
+                       charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
+               }
+               else
+               {
+                       charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
+                                                                                                               this->new_sa);
+               }
        }
-       if (this->new_sa)
+       if (this->ike_init)
        {
-               charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
-                                                                                                       this->new_sa);
+               this->ike_init->task.destroy(&this->ike_init->task);
        }
+       DESTROY_IF(this->collision);
        free(this);
 }
 
@@ -244,6 +300,7 @@ ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator)
 {
        private_ike_rekey_t *this = malloc_thing(private_ike_rekey_t);
 
+       this->public.collide = (void(*)(ike_rekey_t*,task_t*))collide;
        this->public.task.get_type = (task_type_t(*)(task_t*))get_type;
        this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate;
        this->public.task.destroy = (void(*)(task_t*))destroy;
@@ -262,6 +319,7 @@ ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator)
        this->new_sa = NULL;
        this->ike_init = NULL;
        this->initiator = initiator;
+       this->collision = NULL;
        
        return &this->public;
 }

diff --git a/src/charon/sa/tasks/ike_rekey.h b/src/charon/sa/tasks/ike_rekey.h
index f1caf57586abf7a7c75d06097109730bd7f4dc8d..125422efdaaa60fe801f21f855664521e732a847 100644 (file)
--- a/src/charon/sa/tasks/ike_rekey.h
+++ b/src/charon/sa/tasks/ike_rekey.h
@@ -43,6 +43,18 @@ struct ike_rekey_t {
         * Implements the task_t interface
         */
        task_t task;
+       
+       /**
+        * @brief Register a rekeying task which collides with this one.
+        *
+        * If two peers initiate rekeying at the same time, the collision must
+        * be handled gracefully. The task manager is aware of what exchanges
+        * are going on and notifies the outgoing task by passing the incoming.
+        *
+        * @param this          task initated by us
+        * @param other         incoming task
+        */
+       void (*collide)(ike_rekey_t* this, task_t *other);
 };
 
 /**