From 0cbf75eb941d5bd6b6ddfbe556c725d5105c0421 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 21 Mar 2017 15:39:10 +0100 Subject: [PATCH] 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) } */