some task queueing improvements:

	- do not pass CHILD_SAs to task constructor, might not
	  be valid anymore during execution (late lookup)
	- use sub-tasks to delete CHILD/IKE_SA after rekeying,
	  as we want to execute the delete before additional
	  queued tasks

diff --git a/src/charon/sa/ike_sa.c b/src/charon/sa/ike_sa.c
index 3e13249..979fe19 100644 (file)
--- a/src/charon/sa/ike_sa.c
+++ b/src/charon/sa/ike_sa.c
@@ -1623,37 +1623,27 @@ static iterator_t* create_child_sa_iterator(private_ike_sa_t *this)
 /**
  * Implementation of ike_sa_t.rekey_child_sa.
  */
-static status_t rekey_child_sa(private_ike_sa_t *this, protocol_id_t protocol, u_int32_t spi)
+static status_t rekey_child_sa(private_ike_sa_t *this, protocol_id_t protocol,
+                                                          u_int32_t spi)
 {
-       child_sa_t *child_sa;
        child_rekey_t *child_rekey;
        
-       child_sa = get_child_sa(this, protocol, spi, TRUE);
-       if (child_sa)
-       {
-               child_rekey = child_rekey_create(&this->public, child_sa);
-               this->task_manager->queue_task(this->task_manager, &child_rekey->task);
-               return this->task_manager->initiate(this->task_manager);
-       }
-       return FAILED;
+       child_rekey = child_rekey_create(&this->public, protocol, spi);
+       this->task_manager->queue_task(this->task_manager, &child_rekey->task);
+       return this->task_manager->initiate(this->task_manager);
 }
 
 /**
  * Implementation of ike_sa_t.delete_child_sa.
  */
-static status_t delete_child_sa(private_ike_sa_t *this, protocol_id_t protocol, u_int32_t spi)
+static status_t delete_child_sa(private_ike_sa_t *this, protocol_id_t protocol,
+                                                               u_int32_t spi)
 {
-       child_sa_t *child_sa;
        child_delete_t *child_delete;
        
-       child_sa = get_child_sa(this, protocol, spi, TRUE);
-       if (child_sa)
-       {
-               child_delete = child_delete_create(&this->public, child_sa);
-               this->task_manager->queue_task(this->task_manager, &child_delete->task);
-               return this->task_manager->initiate(this->task_manager);
-       }
-       return FAILED;
+       child_delete = child_delete_create(&this->public, protocol, spi);
+       this->task_manager->queue_task(this->task_manager, &child_delete->task);
+       return this->task_manager->initiate(this->task_manager);
 }
 
 /**

diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c
index 135dd7f..211e64f 100644 (file)
--- a/src/charon/sa/task_manager.c
+++ b/src/charon/sa/task_manager.c
@@ -417,7 +417,7 @@ static status_t build_request(private_task_manager_t *this)
        message->set_exchange_type(message, exchange);
        this->initiating.type = exchange;
        this->initiating.retransmitted = 0;
-
+       
        iterator = this->active_tasks->create_iterator(this->active_tasks, TRUE);
        while (iterator->iterate(iterator, (void*)&task))
        {
@@ -442,6 +442,9 @@ static status_t build_request(private_task_manager_t *this)
        }
        iterator->destroy(iterator);
        
+       /* update exchange type if a task changed it */
+       this->initiating.type = message->get_exchange_type(message);
+       
        DESTROY_IF(this->initiating.packet);
        status = this->ike_sa->generate_message(this->ike_sa, message,
                                                                                        &this->initiating.packet);
@@ -495,7 +498,9 @@ static status_t process_response(private_task_manager_t *this,
                        case FAILED:
                        default:
                                /* critical failure, destroy IKE_SA */
+                               iterator->remove(iterator);
                                iterator->destroy(iterator);
+                               task->destroy(task);
                                return DESTROY_ME;
                }
                if (this->reset)
@@ -716,7 +721,8 @@ static status_t process_request(private_task_manager_t *this,
                        {
                                if (notify_found)
                                {
-                                       task = (task_t*)child_rekey_create(this->ike_sa, NULL);
+                                       task = (task_t*)child_rekey_create(this->ike_sa,
+                                                                                                          PROTO_NONE, 0);
                                }
                                else
                                {
@@ -773,7 +779,8 @@ static status_t process_request(private_task_manager_t *this,
                                                }
                                                else
                                                {
-                                                       task = (task_t*)child_delete_create(this->ike_sa, NULL);
+                                                       task = (task_t*)child_delete_create(this->ike_sa,
+                                                                                                                               PROTO_NONE, 0);
                                                }
                                                break;
                                        }
@@ -822,7 +829,9 @@ static status_t process_request(private_task_manager_t *this,
                        case FAILED:
                        default:
                                /* critical failure, destroy IKE_SA */
+                               iterator->remove(iterator);
                                iterator->destroy(iterator);
+                               task->destroy(task);
                                return DESTROY_ME;
                }
        }

diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c
index f1453f0..164bc07 100644 (file)
--- a/src/charon/sa/tasks/child_create.c
+++ b/src/charon/sa/tasks/child_create.c
@@ -1119,6 +1119,8 @@ static void migrate(private_child_create_t *this, ike_sa_t *ike_sa)
        }
        
        this->ike_sa = ike_sa;
+       this->keymat = ike_sa->get_keymat(ike_sa);
+       this->proposal = NULL;
        this->proposals = NULL;
        this->tsi = NULL;
        this->tsr = NULL;

diff --git a/src/charon/sa/tasks/child_delete.c b/src/charon/sa/tasks/child_delete.c
index 45c7775..36d260f 100644 (file)
--- a/src/charon/sa/tasks/child_delete.c
+++ b/src/charon/sa/tasks/child_delete.c
@@ -44,9 +44,19 @@ struct private_child_delete_t {
        bool initiator;
        
        /**
-        * wheter to enforce delete action policy
-        */
-       bool check_delete_action;
+        * Protocol of CHILD_SA to delete
+        */
+       protocol_id_t protocol;
+       
+       /**
+        * Inbound SPI of CHILD_SA to delete
+        */
+       u_int32_t spi;
+       
+       /**
+        * wheter to enforce delete action policy
+        */
+       bool check_delete_action;
        
        /**
         * CHILD_SAs which get deleted
@@ -238,6 +248,16 @@ static void log_children(private_child_delete_t *this)
  */
 static status_t build_i(private_child_delete_t *this, message_t *message)
 {
+       child_sa_t *child_sa;
+       
+       child_sa = this->ike_sa->get_child_sa(this->ike_sa, this->protocol,
+                                                                                 this->spi, TRUE);
+       if (!child_sa)
+       {       /* child does not exist anymore */
+               return SUCCESS;
+       }
+       this->child_sas->insert_last(this->child_sas, child_sa);
+       
        log_children(this);
        build_payloads(this, message);
        return NEED_MORE;
@@ -323,7 +343,8 @@ static void destroy(private_child_delete_t *this)
 /*
  * Described in header.
  */
-child_delete_t *child_delete_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
+child_delete_t *child_delete_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+                                                                       u_int32_t spi)
 {
        private_child_delete_t *this = malloc_thing(private_child_delete_t);
 
@@ -335,13 +356,14 @@ child_delete_t *child_delete_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
        this->ike_sa = ike_sa;
        this->check_delete_action = FALSE;
        this->child_sas = linked_list_create();
+       this->protocol = protocol;
+       this->spi = spi;
        
-       if (child_sa != NULL)
+       if (protocol != PROTO_NONE)
        {
                this->public.task.build = (status_t(*)(task_t*,message_t*))build_i;
                this->public.task.process = (status_t(*)(task_t*,message_t*))process_i;
                this->initiator = TRUE;
-               this->child_sas->insert_last(this->child_sas, child_sa);
        }
        else
        {

diff --git a/src/charon/sa/tasks/child_delete.h b/src/charon/sa/tasks/child_delete.h
index 1aa6099..9503ff0 100644 (file)
--- a/src/charon/sa/tasks/child_delete.h
+++ b/src/charon/sa/tasks/child_delete.h
@@ -52,9 +52,11 @@ struct child_delete_t {
  * Create a new child_delete task.
  *
  * @param ike_sa               IKE_SA this task works for
- * @param child_sa             CHILD_SA to delete, or NULL as responder
+ * @param protocol             protocol of CHILD_SA to delete, PROTO_NONE as responder
+ * @param spi                  inbound SPI of CHILD_SA to delete
  * @return                             child_delete task to handle by the task_manager
  */
-child_delete_t *child_delete_create(ike_sa_t *ike_sa, child_sa_t *child_sa);
+child_delete_t *child_delete_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+                                                                       u_int32_t spi);
 
 #endif /* CHILD_DELETE_H_ @} */

diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c
index 3a22d50..127280e 100644 (file)
--- a/src/charon/sa/tasks/child_rekey.c
+++ b/src/charon/sa/tasks/child_rekey.c
@@ -49,11 +49,26 @@ struct private_child_rekey_t {
        bool initiator;
        
        /**
+        * Protocol of CHILD_SA to rekey
+        */
+       protocol_id_t protocol;
+       
+       /**
+        * Inbound SPI of CHILD_SA to rekey
+        */
+       u_int32_t spi;
+       
+       /**
         * the CHILD_CREATE task which is reused to simplify rekeying
         */
        child_create_t *child_create;
        
        /**
+        * the CHILD_DELETE task to delete rekeyed CHILD_SA
+        */
+       child_delete_t *child_delete;
+       
+       /**
         * CHILD_SA which gets rekeyed
         */
        child_sa_t *child_sa;
@@ -65,6 +80,25 @@ struct private_child_rekey_t {
 };
 
 /**
+ * Implementation of task_t.build for initiator, after rekeying
+ */
+static status_t build_i_delete(private_child_rekey_t *this, message_t *message)
+{
+       /* update exchange type to INFORMATIONAL for the delete */
+       message->set_exchange_type(message, INFORMATIONAL);
+       
+       return this->child_delete->task.build(&this->child_delete->task, message);
+}
+
+/**
+ * Implementation of task_t.process for initiator, after rekeying
+ */
+static status_t process_i_delete(private_child_rekey_t *this, message_t *message)
+{
+       return this->child_delete->task.process(&this->child_delete->task, message);
+}
+
+/**
  * find a child using the REKEY_SA notify
  */
 static void find_child(private_child_rekey_t *this, message_t *message)
@@ -104,25 +138,33 @@ static void find_child(private_child_rekey_t *this, message_t *message)
  * Implementation of task_t.build for initiator
  */
 static status_t build_i(private_child_rekey_t *this, message_t *message)
-{      
+{
        notify_payload_t *notify;
-       protocol_id_t protocol;
-       u_int32_t spi, reqid;
+       u_int32_t reqid;
+       child_cfg_t *config;
+       
+       this->child_sa = this->ike_sa->get_child_sa(this->ike_sa, this->protocol,
+                                                                                               this->spi, TRUE);
+       if (!this->child_sa)
+       {       /* CHILD_SA is gone, unable to rekey */
+               return SUCCESS;
+       }
+       config = this->child_sa->get_config(this->child_sa);
        
        /* we just need the rekey notify ... */
-       protocol = this->child_sa->get_protocol(this->child_sa);
-       spi = this->child_sa->get_spi(this->child_sa, TRUE);
-       notify = notify_payload_create_from_protocol_and_type(protocol, REKEY_SA);
-       notify->set_spi(notify, spi);
+       notify = notify_payload_create_from_protocol_and_type(this->protocol,
+                                                                                                                 REKEY_SA);
+       notify->set_spi(notify, this->spi);
        message->add_payload(message, (payload_t*)notify);
-
+       
        /* ... our CHILD_CREATE task does the hard work for us. */
        reqid = this->child_sa->get_reqid(this->child_sa);
+       this->child_create = child_create_create(this->ike_sa, config);
        this->child_create->use_reqid(this->child_create, reqid);
        this->child_create->task.build(&this->child_create->task, message);
        
        this->child_sa->set_state(this->child_sa, CHILD_REKEYING);
-
+       
        return NEED_MORE;
 }
 
@@ -133,7 +175,7 @@ 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);
-
+       
        find_child(this, message);
        
        return NEED_MORE;
@@ -265,11 +307,13 @@ static status_t process_i(private_child_rekey_t *this, message_t *message)
        
        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;
-       }
-       return SUCCESS;
+       
+       /* rekeying done, delete the obsolete CHILD_SA using a subtask */
+       this->child_delete = child_delete_create(this->ike_sa, protocol, spi);
+       this->public.task.build = (status_t(*)(task_t*,message_t*))build_i_delete;
+       this->public.task.process = (status_t(*)(task_t*,message_t*))process_i_delete;
+       
+       return NEED_MORE;
 }
 
 /**
@@ -319,9 +363,16 @@ 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);
+       if (this->child_create)
+       {
+               this->child_create->task.migrate(&this->child_create->task, ike_sa);
+       }
+       if (this->child_delete)
+       {
+               this->child_delete->task.migrate(&this->child_delete->task, ike_sa);
+       }
        DESTROY_IF(this->collision);
-
+       
        this->ike_sa = ike_sa;
        this->collision = NULL;
 }
@@ -331,7 +382,14 @@ 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);
+       if (this->child_create)
+       {
+               this->child_create->task.destroy(&this->child_create->task);
+       }
+       if (this->child_delete)
+       {
+               this->child_delete->task.destroy(&this->child_delete->task);
+       }
        DESTROY_IF(this->collision);
        free(this);
 }
@@ -339,22 +397,21 @@ static void destroy(private_child_rekey_t *this)
 /*
  * Described in header.
  */
-child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
+child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+                                                                 u_int32_t spi)
 {
-       child_cfg_t *config;
        private_child_rekey_t *this = malloc_thing(private_child_rekey_t);
-
+       
        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;
-       if (child_sa != NULL)
+       if (protocol != PROTO_NONE)
        {
                this->public.task.build = (status_t(*)(task_t*,message_t*))build_i;
                this->public.task.process = (status_t(*)(task_t*,message_t*))process_i;
                this->initiator = TRUE;
-               config = child_sa->get_config(child_sa);
-               this->child_create = child_create_create(ike_sa, config);
+               this->child_create = NULL;
        }
        else
        {
@@ -365,8 +422,11 @@ 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->child_sa = NULL;
+       this->protocol = protocol;
+       this->spi = spi;
        this->collision = NULL;
+       this->child_delete = NULL;
        
        return &this->public;
 }

diff --git a/src/charon/sa/tasks/child_rekey.h b/src/charon/sa/tasks/child_rekey.h
index 382cf4a..b0c4cec 100644 (file)
--- a/src/charon/sa/tasks/child_rekey.h
+++ b/src/charon/sa/tasks/child_rekey.h
@@ -56,9 +56,11 @@ struct child_rekey_t {
  * Create a new CHILD_REKEY task.
  *
  * @param ike_sa               IKE_SA this task works for
- * @param child_sa             child_sa to rekey, NULL if responder
+ * @param protocol             protocol of CHILD_SA to rekey, PROTO_NONE as responder
+ * @param spi                  inbound SPI of CHILD_SA to rekey
  * @return                             child_rekey task to handle by the task_manager
  */
-child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa);
+child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, protocol_id_t protocol,
+                                                                 u_int32_t spi);
 
 #endif /* CHILD_REKEY_H_ @} */

diff --git a/src/charon/sa/tasks/ike_rekey.c b/src/charon/sa/tasks/ike_rekey.c
index d094a04..6dbad5f 100644 (file)
--- a/src/charon/sa/tasks/ike_rekey.c
+++ b/src/charon/sa/tasks/ike_rekey.c
@@ -21,6 +21,7 @@
 #include <daemon.h>
 #include <encoding/payloads/notify_payload.h>
 #include <sa/tasks/ike_init.h>
+#include <sa/tasks/ike_delete.h>
 #include <processing/jobs/delete_ike_sa_job.h>
 #include <processing/jobs/rekey_ike_sa_job.h>
 
@@ -58,12 +59,36 @@ struct private_ike_rekey_t {
        ike_init_t *ike_init;
        
        /**
+        * IKE_DELETE task to delete the old IKE_SA after rekeying was successful
+        */
+       ike_delete_t *ike_delete;
+       
+       /**
         * colliding task detected by the task manager
         */
        task_t *collision;
 };
 
 /**
+ * Implementation of task_t.build for initiator, after rekeying
+ */
+static status_t build_i_delete(private_ike_rekey_t *this, message_t *message)
+{
+       /* update exchange type to INFORMATIONAL for the delete */
+       message->set_exchange_type(message, INFORMATIONAL);
+       
+       return this->ike_delete->task.build(&this->ike_delete->task, message);
+}
+
+/**
+ * Implementation of task_t.process for initiator, after rekeying
+ */
+static status_t process_i_delete(private_ike_rekey_t *this, message_t *message)
+{
+       return this->ike_delete->task.process(&this->ike_delete->task, message);
+}
+
+/**
  * Implementation of task_t.build for initiator
  */
 static status_t build_i(private_ike_rekey_t *this, message_t *message)
@@ -168,7 +193,6 @@ 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;
        iterator_t *iterator;
        payload_t *payload;
@@ -271,10 +295,12 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message)
                charon->bus->set_sa(charon->bus, this->ike_sa);
        }
        
-       job = (job_t*)delete_ike_sa_job_create(to_delete, TRUE);
-       charon->processor->queue_job(charon->processor, job);   
+       /* rekeying successful, delete the IKE_SA using a subtask */
+       this->ike_delete = ike_delete_create(this->ike_sa, TRUE);
+       this->public.task.build = (status_t(*)(task_t*,message_t*))build_i_delete;
+       this->public.task.process = (status_t(*)(task_t*,message_t*))process_i_delete;
        
-       return SUCCESS;
+       return NEED_MORE;
 }
 
 /**
@@ -300,6 +326,10 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
        {
                this->ike_init->task.destroy(&this->ike_init->task);
        }
+       if (this->ike_delete)
+       {
+               this->ike_delete->task.destroy(&this->ike_delete->task);
+       }
        if (this->new_sa)
        {
                charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
@@ -308,11 +338,12 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa)
                charon->bus->set_sa(charon->bus, this->ike_sa);
        }
        DESTROY_IF(this->collision);
-
+       
        this->collision = NULL;
        this->ike_sa = ike_sa;
        this->new_sa = NULL;
        this->ike_init = NULL;
+       this->ike_delete = NULL;
 }
 
 /**
@@ -339,6 +370,10 @@ static void destroy(private_ike_rekey_t *this)
        {
                this->ike_init->task.destroy(&this->ike_init->task);
        }
+       if (this->ike_delete)
+       {
+               this->ike_delete->task.destroy(&this->ike_delete->task);
+       }
        DESTROY_IF(this->collision);
        free(this);
 }
@@ -368,6 +403,7 @@ ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator)
        this->ike_sa = ike_sa;
        this->new_sa = NULL;
        this->ike_init = NULL;
+       this->ike_delete = NULL;
        this->initiator = initiator;
        this->collision = NULL;