ikev2: Let ike/child-rekey tasks indicate if the passive task was adopted

This gives us more flexibility with tasks that return NEED_MORE (currently
none of the colliding tasks do, but that will change with multi-KE
rekeyings).  The active task has to check itself if the passive task is
done and should be removed from the task manager.

diff --git a/src/libcharon/sa/ikev2/task_manager_v2.c b/src/libcharon/sa/ikev2/task_manager_v2.c
index 2fd7a48bb90056f40f8128bf0f4e379a11555a9b..62707cb4653a35fb13db37602524cfb22f1a84a4 100644 (file)
--- a/src/libcharon/sa/ikev2/task_manager_v2.c
+++ b/src/libcharon/sa/ikev2/task_manager_v2.c
@@ -872,13 +872,15 @@ static status_t process_response(private_task_manager_t *this,
 }
 
 /**
- * handle exchange collisions
+ * Handle exchange collisions, returns TRUE if the given passive task was
+ * adopted by the active task and the task manager lost control over it.
  */
 static bool handle_collisions(private_task_manager_t *this, task_t *task)
 {
        enumerator_t *enumerator;
        task_t *active;
        task_type_t type;
+       bool adopted = FALSE;
 
        type = task->get_type(task);
 
@@ -896,7 +898,7 @@ static bool handle_collisions(private_task_manager_t *this, task_t *task)
                                        if (type == TASK_IKE_REKEY || type == TASK_IKE_DELETE)
                                        {
                                                ike_rekey_t *rekey = (ike_rekey_t*)active;
-                                               rekey->collide(rekey, task);
+                                               adopted = rekey->collide(rekey, task);
                                                break;
                                        }
                                        continue;
@@ -904,7 +906,7 @@ static bool handle_collisions(private_task_manager_t *this, task_t *task)
                                        if (type == TASK_CHILD_REKEY || type == TASK_CHILD_DELETE)
                                        {
                                                child_rekey_t *rekey = (child_rekey_t*)active;
-                                               rekey->collide(rekey, task);
+                                               adopted = rekey->collide(rekey, task);
                                                break;
                                        }
                                        continue;
@@ -912,11 +914,11 @@ static bool handle_collisions(private_task_manager_t *this, task_t *task)
                                        continue;
                        }
                        enumerator->destroy(enumerator);
-                       return TRUE;
+                       return adopted;
                }
                enumerator->destroy(enumerator);
        }
-       return FALSE;
+       return adopted;
 }
 
 /**

diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c
index 1448189f1c118a96e54fde99463ec07a211cf373..9281c6339db87684c8f60ea3cf0ac03dd48c5703 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_rekey.c
+++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c
@@ -528,7 +528,7 @@ METHOD(child_rekey_t, is_redundant, bool,
        return FALSE;
 }
 
-METHOD(child_rekey_t, collide, void,
+METHOD(child_rekey_t, collide, bool,
        private_child_rekey_t *this, task_t *other)
 {
        /* the task manager only detects exchange collision, but not if
@@ -540,16 +540,14 @@ METHOD(child_rekey_t, collide, void,
 
                if (rekey->child_sa != this->child_sa)
                {       /* not the same child => no collision */
-                       other->destroy(other);
-                       return;
+                       return FALSE;
                }
                /* ignore passive tasks that did not successfully create a CHILD_SA */
                other_child = rekey->child_create->get_child(rekey->child_create);
                if (!other_child ||
                         other_child->get_state(other_child) != CHILD_INSTALLED)
                {
-                       other->destroy(other);
-                       return;
+                       return FALSE;
                }
        }
        else if (other->get_type(other) == TASK_CHILD_DELETE)
@@ -558,26 +556,24 @@ METHOD(child_rekey_t, collide, void,
                if (is_redundant(this, del->get_child(del)))
                {
                        this->other_child_destroyed = TRUE;
-                       other->destroy(other);
-                       return;
+                       return FALSE;
                }
                if (del->get_child(del) != this->child_sa)
                {
                        /* not the same child => no collision */
-                       other->destroy(other);
-                       return;
+                       return FALSE;
                }
        }
        else
        {
-               /* any other task is not critical for collisions, ignore */
-               other->destroy(other);
-               return;
+               /* shouldn't happen */
+               return FALSE;
        }
        DBG1(DBG_IKE, "detected %N collision with %N", task_type_names,
                 TASK_CHILD_REKEY, task_type_names, other->get_type(other));
        DESTROY_IF(this->collision);
        this->collision = other;
+       return TRUE;
 }
 
 METHOD(task_t, migrate, void,

diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.h b/src/libcharon/sa/ikev2/tasks/child_rekey.h
index 4b584274e072f64805ce5b9b40fda0189ca1e5c0..849df3a2b86753eb90b3d66764d471594ced940b 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_rekey.h
+++ b/src/libcharon/sa/ikev2/tasks/child_rekey.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Tobias Brunner
+ * Copyright (C) 2016-2020 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -59,9 +59,11 @@ struct child_rekey_t {
         * be handled gracefully. The task manager is aware of what exchanges
         * are going on and notifies the active task by passing the passive.
         *
-        * @param other         passive task (adopted)
+        * @param other         passive task
+        * @return                      whether the task was adopted and should be removed from
+        *                                      the task manager's control
         */
-       void (*collide)(child_rekey_t* this, task_t *other);
+       bool (*collide)(child_rekey_t* this, task_t *other);
 };
 
 /**

diff --git a/src/libcharon/sa/ikev2/tasks/ike_rekey.c b/src/libcharon/sa/ikev2/tasks/ike_rekey.c
index f539ae21989f0dc556a51ead6b3d36917c574d89..aac908ee6bf4827b297696fb378004f582355551 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_rekey.c
+++ b/src/libcharon/sa/ikev2/tasks/ike_rekey.c
@@ -430,7 +430,7 @@ METHOD(ike_rekey_t, did_collide, bool,
        return this->collision != NULL;
 }
 
-METHOD(ike_rekey_t, collide, void,
+METHOD(ike_rekey_t, collide, bool,
        private_ike_rekey_t* this, task_t *other)
 {
        DBG1(DBG_IKE, "detected %N collision with %N", task_type_names,
@@ -440,7 +440,6 @@ METHOD(ike_rekey_t, collide, void,
        {
                case TASK_IKE_DELETE:
                        conclude_undetected_collision(this);
-                       other->destroy(other);
                        break;
                case TASK_IKE_REKEY:
                {
@@ -450,17 +449,17 @@ METHOD(ike_rekey_t, collide, void,
                        {
                                DBG1(DBG_IKE, "colliding exchange did not result in an IKE_SA, "
                                         "ignore");
-                               other->destroy(other);
                                break;
                        }
                        DESTROY_IF(&this->collision->public.task);
                        this->collision = rekey;
-                       break;
+                       return TRUE;
                }
                default:
                        /* shouldn't happen */
                        break;
        }
+       return FALSE;
 }
 
 /**

diff --git a/src/libcharon/sa/ikev2/tasks/ike_rekey.h b/src/libcharon/sa/ikev2/tasks/ike_rekey.h
index 30f9919db33278b96ecd7fd999a13f3bf29d4533..5fab3491c1e21abe49f7cb5cfd2ee8f67c12e2b7 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_rekey.h
+++ b/src/libcharon/sa/ikev2/tasks/ike_rekey.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Tobias Brunner
+ * Copyright (C) 2016-2020 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -47,15 +47,17 @@ struct ike_rekey_t {
        bool (*did_collide)(ike_rekey_t *this);
 
        /**
-        * Register a rekeying task which collides with this one.
+        * Register a rekeying/delete 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.
+        * are going on and notifies the active task by passing the passive.
         *
-        * @param other         incoming task
+        * @param other         passive task
+        * @return                      whether the task was adopted and should be removed from
+        *                                      the task manager's control
         */
-       void (*collide)(ike_rekey_t* this, task_t *other);
+       bool (*collide)(ike_rekey_t* this, task_t *other);
 };
 
 /**