child-create: Use more generic method to pass information from previous SA

Besides the previous key exchange method, this will allow us to also
reuse the previous traffic selectors.  Some data is still passed in
separate methods as some are set even when there is no previous SA and
others are not set in all cases.

The interface for queue_child() now optionally takes the previous
Child SA to handle both recreations and initiations from scratch.

diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 77e5721825bfc70b2069505ea54ff24e3966a0cd..3cfcdcf332d3611ad8dd6eed9c5dca39761860e7 100644 (file)
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -1628,7 +1628,7 @@ METHOD(ike_sa_t, initiate, status_t,
        if (child_cfg)
        {
                /* normal IKE_SA with CHILD_SA */
-               this->task_manager->queue_child(this->task_manager, child_cfg, args);
+               this->task_manager->queue_child(this->task_manager, child_cfg, args, NULL);
 #ifdef ME
                if (this->peer_cfg->get_mediated_by(this->peer_cfg))
                {
@@ -2119,20 +2119,12 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new,
                }
                if (action & ACTION_START)
                {
-                       child_init_args_t args = {
-                               .reqid = child_sa->get_reqid_ref(child_sa),
-                               .label = child_sa->get_label(child_sa),
-                       };
                        child_cfg = child_sa->get_config(child_sa);
                        DBG1(DBG_IKE, "restarting CHILD_SA %s",
                                 child_cfg->get_name(child_cfg));
                        other->task_manager->queue_child(other->task_manager,
                                                                                         child_cfg->get_ref(child_cfg),
-                                                                                        &args);
-                       if (args.reqid)
-                       {
-                               charon->kernel->release_reqid(charon->kernel, args.reqid);
-                       }
+                                                                                        NULL, child_sa);
                }
        }
        enumerator->destroy(enumerator);

diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c
index 672a04927d8a8cd728aff3a7ebf035d83bdf3743..5afc66ae8181715e33d0f015a6498ebdf7744aed 100644 (file)
--- a/src/libcharon/sa/ikev1/task_manager_v1.c
+++ b/src/libcharon/sa/ikev1/task_manager_v1.c
@@ -1691,11 +1691,23 @@ METHOD(task_manager_t, queue_mobike, void,
 }
 
 METHOD(task_manager_t, queue_child, void,
-       private_task_manager_t *this, child_cfg_t *cfg, child_init_args_t *args)
+       private_task_manager_t *this, child_cfg_t *cfg, child_init_args_t *args,
+       child_sa_t *child_sa)
 {
        quick_mode_t *task;
+       uint32_t reqid;
 
-       if (args)
+       if (child_sa)
+       {
+               task = quick_mode_create(this->ike_sa, cfg, NULL, NULL, 0);
+               reqid = child_sa->get_reqid_ref(child_sa);
+               if (reqid)
+               {
+                       task->use_reqid(task, reqid);
+                       charon->kernel->release_reqid(charon->kernel, reqid);
+               }
+       }
+       else if (args)
        {
                task = quick_mode_create(this->ike_sa, cfg, args->src, args->dst,
                                                                 args->seq);

diff --git a/src/libcharon/sa/ikev2/task_manager_v2.c b/src/libcharon/sa/ikev2/task_manager_v2.c
index 2180bd9099d4c3b7000fdb8b3e66d7c32b6a6a88..d46559182f8481574254350a6223c21a1639d3bf 100644 (file)
--- a/src/libcharon/sa/ikev2/task_manager_v2.c
+++ b/src/libcharon/sa/ikev2/task_manager_v2.c
@@ -2197,6 +2197,7 @@ static void trigger_mbb_reauth(private_task_manager_t *this)
                cfg = child_sa->get_config(child_sa);
                child_create = child_create_create(new, cfg->get_ref(cfg),
                                                                                   FALSE, NULL, NULL, 0);
+               child_create->recreate_sa(child_create, child_sa);
                reqid = child_sa->get_reqid_ref(child_sa);
                if (reqid)
                {
@@ -2369,11 +2370,25 @@ METHOD(task_manager_t, queue_dpd, void,
 }
 
 METHOD(task_manager_t, queue_child, void,
-       private_task_manager_t *this, child_cfg_t *cfg, child_init_args_t *args)
+       private_task_manager_t *this, child_cfg_t *cfg, child_init_args_t *args,
+       child_sa_t *child_sa)
 {
        child_create_t *task;
+       uint32_t reqid;
 
-       if (args)
+       if (child_sa)
+       {
+               task = child_create_create(this->ike_sa, cfg, FALSE, NULL, NULL, 0);
+               task->recreate_sa(task, child_sa);
+               reqid = child_sa->get_reqid_ref(child_sa);
+               if (reqid)
+               {
+                       task->use_reqid(task, reqid);
+                       charon->kernel->release_reqid(charon->kernel, reqid);
+               }
+               task->use_label(task, child_sa->get_label(child_sa));
+       }
+       else if (args)
        {
                task = child_create_create(this->ike_sa, cfg, FALSE, args->src,
                                                                   args->dst, args->seq);

diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c
index be4662f1c0d91b2c68e5f8cffe90aa2eb2bb496e..3020967fd6dc9c58d40f0775c89fa20b62b018ec 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_create.c
+++ b/src/libcharon/sa/ikev2/tasks/child_create.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2024 Tobias Brunner
+ * Copyright (C) 2008-2025 Tobias Brunner
  * Copyright (C) 2005-2008 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  *
@@ -1574,7 +1574,11 @@ METHOD(task_t, build_i, status_t,
                return FAILED;
        }
 
-       if (!no_ke && !this->retry)
+       if (no_ke)
+       {       /* we might have one set if we are recreating this SA */
+               this->ke_method = KE_NONE;
+       }
+       else if (!this->retry)
        {       /* during a rekeying the method might already be set */
                if (this->ke_method == KE_NONE)
                {
@@ -2080,7 +2084,7 @@ METHOD(task_t, build_r, status_t,
                return SUCCESS;
        }
 
-       /* check if ike_config_t included non-critical error notifies */
+       /* check if ike_config_t task included non-critical error notifies */
        enumerator = message->create_payload_enumerator(message);
        while (enumerator->enumerate(enumerator, &payload))
        {
@@ -2540,10 +2544,22 @@ METHOD(child_create_t, use_label, void,
        this->child.label = label ? label->clone(label) : NULL;
 }
 
-METHOD(child_create_t, use_ke_method, void,
-       private_child_create_t *this, key_exchange_method_t ke_method)
+METHOD(child_create_t, recreate_sa, void,
+       private_child_create_t *this, child_sa_t *old)
 {
-       this->ke_method = ke_method;
+       if (this->initiator)
+       {
+               proposal_t *proposal;
+               uint16_t ke_method;
+
+               proposal = old->get_proposal(old);
+               if (proposal->get_algorithm(proposal, KEY_EXCHANGE_METHOD,
+                                                                       &ke_method, NULL))
+               {
+                       /* reuse the KE method negotiated previously */
+                       this->ke_method = ke_method;
+               }
+       }
 }
 
 METHOD(child_create_t, get_child, child_sa_t*,
@@ -2719,7 +2735,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa,
                        .use_marks = _use_marks,
                        .use_if_ids = _use_if_ids,
                        .use_label = _use_label,
-                       .use_ke_method = _use_ke_method,
+                       .recreate_sa = _recreate_sa,
                        .abort = _abort_,
                        .task = {
                                .get_type = _get_type,

diff --git a/src/libcharon/sa/ikev2/tasks/child_create.h b/src/libcharon/sa/ikev2/tasks/child_create.h
index f3ff0cc7d6f4e666806de70062503e20e53f931a..0c0e6137e7ae609107b4ea52d3d45d7f29e5ae4e 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_create.h
+++ b/src/libcharon/sa/ikev2/tasks/child_create.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2024 Tobias Brunner
+ * Copyright (C) 2018-2025 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -81,13 +81,12 @@ struct child_create_t {
        void (*use_label)(child_create_t *this, sec_label_t *label);
 
        /**
-        * Initially propose a specific KE method to override configuration.
+        * Use data from the given old SA (e.g. KE method and traffic selectors)
+        * when rekeying/recreating it.
         *
-        * This is used during rekeying to prefer the previously negotiated method.
-        *
-        * @param ke_method     KE method to use
+        * @param old                   old CHILD_SA that is getting rekeyed/recreated
         */
-       void (*use_ke_method)(child_create_t *this, key_exchange_method_t ke_method);
+       void (*recreate_sa)(child_create_t *this, child_sa_t *old);
 
        /**
         * Get the lower of the two nonces, used for rekey collisions.

diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c
index 3282a21f7a1b3c8ffd5c9fddc96f316cb9bb2f76..7987bdd534e56d742317020297da363296704eff 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_delete.c
+++ b/src/libcharon/sa/ikev2/tasks/child_delete.c
@@ -151,6 +151,29 @@ static void conclude_rekeying(private_child_delete_t *this, child_sa_t *old)
        child_rekey_conclude_rekeying(old, child_sa);
 }
 
+/**
+ * Queue a task to recreate the given CHILD_SA.
+ */
+static void queue_child_create(ike_sa_t *ike_sa, child_sa_t *child_sa)
+{
+       child_create_t *child_create;
+       child_cfg_t *child_cfg;
+       uint32_t reqid;
+
+       child_cfg = child_sa->get_config(child_sa);
+       child_create = child_create_create(ike_sa, child_cfg->get_ref(child_cfg),
+                                                                          FALSE, NULL, NULL, 0);
+       child_create->recreate_sa(child_create, child_sa);
+       reqid = child_sa->get_reqid_ref(child_sa);
+       if (reqid)
+       {
+               child_create->use_reqid(child_create, reqid);
+               charon->kernel->release_reqid(charon->kernel, reqid);
+       }
+       child_create->use_label(child_create, child_sa->get_label(child_sa));
+       ike_sa->queue_task(ike_sa, (task_t*)child_create);
+}
+
 /**
  * Destroy and optionally reestablish the given CHILD_SA according to config.
  */
@@ -160,12 +183,9 @@ static status_t destroy_and_reestablish_internal(ike_sa_t *ike_sa,
                                                                                                 bool delete_action,
                                                                                                 action_t forced_action)
 {
-       child_init_args_t args = {};
        child_cfg_t *child_cfg;
-       protocol_id_t protocol;
-       uint32_t spi;
        action_t action;
-       status_t status = SUCCESS;
+       bool initiate = FALSE;
 
        child_sa->set_state(child_sa, CHILD_DELETED);
        if (trigger_updown)
@@ -173,44 +193,29 @@ static status_t destroy_and_reestablish_internal(ike_sa_t *ike_sa,
                charon->bus->child_updown(charon->bus, child_sa, FALSE);
        }
 
-       protocol = child_sa->get_protocol(child_sa);
-       spi = child_sa->get_spi(child_sa, TRUE);
-       child_cfg = child_sa->get_config(child_sa);
-       child_cfg->get_ref(child_cfg);
-       args.reqid = child_sa->get_reqid_ref(child_sa);
-       args.label = child_sa->get_label(child_sa);
-       if (args.label)
-       {
-               args.label = args.label->clone(args.label);
-       }
-       action = forced_action ?: child_sa->get_close_action(child_sa);
-
        DBG1(DBG_IKE, "CHILD_SA %s{%u} closed", child_sa->get_name(child_sa),
                 child_sa->get_unique_id(child_sa));
 
-       ike_sa->destroy_child_sa(ike_sa, protocol, spi);
+       action = forced_action ?: child_sa->get_close_action(child_sa);
 
        if (delete_action)
        {
                if (action & ACTION_TRAP)
                {
+                       child_cfg = child_sa->get_config(child_sa);
                        charon->traps->install(charon->traps,
                                                                   ike_sa->get_peer_cfg(ike_sa),
-                                                                  child_cfg);
+                                                                  child_cfg->get_ref(child_cfg));
                }
                if (action & ACTION_START)
                {
-                       child_cfg->get_ref(child_cfg);
-                       status = ike_sa->initiate(ike_sa, child_cfg, &args);
+                       queue_child_create(ike_sa, child_sa);
+                       initiate = TRUE;
                }
        }
-       child_cfg->destroy(child_cfg);
-       if (args.reqid)
-       {
-               charon->kernel->release_reqid(charon->kernel, args.reqid);
-       }
-       DESTROY_IF(args.label);
-       return status;
+       ike_sa->destroy_child_sa(ike_sa, child_sa->get_protocol(child_sa),
+                                                        child_sa->get_spi(child_sa, TRUE));
+       return initiate ? ike_sa->initiate(ike_sa, NULL, NULL) : SUCCESS;
 }
 
 /*
@@ -550,13 +555,8 @@ METHOD(task_t, build_i, status_t,
 
        if (this->expired)
        {
-               child_cfg_t *child_cfg;
-
-               DBG1(DBG_IKE, "scheduling CHILD_SA recreate after hard expire");
-               child_cfg = child_sa->get_config(child_sa);
-               this->ike_sa->queue_task(this->ike_sa, (task_t*)
-                               child_create_create(this->ike_sa, child_cfg->get_ref(child_cfg),
-                                                                       FALSE, NULL, NULL, 0));
+               DBG1(DBG_IKE, "queue CHILD_SA recreate after hard expire");
+               queue_child_create(this->ike_sa, child_sa);
        }
        return NEED_MORE;
 }

diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c
index 86161423b72243683197399fbfe5d50ffac27596..fb4a724f4dc3a84d632874fbd3a9ff75ebb2f7ca 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_rekey.c
+++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c
@@ -269,20 +269,14 @@ METHOD(task_t, build_i, status_t,
        if (!this->child_create)
        {
                child_cfg_t *config;
-               proposal_t *proposal;
-               uint16_t ke_method;
                uint32_t reqid;
 
                config = this->child_sa->get_config(this->child_sa);
                this->child_create = child_create_create(this->ike_sa,
                                                                config->get_ref(config), TRUE, NULL, NULL, 0);
 
-               proposal = this->child_sa->get_proposal(this->child_sa);
-               if (proposal->get_algorithm(proposal, KEY_EXCHANGE_METHOD,
-                                                                       &ke_method, NULL))
-               {       /* reuse the KE method negotiated previously */
-                       this->child_create->use_ke_method(this->child_create, ke_method);
-               }
+               this->child_create->recreate_sa(this->child_create, this->child_sa);
+
                reqid = this->child_sa->get_reqid_ref(this->child_sa);
                if (reqid)
                {
@@ -436,6 +430,7 @@ METHOD(task_t, build_r, status_t,
 
        if (message->get_exchange_type(message) == CREATE_CHILD_SA)
        {
+               this->child_create->recreate_sa(this->child_create, this->child_sa);
                reqid = this->child_sa->get_reqid_ref(this->child_sa);
                if (reqid)
                {

diff --git a/src/libcharon/sa/task_manager.h b/src/libcharon/sa/task_manager.h
index 69e5a812e750fc327c62df15d646dd2709772b3e..a30fab5a9a028b6df3e97c98f180638e061355db 100644 (file)
--- a/src/libcharon/sa/task_manager.h
+++ b/src/libcharon/sa/task_manager.h
@@ -202,9 +202,10 @@ struct task_manager_t {
         *
         * @param cfg                   CHILD_SA config to establish
         * @param args                  optional arguments for the initiation
+        * @param child_sa              optional CHILD_SA when recreating (instead of args)
         */
        void (*queue_child)(task_manager_t *this, child_cfg_t *cfg,
-                                               child_init_args_t *args);
+                                               child_init_args_t *args, child_sa_t *child_sa);
 
        /**
         * Queue CHILD_SA rekeying tasks.