From 7f30e1aea2648478ee5823e90c9a6b5be4f27d1e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 19 May 2021 14:16:20 +0200 Subject: [PATCH] ike-sa: Use a struct to pass optional arguments when initiating CHILD_SAs --- src/charon-nm/nm/nm_service.c | 2 +- .../backend/android_service.c | 2 +- src/libcharon/control/controller.c | 2 +- .../processing/jobs/adopt_children_job.c | 2 +- .../processing/jobs/initiate_tasks_job.c | 2 +- src/libcharon/sa/ike_sa.c | 18 ++++++++--------- src/libcharon/sa/ike_sa.h | 20 ++++++++++++++----- src/libcharon/sa/ikev1/task_manager_v1.c | 17 ++++++++++------ src/libcharon/sa/ikev1/tasks/quick_delete.c | 5 ++++- src/libcharon/sa/ikev2/task_manager_v2.c | 15 ++++++++------ src/libcharon/sa/ikev2/tasks/child_delete.c | 7 ++++--- src/libcharon/sa/ikev2/tasks/child_rekey.c | 7 +++---- src/libcharon/sa/task_manager.h | 8 +++----- src/libcharon/sa/trap_manager.c | 10 ++++++++-- .../tests/suites/test_child_create.c | 4 ++-- src/libcharon/tests/suites/test_childless.c | 10 +++++----- .../tests/suites/test_ike_mid_sync.c | 2 +- .../tests/utils/exchange_test_helper.c | 2 +- 18 files changed, 80 insertions(+), 55 deletions(-) diff --git a/src/charon-nm/nm/nm_service.c b/src/charon-nm/nm/nm_service.c index 2d93b2fae4..6fae7bf9a3 100644 --- a/src/charon-nm/nm/nm_service.c +++ b/src/charon-nm/nm/nm_service.c @@ -903,7 +903,7 @@ static gboolean connect_(NMVpnServicePlugin *plugin, NMConnection *connection, * Initiate */ child_cfg->get_ref(child_cfg); - if (ike_sa->initiate(ike_sa, child_cfg, 0, NULL, NULL) != SUCCESS) + if (ike_sa->initiate(ike_sa, child_cfg, NULL) != SUCCESS) { charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, ike_sa); diff --git a/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_service.c b/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_service.c index 3e686ac839..a99d039f8e 100644 --- a/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_service.c +++ b/src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_service.c @@ -924,7 +924,7 @@ static job_requeue_t initiate(private_android_service_t *this) /* get an additional reference because initiate consumes one */ child_cfg->get_ref(child_cfg); - if (ike_sa->initiate(ike_sa, child_cfg, 0, NULL, NULL) != SUCCESS) + if (ike_sa->initiate(ike_sa, child_cfg, NULL) != SUCCESS) { DBG1(DBG_CFG, "failed to initiate tunnel"); charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, diff --git a/src/libcharon/control/controller.c b/src/libcharon/control/controller.c index 46b065e3fd..cd25b28fef 100644 --- a/src/libcharon/control/controller.c +++ b/src/libcharon/control/controller.c @@ -473,7 +473,7 @@ METHOD(job_t, initiate_execute, job_requeue_t, } } - if (ike_sa->initiate(ike_sa, listener->child_cfg, 0, NULL, NULL) == SUCCESS) + if (ike_sa->initiate(ike_sa, listener->child_cfg, NULL) == SUCCESS) { if (!listener->logger.callback) { diff --git a/src/libcharon/processing/jobs/adopt_children_job.c b/src/libcharon/processing/jobs/adopt_children_job.c index e2a7f6b202..f969419fd2 100644 --- a/src/libcharon/processing/jobs/adopt_children_job.c +++ b/src/libcharon/processing/jobs/adopt_children_job.c @@ -253,7 +253,7 @@ METHOD(job_t, execute, job_requeue_t, task->migrate(task, ike_sa); ike_sa->queue_task(ike_sa, task); } - if (ike_sa->initiate(ike_sa, NULL, 0, NULL, NULL) == DESTROY_ME) + if (ike_sa->initiate(ike_sa, NULL, NULL) == DESTROY_ME) { charon->ike_sa_manager->checkin_and_destroy( charon->ike_sa_manager, ike_sa); diff --git a/src/libcharon/processing/jobs/initiate_tasks_job.c b/src/libcharon/processing/jobs/initiate_tasks_job.c index 001e71fd1f..442cd31635 100644 --- a/src/libcharon/processing/jobs/initiate_tasks_job.c +++ b/src/libcharon/processing/jobs/initiate_tasks_job.c @@ -55,7 +55,7 @@ METHOD(job_t, execute, job_requeue_t, this->ike_sa_id); if (ike_sa) { - if (ike_sa->initiate(ike_sa, NULL, 0, NULL, NULL) == DESTROY_ME) + if (ike_sa->initiate(ike_sa, NULL, NULL) == DESTROY_ME) { charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, ike_sa); diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c index a891aff497..3ef5e020b1 100644 --- a/src/libcharon/sa/ike_sa.c +++ b/src/libcharon/sa/ike_sa.c @@ -1531,8 +1531,7 @@ static void resolve_hosts(private_ike_sa_t *this) } METHOD(ike_sa_t, initiate, status_t, - private_ike_sa_t *this, child_cfg_t *child_cfg, uint32_t reqid, - traffic_selector_t *tsi, traffic_selector_t *tsr) + private_ike_sa_t *this, child_cfg_t *child_cfg, child_init_args_t *args) { bool defer_initiate = FALSE; @@ -1587,8 +1586,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, reqid, - tsi, tsr); + this->task_manager->queue_child(this->task_manager, child_cfg, args); #ifdef ME if (this->peer_cfg->get_mediated_by(this->peer_cfg)) { @@ -1621,7 +1619,7 @@ METHOD(ike_sa_t, retry_initiate, status_t, if (this->retry_initiate_queued) { this->retry_initiate_queued = FALSE; - return initiate(this, NULL, 0, NULL, NULL); + return initiate(this, NULL, NULL); } return SUCCESS; } @@ -2077,13 +2075,15 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new, } if (action == ACTION_RESTART) { + child_init_args_t args = { + .reqid = child_sa->get_reqid(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), - child_sa->get_reqid(child_sa), - NULL, NULL); + &args); } } enumerator->destroy(enumerator); @@ -2091,7 +2091,7 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new, /* adopt any active or queued CHILD-creating tasks */ new->adopt_child_tasks(new, &this->public); - return new->initiate(new, NULL, 0, NULL, NULL); + return new->initiate(new, NULL, NULL); } METHOD(ike_sa_t, reestablish, status_t, @@ -2224,7 +2224,7 @@ METHOD(ike_sa_t, reestablish, status_t, #ifdef ME if (this->peer_cfg->is_mediation(this->peer_cfg)) { - status = new->initiate(new, NULL, 0, NULL, NULL); + status = new->initiate(new, NULL, NULL); } else #endif /* ME */ diff --git a/src/libcharon/sa/ike_sa.h b/src/libcharon/sa/ike_sa.h index ada8e89b60..37aac2d389 100644 --- a/src/libcharon/sa/ike_sa.h +++ b/src/libcharon/sa/ike_sa.h @@ -29,6 +29,7 @@ typedef enum ike_condition_t ike_condition_t; typedef enum ike_sa_state_t ike_sa_state_t; typedef enum statistic_t statistic_t; typedef enum update_hosts_flag_t update_hosts_flag_t; +typedef struct child_init_args_t child_init_args_t; typedef struct ike_sa_t ike_sa_t; #include @@ -369,6 +370,18 @@ enum ike_sa_state_t { */ extern enum_name_t *ike_sa_state_names; +/** + * Optional arguments passed when initiating a CHILD_SA. + */ +struct child_init_args_t { + /** Reqid to use for CHILD_SA, 0 to assign automatically */ + uint32_t reqid; + /** Optional source of triggering packet */ + traffic_selector_t *src; + /** Optional destination of triggering packet */ + traffic_selector_t *dst; +}; + /** * Class ike_sa_t representing an IKE_SA. * @@ -787,16 +800,13 @@ struct ike_sa_t { * to the CHILD_SA. * * @param child_cfg child config to create CHILD from - * @param reqid reqid to use for CHILD_SA, 0 assign uniquely - * @param tsi source of triggering packet - * @param tsr destination of triggering packet. + * @param args optional arguments for the CHILD initiation * @return * - SUCCESS if initialization started * - DESTROY_ME if initialization failed */ status_t (*initiate) (ike_sa_t *this, child_cfg_t *child_cfg, - uint32_t reqid, traffic_selector_t *tsi, - traffic_selector_t *tsr); + child_init_args_t *args); /** * Retry initiation of this IKE_SA after it got deferred previously. diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c index 8df2862dd9..cafeabb841 100644 --- a/src/libcharon/sa/ikev1/task_manager_v1.c +++ b/src/libcharon/sa/ikev1/task_manager_v1.c @@ -1685,7 +1685,7 @@ METHOD(task_manager_t, queue_ike_reauth, void, enumerator->destroy(enumerator); } - if (new->initiate(new, NULL, 0, NULL, NULL) != DESTROY_ME) + if (new->initiate(new, NULL, NULL) != DESTROY_ME) { charon->ike_sa_manager->checkin(charon->ike_sa_manager, new); this->ike_sa->set_state(this->ike_sa, IKE_REKEYING); @@ -1732,14 +1732,19 @@ METHOD(task_manager_t, queue_mobike, void, } METHOD(task_manager_t, queue_child, void, - private_task_manager_t *this, child_cfg_t *cfg, uint32_t reqid, - traffic_selector_t *tsi, traffic_selector_t *tsr) + private_task_manager_t *this, child_cfg_t *cfg, child_init_args_t *args) { quick_mode_t *task; - task = quick_mode_create(this->ike_sa, cfg, tsi, tsr); - task->use_reqid(task, reqid); - + if (args) + { + task = quick_mode_create(this->ike_sa, cfg, args->src, args->dst); + task->use_reqid(task, args->reqid); + } + else + { + task = quick_mode_create(this->ike_sa, cfg, NULL, NULL); + } queue_task(this, &task->task); } diff --git a/src/libcharon/sa/ikev1/tasks/quick_delete.c b/src/libcharon/sa/ikev1/tasks/quick_delete.c index 7dff247fbe..ecfc8dda33 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_delete.c +++ b/src/libcharon/sa/ikev1/tasks/quick_delete.c @@ -149,6 +149,9 @@ static status_t delete_child(private_quick_delete_t *this, if (remote_close) { + child_init_args_t args = { + .reqid = child_sa->get_reqid(child_sa), + }; child_cfg = child_sa->get_config(child_sa); child_cfg->get_ref(child_cfg); @@ -157,7 +160,7 @@ static status_t delete_child(private_quick_delete_t *this, case ACTION_RESTART: child_cfg->get_ref(child_cfg); status = this->ike_sa->initiate(this->ike_sa, child_cfg, - child_sa->get_reqid(child_sa), NULL, NULL); + &args); break; case ACTION_ROUTE: charon->traps->install(charon->traps, diff --git a/src/libcharon/sa/ikev2/task_manager_v2.c b/src/libcharon/sa/ikev2/task_manager_v2.c index f45d074e12..b359c67bb2 100644 --- a/src/libcharon/sa/ikev2/task_manager_v2.c +++ b/src/libcharon/sa/ikev2/task_manager_v2.c @@ -1983,7 +1983,7 @@ static void trigger_mbb_reauth(private_task_manager_t *this) /* suspend online revocation checking until the SA is established */ new->set_condition(new, COND_ONLINE_VALIDATION_SUSPENDED, TRUE); - if (new->initiate(new, NULL, 0, NULL, NULL) != DESTROY_ME) + if (new->initiate(new, NULL, NULL) != DESTROY_ME) { new->queue_task(new, (task_t*)ike_verify_peer_cert_create(new)); new->queue_task(new, (task_t*)ike_reauth_complete_create(new, @@ -2102,15 +2102,18 @@ METHOD(task_manager_t, queue_dpd, void, } METHOD(task_manager_t, queue_child, void, - private_task_manager_t *this, child_cfg_t *cfg, uint32_t reqid, - traffic_selector_t *tsi, traffic_selector_t *tsr) + private_task_manager_t *this, child_cfg_t *cfg, child_init_args_t *args) { child_create_t *task; - task = child_create_create(this->ike_sa, cfg, FALSE, tsi, tsr); - if (reqid) + if (args) { - task->use_reqid(task, reqid); + task = child_create_create(this->ike_sa, cfg, FALSE, args->src, args->dst); + task->use_reqid(task, args->reqid); + } + else + { + task = child_create_create(this->ike_sa, cfg, FALSE, NULL, NULL); } queue_task(this, &task->task); } diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index f3881ceaa2..5ce251b5ff 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -312,12 +312,13 @@ static void process_payloads(private_child_delete_t *this, message_t *message) */ static status_t destroy_and_reestablish(private_child_delete_t *this) { + child_init_args_t args = {}; enumerator_t *enumerator; entry_t *entry; child_sa_t *child_sa; child_cfg_t *child_cfg; protocol_id_t protocol; - uint32_t spi, reqid; + uint32_t spi; action_t action; status_t status = SUCCESS; time_t now, expire; @@ -362,9 +363,9 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) /* no delay and no lifetime, destroy it immediately */ } spi = child_sa->get_spi(child_sa, TRUE); - reqid = child_sa->get_reqid(child_sa); child_cfg = child_sa->get_config(child_sa); child_cfg->get_ref(child_cfg); + args.reqid = child_sa->get_reqid(child_sa); action = child_sa->get_close_action(child_sa); this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi); @@ -376,7 +377,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) case ACTION_RESTART: child_cfg->get_ref(child_cfg); status = this->ike_sa->initiate(this->ike_sa, child_cfg, - reqid, NULL, NULL); + &args); break; case ACTION_ROUTE: charon->traps->install(charon->traps, diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index 37f7ea9bdb..cf103e1941 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -391,7 +391,7 @@ METHOD(task_t, process_i, status_t, if (message->get_notify(message, CHILD_SA_NOT_FOUND)) { child_cfg_t *child_cfg; - uint32_t reqid; + child_init_args_t args = {}; if (this->collision && this->collision->get_type(this->collision) == TASK_CHILD_DELETE) @@ -406,15 +406,14 @@ METHOD(task_t, process_i, status_t, * that (we could go by name, but that might be tricky e.g. due to * narrowing) */ spi = this->child_sa->get_spi(this->child_sa, TRUE); - reqid = this->child_sa->get_reqid(this->child_sa); protocol = this->child_sa->get_protocol(this->child_sa); child_cfg = this->child_sa->get_config(this->child_sa); child_cfg->get_ref(child_cfg); + args.reqid = this->child_sa->get_reqid(this->child_sa); charon->bus->child_updown(charon->bus, this->child_sa, FALSE); this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi); return this->ike_sa->initiate(this->ike_sa, - child_cfg->get_ref(child_cfg), reqid, - NULL, NULL); + child_cfg->get_ref(child_cfg), &args); } if (this->child_create->task.process(&this->child_create->task, diff --git a/src/libcharon/sa/task_manager.h b/src/libcharon/sa/task_manager.h index f9225c56ad..8c6496adfc 100644 --- a/src/libcharon/sa/task_manager.h +++ b/src/libcharon/sa/task_manager.h @@ -172,12 +172,10 @@ struct task_manager_t { * Queue CHILD_SA establishing tasks. * * @param cfg CHILD_SA config to establish - * @param reqid reqid to use for CHILD_SA - * @param tsi initiator traffic selector, if packet-triggered - * @param tsr responder traffic selector, if packet-triggered + * @param args optional arguments for the initiation */ - void (*queue_child)(task_manager_t *this, child_cfg_t *cfg, uint32_t reqid, - traffic_selector_t *tsi, traffic_selector_t *tsr); + void (*queue_child)(task_manager_t *this, child_cfg_t *cfg, + child_init_args_t *args); /** * Queue CHILD_SA rekeying tasks. diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c index f9f78acaba..fc78eb441d 100644 --- a/src/libcharon/sa/trap_manager.c +++ b/src/libcharon/sa/trap_manager.c @@ -542,17 +542,23 @@ METHOD(trap_manager_t, acquire, void, if (ike_sa) { + child_init_args_t args = { + .reqid = reqid, + .src = src, + .dst = dst, + }; + if (this->ignore_acquire_ts || ike_sa->get_version(ike_sa) == IKEV1) { /* in IKEv1, don't prepend the acquiring packet TS, as we only * have a single TS that we can establish in a Quick Mode. */ - src = dst = NULL; + args.src = args.dst = NULL; } this->mutex->lock(this->mutex); acquire->ike_sa = ike_sa; this->mutex->unlock(this->mutex); - if (ike_sa->initiate(ike_sa, child, reqid, src, dst) != DESTROY_ME) + if (ike_sa->initiate(ike_sa, child, &args) != DESTROY_ME) { charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); } diff --git a/src/libcharon/tests/suites/test_child_create.c b/src/libcharon/tests/suites/test_child_create.c index 20a47f6bff..4f6733a920 100644 --- a/src/libcharon/tests/suites/test_child_create.c +++ b/src/libcharon/tests/suites/test_child_create.c @@ -44,7 +44,7 @@ START_TEST(test_collision_ike_rekey) traffic_selector_create_dynamic(0, 0, 65535)); child_cfg->add_traffic_selector(child_cfg, FALSE, traffic_selector_create_dynamic(0, 0, 65535)); - call_ikesa(a, initiate, child_cfg, 0, NULL, NULL); + call_ikesa(a, initiate, child_cfg, NULL); assert_child_sa_count(a, 1); assert_hook(); @@ -81,7 +81,7 @@ START_TEST(test_collision_ike_rekey) ck_assert(!exchange_test_helper->sender->dequeue(exchange_test_helper->sender)); assert_num_tasks(a, 0, TASK_QUEUE_ACTIVE); assert_num_tasks(a, 1, TASK_QUEUE_QUEUED); - call_ikesa(a, initiate, NULL, 0, NULL, NULL); + call_ikesa(a, initiate, NULL, NULL); assert_num_tasks(a, 0, TASK_QUEUE_ACTIVE); assert_sa_idle(b); diff --git a/src/libcharon/tests/suites/test_childless.c b/src/libcharon/tests/suites/test_childless.c index 6ac02aad81..931abbb5ec 100644 --- a/src/libcharon/tests/suites/test_childless.c +++ b/src/libcharon/tests/suites/test_childless.c @@ -44,7 +44,7 @@ START_TEST(test_regular) id_a = a->get_id(a); id_b = b->get_id(b); - call_ikesa(a, initiate, child_cfg, 0, NULL, NULL); + call_ikesa(a, initiate, child_cfg, NULL); /* IKE_SA_INIT --> */ id_b->set_initiator_spi(id_b, id_a->get_initiator_spi(id_a)); @@ -115,7 +115,7 @@ START_TEST(test_regular_manual) id_a = a->get_id(a); id_b = b->get_id(b); - call_ikesa(a, initiate, NULL, 0, NULL, NULL); + call_ikesa(a, initiate, NULL, NULL); /* IKE_SA_INIT --> */ id_b->set_initiator_spi(id_b, id_a->get_initiator_spi(id_a)); @@ -144,7 +144,7 @@ START_TEST(test_regular_manual) assert_sa_idle(a); assert_sa_idle(b); - call_ikesa(a, initiate, child_cfg, 0, NULL, NULL); + call_ikesa(a, initiate, child_cfg, NULL); /* CREATE_CHILD_SA { SA, Ni, KEi, TSi, TSr } --> */ assert_hook_called(child_updown); @@ -192,7 +192,7 @@ START_TEST(test_failure_init) id_a = a->get_id(a); id_b = b->get_id(b); - call_ikesa(a, initiate, child_cfg, 0, NULL, NULL); + call_ikesa(a, initiate, child_cfg, NULL); /* IKE_SA_INIT --> */ id_b->set_initiator_spi(id_b, id_a->get_initiator_spi(id_a)); @@ -233,7 +233,7 @@ START_TEST(test_failure_resp) id_a = a->get_id(a); id_b = b->get_id(b); - call_ikesa(a, initiate, child_cfg, 0, NULL, NULL); + call_ikesa(a, initiate, child_cfg, NULL); /* IKE_SA_INIT --> */ id_b->set_initiator_spi(id_b, id_a->get_initiator_spi(id_a)); diff --git a/src/libcharon/tests/suites/test_ike_mid_sync.c b/src/libcharon/tests/suites/test_ike_mid_sync.c index 3776f39e9d..cfebf4b5a8 100644 --- a/src/libcharon/tests/suites/test_ike_mid_sync.c +++ b/src/libcharon/tests/suites/test_ike_mid_sync.c @@ -493,7 +493,7 @@ START_TEST(test_active) charon->bus->remove_listener(charon->bus, &mid.listener); /* the active task was queued again */ - call_ikesa(a, initiate, NULL, 0, NULL, NULL); + call_ikesa(a, initiate, NULL, NULL); exchange_test_helper->process_message(exchange_test_helper, b, NULL); exchange_test_helper->process_message(exchange_test_helper, a, NULL); send_dpd(b, a); diff --git a/src/libcharon/tests/utils/exchange_test_helper.c b/src/libcharon/tests/utils/exchange_test_helper.c index be55a1387f..1448504abe 100644 --- a/src/libcharon/tests/utils/exchange_test_helper.c +++ b/src/libcharon/tests/utils/exchange_test_helper.c @@ -273,7 +273,7 @@ METHOD(exchange_test_helper_t, establish_sa, void, id_i = sa_i->get_id(sa_i); id_r = sa_r->get_id(sa_r); - call_ikesa(sa_i, initiate, child_i, 0, NULL, NULL); + call_ikesa(sa_i, initiate, child_i, NULL); /* IKE_SA_INIT --> */ id_r->set_initiator_spi(id_r, id_i->get_initiator_spi(id_i)); -- 2.47.3