From: Tobias Brunner Date: Fri, 20 May 2016 08:49:21 +0000 (+0200) Subject: ikev2: Use CHILD_REKEYED for replaced CHILD_SAs after rekeying X-Git-Tag: 5.5.0dr1~4^2~57 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cdbf9428890a324b4cbd8bdbe0f1cfe669870a15;p=thirdparty%2Fstrongswan.git ikev2: Use CHILD_REKEYED for replaced CHILD_SAs after rekeying This allows handling collisions better, in particular with deletions. --- diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index 52661f9547..dd77665f70 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -157,9 +157,8 @@ static void process_payloads(private_child_delete_t *this, message_t *message) switch (child_sa->get_state(child_sa)) { - case CHILD_REKEYING: + case CHILD_REKEYED: this->rekeyed = TRUE; - /* we reply as usual, rekeying will fail */ break; case CHILD_DELETING: /* we don't send back a delete if we initiated ourself */ @@ -168,11 +167,14 @@ static void process_payloads(private_child_delete_t *this, message_t *message) continue; } /* fall through */ + case CHILD_REKEYING: + /* we reply as usual, rekeying will fail */ case CHILD_INSTALLED: if (!this->initiator) { /* reestablish installed children if required */ this->check_delete_action = TRUE; } + break; default: break; } @@ -204,7 +206,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this) enumerator = this->child_sas->create_enumerator(this->child_sas); while (enumerator->enumerate(enumerator, (void**)&child_sa)) { - /* signal child down event if we are not rekeying */ + /* signal child down event if we weren't rekeying */ if (!this->rekeyed) { charon->bus->child_updown(charon->bus, child_sa, FALSE); @@ -306,7 +308,7 @@ METHOD(task_t, build_i, status_t, this->spi = child_sa->get_spi(child_sa, TRUE); } this->child_sas->insert_last(this->child_sas, child_sa); - if (child_sa->get_state(child_sa) == CHILD_REKEYING) + if (child_sa->get_state(child_sa) == CHILD_REKEYED) { this->rekeyed = TRUE; } diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index b32d2d7254..57085e8cdc 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -241,6 +241,7 @@ METHOD(task_t, build_r, status_t, config = this->child_sa->get_config(this->child_sa); this->child_create->set_config(this->child_create, config->get_ref(config)); this->child_create->task.build(&this->child_create->task, message); + this->child_sa->set_state(this->child_sa, CHILD_REKEYING); if (message->get_payload(message, PLV2_SECURITY_ASSOCIATION) == NULL) { @@ -249,7 +250,7 @@ METHOD(task_t, build_r, status_t, return SUCCESS; } - this->child_sa->set_state(this->child_sa, CHILD_REKEYING); + this->child_sa->set_state(this->child_sa, CHILD_REKEYED); /* invoke rekey hook */ charon->bus->child_rekey(charon->bus, this->child_sa, @@ -289,9 +290,9 @@ static child_sa_t *handle_collision(private_child_rekey_t *this) if (child_sa) { child_sa->set_close_action(child_sa, ACTION_NONE); - if (child_sa->get_state(child_sa) != CHILD_REKEYING) + if (child_sa->get_state(child_sa) != CHILD_REKEYED) { - child_sa->set_state(child_sa, CHILD_REKEYING); + child_sa->set_state(child_sa, CHILD_REKEYED); } } } @@ -410,9 +411,9 @@ METHOD(task_t, process_i, status_t, return SUCCESS; } /* disable updown event for redundant CHILD_SA */ - if (to_delete->get_state(to_delete) != CHILD_REKEYING) + if (to_delete->get_state(to_delete) != CHILD_REKEYED) { - to_delete->set_state(to_delete, CHILD_REKEYING); + 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 2cf2b3fd00..2ed1f89d5f 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -60,8 +60,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); - /* FIXME: keeping this in CHILD_REKEYING is not ideal */ - assert_child_sa_state(b, spi_b, CHILD_REKEYING); + assert_child_sa_state(b, spi_b, CHILD_REKEYED); assert_child_sa_state(b, 4, CHILD_INSTALLED); assert_hook(); @@ -145,14 +144,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_REKEYING); + assert_child_sa_state(b, 2, CHILD_REKEYED); assert_child_sa_state(b, 5, CHILD_INSTALLED); 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_REKEYING); + assert_child_sa_state(a, 1, CHILD_REKEYED); assert_child_sa_state(a, 6, CHILD_INSTALLED); assert_hook(); @@ -169,7 +168,7 @@ START_TEST(test_collision) 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_del_b, CHILD_REKEYING); + assert_child_sa_state(a, data[_i].spi_del_b, CHILD_REKEYED); assert_child_sa_state(a, data[_i].spi_a, CHILD_INSTALLED); /* CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } --> */ if (data[_i].spi_del_b == 2) @@ -183,7 +182,7 @@ START_TEST(test_collision) 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_del_a, CHILD_REKEYING); + assert_child_sa_state(b, data[_i].spi_del_a, CHILD_REKEYED); assert_child_sa_state(b, data[_i].spi_b, CHILD_INSTALLED); /* we don't expect this hook to get called anymore */