ikev2: Delay IKE key derivation until next message

In particular as responder, this delays costly cryptographic operations
until the IKE_AUTH request is received, which is preferable to reduce
the impact of DoS attacks.

Another advantage is that the key material is not changed until all tasks
built or processed a message.

diff --git a/src/libcharon/sa/ikev2/task_manager_v2.c b/src/libcharon/sa/ikev2/task_manager_v2.c
index 2e29f027d4ad37c6712f2c67f44132ed40b43d68..f1228ab0729780dab426174df8d67802b69810ac 100644 (file)
--- a/src/libcharon/sa/ikev2/task_manager_v2.c
+++ b/src/libcharon/sa/ikev2/task_manager_v2.c
@@ -475,6 +475,40 @@ METHOD(task_manager_t, retransmit, status_t,
        return INVALID_STATE;
 }
 
+/**
+ * Derive IKE keys if necessary
+ */
+static bool derive_keys(private_task_manager_t *this, array_t *tasks)
+{
+       enumerator_t *enumerator;
+       task_t *task;
+
+       enumerator = array_create_enumerator(tasks);
+       while (enumerator->enumerate(enumerator, (void*)&task))
+       {
+               if (task->get_type(task) == TASK_IKE_INIT)
+               {
+                       ike_init_t *ike_init = (ike_init_t*)task;
+
+                       switch (ike_init->derive_keys(ike_init))
+                       {
+                               case SUCCESS:
+                                       array_remove_at(tasks, enumerator);
+                                       task->destroy(task);
+                                       break;
+                               case NEED_MORE:
+                                       break;
+                               default:
+                                       enumerator->destroy(enumerator);
+                                       return FALSE;
+                       }
+                       break;
+               }
+       }
+       enumerator->destroy(enumerator);
+       return TRUE;
+}
+
 METHOD(task_manager_t, initiate, status_t,
        private_task_manager_t *this)
 {
@@ -609,6 +643,11 @@ METHOD(task_manager_t, initiate, status_t,
        }
        else
        {
+               if (!derive_keys(this, this->active_tasks))
+               {
+                       return DESTROY_ME;
+               }
+
                DBG2(DBG_IKE, "reinitiating already active tasks");
                enumerator = array_create_enumerator(this->active_tasks);
                while (enumerator->enumerate(enumerator, &task))
@@ -1531,6 +1570,36 @@ static status_t send_invalid_syntax(private_task_manager_t *this,
        return DESTROY_ME;
 }
 
+/**
+ * Check for unsupported critical payloads
+ */
+static status_t has_unsupported_critical_payload(message_t *msg, uint8_t *type)
+{
+       enumerator_t *enumerator;
+       unknown_payload_t *unknown;
+       payload_t *payload;
+       status_t status = SUCCESS;
+
+       enumerator = msg->create_payload_enumerator(msg);
+       while (enumerator->enumerate(enumerator, &payload))
+       {
+               if (payload->get_type(payload) == PL_UNKNOWN)
+               {
+                       unknown = (unknown_payload_t*)payload;
+                       if (unknown->is_critical(unknown))
+                       {
+                               *type = unknown->get_type(unknown);
+                               DBG1(DBG_ENC, "payload type %N is not supported, "
+                                        "but payload is critical!", payload_type_names, *type);
+                               status = NOT_SUPPORTED;
+                               break;
+                       }
+               }
+       }
+       enumerator->destroy(enumerator);
+       return status;
+}
+
 /**
  * Parse the given message and verify that it is valid.
  */
@@ -1539,34 +1608,22 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
        status_t parse_status, status;
        uint8_t type = 0;
 
-       parse_status = msg->parse_body(msg, this->ike_sa->get_keymat(this->ike_sa));
-
-       if (parse_status == SUCCESS)
-       {       /* check for unsupported critical payloads */
-               enumerator_t *enumerator;
-               unknown_payload_t *unknown;
-               payload_t *payload;
+       if (derive_keys(this, this->passive_tasks))
+       {
+               parse_status = msg->parse_body(msg, this->ike_sa->get_keymat(this->ike_sa));
 
-               enumerator = msg->create_payload_enumerator(msg);
-               while (enumerator->enumerate(enumerator, &payload))
+               if (parse_status == SUCCESS)
                {
-                       if (payload->get_type(payload) == PL_UNKNOWN)
-                       {
-                               unknown = (unknown_payload_t*)payload;
-                               if (unknown->is_critical(unknown))
-                               {
-                                       type = unknown->get_type(unknown);
-                                       DBG1(DBG_ENC, "payload type %N is not supported, "
-                                                "but payload is critical!", payload_type_names, type);
-                                       parse_status = NOT_SUPPORTED;
-                                       break;
-                               }
-                       }
+                       parse_status = has_unsupported_critical_payload(msg, &type);
                }
-               enumerator->destroy(enumerator);
-       }
 
-       status = parse_status;
+               status = parse_status;
+       }
+       else
+       {       /* there is no point in trying again */
+               parse_status = INVALID_STATE;
+               status = DESTROY_ME;
+       }
 
        if (parse_status != SUCCESS)
        {

diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c
index cae68559fca8f7feb336f31701f6eb0875adde57..a73c98ac9d859f4457458873df4afc920eb18943 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_init.c
+++ b/src/libcharon/sa/ikev2/tasks/ike_init.c
@@ -55,6 +55,16 @@ struct private_ike_init_t {
         */
        bool initiator;
 
+       /**
+        * Whether the key exchange is done
+        */
+       bool ke_done;
+
+       /**
+        * Whether keys have already been derived
+        */
+       bool ke_derived;
+
        /**
         * diffie hellman group to use
         */
@@ -520,6 +530,7 @@ static void process_payloads(private_ike_init_t *this, message_t *message)
 {
        enumerator_t *enumerator;
        payload_t *payload;
+       ike_sa_id_t *id;
        ke_payload_t *ke_payload = NULL;
 
        enumerator = message->create_payload_enumerator(message);
@@ -615,6 +626,21 @@ static void process_payloads(private_ike_init_t *this, message_t *message)
        if (this->proposal)
        {
                this->ike_sa->set_proposal(this->ike_sa, this->proposal);
+
+               if (this->old_sa)
+               {       /* retrieve SPI of new IKE_SA when rekeying */
+                       id = this->ike_sa->get_id(this->ike_sa);
+                       if (this->initiator)
+                       {
+                               id->set_responder_spi(id,
+                                                                         this->proposal->get_spi(this->proposal));
+                       }
+                       else
+                       {
+                               id->set_initiator_spi(id,
+                                                                         this->proposal->get_spi(this->proposal));
+                       }
+               }
        }
 
        if (ke_payload && this->proposal &&
@@ -769,8 +795,8 @@ METHOD(task_t, process_r,  status_t,
 /**
  * Derive the keymat for the IKE_SA
  */
-static bool derive_keys(private_ike_init_t *this,
-                                               chunk_t nonce_i, chunk_t nonce_r)
+static bool derive_keys_internal(private_ike_init_t *this, chunk_t nonce_i,
+                                                                chunk_t nonce_r)
 {
        keymat_v2_t *old_keymat;
        pseudo_random_function_t prf_alg = PRF_UNDEFINED;
@@ -783,14 +809,6 @@ static bool derive_keys(private_ike_init_t *this,
                /* rekeying: Include old SKd, use old PRF, apply SPI */
                old_keymat = (keymat_v2_t*)this->old_sa->get_keymat(this->old_sa);
                prf_alg = old_keymat->get_skd(old_keymat, &skd);
-               if (this->initiator)
-               {
-                       id->set_responder_spi(id, this->proposal->get_spi(this->proposal));
-               }
-               else
-               {
-                       id->set_initiator_spi(id, this->proposal->get_spi(this->proposal));
-               }
        }
        if (!this->keymat->derive_ike_keys(this->keymat, this->proposal, this->dh,
                                                                           nonce_i, nonce_r, id, prf_alg, skd))
@@ -802,6 +820,35 @@ static bool derive_keys(private_ike_init_t *this,
        return TRUE;
 }
 
+METHOD(ike_init_t, derive_keys, status_t,
+       private_ike_init_t *this)
+{
+       bool success;
+
+       if (!this->ke_done || this->ke_derived)
+       {
+               return NEED_MORE;
+       }
+
+       if (this->initiator)
+       {
+               success = derive_keys_internal(this, this->my_nonce, this->other_nonce);
+       }
+       else
+       {
+               success = derive_keys_internal(this, this->other_nonce, this->my_nonce);
+       }
+
+       this->ke_derived = TRUE;
+
+       if (!success)
+       {
+               DBG1(DBG_IKE, "key derivation failed");
+               return FAILED;
+       }
+       return SUCCESS;
+}
+
 METHOD(task_t, build_r, status_t,
        private_ike_init_t *this, message_t *message)
 {
@@ -864,18 +911,25 @@ METHOD(task_t, build_r, status_t,
                return FAILED;
        }
 
-       if (!derive_keys(this, this->other_nonce, this->my_nonce))
+       if (!build_payloads(this, message))
        {
-               DBG1(DBG_IKE, "key derivation failed");
                message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
                return FAILED;
        }
-       if (!build_payloads(this, message))
+       this->ke_done = TRUE;
+
+       if (this->old_sa)
        {
-               message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
-               return FAILED;
+               /* during rekeying, we derive keys here directly */
+               if (derive_keys(this) != SUCCESS)
+               {
+                       message->add_notify(message, FALSE, NO_PROPOSAL_CHOSEN, chunk_empty);
+                       return FAILED;
+               }
+               return SUCCESS;
        }
-       return SUCCESS;
+       /* key derivation is done before the next request is processed */
+       return NEED_MORE;
 }
 
 /**
@@ -1087,13 +1141,15 @@ METHOD(task_t, process_i, status_t,
                DBG1(DBG_IKE, "applying DH public value failed");
                return FAILED;
        }
+       this->ke_done = TRUE;
 
-       if (!derive_keys(this, this->my_nonce, this->other_nonce))
+       if (this->old_sa)
        {
-               DBG1(DBG_IKE, "key derivation failed");
-               return FAILED;
+               /* during rekeying, we derive keys here directly */
+               return derive_keys(this);
        }
-       return SUCCESS;
+       /* key derivation is done before we send the next message */
+       return NEED_MORE;
 }
 
 METHOD(task_t, get_type, task_type_t,
@@ -1107,6 +1163,8 @@ METHOD(task_t, migrate, void,
 {
        DESTROY_IF(this->proposal);
        chunk_free(&this->other_nonce);
+       this->ke_done = FALSE;
+       this->ke_derived = FALSE;
 
        this->ike_sa = ike_sa;
        this->keymat = (keymat_v2_t*)ike_sa->get_keymat(ike_sa);
@@ -1154,6 +1212,7 @@ ike_init_t *ike_init_create(ike_sa_t *ike_sa, bool initiator, ike_sa_t *old_sa)
                                .migrate = _migrate,
                                .destroy = _destroy,
                        },
+                       .derive_keys = _derive_keys,
                        .get_lower_nonce = _get_lower_nonce,
                },
                .ike_sa = ike_sa,

diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.h b/src/libcharon/sa/ikev2/tasks/ike_init.h
index 6f2e47a6cd1ac69959b2a806627e56e1bb14dcef..eea4e58e0ca6d371f444f2e0b2540ab2bbc916aa 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_init.h
+++ b/src/libcharon/sa/ikev2/tasks/ike_init.h
@@ -40,6 +40,18 @@ struct ike_init_t {
         */
        task_t task;
 
+       /**
+        * Derive the keys for this IKE_SA when not used during a rekeying.
+        *
+        * @return
+        *                                      - FAILED if key derivation failed
+        *                                      - NEED_MORE if key drivation was successful but more
+        *                                        key exchanges are necessary
+        *                                      - SUCCESS if key derivation was successful and the task
+        *                                        can be removed from the queue
+        */
+    status_t (*derive_keys)(ike_init_t *this);
+
        /**
         * Get the lower of the two nonces, used for rekey collisions.
         *