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.

diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c
index d4d05c9..db57ee8 100644 (file)
--- 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 6fa8836..d69f44b 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_delete.c
+++ b/src/libcharon/sa/ikev2/tasks/child_delete.c
@@ -147,6 +147,57 @@ static bool is_redundant(private_child_delete_t *this, child_sa_t *child)
 }
 
 /**
+ * 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
  */
 static void process_payloads(private_child_delete_t *this, message_t *message)
@@ -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 c04ec14..afc4644 100644 (file)
--- 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 fcac493..19e5f78 100644 (file)
--- 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);