Merge branch 'ikev1-mm-retransmits'

Fixes retransmit of the last Main Mode or IKE_AUTH message, and correctly
queues Main Mode messages when processing of the last message is still in
progress.

4 files changed, 55 insertions, 45 deletions

diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 258e562d4..ea15ff2ca 100644
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -1250,24 +1250,6 @@ METHOD(ike_sa_t, process_message, status_t,
 	{	/* do not handle messages in passive state */
 		return FAILED;
 	}
-	switch (message->get_exchange_type(message))
-	{
-		case ID_PROT:
-		case AGGRESSIVE:
-		case IKE_SA_INIT:
-		case IKE_AUTH:
-			if (this->state != IKE_CREATED &&
-				this->state != IKE_CONNECTING &&
-				message->get_first_payload_type(message) != FRAGMENT_V1)
-			{
-				DBG1(DBG_IKE, "ignoring %N in established IKE_SA state",
-					 exchange_type_names, message->get_exchange_type(message));
-				return FAILED;
-			}
-			break;
-		default:
-			break;
-	}
 	if (message->get_major_version(message) != this->version)
 	{
 		DBG1(DBG_IKE, "ignoring %N IKEv%u exchange on %N SA",
diff --git a/src/libcharon/sa/ike_sa_manager.c b/src/libcharon/sa/ike_sa_manager.c
index df46b35a2..c5e6bf4bd 100644
--- a/src/libcharon/sa/ike_sa_manager.c
+++ b/src/libcharon/sa/ike_sa_manager.c
@@ -108,9 +108,9 @@ struct entry_t {
 	identification_t *other_id;
 
 	/**
-	 * message ID currently processing, if any
+	 * message ID or hash of currently processing message, -1 if none
 	 */
-	u_int32_t message_id;
+	u_int32_t processing;
 };
 
 /**
@@ -135,23 +135,12 @@ static status_t entry_destroy(entry_t *this)
  */
 static entry_t *entry_create()
 {
-	entry_t *this = malloc_thing(entry_t);
-
-	this->waiting_threads = 0;
-	this->condvar = condvar_create(CONDVAR_TYPE_DEFAULT);
-
-	/* we set checkout flag when we really give it out */
-	this->checked_out = FALSE;
-	this->driveout_new_threads = FALSE;
-	this->driveout_waiting_threads = FALSE;
-	this->message_id = -1;
-	this->init_hash = chunk_empty;
-	this->other = NULL;
-	this->half_open = FALSE;
-	this->my_id = NULL;
-	this->other_id = NULL;
-	this->ike_sa_id = NULL;
-	this->ike_sa = NULL;
+	entry_t *this;
+
+	INIT(this,
+		.condvar = condvar_create(CONDVAR_TYPE_DEFAULT),
+		.processing = -1,
+	);
 
 	return this;
 }
@@ -1171,6 +1160,20 @@ METHOD(ike_sa_manager_t, checkout_new, ike_sa_t*,
 	return ike_sa;
 }
 
+/**
+ * Get the message ID or message hash to detect early retransmissions
+ */
+static u_int32_t get_message_id_or_hash(message_t *message)
+{
+	/* Use the message ID, or the message hash in IKEv1 Main/Aggressive mode */
+	if (message->get_major_version(message) == IKEV1_MAJOR_VERSION &&
+		message->get_message_id(message) == 0)
+	{
+		return chunk_hash(message->get_packet_data(message));
+	}
+	return message->get_message_id(message);
+}
+
 METHOD(ike_sa_manager_t, checkout_by_message, ike_sa_t*,
 	private_ike_sa_manager_t* this, message_t *message)
 {
@@ -1246,7 +1249,7 @@ METHOD(ike_sa_manager_t, checkout_by_message, ike_sa_t*,
 						entry->checked_out = TRUE;
 						unlock_single_segment(this, segment);
 
-						entry->message_id = message->get_message_id(message);
+						entry->processing = get_message_id_or_hash(message);
 						entry->init_hash = hash;
 
 						DBG2(DBG_MGR, "created IKE_SA %s[%u]",
@@ -1290,12 +1293,11 @@ METHOD(ike_sa_manager_t, checkout_by_message, ike_sa_t*,
 
 	if (get_entry_by_id(this, id, &entry, &segment) == SUCCESS)
 	{
-		/* only check out in IKEv2 if we are not already processing it */
-		if (message->get_request(message) &&
-			message->get_message_id(message) == entry->message_id)
+		/* only check out if we are not already processing it. */
+		if (entry->processing == get_message_id_or_hash(message))
 		{
 			DBG1(DBG_MGR, "ignoring request with ID %u, already processing",
-				 entry->message_id);
+				 entry->processing);
 		}
 		else if (wait_for_entry(this, entry, segment))
 		{
@@ -1305,7 +1307,7 @@ METHOD(ike_sa_manager_t, checkout_by_message, ike_sa_t*,
 			entry->checked_out = TRUE;
 			if (message->get_first_payload_type(message) != FRAGMENT_V1)
 			{
-				entry->message_id = message->get_message_id(message);
+				entry->processing = get_message_id_or_hash(message);
 			}
 			if (ike_id->get_responder_spi(ike_id) == 0)
 			{
@@ -1564,7 +1566,7 @@ METHOD(ike_sa_manager_t, checkin, void,
 		entry->ike_sa_id->replace_values(entry->ike_sa_id, ike_sa->get_id(ike_sa));
 		/* signal waiting threads */
 		entry->checked_out = FALSE;
-		entry->message_id = -1;
+		entry->processing = -1;
 		/* check if this SA is half-open */
 		if (entry->half_open && ike_sa->get_state(ike_sa) != IKE_CONNECTING)
 		{
diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c
index 8a4761d5c..7a84d1cef 100644
--- a/src/libcharon/sa/ikev1/task_manager_v1.c
+++ b/src/libcharon/sa/ikev1/task_manager_v1.c
@@ -1471,6 +1471,21 @@ METHOD(task_manager_t, process_message, status_t,
 			charon->bus->alert(charon->bus, ALERT_RETRANSMIT_RECEIVE, msg);
 			return SUCCESS;
 		}
+
+		/* reject Main/Agressive Modes once established */
+		if (msg->get_exchange_type(msg) == ID_PROT ||
+			msg->get_exchange_type(msg) == AGGRESSIVE)
+		{
+			if (this->ike_sa->get_state(this->ike_sa) != IKE_CREATED &&
+				this->ike_sa->get_state(this->ike_sa) != IKE_CONNECTING &&
+				msg->get_first_payload_type(msg) != FRAGMENT_V1)
+			{
+				DBG1(DBG_IKE, "ignoring %N in established IKE_SA state",
+					 exchange_type_names, msg->get_exchange_type(msg));
+				return FAILED;
+			}
+		}
+
 		if (msg->get_exchange_type(msg) == TRANSACTION &&
 			this->active_tasks->get_count(this->active_tasks))
 		{	/* main mode not yet complete, queue XAuth/Mode config tasks */
@@ -2030,4 +2045,3 @@ task_manager_v1_t *task_manager_v1_create(ike_sa_t *ike_sa)
 
 	return &this->public;
 }
-
diff --git a/src/libcharon/sa/ikev2/task_manager_v2.c b/src/libcharon/sa/ikev2/task_manager_v2.c
index ea0117c54..29d8d830e 100644
--- a/src/libcharon/sa/ikev2/task_manager_v2.c
+++ b/src/libcharon/sa/ikev2/task_manager_v2.c
@@ -1123,6 +1123,18 @@ METHOD(task_manager_t, process_message, status_t,
 	{
 		if (mid == this->responding.mid)
 		{
+			/* reject initial messages once established */
+			if (msg->get_exchange_type(msg) == IKE_SA_INIT ||
+				msg->get_exchange_type(msg) == IKE_AUTH)
+			{
+				if (this->ike_sa->get_state(this->ike_sa) != IKE_CREATED &&
+					this->ike_sa->get_state(this->ike_sa) != IKE_CONNECTING)
+				{
+					DBG1(DBG_IKE, "ignoring %N in established IKE_SA state",
+						 exchange_type_names, msg->get_exchange_type(msg));
+					return FAILED;
+				}
+			}
 			if (this->ike_sa->get_state(this->ike_sa) == IKE_CREATED ||
 				this->ike_sa->get_state(this->ike_sa) == IKE_CONNECTING ||
 				msg->get_exchange_type(msg) != IKE_SA_INIT)