diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index 0e3711898..f3881ceaa 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -340,11 +340,13 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) } else { + /* the following two calls are only relevant as responder/loser of + * rekeyings as the initiator/winner already did this right after + * the rekeying was completed, either way, we delay destroying + * the CHILD_SA, by default, so we can process delayed packets */ install_outbound(this, protocol, child_sa->get_rekey_spi(child_sa)); - /* 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))) { diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index 36d7c4bff..37f7ea9bd 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -475,20 +475,18 @@ METHOD(task_t, process_i, status_t, other_ts->destroy(other_ts); } } - if (to_delete != this->child_create->get_child(this->child_create)) - { /* invoke rekey hook if rekeying successful */ + if (to_delete->get_state(to_delete) != CHILD_REKEYED) + { /* disable updown event for old/redundant CHILD_SA */ + to_delete->set_state(to_delete, CHILD_REKEYED); + } + if (to_delete == this->child_sa) + { /* invoke rekey hook if rekeying successful and remove the old + * outbound SA as we installed the new one already above, but might not + * be using it yet depending on how SAs/policies are handled */ + this->child_sa->remove_outbound(this->child_sa); charon->bus->child_rekey(charon->bus, this->child_sa, this->child_create->get_child(this->child_create)); } - if (to_delete == NULL) - { - return SUCCESS; - } - /* disable updown event for redundant CHILD_SA */ - if (to_delete->get_state(to_delete) != CHILD_REKEYED) - { - to_delete->set_state(to_delete, CHILD_REKEYED); - } spi = to_delete->get_spi(to_delete, TRUE); protocol = to_delete->get_protocol(to_delete); diff --git a/src/libcharon/tests/suites/test_child_rekey.c b/src/libcharon/tests/suites/test_child_rekey.c index b9f6ea0bc..563357de6 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -87,9 +87,9 @@ 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, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, spi_a, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(a, spi_a, spi_b, 3, 4); + assert_ipsec_sas_installed(a, spi_a, 3, 4); assert_hook(); /* INFORMATIONAL { D } --> */ @@ -196,9 +196,9 @@ START_TEST(test_regular_ke_invalid) 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_NONE); assert_child_sa_state(a, 5, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(a, spi_a, spi_b, 5, 6); + assert_ipsec_sas_installed(a, spi_a, 5, 6); assert_hook(); /* INFORMATIONAL { D } --> */ @@ -250,9 +250,9 @@ START_TEST(test_regular_ke_invalid) 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, 5, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, 5, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 7, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(a, 5, 6, 7, 8); + assert_ipsec_sas_installed(a, 5, 7, 8); assert_hook(); /* INFORMATIONAL { D } --> */ @@ -320,9 +320,9 @@ START_TEST(test_regular_responder_ignore_soft_expire) 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, 1, CHILD_DELETING); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(a, 1, 2, 3, 4); + assert_ipsec_sas_installed(a, 1, 3, 4); assert_hook(); /* we don't expect this to get called anymore */ @@ -398,9 +398,9 @@ START_TEST(test_regular_responder_handle_hard_expire) 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, 1, CHILD_DELETING); + assert_child_sa_state(a, 1, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(a, 3, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(a, 1, 2, 3, 4); + assert_ipsec_sas_installed(a, 1, 3, 4); assert_hook(); /* we don't expect this to get called anymore */ @@ -446,7 +446,7 @@ START_TEST(test_regular_responder_handle_hard_expire) END_TEST /** - * Both peers initiate the CHILD_SA reekying concurrently and should handle + * Both peers initiate the CHILD_SA rekeying concurrently and should handle * the collision properly depending on the nonces. */ START_TEST(test_collision) @@ -521,8 +521,8 @@ START_TEST(test_collision) assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); 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); + CHILD_OUTBOUND_NONE); + assert_ipsec_sas_installed(a, 1, 3, 5, 6); } else { @@ -548,8 +548,8 @@ START_TEST(test_collision) assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); 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); + CHILD_OUTBOUND_NONE); + assert_ipsec_sas_installed(b, 2, 4, 5, 6); } else { @@ -571,7 +571,7 @@ START_TEST(test_collision) 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, - data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED + data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_NONE : CHILD_OUTBOUND_REGISTERED); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETED, CHILD_OUTBOUND_NONE); @@ -580,7 +580,7 @@ START_TEST(test_collision) assert_child_sa_count(b, 3); if (data[_i].spi_del_b == 2) { - assert_ipsec_sas_installed(b, 1, 2, 4, 5, 6); + assert_ipsec_sas_installed(b, 2, 4, 5, 6); } else { @@ -591,7 +591,7 @@ START_TEST(test_collision) 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, - data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_INSTALLED + data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_NONE : CHILD_OUTBOUND_REGISTERED); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETED, CHILD_OUTBOUND_NONE); @@ -600,7 +600,7 @@ START_TEST(test_collision) assert_child_sa_count(a, 3); if (data[_i].spi_del_a == 1) { - assert_ipsec_sas_installed(a, 1, 2, 3, 5, 6); + assert_ipsec_sas_installed(a, 1, 3, 5, 6); } else { @@ -740,8 +740,8 @@ START_TEST(test_collision_delayed_response) assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); 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); + CHILD_OUTBOUND_NONE); + assert_ipsec_sas_installed(b, 2, 4, 5, 6); } else { @@ -809,8 +809,8 @@ 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, - CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(a, 1, 2, 3, 5, 6); + CHILD_OUTBOUND_NONE); + assert_ipsec_sas_installed(a, 1, 3, 5, 6); } else { @@ -937,9 +937,9 @@ START_TEST(test_collision_delayed_request) /* 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, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(b, 1, 2, 4, 5); + assert_ipsec_sas_installed(b, 2, 4, 5); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -948,7 +948,7 @@ 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, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); /* <-- INFORMATIONAL { D } */ @@ -1060,9 +1060,9 @@ START_TEST(test_collision_delayed_request_more) /* 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, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 4, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); - assert_ipsec_sas_installed(b, 1, 2, 4, 5); + assert_ipsec_sas_installed(b, 2, 4, 5); assert_hook(); /* we don't expect this hook to get called anymore */ @@ -1236,7 +1236,7 @@ START_TEST(test_collision_ke_invalid) assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(a, data[_i].spi_del_a, CHILD_DELETING, - CHILD_OUTBOUND_INSTALLED); + CHILD_OUTBOUND_NONE); } else { @@ -1259,7 +1259,7 @@ START_TEST(test_collision_ke_invalid) assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_child_sa_state(b, data[_i].spi_del_b, CHILD_DELETING, - CHILD_OUTBOUND_INSTALLED); + CHILD_OUTBOUND_NONE); } else { @@ -1279,7 +1279,7 @@ START_TEST(test_collision_ke_invalid) 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, - data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_INSTALLED + data[_i].spi_del_b == 2 ? CHILD_OUTBOUND_NONE : CHILD_OUTBOUND_REGISTERED); assert_child_sa_state(b, data[_i].spi_del_a, CHILD_DELETED, CHILD_OUTBOUND_NONE); @@ -1291,7 +1291,7 @@ START_TEST(test_collision_ke_invalid) 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, - data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_INSTALLED + data[_i].spi_del_a == 1 ? CHILD_OUTBOUND_NONE : CHILD_OUTBOUND_REGISTERED); assert_child_sa_state(a, data[_i].spi_del_b, CHILD_DELETED, CHILD_OUTBOUND_NONE); @@ -1441,7 +1441,7 @@ START_TEST(test_collision_ke_invalid_delayed_retry) /* 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, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); assert_hook(); @@ -1451,7 +1451,7 @@ 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, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 2, CHILD_DELETING, CHILD_OUTBOUND_NONE); assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); /* <-- INFORMATIONAL { D } */