From b00bcb9add6aceec1e2c22856ca1c9b71bc8a9c5 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 1 Mar 2017 12:34:22 +0100 Subject: [PATCH 01/20] child-create: Trigger NARROW_RESPONDER_POST hook before installing SAs This makes sure we use the same set of traffic selectors when installing the SAs and installing the policies. --- src/libcharon/sa/ikev2/tasks/child_create.c | 46 ++++++++++----------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 7180bfd13..03e2c0029 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-2016 Tobias Brunner + * Copyright (C) 2008-2017 Tobias Brunner * Copyright (C) 2005-2008 Martin Willi * Copyright (C) 2005 Jan Hutter * HSR Hochschule fuer Technik Rapperswil @@ -630,6 +630,23 @@ static status_t select_and_install(private_child_create_t *this, default: break; } + /* use a copy of the traffic selectors, as the POST hook should not + * change payloads */ + my_ts = this->tsr->clone_offset(this->tsr, + offsetof(traffic_selector_t, clone)); + other_ts = this->tsi->clone_offset(this->tsi, + offsetof(traffic_selector_t, clone)); + charon->bus->narrow(charon->bus, this->child_sa, + NARROW_RESPONDER_POST, my_ts, other_ts); + + if (my_ts->get_count(my_ts) == 0 || other_ts->get_count(other_ts) == 0) + { + my_ts->destroy_offset(my_ts, + offsetof(traffic_selector_t, destroy)); + other_ts->destroy_offset(other_ts, + offsetof(traffic_selector_t, destroy)); + return NOT_FOUND; + } } this->child_sa->set_state(this->child_sa, CHILD_INSTALLING); @@ -679,31 +696,10 @@ static status_t select_and_install(private_child_create_t *this, } else { - if (this->initiator) + status = this->child_sa->add_policies(this->child_sa, my_ts, other_ts); + + if (!this->initiator) { - status = this->child_sa->add_policies(this->child_sa, - my_ts, other_ts); - } - else - { - /* use a copy of the traffic selectors, as the POST hook should not - * change payloads */ - my_ts = this->tsr->clone_offset(this->tsr, - offsetof(traffic_selector_t, clone)); - other_ts = this->tsi->clone_offset(this->tsi, - offsetof(traffic_selector_t, clone)); - charon->bus->narrow(charon->bus, this->child_sa, - NARROW_RESPONDER_POST, my_ts, other_ts); - if (my_ts->get_count(my_ts) == 0 || - other_ts->get_count(other_ts) == 0) - { - status = FAILED; - } - else - { - status = this->child_sa->add_policies(this->child_sa, - my_ts, other_ts); - } my_ts->destroy_offset(my_ts, offsetof(traffic_selector_t, destroy)); other_ts->destroy_offset(other_ts, From 29ef4cf5f45759d11e8a8c9a891a0c29e4e6f8fc Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 28 Feb 2017 14:44:38 +0100 Subject: [PATCH 02/20] child-sa: Split in- and outbound policy de-/installation Only install outbound fallback policies. --- src/libcharon/sa/child_sa.c | 193 ++++++++++++++++++++++++------------ 1 file changed, 129 insertions(+), 64 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 9dd175b00..a512eb8b8 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -1,6 +1,6 @@ /* + * Copyright (C) 2006-2017 Tobias Brunner * Copyright (C) 2016 Andreas Steffen - * Copyright (C) 2006-2016 Tobias Brunner * Copyright (C) 2005-2008 Martin Willi * Copyright (C) 2006 Daniel Roethlisberger * Copyright (C) 2005 Jan Hutter @@ -888,9 +888,43 @@ static void prepare_sa_cfg(private_child_sa_t *this, ipsec_sa_cfg_t *my_sa, } /** - * Install 3 policies: out, in and forward + * Install inbound policie(s): in, fwd */ -static status_t install_policies_internal(private_child_sa_t *this, +static status_t install_policies_inbound(private_child_sa_t *this, + host_t *my_addr, host_t *other_addr, traffic_selector_t *my_ts, + traffic_selector_t *other_ts, ipsec_sa_cfg_t *my_sa, + ipsec_sa_cfg_t *other_sa, policy_type_t type, + policy_priority_t priority, uint32_t manual_prio) +{ + kernel_ipsec_policy_id_t in_id = { + .dir = POLICY_IN, + .src_ts = other_ts, + .dst_ts = my_ts, + .mark = this->mark_in, + }; + kernel_ipsec_manage_policy_t in_policy = { + .type = type, + .prio = priority, + .manual_prio = manual_prio, + .src = other_addr, + .dst = my_addr, + .sa = my_sa, + }; + status_t status = SUCCESS; + + status |= charon->kernel->add_policy(charon->kernel, &in_id, &in_policy); + if (this->mode != MODE_TRANSPORT) + { + in_id.dir = POLICY_FWD; + status |= charon->kernel->add_policy(charon->kernel, &in_id, &in_policy); + } + return status; +} + +/** + * Install outbound policie(s): out, [fwd] + */ +static status_t install_policies_outbound(private_child_sa_t *this, host_t *my_addr, host_t *other_addr, traffic_selector_t *my_ts, traffic_selector_t *other_ts, ipsec_sa_cfg_t *my_sa, ipsec_sa_cfg_t *other_sa, policy_type_t type, @@ -902,11 +936,6 @@ static status_t install_policies_internal(private_child_sa_t *this, .dst_ts = other_ts, .mark = this->mark_out, .interface = this->config->get_interface(this->config), - }, in_id = { - .dir = POLICY_IN, - .src_ts = other_ts, - .dst_ts = my_ts, - .mark = this->mark_in, }; kernel_ipsec_manage_policy_t out_policy = { .type = type, @@ -915,23 +944,13 @@ static status_t install_policies_internal(private_child_sa_t *this, .src = my_addr, .dst = other_addr, .sa = other_sa, - }, in_policy = { - .type = type, - .prio = priority, - .manual_prio = manual_prio, - .src = other_addr, - .dst = my_addr, - .sa = my_sa, }; status_t status = SUCCESS; status |= charon->kernel->add_policy(charon->kernel, &out_id, &out_policy); - status |= charon->kernel->add_policy(charon->kernel, &in_id, &in_policy); - if (this->mode != MODE_TRANSPORT) - { - in_id.dir = POLICY_FWD; - status |= charon->kernel->add_policy(charon->kernel, &in_id, &in_policy); + if (this->mode != MODE_TRANSPORT && this->policies_fwd_out) + { /* install an "outbound" FWD policy in case there is a drop policy * matching outbound forwarded traffic, to allow another tunnel to use * the reversed subnets and do the same we don't set a reqid (this also @@ -940,27 +959,77 @@ static status_t install_policies_internal(private_child_sa_t *this, * policies of two SAs we install them with reduced priority. As they * basically act as bypass policies for drop policies we use a higher * priority than is used for them. */ - if (this->policies_fwd_out) + out_id.dir = POLICY_FWD; + other_sa->reqid = 0; + if (priority == POLICY_PRIORITY_DEFAULT) { - out_id.dir = POLICY_FWD; - other_sa->reqid = 0; - if (priority == POLICY_PRIORITY_DEFAULT) - { - out_policy.prio = POLICY_PRIORITY_ROUTED; - } - status |= charon->kernel->add_policy(charon->kernel, &out_id, - &out_policy); - /* reset the reqid for any other further policies */ - other_sa->reqid = this->reqid; + out_policy.prio = POLICY_PRIORITY_ROUTED; } + status |= charon->kernel->add_policy(charon->kernel, &out_id, + &out_policy); + /* reset the reqid for any other further policies */ + other_sa->reqid = this->reqid; } return status; } /** - * Delete 3 policies: out, in and forward + * Install all policies */ -static void del_policies_internal(private_child_sa_t *this, +static status_t install_policies_internal(private_child_sa_t *this, + host_t *my_addr, host_t *other_addr, traffic_selector_t *my_ts, + traffic_selector_t *other_ts, ipsec_sa_cfg_t *my_sa, + ipsec_sa_cfg_t *other_sa, policy_type_t type, + policy_priority_t priority, uint32_t manual_prio) +{ + status_t status = SUCCESS; + + status |= install_policies_inbound(this, my_addr, other_addr, my_ts, + other_ts, my_sa, other_sa, type, + priority, manual_prio); + status |= install_policies_outbound(this, my_addr, other_addr, my_ts, + other_ts, my_sa, other_sa, type, + priority, manual_prio); + return status; +} + +/** + * Delete inbound policies: in, fwd + */ +static void del_policies_inbound(private_child_sa_t *this, + host_t *my_addr, host_t *other_addr, traffic_selector_t *my_ts, + traffic_selector_t *other_ts, ipsec_sa_cfg_t *my_sa, + ipsec_sa_cfg_t *other_sa, policy_type_t type, + policy_priority_t priority, uint32_t manual_prio) +{ + kernel_ipsec_policy_id_t in_id = { + .dir = POLICY_IN, + .src_ts = other_ts, + .dst_ts = my_ts, + .mark = this->mark_in, + }; + kernel_ipsec_manage_policy_t in_policy = { + .type = type, + .prio = priority, + .manual_prio = manual_prio, + .src = other_addr, + .dst = my_addr, + .sa = my_sa, + }; + + charon->kernel->del_policy(charon->kernel, &in_id, &in_policy); + + if (this->mode != MODE_TRANSPORT) + { + in_id.dir = POLICY_FWD; + charon->kernel->del_policy(charon->kernel, &in_id, &in_policy); + } +} + +/** + * Delete outbound policies: out, [fwd] + */ +static void del_policies_outbound(private_child_sa_t *this, host_t *my_addr, host_t *other_addr, traffic_selector_t *my_ts, traffic_selector_t *other_ts, ipsec_sa_cfg_t *my_sa, ipsec_sa_cfg_t *other_sa, policy_type_t type, @@ -972,11 +1041,6 @@ static void del_policies_internal(private_child_sa_t *this, .dst_ts = other_ts, .mark = this->mark_out, .interface = this->config->get_interface(this->config), - }, in_id = { - .dir = POLICY_IN, - .src_ts = other_ts, - .dst_ts = my_ts, - .mark = this->mark_in, }; kernel_ipsec_manage_policy_t out_policy = { .type = type, @@ -985,36 +1049,38 @@ static void del_policies_internal(private_child_sa_t *this, .src = my_addr, .dst = other_addr, .sa = other_sa, - }, in_policy = { - .type = type, - .prio = priority, - .manual_prio = manual_prio, - .src = other_addr, - .dst = my_addr, - .sa = my_sa, }; charon->kernel->del_policy(charon->kernel, &out_id, &out_policy); - charon->kernel->del_policy(charon->kernel, &in_id, &in_policy); - if (this->mode != MODE_TRANSPORT) - { - in_id.dir = POLICY_FWD; - charon->kernel->del_policy(charon->kernel, &in_id, &in_policy); - if (this->policies_fwd_out) + if (this->mode != MODE_TRANSPORT && this->policies_fwd_out) + { + out_id.dir = POLICY_FWD; + other_sa->reqid = 0; + if (priority == POLICY_PRIORITY_DEFAULT) { - out_id.dir = POLICY_FWD; - other_sa->reqid = 0; - if (priority == POLICY_PRIORITY_DEFAULT) - { - out_policy.prio = POLICY_PRIORITY_ROUTED; - } - charon->kernel->del_policy(charon->kernel, &out_id, &out_policy); - other_sa->reqid = this->reqid; + out_policy.prio = POLICY_PRIORITY_ROUTED; } + charon->kernel->del_policy(charon->kernel, &out_id, &out_policy); + other_sa->reqid = this->reqid; } } +/** + * Delete in- and outbound policies + */ +static void del_policies_internal(private_child_sa_t *this, + host_t *my_addr, host_t *other_addr, traffic_selector_t *my_ts, + traffic_selector_t *other_ts, ipsec_sa_cfg_t *my_sa, + ipsec_sa_cfg_t *other_sa, policy_type_t type, + policy_priority_t priority, uint32_t manual_prio) +{ + del_policies_outbound(this, my_addr, other_addr, my_ts, other_ts, my_sa, + other_sa, type, priority, manual_prio); + del_policies_inbound(this, my_addr, other_addr, my_ts, other_ts, my_sa, + other_sa, type, priority, manual_prio); +} + METHOD(child_sa_t, add_policies, status_t, private_child_sa_t *this, linked_list_t *my_ts_list, linked_list_t *other_ts_list) @@ -1077,13 +1143,12 @@ METHOD(child_sa_t, add_policies, status_t, if (priority == POLICY_PRIORITY_DEFAULT && manual_prio == 0 && require_policy_update()) { - status |= install_policies_internal(this, this->my_addr, + status |= install_policies_outbound(this, this->my_addr, this->other_addr, my_ts, other_ts, &my_sa, &other_sa, POLICY_DROP, POLICY_PRIORITY_FALLBACK, 0); } - /* install policies */ status |= install_policies_internal(this, this->my_addr, this->other_addr, my_ts, other_ts, &my_sa, &other_sa, POLICY_IPSEC, @@ -1242,12 +1307,12 @@ METHOD(child_sa_t, update, status_t, /* update fallback policies after the new policy is in place */ if (manual_prio == 0) { - del_policies_internal(this, this->my_addr, this->other_addr, + del_policies_outbound(this, this->my_addr, this->other_addr, old_my_ts ?: my_ts, old_other_ts ?: other_ts, &my_sa, &other_sa, POLICY_DROP, POLICY_PRIORITY_FALLBACK, 0); - install_policies_internal(this, me, other, my_ts, other_ts, + install_policies_outbound(this, me, other, my_ts, other_ts, &my_sa, &other_sa, POLICY_DROP, POLICY_PRIORITY_FALLBACK, 0); } @@ -1308,7 +1373,7 @@ METHOD(child_sa_t, destroy, void, if (priority == POLICY_PRIORITY_DEFAULT && manual_prio == 0 && require_policy_update()) { - del_policies_internal(this, this->my_addr, this->other_addr, + del_policies_outbound(this, this->my_addr, this->other_addr, my_ts, other_ts, &my_sa, &other_sa, POLICY_DROP, POLICY_PRIORITY_FALLBACK, 0); } From 4989aba8225f6d710ee69e56a98b90d13c37319a Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 1 Mar 2017 14:40:15 +0100 Subject: [PATCH 03/20] child-sa: Change API used to set/install policies This way we only have to pass the traffic selectors once. --- src/libcharon/plugins/ha/ha_dispatcher.c | 16 ++-- src/libcharon/sa/child_sa.c | 86 ++++++++++++++------- src/libcharon/sa/child_sa.h | 34 +++++--- src/libcharon/sa/ikev1/tasks/quick_mode.c | 33 ++++---- src/libcharon/sa/ikev2/tasks/child_create.c | 26 ++++--- src/libcharon/sa/trap_manager.c | 3 +- 6 files changed, 119 insertions(+), 79 deletions(-) diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c index ee66b8442..7d22257c6 100644 --- a/src/libcharon/plugins/ha/ha_dispatcher.c +++ b/src/libcharon/plugins/ha/ha_dispatcher.c @@ -818,14 +818,14 @@ static void process_child_add(private_ha_dispatcher_t *this, } enumerator->destroy(enumerator); + child_sa->set_policies(child_sa, local_ts, remote_ts); + if (initiator) { if (child_sa->install(child_sa, encr_r, integ_r, inbound_spi, - inbound_cpi, initiator, TRUE, TRUE, - local_ts, remote_ts) != SUCCESS || + inbound_cpi, initiator, TRUE, TRUE) != SUCCESS || child_sa->install(child_sa, encr_i, integ_i, outbound_spi, - outbound_cpi, initiator, FALSE, TRUE, - local_ts, remote_ts) != SUCCESS) + outbound_cpi, initiator, FALSE, TRUE) != SUCCESS) { failed = TRUE; } @@ -833,11 +833,9 @@ static void process_child_add(private_ha_dispatcher_t *this, else { if (child_sa->install(child_sa, encr_i, integ_i, inbound_spi, - inbound_cpi, initiator, TRUE, TRUE, - local_ts, remote_ts) != SUCCESS || + inbound_cpi, initiator, TRUE, TRUE) != SUCCESS || child_sa->install(child_sa, encr_r, integ_r, outbound_spi, - outbound_cpi, initiator, FALSE, TRUE, - local_ts, remote_ts) != SUCCESS) + outbound_cpi, initiator, FALSE, TRUE) != SUCCESS) { failed = TRUE; } @@ -868,7 +866,7 @@ static void process_child_add(private_ha_dispatcher_t *this, child_sa->get_unique_id(child_sa), local_ts, remote_ts, seg_i, this->segments->is_active(this->segments, seg_i) ? "*" : "", seg_o, this->segments->is_active(this->segments, seg_o) ? "*" : ""); - child_sa->add_policies(child_sa, local_ts, remote_ts); + child_sa->install_policies(child_sa); local_ts->destroy_offset(local_ts, offsetof(traffic_selector_t, destroy)); remote_ts->destroy_offset(remote_ts, offsetof(traffic_selector_t, destroy)); diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index a512eb8b8..dc1539fa0 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -693,12 +693,11 @@ METHOD(child_sa_t, alloc_cpi, uint16_t, METHOD(child_sa_t, install, status_t, private_child_sa_t *this, chunk_t encr, chunk_t integ, uint32_t spi, - uint16_t cpi, bool initiator, bool inbound, bool tfcv3, - linked_list_t *my_ts, linked_list_t *other_ts) + uint16_t cpi, bool initiator, bool inbound, bool tfcv3) { uint16_t enc_alg = ENCR_UNDEFINED, int_alg = AUTH_UNDEFINED, size; uint16_t esn = NO_EXT_SEQ_NUMBERS; - linked_list_t *src_ts = NULL, *dst_ts = NULL; + linked_list_t *my_ts, *other_ts, *src_ts, *dst_ts; time_t now; kernel_ipsec_sa_id_t id; kernel_ipsec_add_sa_t sa; @@ -708,6 +707,12 @@ METHOD(child_sa_t, install, status_t, status_t status; bool update = FALSE; + /* BEET requires the bound address from the traffic selectors */ + my_ts = linked_list_create_from_enumerator( + array_create_enumerator(this->my_ts)); + other_ts = linked_list_create_from_enumerator( + array_create_enumerator(this->other_ts)); + /* now we have to decide which spi to use. Use self allocated, if "in", * or the one in the proposal, if not "in" (others). Additionally, * source and dest host switch depending on the role */ @@ -721,6 +726,8 @@ METHOD(child_sa_t, install, status_t, } this->my_spi = spi; this->my_cpi = cpi; + dst_ts = my_ts; + src_ts = other_ts; } else { @@ -728,6 +735,8 @@ METHOD(child_sa_t, install, status_t, dst = this->other_addr; this->other_spi = spi; this->other_cpi = cpi; + src_ts = my_ts; + dst_ts = other_ts; if (tfcv3) { @@ -754,6 +763,8 @@ METHOD(child_sa_t, install, status_t, this->mark_in, this->mark_out, &this->reqid); if (status != SUCCESS) { + my_ts->destroy(my_ts); + other_ts->destroy(other_ts); return status; } this->reqid_allocated = TRUE; @@ -783,18 +794,6 @@ METHOD(child_sa_t, install, status_t, lifetime->time.rekey = 0; } - /* BEET requires the bound address from the traffic selectors */ - if (inbound) - { - dst_ts = my_ts; - src_ts = other_ts; - } - else - { - src_ts = my_ts; - dst_ts = other_ts; - } - id = (kernel_ipsec_sa_id_t){ .src = src, .dst = dst, @@ -827,6 +826,8 @@ METHOD(child_sa_t, install, status_t, status = charon->kernel->add_sa(charon->kernel, &id, &sa); + my_ts->destroy(my_ts); + other_ts->destroy(other_ts); free(lifetime); return status; @@ -1081,28 +1082,19 @@ static void del_policies_internal(private_child_sa_t *this, other_sa, type, priority, manual_prio); } -METHOD(child_sa_t, add_policies, status_t, +METHOD(child_sa_t, set_policies, void, private_child_sa_t *this, linked_list_t *my_ts_list, linked_list_t *other_ts_list) { enumerator_t *enumerator; traffic_selector_t *my_ts, *other_ts; - status_t status = SUCCESS; - if (!this->reqid_allocated && !this->static_reqid) + if (array_count(this->my_ts)) { - /* trap policy, get or confirm reqid */ - status = charon->kernel->alloc_reqid( - charon->kernel, my_ts_list, other_ts_list, - this->mark_in, this->mark_out, &this->reqid); - if (status != SUCCESS) - { - return status; - } - this->reqid_allocated = TRUE; + array_destroy_offset(this->my_ts, + offsetof(traffic_selector_t, destroy)); + this->my_ts = array_create(0, 0); } - - /* apply traffic selectors */ enumerator = my_ts_list->create_enumerator(my_ts_list); while (enumerator->enumerate(enumerator, &my_ts)) { @@ -1111,6 +1103,12 @@ METHOD(child_sa_t, add_policies, status_t, enumerator->destroy(enumerator); array_sort(this->my_ts, (void*)traffic_selector_cmp, NULL); + if (array_count(this->other_ts)) + { + array_destroy_offset(this->other_ts, + offsetof(traffic_selector_t, destroy)); + this->other_ts = array_create(0, 0); + } enumerator = other_ts_list->create_enumerator(other_ts_list); while (enumerator->enumerate(enumerator, &other_ts)) { @@ -1118,6 +1116,33 @@ METHOD(child_sa_t, add_policies, status_t, } enumerator->destroy(enumerator); array_sort(this->other_ts, (void*)traffic_selector_cmp, NULL); +} + +METHOD(child_sa_t, install_policies, status_t, + private_child_sa_t *this) +{ + enumerator_t *enumerator; + linked_list_t *my_ts_list, *other_ts_list; + traffic_selector_t *my_ts, *other_ts; + status_t status = SUCCESS; + + if (!this->reqid_allocated && !this->static_reqid) + { + my_ts_list = linked_list_create_from_enumerator( + array_create_enumerator(this->my_ts)); + other_ts_list = linked_list_create_from_enumerator( + array_create_enumerator(this->other_ts)); + status = charon->kernel->alloc_reqid( + charon->kernel, my_ts_list, other_ts_list, + this->mark_in, this->mark_out, &this->reqid); + my_ts_list->destroy(my_ts_list); + other_ts_list->destroy(other_ts_list); + if (status != SUCCESS) + { + return status; + } + this->reqid_allocated = TRUE; + } if (!this->config->has_option(this->config, OPT_NO_POLICIES)) { @@ -1505,7 +1530,8 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .alloc_cpi = _alloc_cpi, .install = _install, .update = _update, - .add_policies = _add_policies, + .set_policies = _set_policies, + .install_policies = _install_policies, .create_ts_enumerator = _create_ts_enumerator, .create_policy_enumerator = _create_policy_enumerator, .destroy = _destroy, diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index bc7df996a..bc5f919ae 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -1,8 +1,8 @@ /* - * Copyright (C) 2006-2008 Tobias Brunner + * Copyright (C) 2006-2017 Tobias Brunner * Copyright (C) 2006-2008 Martin Willi * Copyright (C) 2006 Daniel Roethlisberger - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -347,6 +347,8 @@ struct child_sa_t { /** * Install an IPsec SA for one direction. * + * set_policies() should be called before calling this. + * * @param encr encryption key, if any * @param integ integrity key * @param spi SPI to use, allocated for inbound @@ -354,26 +356,36 @@ struct child_sa_t { * @param initiator TRUE if initiator of exchange resulting in this SA * @param inbound TRUE to install an inbound SA, FALSE for outbound * @param tfcv3 TRUE if peer supports ESPv3 TFC - * @param my_ts negotiated local traffic selector list - * @param other_ts negotiated remote traffic selector list * @return SUCCESS or FAILED */ status_t (*install)(child_sa_t *this, chunk_t encr, chunk_t integ, uint32_t spi, uint16_t cpi, - bool initiator, bool inbound, bool tfcv3, - linked_list_t *my_ts, linked_list_t *other_ts); + bool initiator, bool inbound, bool tfcv3); + /** - * Install the policies using some traffic selectors. + * Configure the policies using some traffic selectors. * * Supplied lists of traffic_selector_t's specify the policies * to use for this child sa. * - * @param my_ts traffic selectors for local site - * @param other_ts traffic selectors for remote site + * Install the policies by calling install_policies(). + * + * This should be called before calling install() so the traffic selectors + * may be passed to the kernel interface when installing the SAs. + * + * @param my_ts traffic selectors for local site (cloned) + * @param other_ts traffic selectors for remote site (cloned) + */ + void (*set_policies)(child_sa_t *this, linked_list_t *my_ts_list, + linked_list_t *other_ts_list); + + /** + * Install the configured policies. + * * @return SUCCESS or FAILED */ - status_t (*add_policies)(child_sa_t *this, linked_list_t *my_ts_list, - linked_list_t *other_ts_list); + status_t (*install_policies)(child_sa_t *this); + /** * Update hosts and ecapulation mode in the kernel SAs and policies. * diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index d65db2875..8be82ebe2 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -325,6 +325,17 @@ static bool install(private_quick_mode_t *this) return FALSE; } + if (this->initiator) + { + this->child_sa->set_policies(this->child_sa, tsi, tsr); + } + else + { + this->child_sa->set_policies(this->child_sa, tsr, tsi); + } + tsi->destroy_offset(tsi, offsetof(traffic_selector_t, destroy)); + tsr->destroy_offset(tsr, offsetof(traffic_selector_t, destroy)); + if (this->keymat->derive_child_keys(this->keymat, this->proposal, this->dh, this->spi_i, this->spi_r, this->nonce_i, this->nonce_r, &encr_i, &integ_i, &encr_r, &integ_r)) @@ -333,19 +344,19 @@ static bool install(private_quick_mode_t *this) { status_i = this->child_sa->install(this->child_sa, encr_r, integ_r, this->spi_i, this->cpi_i, - this->initiator, TRUE, FALSE, tsi, tsr); + this->initiator, TRUE, FALSE); status_o = this->child_sa->install(this->child_sa, encr_i, integ_i, this->spi_r, this->cpi_r, - this->initiator, FALSE, FALSE, tsi, tsr); + this->initiator, FALSE, FALSE); } else { status_i = this->child_sa->install(this->child_sa, encr_i, integ_i, this->spi_r, this->cpi_r, - this->initiator, TRUE, FALSE, tsr, tsi); + this->initiator, TRUE, FALSE); status_o = this->child_sa->install(this->child_sa, encr_r, integ_r, this->spi_i, this->cpi_i, - this->initiator, FALSE, FALSE, tsr, tsi); + this->initiator, FALSE, FALSE); } } @@ -355,22 +366,12 @@ static bool install(private_quick_mode_t *this) (status_i != SUCCESS) ? "inbound " : "", (status_i != SUCCESS && status_o != SUCCESS) ? "and ": "", (status_o != SUCCESS) ? "outbound " : ""); - tsi->destroy_offset(tsi, offsetof(traffic_selector_t, destroy)); - tsr->destroy_offset(tsr, offsetof(traffic_selector_t, destroy)); status = FAILED; } else { - if (this->initiator) - { - status = this->child_sa->add_policies(this->child_sa, tsi, tsr); - } - else - { - status = this->child_sa->add_policies(this->child_sa, tsr, tsi); - } - tsi->destroy_offset(tsi, offsetof(traffic_selector_t, destroy)); - tsr->destroy_offset(tsr, offsetof(traffic_selector_t, destroy)); + status = this->child_sa->install_policies(this->child_sa); + if (status != SUCCESS) { DBG1(DBG_IKE, "unable to install IPsec policies (SPD) in kernel"); diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 03e2c0029..d4d05c9c1 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -649,6 +649,15 @@ static status_t select_and_install(private_child_create_t *this, } } + this->child_sa->set_policies(this->child_sa, my_ts, other_ts); + if (!this->initiator) + { + my_ts->destroy_offset(my_ts, + offsetof(traffic_selector_t, destroy)); + other_ts->destroy_offset(other_ts, + offsetof(traffic_selector_t, destroy)); + } + this->child_sa->set_state(this->child_sa, CHILD_INSTALLING); this->child_sa->set_ipcomp(this->child_sa, this->ipcomp); this->child_sa->set_mode(this->child_sa, this->mode); @@ -668,19 +677,19 @@ static status_t select_and_install(private_child_create_t *this, { status_i = this->child_sa->install(this->child_sa, encr_r, integ_r, this->my_spi, this->my_cpi, this->initiator, - TRUE, this->tfcv3, my_ts, other_ts); + TRUE, this->tfcv3); status_o = this->child_sa->install(this->child_sa, encr_i, integ_i, this->other_spi, this->other_cpi, this->initiator, - FALSE, this->tfcv3, my_ts, other_ts); + FALSE, this->tfcv3); } else { status_i = this->child_sa->install(this->child_sa, encr_i, integ_i, this->my_spi, this->my_cpi, this->initiator, - TRUE, this->tfcv3, my_ts, other_ts); + TRUE, this->tfcv3); status_o = this->child_sa->install(this->child_sa, encr_r, integ_r, this->other_spi, this->other_cpi, this->initiator, - FALSE, this->tfcv3, my_ts, other_ts); + FALSE, this->tfcv3); } } @@ -696,15 +705,8 @@ static status_t select_and_install(private_child_create_t *this, } else { - status = this->child_sa->add_policies(this->child_sa, my_ts, other_ts); + status = this->child_sa->install_policies(this->child_sa); - if (!this->initiator) - { - my_ts->destroy_offset(my_ts, - offsetof(traffic_selector_t, destroy)); - other_ts->destroy_offset(other_ts, - offsetof(traffic_selector_t, destroy)); - } if (status != SUCCESS) { DBG1(DBG_IKE, "unable to install IPsec policies (SPD) in kernel"); diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c index 40a0682f2..51df7a0db 100644 --- a/src/libcharon/sa/trap_manager.c +++ b/src/libcharon/sa/trap_manager.c @@ -272,7 +272,8 @@ METHOD(trap_manager_t, install, uint32_t, proposals->destroy_offset(proposals, offsetof(proposal_t, destroy)); child_sa->set_protocol(child_sa, proto); child_sa->set_mode(child_sa, child->get_mode(child)); - status = child_sa->add_policies(child_sa, my_ts, other_ts); + child_sa->set_policies(child_sa, my_ts, other_ts); + status = child_sa->install_policies(child_sa); my_ts->destroy_offset(my_ts, offsetof(traffic_selector_t, destroy)); other_ts->destroy_offset(other_ts, offsetof(traffic_selector_t, destroy)); if (status != SUCCESS) From 83186d3cf7fc9d715e7c1bf509f22733feeec1e5 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 28 Feb 2017 14:57:51 +0100 Subject: [PATCH 04/20] child-sa: Add new state to track installation of only the inbound SA --- src/libcharon/sa/child_sa.c | 1 + src/libcharon/sa/child_sa.h | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index dc1539fa0..9fe936a4e 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -31,6 +31,7 @@ ENUM(child_sa_state_names, CHILD_CREATED, CHILD_DESTROYING, "CREATED", "ROUTED", "INSTALLING", + "INSTALLED_INBOUND", "INSTALLED", "UPDATING", "REKEYING", diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index bc5f919ae..4d6409175 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -53,7 +53,12 @@ enum child_sa_state_t { CHILD_INSTALLING, /** - * Installed an in-use CHILD_SA + * Installed the inbound SA of a CHILD_SA during rekeying + */ + CHILD_INSTALLED_INBOUND, + + /** + * Installed both SAs of a CHILD_SA */ CHILD_INSTALLED, From 8a3a389e3dd2cbf4df916145df0e02b2fdbe4edb Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 28 Feb 2017 15:03:12 +0100 Subject: [PATCH 05/20] child-sa: Add methods that allow partial installation of CHILD_SA Using install() for the inbound SA and register_outbound() for the outbound SA followed by install_policies(), will delay the installation of the outbound SA as well as the installation of the outbound policies in the kernel until install_outbound() is called later. --- src/libcharon/sa/child_sa.c | 122 ++++++++++++++++++++++++++++++++++-- src/libcharon/sa/child_sa.h | 27 ++++++++ 2 files changed, 144 insertions(+), 5 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 9fe936a4e..93a2120e6 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -92,6 +92,26 @@ struct private_child_sa_t { */ array_t *other_ts; + /** + * Outbound encryption key cached during a rekeying + */ + chunk_t encr_r; + + /** + * Outbound integrity key cached during a rekeying + */ + chunk_t integ_r; + + /** + * Whether the outbound SA has only been registered yet during a rekeying + */ + bool outbound_registered; + + /** + * Whether the peer supports TFCv3 + */ + bool tfcv3; + /** * Protocol used to protect this SA, ESP|AH */ @@ -692,9 +712,12 @@ METHOD(child_sa_t, alloc_cpi, uint16_t, return 0; } -METHOD(child_sa_t, install, status_t, - private_child_sa_t *this, chunk_t encr, chunk_t integ, uint32_t spi, - uint16_t cpi, bool initiator, bool inbound, bool tfcv3) +/** + * Install the given SA in the kernel + */ +static status_t install_internal(private_child_sa_t *this, chunk_t encr, + chunk_t integ, uint32_t spi, uint16_t cpi, bool initiator, bool inbound, + bool tfcv3) { uint16_t enc_alg = ENCR_UNDEFINED, int_alg = AUTH_UNDEFINED, size; uint16_t esn = NO_EXT_SEQ_NUMBERS; @@ -834,6 +857,14 @@ METHOD(child_sa_t, install, status_t, return status; } +METHOD(child_sa_t, install, status_t, + private_child_sa_t *this, chunk_t encr, chunk_t integ, uint32_t spi, + uint16_t cpi, bool initiator, bool inbound, bool tfcv3) +{ + return install_internal(this, encr, integ, spi, cpi, initiator, inbound, + tfcv3); +} + /** * Check kernel interface if policy updates are required */ @@ -1167,7 +1198,7 @@ METHOD(child_sa_t, install_policies, status_t, /* install outbound drop policy to avoid packets leaving unencrypted * when updating policies */ if (priority == POLICY_PRIORITY_DEFAULT && manual_prio == 0 && - require_policy_update()) + require_policy_update() && !this->outbound_registered) { status |= install_policies_outbound(this, this->my_addr, this->other_addr, my_ts, other_ts, @@ -1175,11 +1206,19 @@ METHOD(child_sa_t, install_policies, status_t, POLICY_PRIORITY_FALLBACK, 0); } - status |= install_policies_internal(this, this->my_addr, + status |= install_policies_inbound(this, this->my_addr, this->other_addr, my_ts, other_ts, &my_sa, &other_sa, POLICY_IPSEC, priority, manual_prio); + if (!this->outbound_registered) + { + status |= install_policies_outbound(this, this->my_addr, + this->other_addr, my_ts, other_ts, + &my_sa, &other_sa, POLICY_IPSEC, + priority, manual_prio); + + } if (status != SUCCESS) { break; @@ -1195,6 +1234,75 @@ METHOD(child_sa_t, install_policies, status_t, return status; } +METHOD(child_sa_t, register_outbound, void, + private_child_sa_t *this, chunk_t encr, chunk_t integ, uint32_t spi, + uint16_t cpi, bool tfcv3) +{ + DBG2(DBG_CHD, "registering outbound %N SA", protocol_id_names, + this->protocol); + DBG2(DBG_CHD, " SPI 0x%.8x, src %H dst %H", ntohl(spi), this->my_addr, + this->other_addr); + + this->other_spi = spi; + this->other_cpi = cpi; + this->encr_r = chunk_clone(encr); + this->integ_r = chunk_clone(integ); + this->tfcv3 = tfcv3; + this->outbound_registered = TRUE; +} + +METHOD(child_sa_t, install_outbound, status_t, + private_child_sa_t *this) +{ + enumerator_t *enumerator; + traffic_selector_t *my_ts, *other_ts; + status_t status; + + this->outbound_registered = FALSE; + + status = install_internal(this, this->encr_r, this->integ_r, + this->other_spi, this->other_cpi, FALSE, FALSE, + this->tfcv3); + chunk_clear(&this->encr_r); + chunk_clear(&this->integ_r); + if (status != SUCCESS) + { + return status; + } + if (!this->config->has_option(this->config, OPT_NO_POLICIES)) + { + ipsec_sa_cfg_t my_sa, other_sa; + uint32_t manual_prio; + + prepare_sa_cfg(this, &my_sa, &other_sa); + manual_prio = this->config->get_manual_prio(this->config); + + enumerator = create_policy_enumerator(this); + while (enumerator->enumerate(enumerator, &my_ts, &other_ts)) + { + /* install outbound drop policy to avoid packets leaving unencrypted + * when updating policies */ + if (manual_prio == 0 && require_policy_update()) + { + status |= install_policies_outbound(this, this->my_addr, + this->other_addr, my_ts, other_ts, + &my_sa, &other_sa, POLICY_DROP, + POLICY_PRIORITY_FALLBACK, 0); + } + status |= install_policies_outbound(this, this->my_addr, + this->other_addr, my_ts, other_ts, + &my_sa, &other_sa, POLICY_IPSEC, + POLICY_PRIORITY_DEFAULT, manual_prio); + if (status != SUCCESS) + { + break; + } + } + enumerator->destroy(enumerator); + } + return status; +} + /** * Callback to reinstall a virtual IP */ @@ -1451,6 +1559,8 @@ METHOD(child_sa_t, destroy, void, this->other_addr->destroy(this->other_addr); DESTROY_IF(this->proposal); this->config->destroy(this->config); + chunk_clear(&this->encr_r); + chunk_clear(&this->integ_r); free(this); } @@ -1530,6 +1640,8 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .alloc_spi = _alloc_spi, .alloc_cpi = _alloc_cpi, .install = _install, + .register_outbound = _register_outbound, + .install_outbound = _install_outbound, .update = _update, .set_policies = _set_policies, .install_policies = _install_policies, diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 4d6409175..98fa24701 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -367,6 +367,29 @@ struct child_sa_t { uint32_t spi, uint16_t cpi, bool initiator, bool inbound, bool tfcv3); + /** + * Register data for the installation of an outbound SA as responder during + * a rekeying. + * + * The SA is not installed until install_outbound() is called. + * + * @param encr encryption key, if any (cloned) + * @param integ integrity key (cloned) + * @param spi SPI to use, allocated for inbound + * @param cpi CPI to use, allocated for outbound + * @param tfcv3 TRUE if peer supports ESPv3 TFC + */ + void (*register_outbound)(child_sa_t *this, chunk_t encr, chunk_t integ, + uint32_t spi, uint16_t cpi, bool tfcv3); + + /** + * Install the outbound SA and the outbound policies as responder during a + * rekeying. + * + * @return SUCCESS or FAILED + */ + status_t (*install_outbound)(child_sa_t *this); + /** * Configure the policies using some traffic selectors. * @@ -387,6 +410,10 @@ struct child_sa_t { /** * Install the configured policies. * + * If register_outbound() was called previously this only installs the + * inbound and forward policies, the outbound policies are installed when + * install_outbound() is called. + * * @return SUCCESS or FAILED */ status_t (*install_policies)(child_sa_t *this); From cad13450bed9c082f58c1acddd9c4f197aebd8e5 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 28 Feb 2017 15:03:45 +0100 Subject: [PATCH 06/20] child-sa: Add method to associate rekeyed CHILD_SAs with their replacement --- src/libcharon/sa/child_sa.c | 19 +++++++++++++++++++ src/libcharon/sa/child_sa.h | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 93a2120e6..29d4680f2 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -112,6 +112,11 @@ struct private_child_sa_t { */ bool tfcv3; + /** + * The outbound SPI of the CHILD_SA that replaced this one during a rekeying + */ + uint32_t rekey_spi; + /** * Protocol used to protect this SA, ESP|AH */ @@ -1303,6 +1308,18 @@ METHOD(child_sa_t, install_outbound, status_t, return status; } +METHOD(child_sa_t, set_rekey_spi, void, + private_child_sa_t *this, uint32_t spi) +{ + this->rekey_spi = spi; +} + +METHOD(child_sa_t, get_rekey_spi, uint32_t, + private_child_sa_t *this) +{ + return this->rekey_spi; +} + /** * Callback to reinstall a virtual IP */ @@ -1642,6 +1659,8 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .install = _install, .register_outbound = _register_outbound, .install_outbound = _install_outbound, + .set_rekey_spi = _set_rekey_spi, + .get_rekey_spi = _get_rekey_spi, .update = _update, .set_policies = _set_policies, .install_policies = _install_policies, diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 98fa24701..4483346b1 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -418,6 +418,22 @@ struct child_sa_t { */ status_t (*install_policies)(child_sa_t *this); + /** + * Set the outbound SPI of the CHILD_SA that replaced this CHILD_SA during + * a rekeying. + * + * @param spi outbound SPI of the CHILD_SA that replaced this CHILD_SA + */ + void (*set_rekey_spi)(child_sa_t *this, uint32_t spi); + + /** + * Get the outbound SPI of the CHILD_SA that replaced this CHILD_SA during + * a rekeying. + * + * @return outbound SPI of the CHILD_SA that replaced this CHILD_SA + */ + uint32_t (*get_rekey_spi)(child_sa_t *this); + /** * Update hosts and ecapulation mode in the kernel SAs and policies. * From f84757f2e6f253ed2ed6045fe53cce470acc3101 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 1 Mar 2017 16:07:57 +0100 Subject: [PATCH 07/20] child-sa: Add log message for CHILD_SA state changes --- src/libcharon/sa/child_sa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 29d4680f2..2671fb7f3 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -291,6 +291,10 @@ METHOD(child_sa_t, get_config, child_cfg_t*, METHOD(child_sa_t, set_state, void, private_child_sa_t *this, child_sa_state_t state) { + DBG2(DBG_CHD, "CHILD_SA %s{%d} state change: %N => %N", + get_name(this), this->unique_id, + child_sa_state_names, this->state, + child_sa_state_names, state); charon->bus->child_state_change(charon->bus, &this->public, state); this->state = state; } From dc3710e987b49c96ee2e81c1979fb2a6133bd30d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 1 Mar 2017 18:02:38 +0100 Subject: [PATCH 08/20] ikev2: Delay installation of outbound SAs during rekeying on the responder The responder has all the information needed to install both SAs before the initiator does. So if the responder immediately installs the outbound SA it might send packets using the new SA which the initiator is not yet able to process. This can be avoided by delaying the installation of the outbound SA until the replaced SA is deleted. --- src/libcharon/sa/ikev2/tasks/child_create.c | 31 +++++++-- src/libcharon/sa/ikev2/tasks/child_delete.c | 64 ++++++++++++++++++- src/libcharon/sa/ikev2/tasks/child_rekey.c | 7 +- src/libcharon/tests/suites/test_child_rekey.c | 52 +++++++++------ 4 files changed, 124 insertions(+), 30 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index d4d05c9c1..db57ee815 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -682,7 +682,7 @@ static status_t select_and_install(private_child_create_t *this, this->other_spi, this->other_cpi, this->initiator, FALSE, this->tfcv3); } - else + else if (!this->rekey) { status_i = this->child_sa->install(this->child_sa, encr_i, integ_i, this->my_spi, this->my_cpi, this->initiator, @@ -691,6 +691,17 @@ static status_t select_and_install(private_child_create_t *this, this->other_spi, this->other_cpi, this->initiator, FALSE, this->tfcv3); } + else + { /* as responder during a rekeying we only install the inbound + * SA now, the outbound SA and policies are installed when we + * receive the delete for the old SA */ + status_i = this->child_sa->install(this->child_sa, encr_i, integ_i, + this->my_spi, this->my_cpi, this->initiator, + TRUE, this->tfcv3); + this->child_sa->register_outbound(this->child_sa, encr_r, integ_r, + this->other_spi, this->other_cpi, this->tfcv3); + status_o = SUCCESS; + } } if (status_i != SUCCESS || status_o != SUCCESS) @@ -734,8 +745,14 @@ static status_t select_and_install(private_child_create_t *this, charon->bus->child_keys(charon->bus, this->child_sa, this->initiator, this->dh, nonce_i, nonce_r); - /* add to IKE_SA, and remove from task */ - this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); + if (this->rekey && !this->initiator) + { + this->child_sa->set_state(this->child_sa, CHILD_INSTALLED_INBOUND); + } + else + { + this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); + } this->ike_sa->add_child_sa(this->ike_sa, this->child_sa); this->established = TRUE; @@ -746,16 +763,17 @@ static status_t select_and_install(private_child_create_t *this, other_ts = linked_list_create_from_enumerator( this->child_sa->create_ts_enumerator(this->child_sa, FALSE)); - DBG0(DBG_IKE, "CHILD_SA %s{%d} established " + DBG0(DBG_IKE, "%sCHILD_SA %s{%d} established " "with SPIs %.8x_i %.8x_o and TS %#R === %#R", + this->rekey && !this->initiator ? "inbound " : "", this->child_sa->get_name(this->child_sa), this->child_sa->get_unique_id(this->child_sa), ntohl(this->child_sa->get_spi(this->child_sa, TRUE)), - ntohl(this->child_sa->get_spi(this->child_sa, FALSE)), my_ts, other_ts); + ntohl(this->child_sa->get_spi(this->child_sa, FALSE)), + my_ts, other_ts); my_ts->destroy(my_ts); other_ts->destroy(other_ts); - return SUCCESS; } @@ -1688,7 +1706,6 @@ METHOD(task_t, destroy, void, { this->proposals->destroy_offset(this->proposals, offsetof(proposal_t, destroy)); } - DESTROY_IF(this->config); DESTROY_IF(this->nonceg); free(this); diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index 6fa8836ac..d69f44b0d 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -146,6 +146,57 @@ static bool is_redundant(private_child_delete_t *this, child_sa_t *child) return FALSE; } +/** + * Install the outbound CHILD_SA with the given SPI + */ +static void install_outbound(private_child_delete_t *this, + protocol_id_t protocol, uint32_t spi) +{ + child_sa_t *child_sa; + linked_list_t *my_ts, *other_ts; + status_t status; + + child_sa = this->ike_sa->get_child_sa(this->ike_sa, protocol, + spi, FALSE); + if (!child_sa) + { + DBG1(DBG_IKE, "CHILD_SA not found after rekeying"); + return; + } + if (this->initiator && is_redundant(this, child_sa)) + { /* if we won the rekey collision we don't want to install the + * redundant SA created by the peer */ + return; + } + + status = child_sa->install_outbound(child_sa); + if (status != SUCCESS) + { + DBG1(DBG_IKE, "unable to install outbound IPsec SA (SAD) in kernel"); + charon->bus->alert(charon->bus, ALERT_INSTALL_CHILD_SA_FAILED, + child_sa); + /* FIXME: delete the new child_sa? */ + return; + } + child_sa->set_state(child_sa, CHILD_INSTALLED); + + my_ts = linked_list_create_from_enumerator( + child_sa->create_ts_enumerator(child_sa, TRUE)); + other_ts = linked_list_create_from_enumerator( + child_sa->create_ts_enumerator(child_sa, FALSE)); + + DBG0(DBG_IKE, "outbound CHILD_SA %s{%d} established " + "with SPIs %.8x_i %.8x_o and TS %#R === %#R", + child_sa->get_name(child_sa), + child_sa->get_unique_id(child_sa), + ntohl(child_sa->get_spi(child_sa, TRUE)), + ntohl(child_sa->get_spi(child_sa, FALSE)), + my_ts, other_ts); + + my_ts->destroy(my_ts); + other_ts->destroy(other_ts); +} + /** * read in payloads and find the children to delete */ @@ -197,6 +248,7 @@ static void process_payloads(private_child_delete_t *this, message_t *message) /* fall through */ case CHILD_REKEYING: /* we reply as usual, rekeying will fail */ + case CHILD_INSTALLED_INBOUND: case CHILD_INSTALLED: if (!this->initiator) { @@ -234,7 +286,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) child_sa_t *child_sa; child_cfg_t *child_cfg; protocol_id_t protocol; - uint32_t spi, reqid; + uint32_t spi, reqid, rekey_spi; action_t action; status_t status = SUCCESS; @@ -242,13 +294,21 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) while (enumerator->enumerate(enumerator, (void**)&child_sa)) { /* signal child down event if we weren't rekeying */ + protocol = child_sa->get_protocol(child_sa); if (!this->rekeyed) { charon->bus->child_updown(charon->bus, child_sa, FALSE); } + else + { + rekey_spi = child_sa->get_rekey_spi(child_sa); + if (rekey_spi) + { + install_outbound(this, protocol, rekey_spi); + } + } spi = child_sa->get_spi(child_sa, TRUE); reqid = child_sa->get_reqid(child_sa); - protocol = child_sa->get_protocol(child_sa); child_cfg = child_sa->get_config(child_sa); child_cfg->get_ref(child_cfg); action = child_sa->get_close_action(child_sa); diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index c04ec141f..afc4644a2 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -227,6 +227,7 @@ METHOD(task_t, build_r, status_t, child_cfg_t *config; uint32_t reqid; child_sa_state_t state; + child_sa_t *child_sa; if (!this->child_sa) { @@ -260,7 +261,10 @@ METHOD(task_t, build_r, status_t, return SUCCESS; } + child_sa = this->child_create->get_child(this->child_create); this->child_sa->set_state(this->child_sa, CHILD_REKEYED); + this->child_sa->set_rekey_spi(this->child_sa, + child_sa->get_spi(child_sa, FALSE)); /* invoke rekey hook */ charon->bus->child_rekey(charon->bus, this->child_sa, @@ -472,7 +476,8 @@ METHOD(child_rekey_t, collide, void, /* 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_child->get_state(other_child) != CHILD_INSTALLED && + other_child->get_state(other_child) != CHILD_INSTALLED_INBOUND)) { other->destroy(other); return; diff --git a/src/libcharon/tests/suites/test_child_rekey.c b/src/libcharon/tests/suites/test_child_rekey.c index fcac49388..19e5f784a 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -62,7 +62,7 @@ START_TEST(test_regular) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_REKEYED); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -150,7 +150,7 @@ START_TEST(test_regular_ke_invalid) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_REKEYED); - assert_child_sa_state(b, 6, CHILD_INSTALLED); + assert_child_sa_state(b, 6, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -204,7 +204,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -263,7 +263,7 @@ START_TEST(test_regular_responder_handle_hard_expire) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -284,7 +284,7 @@ START_TEST(test_regular_responder_handle_hard_expire) /* INFORMATIONAL { D } --> */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); assert_child_sa_state(a, 2, CHILD_DELETING); /* <-- INFORMATIONAL { D } */ assert_single_payload(IN, PLV2_DELETE); @@ -361,14 +361,14 @@ START_TEST(test_collision) assert_hook_rekey(child_rekey, 2, 5); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 5, CHILD_INSTALLED); + assert_child_sa_state(b, 5, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 6); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 6, CHILD_INSTALLED); + assert_child_sa_state(a, 6, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -387,7 +387,9 @@ START_TEST(test_collision) } assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); + assert_child_sa_state(a, data[_i].spi_a, + data[_i].spi_del_a == 1 ? CHILD_INSTALLED + : CHILD_INSTALLED_INBOUND); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { @@ -403,7 +405,9 @@ START_TEST(test_collision) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, + data[_i].spi_del_b == 2 ? CHILD_INSTALLED + : CHILD_INSTALLED_INBOUND); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); @@ -494,14 +498,14 @@ START_TEST(test_collision_delayed_response) assert_hook_rekey(child_rekey, 2, 5); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 5, CHILD_INSTALLED); + assert_child_sa_state(b, 5, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 6); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 6, CHILD_INSTALLED); + assert_child_sa_state(a, 6, CHILD_INSTALLED_INBOUND); assert_hook(); /* delay the CREATE_CHILD_SA response from b to a */ @@ -522,7 +526,9 @@ START_TEST(test_collision_delayed_response) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, + data[_i].spi_del_b == 2 ? CHILD_INSTALLED + : CHILD_INSTALLED_INBOUND); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); @@ -540,7 +546,9 @@ START_TEST(test_collision_delayed_response) /* INFORMATIONAL { D } --> */ exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, + data[_i].spi_del_b == 2 ? CHILD_INSTALLED + : CHILD_INSTALLED_INBOUND); assert_child_sa_count(b, 2); assert_hook(); @@ -635,7 +643,7 @@ START_TEST(test_collision_delayed_request) assert_hook_rekey(child_rekey, 1, 5); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 5, CHILD_INSTALLED); + assert_child_sa_state(a, 5, CHILD_INSTALLED_INBOUND); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); @@ -736,7 +744,7 @@ START_TEST(test_collision_delayed_request_more) assert_hook_rekey(child_rekey, 1, 5); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 5, CHILD_INSTALLED); + assert_child_sa_state(a, 5, CHILD_INSTALLED_INBOUND); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); @@ -874,14 +882,14 @@ START_TEST(test_collision_ke_invalid) assert_hook_rekey(child_rekey, 2, 9); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 9, CHILD_INSTALLED); + assert_child_sa_state(b, 9, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 10); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a,10, CHILD_INSTALLED); + assert_child_sa_state(a,10, CHILD_INSTALLED_INBOUND); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -898,7 +906,9 @@ START_TEST(test_collision_ke_invalid) } assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); + assert_child_sa_state(a, data[_i].spi_a, + data[_i].spi_del_a == 1 ? CHILD_INSTALLED + : CHILD_INSTALLED_INBOUND); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { @@ -912,7 +922,9 @@ START_TEST(test_collision_ke_invalid) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, + data[_i].spi_del_b == 2 ? CHILD_INSTALLED + : CHILD_INSTALLED_INBOUND); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); @@ -1039,7 +1051,7 @@ START_TEST(test_collision_ke_invalid_delayed_retry) assert_hook_rekey(child_rekey, 1, 9); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 9, CHILD_INSTALLED); + assert_child_sa_state(a, 9, CHILD_INSTALLED_INBOUND); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 8); From 9a5f7a30f8ca60c25ab7a46b3e6506028806d8db Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 12:18:12 +0100 Subject: [PATCH 09/20] child-delete: Track flags per individual CHILD_SA --- src/libcharon/sa/ikev2/tasks/child_delete.c | 125 ++++++++++++-------- 1 file changed, 78 insertions(+), 47 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index d69f44b0d..6ab59962f 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -39,41 +39,51 @@ struct private_child_delete_t { ike_sa_t *ike_sa; /** - * Are we the initiator? + * Whether we are the initiator of the exchange */ bool initiator; /** - * Protocol of CHILD_SA to delete + * Protocol of CHILD_SA to delete (as initiator) */ protocol_id_t protocol; /** - * Inbound SPI of CHILD_SA to delete + * Inbound SPI of CHILD_SA to delete (as initiator) */ uint32_t spi; /** - * whether to enforce delete action policy - */ - bool check_delete_action; - - /** - * is this delete exchange following a rekey? - */ - bool rekeyed; - - /** - * CHILD_SA already expired? + * CHILD_SA already expired (as initiator) */ bool expired; /** - * CHILD_SAs which get deleted + * CHILD_SAs which get deleted, entry_t* */ linked_list_t *child_sas; }; +/** + * Information about a deleted CHILD_SA + */ +typedef struct { + /** Deleted CHILD_SA */ + child_sa_t *child_sa; + /** Whether the CHILD_SA was rekeyed */ + bool rekeyed; + /** Whether to enforce any delete action policy */ + bool check_delete_action; +} entry_t; + +/** + * Check if the given entry is for the same CHILD_SA + */ +static bool match_child(entry_t *entry, child_sa_t *child_sa) +{ + return entry->child_sa == child_sa; +} + /** * build the delete payloads from the listed child_sas */ @@ -81,25 +91,27 @@ static void build_payloads(private_child_delete_t *this, message_t *message) { delete_payload_t *ah = NULL, *esp = NULL; enumerator_t *enumerator; - child_sa_t *child_sa; + entry_t *entry; + protocol_id_t protocol; + uint32_t spi; enumerator = this->child_sas->create_enumerator(this->child_sas); - while (enumerator->enumerate(enumerator, (void**)&child_sa)) + while (enumerator->enumerate(enumerator, (void**)&entry)) { - protocol_id_t protocol = child_sa->get_protocol(child_sa); - uint32_t spi = child_sa->get_spi(child_sa, TRUE); + protocol = entry->child_sa->get_protocol(entry->child_sa); + spi = entry->child_sa->get_spi(entry->child_sa, TRUE); switch (protocol) { case PROTO_ESP: - if (esp == NULL) + if (!esp) { esp = delete_payload_create(PLV2_DELETE, PROTO_ESP); message->add_payload(message, (payload_t*)esp); } esp->add_spi(esp, spi); DBG1(DBG_IKE, "sending DELETE for %N CHILD_SA with SPI %.8x", - protocol_id_names, protocol, ntohl(spi)); + protocol_id_names, protocol, ntohl(spi)); break; case PROTO_AH: if (ah == NULL) @@ -109,12 +121,12 @@ static void build_payloads(private_child_delete_t *this, message_t *message) } ah->add_spi(ah, spi); DBG1(DBG_IKE, "sending DELETE for %N CHILD_SA with SPI %.8x", - protocol_id_names, protocol, ntohl(spi)); + protocol_id_names, protocol, ntohl(spi)); break; default: break; } - child_sa->set_state(child_sa, CHILD_DELETING); + entry->child_sa->set_state(entry->child_sa, CHILD_DELETING); } enumerator->destroy(enumerator); } @@ -208,6 +220,7 @@ static void process_payloads(private_child_delete_t *this, message_t *message) uint32_t spi; protocol_id_t protocol; child_sa_t *child_sa; + entry_t *entry; payloads = message->create_payload_enumerator(message); while (payloads->enumerate(payloads, &payload)) @@ -225,27 +238,37 @@ static void process_payloads(private_child_delete_t *this, message_t *message) { child_sa = this->ike_sa->get_child_sa(this->ike_sa, protocol, spi, FALSE); - if (child_sa == NULL) + if (!child_sa) { - DBG1(DBG_IKE, "received DELETE for %N CHILD_SA with SPI %.8x, " - "but no such SA", protocol_id_names, protocol, ntohl(spi)); + DBG1(DBG_IKE, "received DELETE for unknown %N CHILD_SA with" + " SPI %.8x", protocol_id_names, protocol, ntohl(spi)); continue; } DBG1(DBG_IKE, "received DELETE for %N CHILD_SA with SPI %.8x", protocol_id_names, protocol, ntohl(spi)); + if (this->child_sas->find_first(this->child_sas, + (void*)match_child, NULL, child_sa) == SUCCESS) + { + continue; + } + INIT(entry, + .child_sa = child_sa + ); switch (child_sa->get_state(child_sa)) { case CHILD_REKEYED: - this->rekeyed = TRUE; + entry->rekeyed = TRUE; break; case CHILD_DELETING: - /* we don't send back a delete if we initiated ourself */ + /* we don't send back a delete if we already initiated + * a delete ourself */ if (!this->initiator) { + free(entry); continue; } - /* fall through */ + break; case CHILD_REKEYING: /* we reply as usual, rekeying will fail */ case CHILD_INSTALLED_INBOUND: @@ -254,22 +277,18 @@ static void process_payloads(private_child_delete_t *this, message_t *message) { if (is_redundant(this, child_sa)) { - this->rekeyed = TRUE; + entry->rekeyed = TRUE; } else { - this->check_delete_action = TRUE; + entry->check_delete_action = TRUE; } } break; default: break; } - if (this->child_sas->find_first(this->child_sas, NULL, - (void**)&child_sa) != SUCCESS) - { - this->child_sas->insert_last(this->child_sas, child_sa); - } + this->child_sas->insert_last(this->child_sas, entry); } spis->destroy(spis); } @@ -283,6 +302,7 @@ static void process_payloads(private_child_delete_t *this, message_t *message) static status_t destroy_and_reestablish(private_child_delete_t *this) { enumerator_t *enumerator; + entry_t *entry; child_sa_t *child_sa; child_cfg_t *child_cfg; protocol_id_t protocol; @@ -291,11 +311,12 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) status_t status = SUCCESS; enumerator = this->child_sas->create_enumerator(this->child_sas); - while (enumerator->enumerate(enumerator, (void**)&child_sa)) + while (enumerator->enumerate(enumerator, (void**)&entry)) { + child_sa = entry->child_sa; /* signal child down event if we weren't rekeying */ protocol = child_sa->get_protocol(child_sa); - if (!this->rekeyed) + if (!entry->rekeyed) { charon->bus->child_updown(charon->bus, child_sa, FALSE); } @@ -313,7 +334,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) child_cfg->get_ref(child_cfg); action = child_sa->get_close_action(child_sa); this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi); - if (this->check_delete_action) + if (entry->check_delete_action) { /* enforce child_cfg policy if deleted passively */ switch (action) { @@ -348,12 +369,14 @@ static void log_children(private_child_delete_t *this) { linked_list_t *my_ts, *other_ts; enumerator_t *enumerator; + entry_t *entry; child_sa_t *child_sa; uint64_t bytes_in, bytes_out; enumerator = this->child_sas->create_enumerator(this->child_sas); - while (enumerator->enumerate(enumerator, (void**)&child_sa)) + while (enumerator->enumerate(enumerator, (void**)&entry)) { + child_sa = entry->child_sa; my_ts = linked_list_create_from_enumerator( child_sa->create_ts_enumerator(child_sa, TRUE)); other_ts = linked_list_create_from_enumerator( @@ -388,6 +411,7 @@ METHOD(task_t, build_i, status_t, private_child_delete_t *this, message_t *message) { child_sa_t *child_sa; + entry_t *entry; child_sa = this->ike_sa->get_child_sa(this->ike_sa, this->protocol, this->spi, TRUE); @@ -402,15 +426,18 @@ METHOD(task_t, build_i, status_t, /* we work only with the inbound SPI */ this->spi = child_sa->get_spi(child_sa, TRUE); } - this->child_sas->insert_last(this->child_sas, child_sa); + INIT(entry, + .child_sa = child_sa, + ); + this->child_sas->insert_last(this->child_sas, entry); if (child_sa->get_state(child_sa) == CHILD_REKEYED) { - this->rekeyed = TRUE; + entry->rekeyed = TRUE; } log_children(this); build_payloads(this, message); - if (!this->rekeyed && this->expired) + if (!entry->rekeyed && this->expired) { child_cfg_t *child_cfg; @@ -457,24 +484,28 @@ METHOD(child_delete_t , get_child, child_sa_t*, private_child_delete_t *this) { child_sa_t *child_sa = NULL; - this->child_sas->get_first(this->child_sas, (void**)&child_sa); + entry_t *entry; + + if (this->child_sas->get_first(this->child_sas, (void**)&entry) == SUCCESS) + { + child_sa = entry->child_sa; + } return child_sa; } METHOD(task_t, migrate, void, private_child_delete_t *this, ike_sa_t *ike_sa) { - this->check_delete_action = FALSE; this->ike_sa = ike_sa; - this->child_sas->destroy(this->child_sas); + this->child_sas->destroy_function(this->child_sas, free); this->child_sas = linked_list_create(); } METHOD(task_t, destroy, void, private_child_delete_t *this) { - this->child_sas->destroy(this->child_sas); + this->child_sas->destroy_function(this->child_sas, free); free(this); } From 4ba07a865244e3aa3b180ffb064264bc933df389 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 8 Mar 2017 09:40:40 +0100 Subject: [PATCH 10/20] child-sa: Keep track whether the outbound SA has been installed or not --- src/libcharon/sa/child_sa.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 2671fb7f3..a5d172dc6 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -105,7 +105,11 @@ struct private_child_sa_t { /** * Whether the outbound SA has only been registered yet during a rekeying */ - bool outbound_registered; + enum { + OUTBOUND_NONE, + OUTBOUND_REGISTERED, + OUTBOUND_INSTALLED, + } outbound_state; /** * Whether the peer supports TFCv3 @@ -534,7 +538,7 @@ static status_t update_usebytes(private_child_sa_t *this, bool inbound) } else { - if (this->other_spi) + if (this->other_spi && this->outbound_state == OUTBOUND_INSTALLED) { kernel_ipsec_sa_id_t id = { .src = this->my_addr, @@ -775,6 +779,7 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr, { tfc = this->config->get_tfc(this->config); } + this->outbound_state = OUTBOUND_INSTALLED; } DBG2(DBG_CHD, "adding %s %N SA", inbound ? "inbound" : "outbound", @@ -1190,6 +1195,7 @@ METHOD(child_sa_t, install_policies, status_t, policy_priority_t priority; ipsec_sa_cfg_t my_sa, other_sa; uint32_t manual_prio; + bool install_outbound; prepare_sa_cfg(this, &my_sa, &other_sa); manual_prio = this->config->get_manual_prio(this->config); @@ -1199,6 +1205,7 @@ METHOD(child_sa_t, install_policies, status_t, this->trap = this->state == CHILD_CREATED; priority = this->trap ? POLICY_PRIORITY_ROUTED : POLICY_PRIORITY_DEFAULT; + install_outbound = this->outbound_state != OUTBOUND_REGISTERED; /* enumerate pairs of traffic selectors */ enumerator = create_policy_enumerator(this); @@ -1207,7 +1214,7 @@ METHOD(child_sa_t, install_policies, status_t, /* install outbound drop policy to avoid packets leaving unencrypted * when updating policies */ if (priority == POLICY_PRIORITY_DEFAULT && manual_prio == 0 && - require_policy_update() && !this->outbound_registered) + require_policy_update() && install_outbound) { status |= install_policies_outbound(this, this->my_addr, this->other_addr, my_ts, other_ts, @@ -1220,7 +1227,7 @@ METHOD(child_sa_t, install_policies, status_t, &my_sa, &other_sa, POLICY_IPSEC, priority, manual_prio); - if (!this->outbound_registered) + if (install_outbound) { status |= install_policies_outbound(this, this->my_addr, this->other_addr, my_ts, other_ts, @@ -1257,7 +1264,7 @@ METHOD(child_sa_t, register_outbound, void, this->encr_r = chunk_clone(encr); this->integ_r = chunk_clone(integ); this->tfcv3 = tfcv3; - this->outbound_registered = TRUE; + this->outbound_state = OUTBOUND_REGISTERED; } METHOD(child_sa_t, install_outbound, status_t, @@ -1267,8 +1274,6 @@ METHOD(child_sa_t, install_outbound, status_t, traffic_selector_t *my_ts, *other_ts; status_t status; - this->outbound_registered = FALSE; - status = install_internal(this, this->encr_r, this->integ_r, this->other_spi, this->other_cpi, FALSE, FALSE, this->tfcv3); @@ -1550,7 +1555,7 @@ METHOD(child_sa_t, destroy, void, }; charon->kernel->del_sa(charon->kernel, &id, &sa); } - if (this->other_spi) + if (this->other_spi && this->outbound_state == OUTBOUND_INSTALLED) { kernel_ipsec_sa_id_t id = { .src = this->my_addr, From c5fed4cdeee65dd44b5a6f968871d5ae64dbf10e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 20 Mar 2017 13:35:56 +0100 Subject: [PATCH 11/20] child-sa: Add method to remove the outbound SA and policies --- src/libcharon/sa/child_sa.c | 78 ++++++++++++++++++++++++++++++++++--- src/libcharon/sa/child_sa.h | 5 +++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index a5d172dc6..bd106f232 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -1317,6 +1317,64 @@ METHOD(child_sa_t, install_outbound, status_t, return status; } +METHOD(child_sa_t, remove_outbound, void, + private_child_sa_t *this) +{ + enumerator_t *enumerator; + traffic_selector_t *my_ts, *other_ts; + + switch (this->outbound_state) + { + case OUTBOUND_INSTALLED: + break; + case OUTBOUND_REGISTERED: + chunk_clear(&this->encr_r); + chunk_clear(&this->integ_r); + this->outbound_state = OUTBOUND_NONE; + /* fall-through */ + case OUTBOUND_NONE: + return; + } + + if (!this->config->has_option(this->config, OPT_NO_POLICIES)) + { + ipsec_sa_cfg_t my_sa, other_sa; + uint32_t manual_prio; + + prepare_sa_cfg(this, &my_sa, &other_sa); + manual_prio = this->config->get_manual_prio(this->config); + + enumerator = create_policy_enumerator(this); + while (enumerator->enumerate(enumerator, &my_ts, &other_ts)) + { + del_policies_outbound(this, this->my_addr, this->other_addr, + my_ts, other_ts, &my_sa, &other_sa, + POLICY_IPSEC, POLICY_PRIORITY_DEFAULT, + manual_prio); + if (manual_prio == 0 && require_policy_update()) + { + del_policies_outbound(this, this->my_addr, this->other_addr, + my_ts, other_ts, &my_sa, &other_sa, + POLICY_DROP, POLICY_PRIORITY_FALLBACK, 0); + } + } + enumerator->destroy(enumerator); + } + + kernel_ipsec_sa_id_t id = { + .src = this->my_addr, + .dst = this->other_addr, + .spi = this->other_spi, + .proto = proto_ike2ip(this->protocol), + .mark = this->mark_out, + }; + kernel_ipsec_del_sa_t sa = { + .cpi = this->other_cpi, + }; + charon->kernel->del_sa(charon->kernel, &id, &sa); + this->outbound_state = OUTBOUND_NONE; +} + METHOD(child_sa_t, set_rekey_spi, void, private_child_sa_t *this, uint32_t spi) { @@ -1519,19 +1577,28 @@ METHOD(child_sa_t, destroy, void, { ipsec_sa_cfg_t my_sa, other_sa; uint32_t manual_prio; + bool del_outbound; prepare_sa_cfg(this, &my_sa, &other_sa); manual_prio = this->config->get_manual_prio(this->config); + del_outbound = this->trap || this->outbound_state == OUTBOUND_INSTALLED; /* delete all policies in the kernel */ enumerator = create_policy_enumerator(this); while (enumerator->enumerate(enumerator, &my_ts, &other_ts)) { - del_policies_internal(this, this->my_addr, this->other_addr, - my_ts, other_ts, &my_sa, &other_sa, - POLICY_IPSEC, priority, manual_prio); - if (priority == POLICY_PRIORITY_DEFAULT && manual_prio == 0 && - require_policy_update()) + if (del_outbound) + { + del_policies_outbound(this, this->my_addr, + this->other_addr, my_ts, other_ts, + &my_sa, &other_sa, POLICY_IPSEC, + priority, manual_prio); + } + del_policies_inbound(this, this->my_addr, this->other_addr, + my_ts, other_ts, &my_sa, &other_sa, + POLICY_IPSEC, priority, manual_prio); + if (!this->trap && manual_prio == 0 && require_policy_update() && + del_outbound) { del_policies_outbound(this, this->my_addr, this->other_addr, my_ts, other_ts, &my_sa, &other_sa, @@ -1668,6 +1735,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .install = _install, .register_outbound = _register_outbound, .install_outbound = _install_outbound, + .remove_outbound = _remove_outbound, .set_rekey_spi = _set_rekey_spi, .get_rekey_spi = _get_rekey_spi, .update = _update, diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 4483346b1..06c84d564 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -390,6 +390,11 @@ struct child_sa_t { */ status_t (*install_outbound)(child_sa_t *this); + /** + * Remove the outbound SA and the outbound policies after a rekeying. + */ + void (*remove_outbound)(child_sa_t *this); + /** * Configure the policies using some traffic selectors. * From afbea8ce3cf21cb676dce337c2eb1095f80ad693 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 15:08:14 +0100 Subject: [PATCH 12/20] child-sa: Expose state of the outbound SA --- src/libcharon/sa/child_sa.c | 42 +++++++++++++++++++++++-------------- src/libcharon/sa/child_sa.h | 36 ++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index bd106f232..22ee5c226 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -41,6 +41,12 @@ ENUM(child_sa_state_names, CHILD_CREATED, CHILD_DESTROYING, "DESTROYING", ); +ENUM(child_sa_outbound_state_names, CHILD_OUTBOUND_NONE, CHILD_OUTBOUND_INSTALLED, + "NONE", + "REGISTERED", + "INSTALLED", +); + typedef struct private_child_sa_t private_child_sa_t; /** @@ -105,11 +111,7 @@ struct private_child_sa_t { /** * Whether the outbound SA has only been registered yet during a rekeying */ - enum { - OUTBOUND_NONE, - OUTBOUND_REGISTERED, - OUTBOUND_INSTALLED, - } outbound_state; + child_sa_outbound_state_t outbound_state; /** * Whether the peer supports TFCv3 @@ -309,6 +311,12 @@ METHOD(child_sa_t, get_state, child_sa_state_t, return this->state; } +METHOD(child_sa_t, get_outbound_state, child_sa_outbound_state_t, + private_child_sa_t *this) +{ + return this->outbound_state; +} + METHOD(child_sa_t, get_spi, uint32_t, private_child_sa_t *this, bool inbound) { @@ -538,7 +546,7 @@ static status_t update_usebytes(private_child_sa_t *this, bool inbound) } else { - if (this->other_spi && this->outbound_state == OUTBOUND_INSTALLED) + if (this->other_spi && this->outbound_state == CHILD_OUTBOUND_INSTALLED) { kernel_ipsec_sa_id_t id = { .src = this->my_addr, @@ -779,7 +787,7 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr, { tfc = this->config->get_tfc(this->config); } - this->outbound_state = OUTBOUND_INSTALLED; + this->outbound_state = CHILD_OUTBOUND_INSTALLED; } DBG2(DBG_CHD, "adding %s %N SA", inbound ? "inbound" : "outbound", @@ -1205,7 +1213,7 @@ METHOD(child_sa_t, install_policies, status_t, this->trap = this->state == CHILD_CREATED; priority = this->trap ? POLICY_PRIORITY_ROUTED : POLICY_PRIORITY_DEFAULT; - install_outbound = this->outbound_state != OUTBOUND_REGISTERED; + install_outbound = this->outbound_state != CHILD_OUTBOUND_REGISTERED; /* enumerate pairs of traffic selectors */ enumerator = create_policy_enumerator(this); @@ -1264,7 +1272,7 @@ METHOD(child_sa_t, register_outbound, void, this->encr_r = chunk_clone(encr); this->integ_r = chunk_clone(integ); this->tfcv3 = tfcv3; - this->outbound_state = OUTBOUND_REGISTERED; + this->outbound_state = CHILD_OUTBOUND_REGISTERED; } METHOD(child_sa_t, install_outbound, status_t, @@ -1325,14 +1333,14 @@ METHOD(child_sa_t, remove_outbound, void, switch (this->outbound_state) { - case OUTBOUND_INSTALLED: + case CHILD_OUTBOUND_INSTALLED: break; - case OUTBOUND_REGISTERED: + case CHILD_OUTBOUND_REGISTERED: chunk_clear(&this->encr_r); chunk_clear(&this->integ_r); - this->outbound_state = OUTBOUND_NONE; + this->outbound_state = CHILD_OUTBOUND_NONE; /* fall-through */ - case OUTBOUND_NONE: + case CHILD_OUTBOUND_NONE: return; } @@ -1372,7 +1380,7 @@ METHOD(child_sa_t, remove_outbound, void, .cpi = this->other_cpi, }; charon->kernel->del_sa(charon->kernel, &id, &sa); - this->outbound_state = OUTBOUND_NONE; + this->outbound_state = CHILD_OUTBOUND_NONE; } METHOD(child_sa_t, set_rekey_spi, void, @@ -1581,7 +1589,8 @@ METHOD(child_sa_t, destroy, void, prepare_sa_cfg(this, &my_sa, &other_sa); manual_prio = this->config->get_manual_prio(this->config); - del_outbound = this->trap || this->outbound_state == OUTBOUND_INSTALLED; + del_outbound = this->trap || + this->outbound_state == CHILD_OUTBOUND_INSTALLED; /* delete all policies in the kernel */ enumerator = create_policy_enumerator(this); @@ -1622,7 +1631,7 @@ METHOD(child_sa_t, destroy, void, }; charon->kernel->del_sa(charon->kernel, &id, &sa); } - if (this->other_spi && this->outbound_state == OUTBOUND_INSTALLED) + if (this->other_spi && this->outbound_state == CHILD_OUTBOUND_INSTALLED) { kernel_ipsec_sa_id_t id = { .src = this->my_addr, @@ -1711,6 +1720,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .get_config = _get_config, .get_state = _get_state, .set_state = _set_state, + .get_outbound_state = _get_outbound_state, .get_spi = _get_spi, .get_cpi = _get_cpi, .get_protocol = _get_protocol, diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 06c84d564..70d11ec9d 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -24,6 +24,7 @@ #define CHILD_SA_H_ typedef enum child_sa_state_t child_sa_state_t; +typedef enum child_sa_outbound_state_t child_sa_outbound_state_t; typedef struct child_sa_t child_sa_t; #include @@ -98,6 +99,32 @@ enum child_sa_state_t { */ extern enum_name_t *child_sa_state_names; +/** + * States of the outbound SA of a CHILD_SA + */ +enum child_sa_outbound_state_t { + + /** + * Outbound SA is not installed + */ + CHILD_OUTBOUND_NONE, + + /** + * Data for the outbound SA has been registered, but not installed yet + */ + CHILD_OUTBOUND_REGISTERED, + + /** + * The outbound SA is currently installed + */ + CHILD_OUTBOUND_INSTALLED, +}; + +/** + * enum strings for child_sa_outbound_state_t. + */ +extern enum_name_t *child_sa_outbound_state_names; + /** * Represents an IPsec SAs between two hosts. * @@ -157,7 +184,14 @@ struct child_sa_t { * * @return CHILD_SA state */ - child_sa_state_t (*get_state) (child_sa_t *this); + child_sa_state_t (*get_state)(child_sa_t *this); + + /** + * Get the state of the outbound SA. + * + * @return outbound SA state + */ + child_sa_outbound_state_t (*get_outbound_state)(child_sa_t *this); /** * Set the state of the CHILD_SA. From d94c1224390027902f62534af5946a3259fb3907 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 15:35:30 +0100 Subject: [PATCH 13/20] unit-tests: Overload helper macro to check for outbound SA state --- src/libcharon/tests/utils/sa_asserts.h | 32 ++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/libcharon/tests/utils/sa_asserts.h b/src/libcharon/tests/utils/sa_asserts.h index 7afa3b55b..d23f724f1 100644 --- a/src/libcharon/tests/utils/sa_asserts.h +++ b/src/libcharon/tests/utils/sa_asserts.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2017 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -81,10 +81,16 @@ _sa->get_child_count(_sa)); \ }) +/** + * Check if the CHILD_SA with the given SPI is in the expected state, optionally + * check the state of the outbound SA. + */ +#define assert_child_sa_state(...) VA_ARGS_DISPATCH(assert_child_sa_state, __VA_ARGS__)(__VA_ARGS__) + /** * Check if the CHILD_SA with the given SPI is in the expected state. */ -#define assert_child_sa_state(ike_sa, spi, state) \ +#define assert_child_sa_state3(ike_sa, spi, state) \ ({ \ typeof(ike_sa) _sa = ike_sa; \ typeof(spi) _spi = spi; \ @@ -98,6 +104,28 @@ child_sa_state_names, _child->get_state(_child)); \ }) +/** + * Check if the outbound SA of a CHILD_SA with the given SPI is in the + * expected state. + */ +#define assert_child_sa_state4(ike_sa, spi, state, outbound) \ +({ \ + typeof(ike_sa) _sa = ike_sa; \ + typeof(spi) _spi = spi; \ + typeof(state) _state = state; \ + typeof(outbound) _outbound = outbound; \ + child_sa_t *_child = _sa->get_child_sa(_sa, PROTO_ESP, _spi, TRUE) ?: \ + _sa->get_child_sa(_sa, PROTO_ESP, _spi, FALSE); \ + test_assert_msg(_child, "CHILD_SA with SPI %.8x does not exist", \ + ntohl(_spi)); \ + test_assert_msg(_state == _child->get_state(_child), "%N != %N", \ + child_sa_state_names, _state, \ + child_sa_state_names, _child->get_state(_child)); \ + test_assert_msg(_outbound == _child->get_outbound_state(_child), "%N != %N", \ + child_sa_outbound_state_names, _outbound, \ + child_sa_outbound_state_names, _child->get_outbound_state(_child)); \ +}) + /** * Assert that the CHILD_SA with the given inbound SPI does not exist. */ From 0cbf75eb941d5bd6b6ddfbe556c725d5105c0421 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 15:39:10 +0100 Subject: [PATCH 14/20] child-sa: Remove state to track installation of half the SA again --- src/libcharon/sa/child_sa.c | 1 - src/libcharon/sa/child_sa.h | 5 -- src/libcharon/sa/ikev2/tasks/child_create.c | 9 +- src/libcharon/sa/ikev2/tasks/child_delete.c | 1 - src/libcharon/sa/ikev2/tasks/child_rekey.c | 3 +- src/libcharon/tests/suites/test_child_rekey.c | 90 +++++++++---------- 6 files changed, 47 insertions(+), 62 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 22ee5c226..1d615915f 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -31,7 +31,6 @@ ENUM(child_sa_state_names, CHILD_CREATED, CHILD_DESTROYING, "CREATED", "ROUTED", "INSTALLING", - "INSTALLED_INBOUND", "INSTALLED", "UPDATING", "REKEYING", diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 70d11ec9d..b9a913da1 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -53,11 +53,6 @@ enum child_sa_state_t { */ CHILD_INSTALLING, - /** - * Installed the inbound SA of a CHILD_SA during rekeying - */ - CHILD_INSTALLED_INBOUND, - /** * Installed both SAs of a CHILD_SA */ diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index db57ee815..896cabb2b 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -745,14 +745,7 @@ static status_t select_and_install(private_child_create_t *this, charon->bus->child_keys(charon->bus, this->child_sa, this->initiator, this->dh, nonce_i, nonce_r); - if (this->rekey && !this->initiator) - { - this->child_sa->set_state(this->child_sa, CHILD_INSTALLED_INBOUND); - } - else - { - this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); - } + this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); this->ike_sa->add_child_sa(this->ike_sa, this->child_sa); this->established = TRUE; diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index 6ab59962f..0954b7b94 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -271,7 +271,6 @@ static void process_payloads(private_child_delete_t *this, message_t *message) break; case CHILD_REKEYING: /* we reply as usual, rekeying will fail */ - case CHILD_INSTALLED_INBOUND: case CHILD_INSTALLED: if (!this->initiator) { diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index afc4644a2..5a703bffb 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -476,8 +476,7 @@ METHOD(child_rekey_t, collide, void, /* 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_child->get_state(other_child) != CHILD_INSTALLED_INBOUND)) + other_child->get_state(other_child) != CHILD_INSTALLED) { other->destroy(other); return; diff --git a/src/libcharon/tests/suites/test_child_rekey.c b/src/libcharon/tests/suites/test_child_rekey.c index 19e5f784a..4ac27405d 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -62,7 +62,7 @@ START_TEST(test_regular) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_REKEYED); - assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -70,14 +70,14 @@ START_TEST(test_regular) assert_no_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, spi_a, CHILD_DELETING); - assert_child_sa_state(a, 3, CHILD_INSTALLED); + assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* INFORMATIONAL { D } --> */ assert_hook_not_called(child_rekey); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); assert_hook(); /* <-- INFORMATIONAL { D } */ @@ -150,7 +150,7 @@ START_TEST(test_regular_ke_invalid) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_REKEYED); - assert_child_sa_state(b, 6, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -158,14 +158,14 @@ START_TEST(test_regular_ke_invalid) assert_no_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, spi_a, CHILD_DELETING); - assert_child_sa_state(a, 5, CHILD_INSTALLED); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* INFORMATIONAL { D } --> */ assert_hook_not_called(child_rekey); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 6, CHILD_INSTALLED); + assert_child_sa_state(b, 6, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); assert_hook(); /* <-- INFORMATIONAL { D } */ @@ -204,7 +204,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -212,7 +212,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) assert_no_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_DELETING); - assert_child_sa_state(a, 3, CHILD_INSTALLED); + assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* we don't expect this to get called anymore */ @@ -225,7 +225,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) /* INFORMATIONAL { D } --> */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); /* <-- INFORMATIONAL { D } */ assert_single_payload(IN, PLV2_DELETE); @@ -263,7 +263,7 @@ START_TEST(test_regular_responder_handle_hard_expire) assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -271,7 +271,7 @@ START_TEST(test_regular_responder_handle_hard_expire) assert_no_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_DELETING); - assert_child_sa_state(a, 3, CHILD_INSTALLED); + assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* we don't expect this to get called anymore */ @@ -284,12 +284,12 @@ START_TEST(test_regular_responder_handle_hard_expire) /* INFORMATIONAL { D } --> */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_child_sa_state(a, 2, CHILD_DELETING); /* <-- INFORMATIONAL { D } */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 3, CHILD_INSTALLED); + assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 1, CHILD_DELETING); /* <-- INFORMATIONAL { } */ assert_message_empty(IN); @@ -361,14 +361,14 @@ START_TEST(test_collision) assert_hook_rekey(child_rekey, 2, 5); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 5, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 6); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 6, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -387,9 +387,9 @@ START_TEST(test_collision) } assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); - assert_child_sa_state(a, data[_i].spi_a, - data[_i].spi_del_a == 1 ? CHILD_INSTALLED - : CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_INSTALLED + : CHILD_OUTBOUND_REGISTERED); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { @@ -405,9 +405,9 @@ START_TEST(test_collision) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, - data[_i].spi_del_b == 2 ? CHILD_INSTALLED - : CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED + : CHILD_OUTBOUND_REGISTERED); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); @@ -498,14 +498,14 @@ START_TEST(test_collision_delayed_response) assert_hook_rekey(child_rekey, 2, 5); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 5, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 6); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 6, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* delay the CREATE_CHILD_SA response from b to a */ @@ -526,9 +526,9 @@ START_TEST(test_collision_delayed_response) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, - data[_i].spi_del_b == 2 ? CHILD_INSTALLED - : CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED + : CHILD_OUTBOUND_REGISTERED); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); @@ -546,9 +546,9 @@ START_TEST(test_collision_delayed_response) /* INFORMATIONAL { D } --> */ exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, - data[_i].spi_del_b == 2 ? CHILD_INSTALLED - : CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED + : CHILD_OUTBOUND_REGISTERED); assert_child_sa_count(b, 2); assert_hook(); @@ -643,13 +643,13 @@ START_TEST(test_collision_delayed_request) assert_hook_rekey(child_rekey, 1, 5); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 5, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_DELETING); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -663,7 +663,7 @@ START_TEST(test_collision_delayed_request) /* <-- INFORMATIONAL { D } */ exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 5, CHILD_INSTALLED); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); /* <-- CREATE_CHILD_SA { N(TEMP_FAIL) } */ @@ -744,13 +744,13 @@ START_TEST(test_collision_delayed_request_more) assert_hook_rekey(child_rekey, 1, 5); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 5, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_DELETING); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -758,7 +758,7 @@ START_TEST(test_collision_delayed_request_more) /* <-- INFORMATIONAL { D } */ exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 5, CHILD_INSTALLED); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); /* INFORMATIONAL { D } --> */ exchange_test_helper->process_message(exchange_test_helper, b, NULL); @@ -882,14 +882,14 @@ START_TEST(test_collision_ke_invalid) assert_hook_rekey(child_rekey, 2, 9); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); - assert_child_sa_state(b, 9, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, 9, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 10); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a,10, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a,10, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -906,9 +906,9 @@ START_TEST(test_collision_ke_invalid) } assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); - assert_child_sa_state(a, data[_i].spi_a, - data[_i].spi_del_a == 1 ? CHILD_INSTALLED - : CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_INSTALLED + : CHILD_OUTBOUND_REGISTERED); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { @@ -922,9 +922,9 @@ START_TEST(test_collision_ke_invalid) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, - data[_i].spi_del_b == 2 ? CHILD_INSTALLED - : CHILD_INSTALLED_INBOUND); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED + : CHILD_OUTBOUND_REGISTERED); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); @@ -1051,7 +1051,7 @@ START_TEST(test_collision_ke_invalid_delayed_retry) assert_hook_rekey(child_rekey, 1, 9); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_state(a, 9, CHILD_INSTALLED_INBOUND); + assert_child_sa_state(a, 9, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 8); @@ -1071,7 +1071,7 @@ START_TEST(test_collision_ke_invalid_delayed_retry) /* <-- INFORMATIONAL { D } */ exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 9, CHILD_INSTALLED); + assert_child_sa_state(a, 9, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); /* <-- CREATE_CHILD_SA { N(TEMP_FAIL) } */ From ba0796fe75b1a8b6e23ff8543058baa909beae8f Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 15:56:02 +0100 Subject: [PATCH 15/20] delete-child-sa-job: Add new constructor that takes the unique ID of a CHILD_SA This makes sure we delete the right SA in case the addresses got updated in the mean time. --- .../processing/jobs/delete_child_sa_job.c | 69 ++++++++++++++++--- .../processing/jobs/delete_child_sa_job.h | 13 +++- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/libcharon/processing/jobs/delete_child_sa_job.c b/src/libcharon/processing/jobs/delete_child_sa_job.c index 70dbc1b4a..048b879f1 100644 --- a/src/libcharon/processing/jobs/delete_child_sa_job.c +++ b/src/libcharon/processing/jobs/delete_child_sa_job.c @@ -1,6 +1,7 @@ /* + * Copyright (C) 2017 Tobias Brunner * Copyright (C) 2006 Martin Willi - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -24,19 +25,19 @@ typedef struct private_delete_child_sa_job_t private_delete_child_sa_job_t; * Private data of an delete_child_sa_job_t object. */ struct private_delete_child_sa_job_t { - /** + /** * Public delete_child_sa_job_t interface. */ delete_child_sa_job_t public; /** - * protocol of the CHILD_SA (ESP/AH) + * Protocol of the CHILD_SA (ESP/AH) */ protocol_id_t protocol; /** - * inbound SPI of the CHILD_SA + * Inbound SPI of the CHILD_SA */ uint32_t spi; @@ -49,12 +50,17 @@ struct private_delete_child_sa_job_t { * Delete for an expired CHILD_SA */ bool expired; + + /** + * Unique ID of the CHILD_SA + */ + uint32_t id; }; METHOD(job_t, destroy, void, private_delete_child_sa_job_t *this) { - this->dst->destroy(this->dst); + DESTROY_IF(this->dst); free(this); } @@ -63,17 +69,37 @@ METHOD(job_t, execute, job_requeue_t, { ike_sa_t *ike_sa; - ike_sa = charon->child_sa_manager->checkout(charon->child_sa_manager, - this->protocol, this->spi, this->dst, NULL); - if (ike_sa == NULL) + if (this->id) { - DBG1(DBG_JOB, "CHILD_SA %N/0x%08x/%H not found for delete", - protocol_id_names, this->protocol, htonl(this->spi), this->dst); + child_sa_t *child_sa; + + ike_sa = charon->child_sa_manager->checkout_by_id( + charon->child_sa_manager, this->id, &child_sa); + if (!ike_sa) + { + DBG1(DBG_JOB, "CHILD_SA {%d} not found for delete", this->id); + } + else + { + this->spi = child_sa->get_spi(child_sa, TRUE); + this->protocol = child_sa->get_protocol(child_sa); + } } else { - ike_sa->delete_child_sa(ike_sa, this->protocol, this->spi, this->expired); + ike_sa = charon->child_sa_manager->checkout(charon->child_sa_manager, + this->protocol, this->spi, this->dst, NULL); + if (!ike_sa) + { + DBG1(DBG_JOB, "CHILD_SA %N/0x%08x/%H not found for delete", + protocol_id_names, this->protocol, htonl(this->spi), this->dst); + } + } + if (ike_sa) + { + ike_sa->delete_child_sa(ike_sa, this->protocol, this->spi, + this->expired); charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); } return JOB_REQUEUE_NONE; @@ -109,3 +135,24 @@ delete_child_sa_job_t *delete_child_sa_job_create(protocol_id_t protocol, return &this->public; } + +/* + * Described in header + */ +delete_child_sa_job_t *delete_child_sa_job_create_id(uint32_t id) +{ + private_delete_child_sa_job_t *this; + + INIT(this, + .public = { + .job_interface = { + .execute = _execute, + .get_priority = _get_priority, + .destroy = _destroy, + }, + }, + .id = id, + ); + + return &this->public; +} diff --git a/src/libcharon/processing/jobs/delete_child_sa_job.h b/src/libcharon/processing/jobs/delete_child_sa_job.h index 349f5debb..b2d5a11f6 100644 --- a/src/libcharon/processing/jobs/delete_child_sa_job.h +++ b/src/libcharon/processing/jobs/delete_child_sa_job.h @@ -1,6 +1,7 @@ /* + * Copyright (C) 2017 Tobias Brunner * Copyright (C) 2006 Martin Willi - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -42,7 +43,7 @@ struct delete_child_sa_job_t { }; /** - * Creates a job of type DELETE_CHILD_SA. + * Creates a job that deletes a CHILD_SA. * * @param protocol protocol of the CHILD_SA * @param spi security parameter index of the CHILD_SA @@ -53,4 +54,12 @@ struct delete_child_sa_job_t { delete_child_sa_job_t *delete_child_sa_job_create(protocol_id_t protocol, uint32_t spi, host_t *dst, bool expired); +/** + * Creates a job that deletes a CHILD_SA identified by its unique ID. + * + * @param id unique ID of the CHILD_SA + * @return delete_child_sa_job_t object + */ +delete_child_sa_job_t *delete_child_sa_job_create_id(uint32_t id); + #endif /** DELETE_CHILD_SA_JOB_H_ @}*/ From 44107cb7b75551e02afd56061534495c10b94de3 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 16:03:54 +0100 Subject: [PATCH 16/20] child-delete: Delay the removal of the inbound SA of rekeyed CHILD_SAs After deleting a rekeyed CHILD_SA we uninstall the outbound SA but don't destroy the CHILD_SA (and the inbound SA) immediately. We delay it a few seconds or until the SA expires to allow delayed packets to get processed. The CHILD_SA remains in state CHILD_DELETING until it finally gets destroyed. --- conf/options/charon.opt | 10 + src/libcharon/sa/ikev2/tasks/child_delete.c | 44 +- src/libcharon/sa/ikev2/tasks/child_rekey.c | 12 +- src/libcharon/tests/suites/test_child_rekey.c | 484 +++++++++++++----- 4 files changed, 422 insertions(+), 128 deletions(-) diff --git a/conf/options/charon.opt b/conf/options/charon.opt index a5f03f272..3593c6a5f 100644 --- a/conf/options/charon.opt +++ b/conf/options/charon.opt @@ -75,6 +75,16 @@ charon.delete_rekeyed = no However, this might cause problems with implementations that continue to use rekeyed SAs until they expire. +charon.delete_rekeyed_delay = 5 + Delay in seconds until inbound IPsec SAs are deleted after rekeyings (IKEv2 + only). + + Delay in seconds until inbound IPsec SAs are deleted after rekeyings (IKEv2 + only). To process delayed packets the inbound part of a CHILD_SA is kept + installed up to the configured number of seconds after it got replaced + during a rekeying. If set to 0 the CHILD_SA will be kept installed until it + expires (if no lifetime is set it will be destroyed immediately). + charon.dh_exponent_ansi_x9_42 = yes Use ANSI X9.42 DH exponent size or optimum size matched to cryptographic strength. diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index 0954b7b94..160865636 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -18,9 +18,14 @@ #include #include +#include #include #include +#ifndef DELETE_REKEYED_DELAY +#define DELETE_REKEYED_DELAY 5 +#endif + typedef struct private_child_delete_t private_child_delete_t; /** @@ -308,6 +313,12 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) uint32_t spi, reqid, rekey_spi; action_t action; status_t status = SUCCESS; + time_t now, expire; + u_int delay; + + now = time_monotonic(NULL); + delay = lib->settings->get_int(lib->settings, "%s.delete_rekeyed_delay", + DELETE_REKEYED_DELAY, lib->ns); enumerator = this->child_sas->create_enumerator(this->child_sas); while (enumerator->enumerate(enumerator, (void**)&entry)) @@ -326,13 +337,32 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) { install_outbound(this, protocol, rekey_spi); } + /* for rekeyed CHILD_SAs we uninstall the outbound SA but don't + * immediately destroy it, by default, so we can process delayed + * packets */ + child_sa->remove_outbound(child_sa); + expire = child_sa->get_lifetime(child_sa, TRUE); + if (delay && (!expire || ((now + delay) < expire))) + { + lib->scheduler->schedule_job(lib->scheduler, + (job_t*)delete_child_sa_job_create_id( + child_sa->get_unique_id(child_sa)), delay); + continue; + } + else if (expire) + { /* let it expire naturally */ + continue; + } + /* 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); action = child_sa->get_close_action(child_sa); + this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi); + if (entry->check_delete_action) { /* enforce child_cfg policy if deleted passively */ switch (action) @@ -425,14 +455,20 @@ METHOD(task_t, build_i, status_t, /* we work only with the inbound SPI */ this->spi = child_sa->get_spi(child_sa, TRUE); } + + if (child_sa->get_state(child_sa) == CHILD_DELETING) + { /* DELETEs for this CHILD_SA were already exchanged, but it was not yet + * destroyed to allow delayed packets to get processed */ + this->ike_sa->destroy_child_sa(this->ike_sa, this->protocol, this->spi); + message->set_exchange_type(message, EXCHANGE_TYPE_UNDEFINED); + return SUCCESS; + } + INIT(entry, .child_sa = child_sa, + .rekeyed = child_sa->get_state(child_sa) == CHILD_REKEYED, ); this->child_sas->insert_last(this->child_sas, entry); - if (child_sa->get_state(child_sa) == CHILD_REKEYED) - { - entry->rekeyed = TRUE; - } log_children(this); build_payloads(this, message); diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index 5a703bffb..761c860e7 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -132,6 +132,7 @@ static void find_child(private_child_rekey_t *this, message_t *message) notify_payload_t *notify; protocol_id_t protocol; uint32_t spi; + child_sa_t *child_sa; notify = message->get_notify(message, REKEY_SA); if (notify) @@ -141,8 +142,15 @@ static void find_child(private_child_rekey_t *this, message_t *message) if (protocol == PROTO_ESP || protocol == PROTO_AH) { - this->child_sa = this->ike_sa->get_child_sa(this->ike_sa, protocol, - spi, FALSE); + child_sa = this->ike_sa->get_child_sa(this->ike_sa, protocol, + spi, FALSE); + if (child_sa && + child_sa->get_state(child_sa) == CHILD_DELETING && + child_sa->get_outbound_state(child_sa) == CHILD_OUTBOUND_NONE) + { /* ignore rekeyed CHILD_SAs we keep around */ + return; + } + this->child_sa = child_sa; } } } diff --git a/src/libcharon/tests/suites/test_child_rekey.c b/src/libcharon/tests/suites/test_child_rekey.c index 4ac27405d..1c100b624 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2017 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -28,7 +28,23 @@ assert_hook_not_called(child_updown); \ assert_hook_not_called(child_rekey); \ call_ikesa(sa, rekey_child_sa, PROTO_ESP, spi); \ - assert_child_sa_state(sa, spi, CHILD_REKEYING); \ + assert_child_sa_state(sa, spi, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); \ + assert_hook(); \ + assert_hook(); \ +}) + +/** + * Destroy a rekeyed CHILD_SA that was kept around to accept inbound traffic. + * Simulates the job that's scheduled to do this. + */ +#define destroy_rekeyed(sa, spi) ({ \ + assert_hook_not_called(child_updown); \ + assert_hook_not_called(child_rekey); \ + assert_no_jobs_scheduled(); \ + assert_child_sa_state(sa, spi, CHILD_DELETING, CHILD_OUTBOUND_NONE); \ + call_ikesa(sa, delete_child_sa, PROTO_ESP, spi, FALSE); \ + assert_child_sa_not_exists(sa, spi); \ + assert_scheduler(); \ assert_hook(); \ assert_hook(); \ }) @@ -61,7 +77,7 @@ START_TEST(test_regular) assert_hook_called(child_rekey); assert_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, spi_b, CHILD_REKEYED); + assert_child_sa_state(b, spi_b, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); @@ -69,25 +85,37 @@ START_TEST(test_regular) assert_hook_called(child_rekey); assert_no_notify(IN, REKEY_SA); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, spi_a, CHILD_DELETING); + assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* INFORMATIONAL { D } --> */ assert_hook_not_called(child_rekey); + assert_jobs_scheduled(1); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(b, 1); + assert_child_sa_count(b, 2); + assert_scheduler(); assert_hook(); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); + assert_jobs_scheduled(1); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 3, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 2); + assert_scheduler(); assert_hook(); + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, spi_a); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, spi_b); + assert_child_sa_count(b, 1); + /* child_updown */ assert_hook(); @@ -165,17 +193,25 @@ START_TEST(test_regular_ke_invalid) assert_hook_not_called(child_rekey); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 6, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(b, 1); + assert_child_sa_count(b, 2); assert_hook(); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_count(a, 2); assert_hook(); + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, spi_a); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, spi_b); + assert_child_sa_count(b, 1); + /* child_updown */ assert_hook(); @@ -223,15 +259,27 @@ START_TEST(test_regular_responder_ignore_soft_expire) assert_child_sa_state(b, 2, CHILD_REKEYED); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(b, 1); + assert_child_sa_count(b, 2); + assert_scheduler(); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED); + assert_child_sa_count(a, 2); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, 1); assert_child_sa_count(a, 1); + destroy_rekeyed(b, 2); + assert_child_sa_count(b, 1); /* child_rekey/child_updown */ assert_hook(); @@ -279,27 +327,40 @@ START_TEST(test_regular_responder_handle_hard_expire) /* this is similar to a regular delete collision */ assert_single_payload(OUT, PLV2_DELETE); call_ikesa(b, delete_child_sa, PROTO_ESP, 2, TRUE); - assert_child_sa_state(b, 2, CHILD_DELETING); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); /* INFORMATIONAL { D } --> */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(a, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); - assert_child_sa_state(a, 2, CHILD_DELETING); /* <-- INFORMATIONAL { D } */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_state(a, 1, CHILD_DELETING); /* <-- INFORMATIONAL { } */ + assert_jobs_scheduled(1); assert_message_empty(IN); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 3, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 2); + assert_scheduler(); /* INFORMATIONAL { } --> */ + assert_jobs_scheduled(1); assert_message_empty(IN); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 2); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, 1); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); /* child_rekey/child_updown */ @@ -360,14 +421,14 @@ START_TEST(test_collision) exchange_test_helper->nonce_first_byte = data[_i].nonces[2]; assert_hook_rekey(child_rekey, 2, 5); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYED); + assert_child_sa_state(b, 2, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 6); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYED); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); @@ -378,56 +439,100 @@ START_TEST(test_collision) assert_hook_rekey(child_rekey, 1, data[_i].spi_a); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_hook(); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED, + CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); } else { assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_hook(); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_REGISTERED); } - assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); - assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, - data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_INSTALLED - : CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { assert_hook_rekey(child_rekey, 2, data[_i].spi_b); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_hook(); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); } else { assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_hook(); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_REGISTERED); } - assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); - assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, - data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED - : CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); - assert_child_sa_count(b, 2); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 3); + assert_scheduler(); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); - assert_child_sa_count(a, 2); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 3); + assert_scheduler(); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 3); + assert_scheduler(); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 3); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, data[_i].spi_del_a); + destroy_rekeyed(a, data[_i].spi_del_b); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, data[_i].spi_del_a); + destroy_rekeyed(b, data[_i].spi_del_b); assert_child_sa_count(b, 1); /* child_rekey/child_updown */ @@ -497,14 +602,14 @@ START_TEST(test_collision_delayed_response) exchange_test_helper->nonce_first_byte = data[_i].nonces[2]; assert_hook_rekey(child_rekey, 2, 5); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYED); + assert_child_sa_state(b, 2, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 6); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYED); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); @@ -517,39 +622,64 @@ START_TEST(test_collision_delayed_response) assert_hook_rekey(child_rekey, 2, data[_i].spi_b); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_hook(); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); } else { assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_hook(); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_REGISTERED); } - assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); - assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, - data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED - : CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); if (data[_i].spi_del_b == 2) { - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 2); } else { - assert_child_sa_state(a, 1, CHILD_REKEYED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_count(a, 2); } + assert_scheduler(); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, - data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED - : CHILD_OUTBOUND_REGISTERED); - assert_child_sa_count(b, 2); + if (data[_i].spi_del_b == 2) + { + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + } + else + { + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_REGISTERED); + } + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_count(b, 3); + assert_scheduler(); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } (delayed) */ @@ -565,20 +695,46 @@ START_TEST(test_collision_delayed_response) exchange_test_helper->process_message(exchange_test_helper, a, msg); assert_hook(); } - assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); - assert_child_sa_count(a, 2); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 3); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); - assert_child_sa_count(b, 1); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 3); + assert_scheduler(); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 3); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, data[_i].spi_del_a); + destroy_rekeyed(a, data[_i].spi_del_b); assert_child_sa_count(a, 1); + destroy_rekeyed(b, data[_i].spi_del_a); + destroy_rekeyed(b, data[_i].spi_del_b); + assert_child_sa_count(b, 1); /* child_rekey/child_updown */ assert_hook(); @@ -642,13 +798,13 @@ START_TEST(test_collision_delayed_request) exchange_test_helper->nonce_first_byte = data[_i].nonces[2]; assert_hook_rekey(child_rekey, 1, 5); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYED); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_DELETING); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); @@ -658,24 +814,37 @@ START_TEST(test_collision_delayed_request) /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> (delayed) */ assert_single_notify(OUT, TEMPORARY_FAILURE); exchange_test_helper->process_message(exchange_test_helper, b, msg); - assert_child_sa_state(b, 2, CHILD_DELETING); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_count(a, 2); + assert_scheduler(); /* <-- CREATE_CHILD_SA { N(TEMP_FAIL) } */ assert_no_jobs_scheduled(); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 5, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 2); assert_scheduler(); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 2); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, 1); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); /* child_rekey/child_updown */ @@ -743,13 +912,13 @@ START_TEST(test_collision_delayed_request_more) exchange_test_helper->nonce_first_byte = data[_i].nonces[2]; assert_hook_rekey(child_rekey, 1, 5); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYED); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_DELETING); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); @@ -757,26 +926,40 @@ START_TEST(test_collision_delayed_request_more) assert_hook_not_called(child_rekey); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_count(a, 2); + assert_scheduler(); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 4, CHILD_INSTALLED); - assert_child_sa_count(b, 1); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 2); + assert_scheduler(); /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> */ assert_single_notify(OUT, CHILD_SA_NOT_FOUND); exchange_test_helper->process_message(exchange_test_helper, b, msg); - assert_child_sa_state(b, 4, CHILD_INSTALLED); - assert_child_sa_count(b, 1); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 2); /* <-- CREATE_CHILD_SA { N(NO_CHILD_SA) } */ assert_no_jobs_scheduled(); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 5, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 2); assert_scheduler(); + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, 1); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, 2); + assert_child_sa_count(b, 1); + /* child_rekey/child_updown */ assert_hook(); assert_hook(); @@ -850,13 +1033,13 @@ START_TEST(test_collision_ke_invalid) /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> */ assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYING); + assert_child_sa_state(b, 2, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYING); + assert_child_sa_state(a, 1, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); assert_hook(); @@ -865,7 +1048,7 @@ START_TEST(test_collision_ke_invalid) assert_hook_not_called(child_rekey); assert_single_notify(IN, INVALID_KE_PAYLOAD); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYING); + assert_child_sa_state(a, 1, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); assert_hook(); /* CREATE_CHILD_SA { N(INVAL_KE) } --> */ @@ -873,7 +1056,7 @@ START_TEST(test_collision_ke_invalid) assert_hook_not_called(child_rekey); assert_single_notify(IN, INVALID_KE_PAYLOAD); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYING); + assert_child_sa_state(b, 2, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); assert_hook(); @@ -881,14 +1064,14 @@ START_TEST(test_collision_ke_invalid) exchange_test_helper->nonce_first_byte = data[_i].nonces[2]; assert_hook_rekey(child_rekey, 2, 9); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYED); + assert_child_sa_state(b, 2, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 9, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; assert_hook_rekey(child_rekey, 1, 10); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYED); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a,10, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); @@ -899,52 +1082,96 @@ START_TEST(test_collision_ke_invalid) assert_hook_rekey(child_rekey, 1, data[_i].spi_a); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_hook(); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED, + CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); } else { exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_REGISTERED); } - assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); - assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, - data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_INSTALLED - : CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { assert_hook_rekey(child_rekey, 2, data[_i].spi_b); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_hook(); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); } else { exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_REGISTERED); } - assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); - assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, - data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED - : CHILD_OUTBOUND_REGISTERED); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); - assert_child_sa_count(b, 2); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 3); + assert_scheduler(); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); - assert_child_sa_count(a, 2); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 3); + assert_scheduler(); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 3); + assert_scheduler(); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); + assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETING, + CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, + CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 3); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, data[_i].spi_del_a); + destroy_rekeyed(a, data[_i].spi_del_b); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, data[_i].spi_del_a); + destroy_rekeyed(b, data[_i].spi_del_b); assert_child_sa_count(b, 1); /* child_rekey/child_updown */ @@ -1016,13 +1243,13 @@ START_TEST(test_collision_ke_invalid_delayed_retry) /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> */ assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYING); + assert_child_sa_state(b, 2, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ assert_hook_not_called(child_rekey); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYING); + assert_child_sa_state(a, 1, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); assert_hook(); @@ -1031,7 +1258,7 @@ START_TEST(test_collision_ke_invalid_delayed_retry) assert_hook_not_called(child_rekey); assert_single_notify(IN, INVALID_KE_PAYLOAD); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYING); + assert_child_sa_state(a, 1, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 1); assert_hook(); /* CREATE_CHILD_SA { N(INVAL_KE) } --> */ @@ -1039,7 +1266,7 @@ START_TEST(test_collision_ke_invalid_delayed_retry) assert_hook_not_called(child_rekey); assert_single_notify(IN, INVALID_KE_PAYLOAD); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_REKEYING); + assert_child_sa_state(b, 2, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 1); assert_hook(); @@ -1050,14 +1277,14 @@ START_TEST(test_collision_ke_invalid_delayed_retry) exchange_test_helper->nonce_first_byte = data[_i].nonces[2]; assert_hook_rekey(child_rekey, 1, 9); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 1, CHILD_REKEYED); + assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 9, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 8); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 2, CHILD_DELETING); - assert_child_sa_state(b, 8, CHILD_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -1066,24 +1293,37 @@ START_TEST(test_collision_ke_invalid_delayed_retry) /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> (delayed) */ assert_single_notify(OUT, TEMPORARY_FAILURE); exchange_test_helper->process_message(exchange_test_helper, b, msg); - assert_child_sa_state(b, 2, CHILD_DELETING); - assert_child_sa_state(b, 8, CHILD_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); /* <-- INFORMATIONAL { D } */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 9, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_count(a, 2); + assert_scheduler(); /* <-- CREATE_CHILD_SA { N(TEMP_FAIL) } */ assert_no_jobs_scheduled(); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, 9, CHILD_INSTALLED); - assert_child_sa_count(a, 1); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, 9, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(a, 2); assert_scheduler(); /* INFORMATIONAL { D } --> */ + assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, 8, CHILD_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 2); + assert_scheduler(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, 1); + assert_child_sa_count(a, 1); + destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); /* child_rekey/child_updown */ @@ -1126,7 +1366,7 @@ START_TEST(test_collision_delete) } initiate_rekey(a, spi_a); call_ikesa(b, delete_child_sa, PROTO_ESP, spi_b, FALSE); - assert_child_sa_state(b, spi_b, CHILD_DELETING); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); /* this should never get called as there is no successful rekeying on * either side */ @@ -1141,7 +1381,7 @@ START_TEST(test_collision_delete) assert_notify(IN, REKEY_SA); assert_single_notify(OUT, TEMPORARY_FAILURE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, spi_b, CHILD_DELETING); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* RFC 7296, 2.25.1: If a peer receives a request to delete a CHILD_SA that @@ -1213,7 +1453,7 @@ START_TEST(test_collision_delete_drop_delete) } initiate_rekey(a, spi_a); call_ikesa(b, delete_child_sa, PROTO_ESP, spi_b, FALSE); - assert_child_sa_state(b, spi_b, CHILD_DELETING); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); /* this should never get called as there is no successful rekeying on * either side */ @@ -1228,7 +1468,7 @@ START_TEST(test_collision_delete_drop_delete) assert_notify(IN, REKEY_SA); assert_single_notify(OUT, TEMPORARY_FAILURE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(b, spi_b, CHILD_DELETING); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_hook(); /* delay the DELETE request */ @@ -1239,7 +1479,7 @@ START_TEST(test_collision_delete_drop_delete) /* we expect a job to retry the rekeying is scheduled */ assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, spi_a, CHILD_INSTALLED); + assert_child_sa_state(a, spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_scheduler(); assert_hook(); @@ -1298,7 +1538,7 @@ END_TEST } initiate_rekey(a, spi_a); call_ikesa(b, delete_child_sa, PROTO_ESP, spi_b, FALSE); - assert_child_sa_state(b, spi_b, CHILD_DELETING); + assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); /* this should never get called as there is no successful rekeying on * either side */ @@ -1431,13 +1671,13 @@ START_TEST(test_collision_ike_rekey) /* <-- CREATE_CHILD_SA { SA, Ni, KEi } */ assert_single_notify(OUT, TEMPORARY_FAILURE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, spi_a, CHILD_REKEYING); + assert_child_sa_state(a, spi_a, CHILD_REKEYING, CHILD_OUTBOUND_INSTALLED); /* <-- CREATE_CHILD_SA { N(TEMP_FAIL) } */ /* we expect a job to retry the rekeying is scheduled */ assert_jobs_scheduled(1); exchange_test_helper->process_message(exchange_test_helper, a, NULL); - assert_child_sa_state(a, spi_a, CHILD_INSTALLED); + assert_child_sa_state(a, spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_scheduler(); /* CREATE_CHILD_SA { N(TEMP_FAIL) } --> */ From d80055baae6719a28fe1ec872e35086dd3f23870 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 8 Mar 2017 15:45:41 +0100 Subject: [PATCH 17/20] unit-tests: Keep track of installed IPsec SAs in mock kernel_ipsec_t implementation --- src/libcharon/tests/utils/mock_ipsec.c | 129 ++++++++++++++++++++++++- src/libcharon/tests/utils/mock_ipsec.h | 11 ++- 2 files changed, 136 insertions(+), 4 deletions(-) diff --git a/src/libcharon/tests/utils/mock_ipsec.c b/src/libcharon/tests/utils/mock_ipsec.c index d57a26a87..c11f5c966 100644 --- a/src/libcharon/tests/utils/mock_ipsec.c +++ b/src/libcharon/tests/utils/mock_ipsec.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2017 Tobias Brunner * Copyright (C) 2008 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -16,6 +16,11 @@ #include "mock_ipsec.h" +#include +#include + +#include + typedef struct private_kernel_ipsec_t private_kernel_ipsec_t; /** @@ -32,13 +37,72 @@ struct private_kernel_ipsec_t { * Allocated SPI */ refcount_t spi; + + /** + * Installed SAs + */ + hashtable_t *sas; }; +/** + * Global instance + */ +static private_kernel_ipsec_t *instance; + +/** + * Data about installed IPsec SAs + */ +typedef struct { + /** + * SPI of the SA + */ + uint32_t spi; + + /** + * Associated IKE_SA + */ + ike_sa_t *ike_sa; + + /** + * TRUE if this was an allocated SPI + */ + bool alloc; + +} entry_t; + +/** + * Hash an IPsec SA entry + */ +static u_int entry_hash(const void *key) +{ + entry_t *entry = (entry_t*)key; + return chunk_hash_inc(chunk_from_thing(entry->spi), + chunk_hash(chunk_from_thing(entry->ike_sa))); +} + +/** + * Compare an IPsec SA entry + */ +static bool entry_equals(const void *key, const void *other_key) +{ + entry_t *a = (entry_t*)key, *b = (entry_t*)other_key; + return a->spi == b->spi && a->ike_sa == b->ike_sa; +} + METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_ipsec_t *this, host_t *src, host_t *dst, uint8_t protocol, uint32_t *spi) { + entry_t *entry; + *spi = (uint32_t)ref_get(&this->spi); + INIT(entry, + .spi = *spi, + .ike_sa = charon->bus->get_sa(charon->bus), + .alloc = TRUE, + ); + entry = this->sas->put(this->sas, entry, entry); + assert(!entry); return SUCCESS; } @@ -52,6 +116,23 @@ METHOD(kernel_ipsec_t, add_sa, status_t, private_kernel_ipsec_t *this, kernel_ipsec_sa_id_t *id, kernel_ipsec_add_sa_t *data) { + entry_t *entry; + + INIT(entry, + .spi = id->spi, + .ike_sa = charon->bus->get_sa(charon->bus), + ); + if (data->inbound) + { + entry = this->sas->put(this->sas, entry, entry); + assert(entry && entry->alloc); + free(entry); + } + else + { + entry = this->sas->put(this->sas, entry, entry); + assert(!entry); + } return SUCCESS; } @@ -74,6 +155,14 @@ METHOD(kernel_ipsec_t, del_sa, status_t, private_kernel_ipsec_t *this, kernel_ipsec_sa_id_t *id, kernel_ipsec_del_sa_t *data) { + entry_t *entry, lookup = { + .spi = id->spi, + .ike_sa = charon->bus->get_sa(charon->bus), + }; + + entry = this->sas->remove(this->sas, &lookup); + assert(entry); + free(entry); return SUCCESS; } @@ -99,6 +188,13 @@ METHOD(kernel_ipsec_t, del_policy, status_t, return SUCCESS; } +METHOD(kernel_ipsec_t, destroy, void, + private_kernel_ipsec_t *this) +{ + this->sas->destroy(this->sas); + free(this); +} + /* * Described in header */ @@ -121,8 +217,37 @@ kernel_ipsec_t *mock_ipsec_create() .flush_policies = (void*)return_failed, .bypass_socket = (void*)return_true, .enable_udp_decap = (void*)return_true, - .destroy = (void*)free, + .destroy = _destroy, }, + .sas = hashtable_create(entry_hash, entry_equals, 8), ); + + instance = this; + return &this->public; } + +/** + * Filter SAs + */ +static bool filter_sas(void *data, entry_t **entry, ike_sa_t **ike_sa, + void *unused, uint32_t *spi) +{ + if ((*entry)->alloc) + { + return FALSE; + } + *ike_sa = (*entry)->ike_sa; + *spi = (*entry)->spi; + return TRUE; +} + +/* + * Described in header + */ +enumerator_t *mock_ipsec_create_sa_enumerator() +{ + return enumerator_create_filter( + instance->sas->create_enumerator(instance->sas), + (void*)filter_sas, NULL, NULL); +} diff --git a/src/libcharon/tests/utils/mock_ipsec.h b/src/libcharon/tests/utils/mock_ipsec.h index cbf21524a..95038a561 100644 --- a/src/libcharon/tests/utils/mock_ipsec.h +++ b/src/libcharon/tests/utils/mock_ipsec.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2017 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -15,7 +15,7 @@ /** * kernel_ipsec_t implementation used for exchange unit tests. Currently - * returns sequential SPIs, all other methods are noops. + * returns sequential SPIs, and keeps track of installed SAs. * * @defgroup mock_ipsec mock_ipsec * @{ @ingroup test_utils_c @@ -33,4 +33,11 @@ */ kernel_ipsec_t *mock_ipsec_create(); +/** + * Enumerate the installed SAs + * + * @return enumerator over (ike_sa_t*, uint32_t) + */ +enumerator_t *mock_ipsec_create_sa_enumerator(); + #endif /** MOCK_IPSEC_H_ @}*/ From 2b581b59f069828f26542cc6b5df33482a36e9ac Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 9 Mar 2017 10:59:31 +0100 Subject: [PATCH 18/20] unit-tests: Migrate cached IPsec SAs to new IKE_SAs during rekeying --- src/libcharon/tests/utils/mock_ipsec.c | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/libcharon/tests/utils/mock_ipsec.c b/src/libcharon/tests/utils/mock_ipsec.c index c11f5c966..68daaac32 100644 --- a/src/libcharon/tests/utils/mock_ipsec.c +++ b/src/libcharon/tests/utils/mock_ipsec.c @@ -18,6 +18,7 @@ #include #include +#include #include @@ -33,6 +34,11 @@ struct private_kernel_ipsec_t { */ kernel_ipsec_t public; + /** + * Rekey listener + */ + listener_t listener; + /** * Allocated SPI */ @@ -166,6 +172,36 @@ METHOD(kernel_ipsec_t, del_sa, status_t, return SUCCESS; } +METHOD(listener_t, ike_rekey, bool, + listener_t *listener, ike_sa_t *old, ike_sa_t *new) +{ + enumerator_t *enumerator; + array_t *sas = NULL; + entry_t *entry; + + enumerator = instance->sas->create_enumerator(instance->sas); + while (enumerator->enumerate(enumerator, &entry, NULL)) + { + if (entry->ike_sa == old) + { + instance->sas->remove_at(instance->sas, enumerator); + array_insert_create(&sas, ARRAY_TAIL, entry); + } + } + enumerator->destroy(enumerator); + enumerator = array_create_enumerator(sas); + while (enumerator->enumerate(enumerator, &entry)) + { + array_remove_at(sas, enumerator); + entry->ike_sa = new; + entry = instance->sas->put(instance->sas, entry, entry); + assert(!entry); + } + enumerator->destroy(enumerator); + array_destroy(sas); + return TRUE; +} + METHOD(kernel_ipsec_t, add_policy, status_t, private_kernel_ipsec_t *this, kernel_ipsec_policy_id_t *id, kernel_ipsec_manage_policy_t *data) @@ -191,6 +227,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t, METHOD(kernel_ipsec_t, destroy, void, private_kernel_ipsec_t *this) { + charon->bus->remove_listener(charon->bus, &this->listener); this->sas->destroy(this->sas); free(this); } @@ -219,11 +256,16 @@ kernel_ipsec_t *mock_ipsec_create() .enable_udp_decap = (void*)return_true, .destroy = _destroy, }, + .listener = { + .ike_rekey = _ike_rekey, + }, .sas = hashtable_create(entry_hash, entry_equals, 8), ); instance = this; + charon->bus->add_listener(charon->bus, &this->listener); + return &this->public; } From 72655fe411ee0fad4361af16cf914d55b081f149 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 8 Mar 2017 15:46:39 +0100 Subject: [PATCH 19/20] unit-tests: Add assert to check for installed IPsec SAs --- .../tests/utils/exchange_test_asserts.c | 57 ++++++++++++++++- .../tests/utils/exchange_test_asserts.h | 61 ++++++++++++++++++- 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/src/libcharon/tests/utils/exchange_test_asserts.c b/src/libcharon/tests/utils/exchange_test_asserts.c index 2602b97b7..8042d0b63 100644 --- a/src/libcharon/tests/utils/exchange_test_asserts.c +++ b/src/libcharon/tests/utils/exchange_test_asserts.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2017 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -18,6 +18,7 @@ #include #include "exchange_test_asserts.h" +#include "mock_ipsec.h" /* * Described in header @@ -180,3 +181,57 @@ bool exchange_test_asserts_message(listener_t *listener, ike_sa_t *ike_sa, } return TRUE; } + +/** + * Compare two SPIs + */ +static int spis_cmp(const void *a, const void *b) +{ + return *(const uint32_t*)a - *(const uint32_t*)b; +} + +/** + * Compare two SPIs to sort them + */ +static int spis_sort(const void *a, const void *b, void *data) +{ + return spis_cmp(a, b); +} + + +/* + * Described in header + */ +void exchange_test_asserts_ipsec_sas(ipsec_sas_assert_t *sas) +{ + enumerator_t *enumerator; + array_t *spis; + ike_sa_t *ike_sa; + uint32_t spi; + int i; + + spis = array_create(sizeof(uint32_t), 0); + for (i = 0; i < sas->count; i++) + { + array_insert(spis, ARRAY_TAIL, &sas->spis[i]); + } + array_sort(spis, spis_sort, NULL); + + enumerator = mock_ipsec_create_sa_enumerator(); + while (enumerator->enumerate(enumerator, &ike_sa, &spi)) + { + if (ike_sa == sas->ike_sa) + { + i = array_bsearch(spis, &spi, spis_cmp, NULL); + assert_listener_msg(i != -1, sas, "unexpected IPsec SA %.8x", spi); + array_remove(spis, i, NULL); + } + } + enumerator->destroy(enumerator); + for (i = 0; i < array_count(spis); i++) + { + array_get(spis, i, &spi); + assert_listener_msg(!spi, sas, "expected IPsec SA %.8x not found", spi); + } + array_destroy(spis); +} diff --git a/src/libcharon/tests/utils/exchange_test_asserts.h b/src/libcharon/tests/utils/exchange_test_asserts.h index 32afcc2e4..4d363edfd 100644 --- a/src/libcharon/tests/utils/exchange_test_asserts.h +++ b/src/libcharon/tests/utils/exchange_test_asserts.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2017 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -14,7 +14,7 @@ */ /** - * Special assertions using listener_t. + * Special assertions using listener_t etc. * * @defgroup exchange_test_asserts exchange_test_asserts * @{ @ingroup test_utils_c @@ -28,6 +28,7 @@ typedef struct listener_hook_assert_t listener_hook_assert_t; typedef struct listener_message_assert_t listener_message_assert_t; typedef struct listener_message_rule_t listener_message_rule_t; +typedef struct ipsec_sas_assert_t ipsec_sas_assert_t; struct listener_hook_assert_t { @@ -340,4 +341,60 @@ bool exchange_test_asserts_message(listener_t *this, ike_sa_t *ike_sa, exchange_test_helper->add_listener(exchange_test_helper, &_listener.listener); \ }) +/** + * Data used to check IPsec SAs + */ +struct ipsec_sas_assert_t { + + /** + * Original source file + */ + const char *file; + + /** + * Source line + */ + int line; + + /** + * IKE_SA that installed the IPsec SAs + */ + ike_sa_t *ike_sa; + + /** + * SPIs to check + */ + uint32_t *spis; + + /** + * Number of SPIs for IPsec SAs to check + */ + int count; +}; + +/** + * Assert that all given IPsec SAs (and only these) are installed for the given + * IKE_SA. + */ +void exchange_test_asserts_ipsec_sas(ipsec_sas_assert_t *sas); + +/** + * Assert that the IPsec SAs with the given SPIs (and none other) are currently + * installed by the given IKE_SA. + * + * @param sa IKE_SA + * @param ... list of SPIs + */ +#define assert_ipsec_sas_installed(sa, ...) ({ \ + uint32_t _spis[] = { __VA_ARGS__ }; \ + ipsec_sas_assert_t _sas_assert = { \ + .file = __FILE__, \ + .line = __LINE__, \ + .ike_sa = sa, \ + .spis = _spis, \ + .count = countof(_spis), \ + }; \ + exchange_test_asserts_ipsec_sas(&_sas_assert); \ +}) + #endif /** EXCHANGE_TEST_ASSERTS_H_ @}*/ From 10c7a668067b2657e8dffef70812d81b6408f12c Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 8 Mar 2017 15:47:58 +0100 Subject: [PATCH 20/20] unit-tests: Check installed IPsec SAs in child-rekey tests --- src/libcharon/tests/suites/test_child_rekey.c | 97 ++++++++++++++++++- 1 file changed, 94 insertions(+), 3 deletions(-) diff --git a/src/libcharon/tests/suites/test_child_rekey.c b/src/libcharon/tests/suites/test_child_rekey.c index 1c100b624..76b23f589 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -69,6 +69,7 @@ START_TEST(test_regular) &a, &b, NULL); } initiate_rekey(a, spi_a); + assert_ipsec_sas_installed(a, spi_a, spi_b); /* this should never get called as this results in a successful rekeying */ assert_hook_not_called(child_updown); @@ -79,6 +80,7 @@ START_TEST(test_regular) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, spi_a, spi_b, 4); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -87,6 +89,7 @@ START_TEST(test_regular) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, spi_a, spi_b, 3, 4); assert_hook(); /* INFORMATIONAL { D } --> */ @@ -97,6 +100,7 @@ START_TEST(test_regular) assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, spi_b, 3, 4); assert_scheduler(); assert_hook(); /* <-- INFORMATIONAL { D } */ @@ -107,14 +111,17 @@ START_TEST(test_regular) assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, spi_a, 3, 4); assert_scheduler(); assert_hook(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, spi_a); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 3, 4); destroy_rekeyed(b, spi_b); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(a, 3, 4); /* child_updown */ assert_hook(); @@ -153,6 +160,7 @@ START_TEST(test_regular_ke_invalid) &a, &b, &conf); } initiate_rekey(a, spi_a); + assert_ipsec_sas_installed(a, spi_a, spi_b); /* this should never get called as this results in a successful rekeying */ assert_hook_not_called(child_updown); @@ -163,6 +171,7 @@ START_TEST(test_regular_ke_invalid) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_INSTALLED); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, spi_a, spi_b); assert_hook(); /* <-- CREATE_CHILD_SA { N(INVAL_KE) } */ @@ -171,6 +180,7 @@ START_TEST(test_regular_ke_invalid) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, spi_a, CHILD_REKEYING); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, spi_a, spi_b); assert_hook(); /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> */ @@ -179,6 +189,7 @@ START_TEST(test_regular_ke_invalid) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, spi_b, CHILD_REKEYED); assert_child_sa_state(b, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, spi_a, spi_b, 6); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -187,6 +198,7 @@ START_TEST(test_regular_ke_invalid) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, spi_a, CHILD_DELETING); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, spi_a, spi_b, 5, 6); assert_hook(); /* INFORMATIONAL { D } --> */ @@ -196,6 +208,7 @@ START_TEST(test_regular_ke_invalid) assert_child_sa_state(b, spi_b, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 6, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, spi_b, 5, 6); assert_hook(); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); @@ -204,13 +217,16 @@ START_TEST(test_regular_ke_invalid) assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, spi_a, 5, 6); assert_hook(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, spi_a); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 5, 6); destroy_rekeyed(b, spi_b); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 5, 6); /* child_updown */ assert_hook(); @@ -231,6 +247,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) exchange_test_helper->establish_sa(exchange_test_helper, &a, &b, NULL); initiate_rekey(a, 1); + assert_ipsec_sas_installed(a, 1, 2); /* this should never get called as this results in a successful rekeying */ assert_hook_not_called(child_updown); @@ -241,6 +258,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 1, 2, 4); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -249,6 +267,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_DELETING); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, 1, 2, 3, 4); assert_hook(); /* we don't expect this to get called anymore */ @@ -265,6 +284,7 @@ START_TEST(test_regular_responder_ignore_soft_expire) assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, 2, 3, 4); assert_scheduler(); /* <-- INFORMATIONAL { D } */ assert_jobs_scheduled(1); @@ -273,13 +293,16 @@ START_TEST(test_regular_responder_ignore_soft_expire) assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 3, 4); assert_scheduler(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, 1); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 3, 4); destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 3, 4); /* child_rekey/child_updown */ assert_hook(); @@ -302,6 +325,7 @@ START_TEST(test_regular_responder_handle_hard_expire) exchange_test_helper->establish_sa(exchange_test_helper, &a, &b, NULL); initiate_rekey(a, 1); + assert_ipsec_sas_installed(a, 1, 2); /* this should never get called as this results in a successful rekeying */ assert_hook_not_called(child_updown); @@ -312,6 +336,7 @@ START_TEST(test_regular_responder_handle_hard_expire) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 1, 2, 4); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -320,6 +345,7 @@ START_TEST(test_regular_responder_handle_hard_expire) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_DELETING); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, 1, 2, 3, 4); assert_hook(); /* we don't expect this to get called anymore */ @@ -329,17 +355,23 @@ START_TEST(test_regular_responder_handle_hard_expire) call_ikesa(b, delete_child_sa, PROTO_ESP, 2, TRUE); assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + /* since the SAs expired they would not actually be installed in the kernel + * anymore and since we have not yet installed a new outbound SA this + * will result in dropped packets and possibly acquires */ + assert_ipsec_sas_installed(b, 1, 2, 4); /* INFORMATIONAL { D } --> */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, b, NULL); - assert_child_sa_state(a, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 1, 2, 4); /* <-- INFORMATIONAL { D } */ assert_single_payload(IN, PLV2_DELETE); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, 1, 2, 3, 4); /* <-- INFORMATIONAL { } */ assert_jobs_scheduled(1); assert_message_empty(IN); @@ -347,6 +379,7 @@ START_TEST(test_regular_responder_handle_hard_expire) assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 3, 4); assert_scheduler(); /* INFORMATIONAL { } --> */ assert_jobs_scheduled(1); @@ -355,13 +388,16 @@ START_TEST(test_regular_responder_handle_hard_expire) assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, 2, 3, 4); assert_scheduler(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, 1); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 3, 4); destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 3, 4); /* child_rekey/child_updown */ assert_hook(); @@ -411,8 +447,10 @@ START_TEST(test_collision) exchange_test_helper->nonce_first_byte = data[_i].nonces[0]; initiate_rekey(a, 1); + assert_ipsec_sas_installed(a, 1, 2); exchange_test_helper->nonce_first_byte = data[_i].nonces[1]; initiate_rekey(b, 2); + assert_ipsec_sas_installed(b, 1, 2); /* this should never get called as this results in a successful rekeying */ assert_hook_not_called(child_updown); @@ -423,6 +461,7 @@ START_TEST(test_collision) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 1, 2, 5); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; @@ -430,6 +469,7 @@ START_TEST(test_collision) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(a, 1, 2, 6); assert_hook(); /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ @@ -456,6 +496,7 @@ START_TEST(test_collision) } assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, 1, 2, 3, 5, 6); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) { @@ -479,6 +520,7 @@ START_TEST(test_collision) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(b, 1, 2, 4, 5, 6); /* we don't expect this hook to get called anymore */ assert_hook_not_called(child_rekey); @@ -492,6 +534,8 @@ START_TEST(test_collision) assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 3); + assert_ipsec_sas_installed(b, 2, 4, 5, 6, + data[_i].spi_del_b == 2 ? 1 : 3); assert_scheduler(); /* <-- INFORMATIONAL { D } */ assert_jobs_scheduled(1); @@ -503,6 +547,8 @@ START_TEST(test_collision) assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 3); + assert_ipsec_sas_installed(a, 1, 3, 5, 6, + data[_i].spi_del_a == 1 ? 2 : 4); assert_scheduler(); /* <-- INFORMATIONAL { D } */ assert_jobs_scheduled(1); @@ -514,6 +560,8 @@ START_TEST(test_collision) assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 3); + assert_ipsec_sas_installed(a, 1, 3, 6, + data[_i].spi_del_a == 1 ? 5 : 4); assert_scheduler(); /* INFORMATIONAL { D } --> */ assert_jobs_scheduled(1); @@ -525,15 +573,19 @@ START_TEST(test_collision) assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 3); + assert_ipsec_sas_installed(b, 2, 4, 5, + data[_i].spi_del_b == 2 ? 6 : 3); assert_scheduler(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, data[_i].spi_del_a); destroy_rekeyed(a, data[_i].spi_del_b); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, data[_i].spi_a, data[_i].spi_b); destroy_rekeyed(b, data[_i].spi_del_a); destroy_rekeyed(b, data[_i].spi_del_b); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, data[_i].spi_a, data[_i].spi_b); /* child_rekey/child_updown */ assert_hook(); @@ -592,8 +644,10 @@ START_TEST(test_collision_delayed_response) exchange_test_helper->nonce_first_byte = data[_i].nonces[0]; initiate_rekey(a, 1); + assert_ipsec_sas_installed(a, 1, 2); exchange_test_helper->nonce_first_byte = data[_i].nonces[1]; initiate_rekey(b, 2); + assert_ipsec_sas_installed(b, 1, 2); /* this should never get called as this results in a successful rekeying */ assert_hook_not_called(child_updown); @@ -604,6 +658,7 @@ START_TEST(test_collision_delayed_response) exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 1, 2, 5); assert_hook(); /* <-- CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } */ exchange_test_helper->nonce_first_byte = data[_i].nonces[3]; @@ -611,6 +666,7 @@ START_TEST(test_collision_delayed_response) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 6, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(a, 1, 2, 6); assert_hook(); /* delay the CREATE_CHILD_SA response from b to a */ @@ -639,6 +695,7 @@ START_TEST(test_collision_delayed_response) } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(b, 1, 2, 4, 5, 6); /* <-- INFORMATIONAL { D } */ assert_hook_not_called(child_rekey); @@ -649,15 +706,16 @@ START_TEST(test_collision_delayed_response) assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 4, 6); } else { assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETING, CHILD_OUTBOUND_NONE); - assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 2, 6); } + assert_child_sa_count(a, 2); assert_scheduler(); /* INFORMATIONAL { D } --> */ assert_jobs_scheduled(1); @@ -668,6 +726,7 @@ START_TEST(test_collision_delayed_response) CHILD_OUTBOUND_REGISTERED); assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(b, 2, 4, 5, 6); } else { @@ -675,6 +734,7 @@ START_TEST(test_collision_delayed_response) CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 1, 2, 4, 5); } assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, CHILD_OUTBOUND_NONE); @@ -701,6 +761,8 @@ START_TEST(test_collision_delayed_response) CHILD_OUTBOUND_NONE); assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, 1, 3, 5, 6, + data[_i].spi_del_a == 1 ? 2 : 4); assert_child_sa_count(a, 3); /* we don't expect this hook to get called anymore */ @@ -714,6 +776,8 @@ START_TEST(test_collision_delayed_response) CHILD_OUTBOUND_NONE); assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(b, 2, 4, 5, + data[_i].spi_del_b == 2 ? 6 : 3); assert_child_sa_count(b, 3); assert_scheduler(); /* <-- INFORMATIONAL { D } */ @@ -726,15 +790,19 @@ START_TEST(test_collision_delayed_response) assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 3); + assert_ipsec_sas_installed(a, 1, 3, 6, + data[_i].spi_del_a == 1 ? 5 : 4); assert_scheduler(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, data[_i].spi_del_a); destroy_rekeyed(a, data[_i].spi_del_b); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, data[_i].spi_a, data[_i].spi_b); destroy_rekeyed(b, data[_i].spi_del_a); destroy_rekeyed(b, data[_i].spi_del_b); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, data[_i].spi_a, data[_i].spi_b); /* child_rekey/child_updown */ assert_hook(); @@ -785,8 +853,10 @@ START_TEST(test_collision_delayed_request) exchange_test_helper->nonce_first_byte = data[_i].nonces[0]; initiate_rekey(a, 1); + assert_ipsec_sas_installed(a, 1, 2); exchange_test_helper->nonce_first_byte = data[_i].nonces[1]; initiate_rekey(b, 2); + assert_ipsec_sas_installed(b, 1, 2); /* delay the CREATE_CHILD_SA request from a to b */ msg = exchange_test_helper->sender->dequeue(exchange_test_helper->sender); @@ -800,12 +870,14 @@ START_TEST(test_collision_delayed_request) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(a, 1, 2, 5); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(b, 1, 2, 4, 5); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -823,6 +895,7 @@ START_TEST(test_collision_delayed_request) assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 4, 5); assert_scheduler(); /* <-- CREATE_CHILD_SA { N(TEMP_FAIL) } */ @@ -831,6 +904,7 @@ START_TEST(test_collision_delayed_request) assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 4, 5); assert_scheduler(); /* INFORMATIONAL { D } --> */ @@ -839,13 +913,16 @@ START_TEST(test_collision_delayed_request) assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, 2, 4, 5); assert_scheduler(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, 1); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 4, 5); destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 4, 5); /* child_rekey/child_updown */ assert_hook(); @@ -899,8 +976,10 @@ START_TEST(test_collision_delayed_request_more) exchange_test_helper->nonce_first_byte = data[_i].nonces[0]; initiate_rekey(a, 1); + assert_ipsec_sas_installed(a, 1, 2); exchange_test_helper->nonce_first_byte = data[_i].nonces[1]; initiate_rekey(b, 2); + assert_ipsec_sas_installed(b, 1, 2); /* delay the CREATE_CHILD_SA request from a to b */ msg = exchange_test_helper->sender->dequeue(exchange_test_helper->sender); @@ -914,12 +993,14 @@ START_TEST(test_collision_delayed_request_more) exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(a, 1, 2, 5); assert_hook(); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ assert_hook_rekey(child_rekey, 2, 4); exchange_test_helper->process_message(exchange_test_helper, b, NULL); assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(b, 1, 2, 4, 5); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -931,6 +1012,7 @@ START_TEST(test_collision_delayed_request_more) assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 4, 5); assert_scheduler(); /* INFORMATIONAL { D } --> */ assert_jobs_scheduled(1); @@ -938,6 +1020,7 @@ START_TEST(test_collision_delayed_request_more) assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, 2, 4, 5); assert_scheduler(); /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> */ @@ -946,19 +1029,23 @@ START_TEST(test_collision_delayed_request_more) assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, 2, 4, 5); /* <-- CREATE_CHILD_SA { N(NO_CHILD_SA) } */ assert_no_jobs_scheduled(); exchange_test_helper->process_message(exchange_test_helper, a, NULL); assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 1, 4, 5); assert_scheduler(); /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, 1); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 4, 5); destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 4, 5); /* child_rekey/child_updown */ assert_hook(); @@ -1170,9 +1257,11 @@ START_TEST(test_collision_ke_invalid) destroy_rekeyed(a, data[_i].spi_del_a); destroy_rekeyed(a, data[_i].spi_del_b); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, data[_i].spi_a, data[_i].spi_b); destroy_rekeyed(b, data[_i].spi_del_a); destroy_rekeyed(b, data[_i].spi_del_b); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, data[_i].spi_a, data[_i].spi_b); /* child_rekey/child_updown */ assert_hook(); @@ -1323,8 +1412,10 @@ START_TEST(test_collision_ke_invalid_delayed_retry) /* simulate the execution of the scheduled jobs */ destroy_rekeyed(a, 1); assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 8, 9); destroy_rekeyed(b, 2); assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 8, 9); /* child_rekey/child_updown */ assert_hook();