ikev2: Destroy IKE_SA when receiving invalid authenticated requests

RFC 7296, section 2.21.3:

   If a peer parsing a request notices that it is badly formatted (after
   it has passed the message authentication code checks and window
   checks) and it returns an INVALID_SYNTAX notification, then this
   error notification is considered fatal in both peers, meaning that
   the IKE SA is deleted without needing an explicit Delete payload.

src/libcharon/sa/ikev2/task_manager_v2.c

@@ -1324,17 +1324,36 @@ static void send_notify_response(private_task_manager_t *this,
 	response->destroy(response);
 }
 
+/**
+ * Send an INVALID_SYNTAX notify and destroy the IKE_SA for authenticated
+ * messages.
+ */
+static status_t send_invalid_syntax(private_task_manager_t *this,
+									message_t *msg)
+{
+	send_notify_response(this, msg, INVALID_SYNTAX, chunk_empty);
+	incr_mid(this, FALSE);
+
+	/* IKE_SA_INIT is currently the only type the parser accepts unprotected,
+	 * don't destroy the IKE_SA if such a message is invalid */
+	if (msg->get_exchange_type(msg) == IKE_SA_INIT)
+	{
+		return FAILED;
+	}
+	return DESTROY_ME;
+}
+
 /**
  * Parse the given message and verify that it is valid.
  */
 static status_t parse_message(private_task_manager_t *this, message_t *msg)
 {
-	status_t status;
+	status_t parse_status, status;
 	uint8_t type = 0;
 
-	status = msg->parse_body(msg, this->ike_sa->get_keymat(this->ike_sa));
+	parse_status = msg->parse_body(msg, this->ike_sa->get_keymat(this->ike_sa));
 
-	if (status == SUCCESS)
+	if (parse_status == SUCCESS)
 	{	/* check for unsupported critical payloads */
 		enumerator_t *enumerator;
 		unknown_payload_t *unknown;
@@ -1350,8 +1369,8 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
 				{
 					type = unknown->get_type(unknown);
 					DBG1(DBG_ENC, "payload type %N is not supported, "
-						 "but its critical!", payload_type_names, type);
+						 "but payload is critical!", payload_type_names, type);
-					status = NOT_SUPPORTED;
+					parse_status = NOT_SUPPORTED;
 					break;
 				}
 			}
@@ -1359,11 +1378,13 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
 		enumerator->destroy(enumerator);
 	}
 
-	if (status != SUCCESS)
+	status = parse_status;
+
+	if (parse_status != SUCCESS)
 	{
 		bool is_request = msg->get_request(msg);
 
-		switch (status)
+		switch (parse_status)
 		{
 			case NOT_SUPPORTED:
 				DBG1(DBG_IKE, "critical unknown payloads found");
@@ -1379,18 +1400,14 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
 				DBG1(DBG_IKE, "message parsing failed");
 				if (is_request)
 				{
-					send_notify_response(this, msg,
+					status = send_invalid_syntax(this, msg);
-										 INVALID_SYNTAX, chunk_empty);
-					incr_mid(this, FALSE);
 				}
 				break;
 			case VERIFY_ERROR:
 				DBG1(DBG_IKE, "message verification failed");
 				if (is_request)
 				{
-					send_notify_response(this, msg,
+					status = send_invalid_syntax(this, msg);
-										 INVALID_SYNTAX, chunk_empty);
-					incr_mid(this, FALSE);
 				}
 				break;
 			case FAILED:
@@ -1407,11 +1424,25 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
 			 is_request ? "request" : "response",
 			 msg->get_message_id(msg));
 
-		charon->bus->alert(charon->bus, ALERT_PARSE_ERROR_BODY, msg, status);
+		charon->bus->alert(charon->bus, ALERT_PARSE_ERROR_BODY, msg,
+						   parse_status);
 
-		if (this->ike_sa->get_state(this->ike_sa) == IKE_CREATED)
+		switch (this->ike_sa->get_state(this->ike_sa))
-		{	/* invalid initiation attempt, close SA */
+		{
-			return DESTROY_ME;
+			case IKE_CREATED:
+				/* invalid initiation attempt, close SA */
+				status = DESTROY_ME;
+				break;
+			case IKE_CONNECTING:
+			case IKE_REKEYED:
+				/* don't trigger updown event in these states */
+				break;
+			default:
+				if (status == DESTROY_ME)
+				{
+					charon->bus->ike_updown(charon->bus, this->ike_sa, FALSE);
+				}
+				break;
 		}
 	}
 	return status;