From: Tobias Brunner <tobias@strongswan.org>
Date: Mon, 23 Mar 2015 09:58:30 +0000 (+0100)
Subject: ikev1: Make sure SPIs in an IKEv1 DELETE payload match the current SA
X-Git-Tag: 5.3.0rc1~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a7172ddaff8985b7a044b470ed8f5a571ebf5310;p=thirdparty%2Fstrongswan.git

ikev1: Make sure SPIs in an IKEv1 DELETE payload match the current SA

OpenBSD's isakmpd uses the latest ISAKMP SA to delete other expired SAs.
This caused strongSwan to delete e.g. a rekeyed SA even though isakmpd
meant to delete the old one.

What isakmpd does might not be standard compliant. As RFC 2408 puts
it:

  Deletion which is concerned with an ISAKMP SA will contain a
  Protocol-Id of ISAKMP and the SPIs are the initiator and responder
  cookies from the ISAKMP Header.

This could either be interpreted as "copy the SPIs from the ISAKMP
header of the current message to the DELETE payload" (which is what
strongSwan assumed, and the direction IKEv2 took it, by not sending SPIs
for IKE), or as clarification that ISAKMP "cookies" are actually the
SPIs meant to be put in the payload (but that any ISAKMP SA may be
deleted).
---

diff --git a/src/libcharon/sa/ikev1/tasks/isakmp_delete.c b/src/libcharon/sa/ikev1/tasks/isakmp_delete.c
index bea0428c40..a56805afb2 100644
--- a/src/libcharon/sa/ikev1/tasks/isakmp_delete.c
+++ b/src/libcharon/sa/ikev1/tasks/isakmp_delete.c
@@ -1,4 +1,7 @@
 /*
+ * Copyright (C) 2015 Tobias Brunner
+ * Hochschule fuer Technik Rapperswil
+ *
  * Copyright (C) 2011 Martin Willi
  * Copyright (C) 2011 revosec AG
  *
@@ -74,6 +77,42 @@ METHOD(task_t, process_i, status_t,
 METHOD(task_t, process_r, status_t,
 	private_isakmp_delete_t *this, message_t *message)
 {
+	enumerator_t *payloads;
+	payload_t *payload;
+	delete_payload_t *delete_payload;
+	ike_sa_id_t *id;
+	u_int64_t spi_i, spi_r;
+	bool found = FALSE;
+
+	/* some peers send DELETE payloads for other IKE_SAs, e.g. those for expired
+	 * ones after a rekeyeing, make sure the SPIs match */
+	id = this->ike_sa->get_id(this->ike_sa);
+	payloads = message->create_payload_enumerator(message);
+	while (payloads->enumerate(payloads, &payload))
+	{
+		if (payload->get_type(payload) == PLV1_DELETE)
+		{
+			delete_payload = (delete_payload_t*)payload;
+			if (!delete_payload->get_ike_spi(delete_payload, &spi_i, &spi_r))
+			{
+				continue;
+			}
+			if (id->get_initiator_spi(id) == spi_i &&
+				id->get_responder_spi(id) == spi_r)
+			{
+				found = TRUE;
+				break;
+			}
+		}
+	}
+	payloads->destroy(payloads);
+
+	if (!found)
+	{
+		DBG1(DBG_IKE, "received DELETE for different IKE_SA, ignored");
+		return SUCCESS;
+	}
+
 	DBG1(DBG_IKE, "received DELETE for IKE_SA %s[%d]",
 		 this->ike_sa->get_name(this->ike_sa),
 		 this->ike_sa->get_unique_id(this->ike_sa));