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 52661f9547d4296b4e74fb98c6334449ec8c376f..dd77665f7083e108d4344069bc003a7cb92bc352 100644 (file)
--- 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 b32d2d72549366ce63934afc53f191abe75902ea..57085e8cdc595959afa4289db8d0da00c63d8341 100644 (file)
--- 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 2cf2b3fd003c36e1117154ca80df0919bb9b7534..2ed1f89d5f49153b8a3ff68f01549cc2f00d802f 100644 (file)
--- 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 */