handling of CHILD_SA rekeying collisions

diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c
index c40ec6e..9073bed 100644 (file)
--- a/src/charon/sa/task_manager.c
+++ b/src/charon/sa/task_manager.c
@@ -434,6 +434,56 @@ static status_t process_response(private_task_manager_t *this,
 }
 
 /**
+ * handle exchange collisions
+ */
+static void handle_collisions(private_task_manager_t *this, task_t *task)
+{
+       iterator_t *iterator;
+       task_t *active;
+       task_type_t type;
+       
+       type = task->get_type(task);
+       
+       /* check if we have initiated rekeying ourself */
+       if (this->initiating.type != CREATE_CHILD_SA ||
+               (type != IKE_REKEY && type != CHILD_REKEY && type != CHILD_DELETE))
+       {               
+               task->destroy(task);
+               return;
+       }
+    
+    /* find an exchange collision, and notify these tasks */
+    iterator = this->active_tasks->create_iterator(this->active_tasks, TRUE);
+    while (iterator->iterate(iterator, (void**)&active))
+    {
+       switch (active->get_type(active))
+       {
+               case IKE_REKEY:
+                       if (type == IKE_REKEY)
+                       {
+                               //ike_rekey_t *rekey = (ike_rekey_t*)active;
+                               //rekey->collide(rekey, (ike_rekey_t*)task);
+                               break;
+                       }
+                       continue;
+               case CHILD_REKEY:
+                       /* TODO: check if it is the SAME child we are talking about! */
+                       if (type == CHILD_REKEY || type == CHILD_DELETE)
+                       {
+                               child_rekey_t *rekey = (child_rekey_t*)active;
+                               rekey->collide(rekey, task);
+                               break;
+                       }
+                       continue;
+               default:
+                       continue;
+       }
+       break;
+    }
+    iterator->destroy(iterator);
+}
+
+/**
  * build a response depending on the "passive" task list
  */
 static status_t build_response(private_task_manager_t *this,
@@ -458,8 +508,7 @@ static status_t build_response(private_task_manager_t *this,
                case SUCCESS:
                    /* task completed, remove it */
                    iterator->remove(iterator);
-                   task->destroy(task);
-                   break;
+                               handle_collisions(this, task);
                case NEED_MORE:
                    /* processed, but task needs another exchange */
                    break;

diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c
index 9526146..7fa3c60 100644 (file)
--- a/src/charon/sa/tasks/child_create.c
+++ b/src/charon/sa/tasks/child_create.c
@@ -102,6 +102,11 @@ struct private_child_create_t {
         * CHILD_SA which gets established
         */
        child_sa_t *child_sa;
+       
+       /**
+        * successfully established the CHILD?
+        */
+       bool established;
 };
 
 /**
@@ -306,7 +311,7 @@ static status_t select_and_install(private_child_create_t *this)
        /* add to IKE_SA, and remove from task */
        this->child_sa->set_state(this->child_sa, CHILD_INSTALLED);
        this->ike_sa->add_child_sa(this->ike_sa, this->child_sa);
-       this->child_sa = NULL;
+       this->established = TRUE;
        return SUCCESS;
 }
 
@@ -594,7 +599,6 @@ static status_t process_i(private_child_create_t *this, message_t *message)
 {
        iterator_t *iterator;
        payload_t *payload;
-       status_t status;
 
        switch (message->get_exchange_type(message))
        {
@@ -648,10 +652,10 @@ static status_t process_i(private_child_create_t *this, message_t *message)
        
        process_payloads(this, message);
        
-       status = select_and_install(this);
-       
-       SIG(CHILD_UP_SUCCESS, "established CHILD_SA successfully");
-       
+       if (select_and_install(this) == SUCCESS)
+       {
+               SIG(CHILD_UP_SUCCESS, "established CHILD_SA successfully");
+       }
        return SUCCESS;
 }
 
@@ -672,6 +676,18 @@ static void use_reqid(private_child_create_t *this, u_int32_t reqid)
 }
 
 /**
+ * Implementation of child_create_t.get_child
+ */
+static child_sa_t* get_child(private_child_create_t *this)
+{
+       if (this->established)
+       {
+               return this->child_sa;
+       }
+       return NULL;
+}
+
+/**
  * Implementation of task_t.migrate
  */
 static void migrate(private_child_create_t *this, ike_sa_t *ike_sa)
@@ -700,6 +716,7 @@ static void migrate(private_child_create_t *this, ike_sa_t *ike_sa)
        this->child_sa = NULL;
        this->mode = MODE_TUNNEL;
        this->reqid = 0;
+       this->established = FALSE;
 }
 
 /**
@@ -717,7 +734,10 @@ static void destroy(private_child_create_t *this)
        {
                this->tsi->destroy_offset(this->tsi, offsetof(traffic_selector_t, destroy));
        }
-       DESTROY_IF(this->child_sa);
+       if (!this->established)
+       {
+               DESTROY_IF(this->child_sa);
+       }
        DESTROY_IF(this->proposal);
        if (this->proposals)
        {
@@ -735,6 +755,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.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;
@@ -764,6 +785,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa, policy_t *policy)
        this->child_sa = NULL;
        this->mode = MODE_TUNNEL;
        this->reqid = 0;
+       this->established = FALSE;
        
        return &this->public;
 }

diff --git a/src/charon/sa/tasks/child_create.h b/src/charon/sa/tasks/child_create.h
index 1644865..f3f1a63 100644 (file)
--- a/src/charon/sa/tasks/child_create.h
+++ b/src/charon/sa/tasks/child_create.h
@@ -58,6 +58,16 @@ struct child_create_t {
         * @param reqid         reqid to use
         */
        void (*use_reqid) (child_create_t *this, u_int32_t reqid);
+       
+       /**
+        * @brief Get the CHILD_SA established by this task.
+        *
+        * This call returns a child only when it has been established successfully.
+        *
+        * @param this          calling object
+        * @return                      child_sa
+        */
+       child_sa_t* (*get_child) (child_create_t *this);
 };
 
 /**

diff --git a/src/charon/sa/tasks/child_delete.c b/src/charon/sa/tasks/child_delete.c
index 4fb7214..30270f2 100644 (file)
--- a/src/charon/sa/tasks/child_delete.c
+++ b/src/charon/sa/tasks/child_delete.c
@@ -128,6 +128,8 @@ static void process_payloads(private_child_delete_t *this, message_t *message)
                                                 "but no such SA", protocol_id_names, protocol, ntohl(*spi));
                                        continue;
                                }
+                               DBG2(DBG_IKE, "received DELETE for %N CHILD_SA with SPI 0x%x", 
+                                               protocol_id_names, protocol, ntohl(*spi));
                                
                                switch (child_sa->get_state(child_sa))
                                {

diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c
index 660a9c1..fddfa59 100644 (file)
--- a/src/charon/sa/tasks/child_rekey.c
+++ b/src/charon/sa/tasks/child_rekey.c
@@ -63,46 +63,40 @@ struct private_child_rekey_t {
        child_sa_t *child_sa;
        
        /**
-        * redundandt CHILD_SA created simultaneously
+        * colliding task, may be delete or rekey
         */
-       child_sa_t *simultaneous;
+       task_t *collision;
        
        /**
         * the lowest nonce compared so far
         */
        chunk_t nonce;
-       
-       /**
-        * TRUE if we have the lower nonce
-        */
-       bool winner;
 };
 
 /**
- * get the nonce from a message, return TRUE if it was lower than this->nonce
+ * get the nonce from a message, IF it is lower than a previous one
  */
-static bool get_nonce(private_child_rekey_t *this, message_t *message)
+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 == NULL)
-       {
-               return FALSE;
-       }
-       nonce = payload->get_nonce(payload);
+       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);
-               return FALSE;
+               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;
+               }
        }
-       
-       chunk_free(&this->nonce);
-       this->nonce = nonce;
-       return TRUE;
 }
 
 /**
@@ -192,6 +186,7 @@ static status_t build_r(private_child_rekey_t *this, message_t *message)
        if (this->child_sa == NULL ||
                this->child_sa->get_state(this->child_sa) == CHILD_DELETING)
        {
+               DBG1(DBG_IKE, "unable to rekey, CHILD_SA not found");
                message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
                return SUCCESS;
        }
@@ -210,11 +205,6 @@ static status_t build_r(private_child_rekey_t *this, message_t *message)
        }
        
        get_nonce(this, message);
-
-       if (this->child_sa->get_state(this->child_sa) == CHILD_REKEYING)
-       {
-               /* TODO: handle simultaneous rekeying */
-       }
        
        this->child_sa->set_state(this->child_sa, CHILD_REKEYING);
        
@@ -228,19 +218,53 @@ static status_t process_i(private_child_rekey_t *this, message_t *message)
 {
        protocol_id_t protocol;
        u_int32_t spi;
+       child_sa_t *to_delete;
        
        this->child_create->task.process(&this->child_create->task, message);
        if (message->get_payload(message, SECURITY_ASSOCIATION) == NULL)
        {
-               /* establishing new child failed, fallback */
-               this->child_sa->set_state(this->child_sa, CHILD_INSTALLED);
-               /* TODO: rescedule rekeying */
+               /* establishing new child failed, reuse old. but not when we
+                * recieved a delete in the meantime */
+               if (!(this->collision && 
+                         this->collision->get_type(this->collision) == CHILD_DELETE))
+               {
+                       this->child_sa->set_state(this->child_sa, CHILD_INSTALLED);
+                       /* TODO: rescedule rekeying */
+               }
                return SUCCESS;
        }
        
-       /* TODO: delete the redundant CHILD_SA, instead of the rekeyed */
-       spi = this->child_sa->get_spi(this->child_sa, TRUE);
-       protocol = this->child_sa->get_protocol(this->child_sa);
+       get_nonce(this, message);
+       
+       to_delete = this->child_sa;
+       
+       /* check for rekey collisions */
+       if (this->collision &&
+               this->collision->get_type(this->collision) == CHILD_REKEY)
+       {
+               private_child_rekey_t *other = (private_child_rekey_t*)this->collision;
+               
+               /* 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, "CHILD_SA rekey collision won, deleting rekeyed child");
+               }
+               else
+               {
+                       DBG1(DBG_IKE, "CHILD_SA rekey collision lost, deleting redundant child");
+                       to_delete = this->child_create->get_child(this->child_create);
+                       if (to_delete == NULL)
+                       {
+                               /* ooops, should not happen, fallback */
+                               to_delete = this->child_sa;
+                       }
+               }
+       }
+       
+       spi = to_delete->get_spi(to_delete, TRUE);
+       protocol = to_delete->get_protocol(to_delete);
        if (this->ike_sa->delete_child_sa(this->ike_sa, protocol, spi) != SUCCESS)
        {
                return FAILED;
@@ -257,6 +281,15 @@ static task_type_t get_type(private_child_rekey_t *this)
 }
 
 /**
+ * Implementation of child_rekey_t.collide
+ */
+static void collide(private_child_rekey_t *this, task_t *other)
+{
+       DESTROY_IF(this->collision);
+       this->collision = other;
+}
+
+/**
  * Implementation of task_t.migrate
  */
 static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa)
@@ -265,8 +298,7 @@ static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa)
        chunk_free(&this->nonce);
 
        this->ike_sa = ike_sa;
-       this->winner = TRUE;
-       this->simultaneous = NULL;
+       this->collision = NULL;
 }
 
 /**
@@ -276,6 +308,7 @@ 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);
 }
 
@@ -287,6 +320,7 @@ child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
        private_child_rekey_t *this = malloc_thing(private_child_rekey_t);
        policy_t *policy;
 
+       this->public.collide = (void (*)(child_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;
@@ -309,8 +343,7 @@ 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->winner = TRUE;
-       this->simultaneous = NULL;
+       this->collision = NULL;
        
        return &this->public;
 }

diff --git a/src/charon/sa/tasks/child_rekey.h b/src/charon/sa/tasks/child_rekey.h
index 77297b1..3515f0c 100644 (file)
--- a/src/charon/sa/tasks/child_rekey.h
+++ b/src/charon/sa/tasks/child_rekey.h
@@ -44,6 +44,18 @@ struct child_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)(child_rekey_t* this, task_t *other);
 };
 
 /**