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.

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 (derive_keys(this, this->passive_tasks))
+	{
+		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;
-
-		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)
 	{

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))
-	{
-		DBG1(DBG_IKE, "key derivation failed");
-		message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
-		return FAILED;
-	}
 	if (!build_payloads(this, message))
 	{
 		message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
 		return FAILED;
 	}
-	return SUCCESS;
+	this->ke_done = TRUE;
+
+	if (this->old_sa)
+	{
+		/* 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;
+	}
+	/* 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,

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.
 	 *