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 d4d05c9c1cec65fe7f0e83d21951e06367ce5df4..db57ee815b3e075e19a546497b515e051aa5b40a 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 6fa8836acf6282c598cb2af6e26d9dc7713decb2..d69f44b0d9090cdc72e1ca666ab5f5087cebdf7a 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/child_delete.c
+++ b/src/libcharon/sa/ikev2/tasks/child_delete.c
@@ -146,6 +146,57 @@ static bool is_redundant(private_child_delete_t *this, child_sa_t *child)
        return FALSE;
 }
 
+/**
+ * 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
  */
@@ -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 c04ec141fdd8eec49d25cbe96898dd01719739e9..afc4644a29a7d23a82c98e236fa83f048a2d648b 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 fcac49388efc33df65e2ba9a3369e1bc533bf456..19e5f784a248942d036fd93eea8eb802e9ccec94 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);