From: Tobias Brunner Date: Tue, 1 Apr 2025 15:28:28 +0000 (+0200) Subject: child-create: Use more generic method to pass information from previous SA X-Git-Tag: 6.0.2dr1~6^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=920545835539e32cda3691ba12b47a6fe5634329;p=thirdparty%2Fstrongswan.git 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 77e5721825..3cfcdcf332 100644 --- 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 672a04927d..5afc66ae81 100644 --- 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 2180bd9099..d46559182f 100644 --- 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 be4662f1c0..3020967fd6 100644 --- 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 f3ff0cc7d6..0c0e6137e7 100644 --- 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 3282a21f7a..7987bdd534 100644 --- 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 86161423b7..fb4a724f4d 100644 --- 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 69e5a812e7..a30fab5a9a 100644 --- 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.