Move critical bit checking to ike_sa, notify payload includes unsupported payload type

5 files changed, 62 insertions, 26 deletions

diff --git a/src/libcharon/encoding/message.c b/src/libcharon/encoding/message.c
index 913b57c62..41f0edeaf 100644
--- a/src/libcharon/encoding/message.c
+++ b/src/libcharon/encoding/message.c
@@ -1313,21 +1313,9 @@ static status_t verify(private_message_t *this)
 		while (enumerator->enumerate(enumerator, &payload))
 		{
 			payload_type_t type;
-			unknown_payload_t *unknown;
 
 			type = payload->get_type(payload);
-			if (!payload_is_known(type))
-			{
-				unknown = (unknown_payload_t*)payload;
-				if (unknown->is_critical(unknown))
-				{
-					DBG1(DBG_ENC, "payload type %N is not supported, "
-						 "but its critical!", payload_type_names, type);
-					enumerator->destroy(enumerator);
-					return NOT_SUPPORTED;
-				}
-			}
-			else if (type == rule->type)
+			if (type == rule->type)
 			{
 				found++;
 				DBG2(DBG_ENC, "found payload of type %N",
diff --git a/src/libcharon/encoding/message.h b/src/libcharon/encoding/message.h
index 899ced24a..7db9e0154 100644
--- a/src/libcharon/encoding/message.h
+++ b/src/libcharon/encoding/message.h
@@ -211,8 +211,6 @@ struct message_t {
 	 * @param aead		aead transform to verify/decrypt message
 	 * @return
 	 * 					- SUCCESS if parsing successful
-	 * 					- NOT_SUPPORTED if ciritcal unknown payloads found
-	 * 					- NOT_SUPPORTED if message type is not supported!
 	 *					- PARSE_ERROR if message parsing failed
 	 * 					- VERIFY_ERROR if message verification failed (bad syntax)
 	 * 					- FAILED if integrity check failed
diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 04e3d9ebd..b012c3d39 100644
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -50,6 +50,7 @@
 #include <processing/jobs/send_dpd_job.h>
 #include <processing/jobs/send_keepalive_job.h>
 #include <processing/jobs/rekey_ike_sa_job.h>
+#include <encoding/payloads/unknown_payload.h>
 
 #ifdef ME
 #include <sa/tasks/ike_me.h>
@@ -901,7 +902,7 @@ METHOD(ike_sa_t, generate_message, status_t,
  * send a notify back to the sender
  */
 static void send_notify_response(private_ike_sa_t *this, message_t *request,
-								 notify_type_t type)
+								 notify_type_t type, chunk_t data)
 {
 	message_t *response;
 	packet_t *packet;
@@ -910,7 +911,7 @@ static void send_notify_response(private_ike_sa_t *this, message_t *request,
 	response->set_exchange_type(response, request->get_exchange_type(request));
 	response->set_request(response, FALSE);
 	response->set_message_id(response, request->get_message_id(request));
-	response->add_notify(response, FALSE, type, chunk_empty);
+	response->add_notify(response, FALSE, type, data);
 	if (this->my_host->is_anyaddr(this->my_host))
 	{
 		this->my_host->destroy(this->my_host);
@@ -1175,6 +1176,7 @@ METHOD(ike_sa_t, process_message, status_t,
 {
 	status_t status;
 	bool is_request;
+	u_int8_t type = 0;
 
 	if (this->state == IKE_PASSIVE)
 	{	/* do not handle messages in passive state */
@@ -1185,9 +1187,29 @@ METHOD(ike_sa_t, process_message, status_t,
 
 	status = message->parse_body(message,
 								 this->keymat->get_aead(this->keymat, TRUE));
+	if (status == SUCCESS)
+	{	/* check for unsupported critical payloads */
+		enumerator_t *enumerator;
+		unknown_payload_t *unknown;
+		payload_t *payload;
+
+		enumerator = message->create_payload_enumerator(message);
+		while (enumerator->enumerate(enumerator, &payload))
+		{
+			unknown = (unknown_payload_t*)payload;
+			type = payload->get_type(payload);
+			if (!payload_is_known(type) &&
+				unknown->is_critical(unknown))
+			{
+				DBG1(DBG_ENC, "payload type %N is not supported, "
+					 "but its critical!", payload_type_names, type);
+				status = NOT_SUPPORTED;
+			}
+		}
+		enumerator->destroy(enumerator);
+	}
 	if (status != SUCCESS)
 	{
-
 		if (is_request)
 		{
 			switch (status)
@@ -1196,21 +1218,28 @@ METHOD(ike_sa_t, process_message, status_t,
 					DBG1(DBG_IKE, "critical unknown payloads found");
 					if (is_request)
 					{
-						send_notify_response(this, message, UNSUPPORTED_CRITICAL_PAYLOAD);
+						send_notify_response(this, message,
+											 UNSUPPORTED_CRITICAL_PAYLOAD,
+											 chunk_from_thing(type));
+						this->task_manager->incr_mid(this->task_manager, FALSE);
 					}
 					break;
 				case PARSE_ERROR:
 					DBG1(DBG_IKE, "message parsing failed");
 					if (is_request)
 					{
-						send_notify_response(this, message, INVALID_SYNTAX);
+						send_notify_response(this, message,
+											 INVALID_SYNTAX, chunk_empty);
+						this->task_manager->incr_mid(this->task_manager, FALSE);
 					}
 					break;
 				case VERIFY_ERROR:
 					DBG1(DBG_IKE, "message verification failed");
 					if (is_request)
 					{
-						send_notify_response(this, message, INVALID_SYNTAX);
+						send_notify_response(this, message,
+											 INVALID_SYNTAX, chunk_empty);
+						this->task_manager->incr_mid(this->task_manager, FALSE);
 					}
 					break;
 				case FAILED:
@@ -1219,10 +1248,6 @@ METHOD(ike_sa_t, process_message, status_t,
 					break;
 				case INVALID_STATE:
 					DBG1(DBG_IKE, "found encrypted message, but no keys available");
-					if (is_request)
-					{
-						send_notify_response(this, message, INVALID_SYNTAX);
-					}
 				default:
 					break;
 			}
@@ -1252,7 +1277,8 @@ METHOD(ike_sa_t, process_message, status_t,
 				/* no config found for these hosts, destroy */
 				DBG1(DBG_IKE, "no IKE config found for %H...%H, sending %N",
 					 me, other, notify_type_names, NO_PROPOSAL_CHOSEN);
-				send_notify_response(this, message, NO_PROPOSAL_CHOSEN);
+				send_notify_response(this, message,
+									 NO_PROPOSAL_CHOSEN, chunk_empty);
 				return DESTROY_ME;
 			}
 			/* add a timeout if peer does not establish it completely */
diff --git a/src/libcharon/sa/task_manager.c b/src/libcharon/sa/task_manager.c
index 6dda6d27a..8e4177409 100644
--- a/src/libcharon/sa/task_manager.c
+++ b/src/libcharon/sa/task_manager.c
@@ -1008,6 +1008,19 @@ METHOD(task_manager_t, busy, bool,
 	return (this->active_tasks->get_count(this->active_tasks) > 0);
 }
 
+METHOD(task_manager_t, incr_mid, void,
+	private_task_manager_t *this, bool initiate)
+{
+	if (initiate)
+	{
+		this->initiating.mid++;
+	}
+	else
+	{
+		this->responding.mid++;
+	}
+}
+
 METHOD(task_manager_t, reset, void,
 	private_task_manager_t *this, u_int32_t initiate, u_int32_t respond)
 {
@@ -1091,6 +1104,7 @@ task_manager_t *task_manager_create(ike_sa_t *ike_sa)
 			.queue_task = _queue_task,
 			.initiate = _initiate,
 			.retransmit = _retransmit,
+			.incr_mid = _incr_mid,
 			.reset = _reset,
 			.adopt_tasks = _adopt_tasks,
 			.busy = _busy,
diff --git a/src/libcharon/sa/task_manager.h b/src/libcharon/sa/task_manager.h
index 14fccd5f9..5bc6c80c4 100644
--- a/src/libcharon/sa/task_manager.h
+++ b/src/libcharon/sa/task_manager.h
@@ -149,6 +149,16 @@ struct task_manager_t {
 	void (*adopt_tasks) (task_manager_t *this, task_manager_t *other);
 
 	/**
+	 * Increment a message ID counter, in- or outbound.
+	 *
+	 * If a message is processed outside of the manager, this call increments
+	 * the message ID counters of the task manager.
+	 *
+	 * @param inititate		TRUE to increment the initiating ID
+	 */
+	void (*incr_mid)(task_manager_t *this, bool initiate);
+
+	/**
 	 * Reset message ID counters of the task manager.
 	 *
 	 * The IKEv2 protocol requires to restart exchanges with message IDs