ikev2: Trigger ike_updown() event after all IKE-specific tasks ran

This makes sure the event is only triggered after the IKE_SA is fully
established and e.g. virtual IPs, additional peer addresses or
a modified reauth time (on the initiator) are assigned to it.  This was
e.g. a problem for the selinux plugin if virtual IPs are used.

We use a separate task to trigger the event that's queued before the
child-create task so the event is triggered before the child_updown()
event.  Same goes for the state change to IKE_ESTABLISHED.

A new condition is used to indicate the successful completion of all
authentication rounds, so we don't have to set the IKE_ESTABLISHED state
in the ike-auth task (it was used as condition in other tasks).

Since set_state() also sets the rekey and reauth times, this required
some minor changes in regards to how AUTH_LIFETIME notifies are handled.

src/libcharon/Android.mk

@@ -103,6 +103,7 @@ sa/ikev2/tasks/ike_cert_post.c sa/ikev2/tasks/ike_cert_post.h \
 sa/ikev2/tasks/ike_config.c sa/ikev2/tasks/ike_config.h \
 sa/ikev2/tasks/ike_delete.c sa/ikev2/tasks/ike_delete.h \
 sa/ikev2/tasks/ike_dpd.c sa/ikev2/tasks/ike_dpd.h \
+sa/ikev2/tasks/ike_establish.c sa/ikev2/tasks/ike_establish.h \
 sa/ikev2/tasks/ike_init.c sa/ikev2/tasks/ike_init.h \
 sa/ikev2/tasks/ike_natd.c sa/ikev2/tasks/ike_natd.h \
 sa/ikev2/tasks/ike_mid_sync.c sa/ikev2/tasks/ike_mid_sync.h \

src/libcharon/Makefile.am

@@ -102,6 +102,7 @@ sa/ikev2/tasks/ike_cert_post.c sa/ikev2/tasks/ike_cert_post.h \
 sa/ikev2/tasks/ike_config.c sa/ikev2/tasks/ike_config.h \
 sa/ikev2/tasks/ike_delete.c sa/ikev2/tasks/ike_delete.h \
 sa/ikev2/tasks/ike_dpd.c sa/ikev2/tasks/ike_dpd.h \
+sa/ikev2/tasks/ike_establish.c sa/ikev2/tasks/ike_establish.h \
 sa/ikev2/tasks/ike_init.c sa/ikev2/tasks/ike_init.h \
 sa/ikev2/tasks/ike_natd.c sa/ikev2/tasks/ike_natd.h \
 sa/ikev2/tasks/ike_mid_sync.c sa/ikev2/tasks/ike_mid_sync.h \

src/libcharon/sa/ike_sa.c

@@ -893,6 +893,11 @@ METHOD(ike_sa_t, set_state, void,
 					lib->scheduler->schedule_job(lib->scheduler, job, t);
 					DBG1(DBG_IKE, "scheduling reauthentication in %ds", t);
 				}
+				else if (this->stats[STAT_REAUTH])
+				{
+					t = this->stats[STAT_REAUTH] - this->stats[STAT_ESTABLISHED];
+					DBG1(DBG_IKE, "reauthentication already scheduled in %ds", t);
+				}
 				t = this->peer_cfg->get_over_time(this->peer_cfg);
 				if (this->stats[STAT_REKEY] || this->stats[STAT_REAUTH])
 				{

src/libcharon/sa/ike_sa.h

@@ -245,6 +245,11 @@ enum ike_condition_t {
 	 * A Postquantum Preshared Key was used when this IKE_SA was created
 	 */
 	COND_PPK = (1<<13),
+
+	/**
+	 * All authentication rounds have been completed successfully
+	 */
+	COND_AUTHENTICATED = (1<<14),
 };
 
 /**

src/libcharon/sa/ikev2/task_manager_v2.c

@@ -38,6 +38,7 @@
 #include <sa/ikev2/tasks/ike_mid_sync.h>
 #include <sa/ikev2/tasks/ike_vendor.h>
 #include <sa/ikev2/tasks/ike_verify_peer_cert.h>
+#include <sa/ikev2/tasks/ike_establish.h>
 #include <sa/ikev2/tasks/child_create.h>
 #include <sa/ikev2/tasks/child_rekey.h>
 #include <sa/ikev2/tasks/child_delete.h>
@@ -554,6 +555,8 @@ METHOD(task_manager_t, initiate, status_t,
 					activate_task(this, TASK_IKE_CONFIG);
 					activate_task(this, TASK_IKE_AUTH_LIFETIME);
 					activate_task(this, TASK_IKE_MOBIKE);
+					/* make sure this is the last IKE-related task */
+					activate_task(this, TASK_IKE_ESTABLISH);
 					activate_task(this, TASK_CHILD_CREATE);
 				}
 				break;
@@ -1151,10 +1154,15 @@ static status_t process_request(private_task_manager_t *this,
 #endif /* ME */
 				task = (task_t*)ike_config_create(this->ike_sa, FALSE);
 				array_insert(this->passive_tasks, ARRAY_TAIL, task);
-				task = (task_t*)ike_auth_lifetime_create(this->ike_sa, FALSE);
-				array_insert(this->passive_tasks, ARRAY_TAIL, task);
 				task = (task_t*)ike_mobike_create(this->ike_sa, FALSE);
 				array_insert(this->passive_tasks, ARRAY_TAIL, task);
+				/* this should generally be the last IKE-related task */
+				task = (task_t*)ike_establish_create(this->ike_sa, FALSE);
+				array_insert(this->passive_tasks, ARRAY_TAIL, task);
+				/* make sure this comes after the above task to send the correct
+				 * reauth time, as responder the task doesn't modify it anymore */
+				task = (task_t*)ike_auth_lifetime_create(this->ike_sa, FALSE);
+				array_insert(this->passive_tasks, ARRAY_TAIL, task);
 				task = (task_t*)child_create_create(this->ike_sa, NULL, FALSE,
 													NULL, NULL);
 				array_insert(this->passive_tasks, ARRAY_TAIL, task);
@@ -2129,6 +2137,10 @@ METHOD(task_manager_t, queue_ike, void,
 			queue_task(this, (task_t*)ike_mobike_create(this->ike_sa, TRUE));
 		}
 	}
+	if (!has_queued(this, TASK_IKE_ESTABLISH))
+	{
+		queue_task(this, (task_t*)ike_establish_create(this->ike_sa, TRUE));
+	}
 #ifdef ME
 	if (!has_queued(this, TASK_IKE_ME))
 	{

src/libcharon/sa/ikev2/tasks/child_create.c

@@ -1552,7 +1552,7 @@ METHOD(task_t, build_r, status_t,
 			no_dh = FALSE;
 			break;
 		case IKE_AUTH:
-			if (this->ike_sa->get_state(this->ike_sa) != IKE_ESTABLISHED)
+			if (!this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 			{	/* wait until all authentication round completed */
 				return NEED_MORE;
 			}
@@ -1762,7 +1762,7 @@ METHOD(task_t, process_i, status_t,
 			no_dh = FALSE;
 			break;
 		case IKE_AUTH:
-			if (this->ike_sa->get_state(this->ike_sa) != IKE_ESTABLISHED)
+			if (!this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 			{	/* wait until all authentication round completed */
 				return NEED_MORE;
 			}

src/libcharon/sa/ikev2/tasks/ike_auth.c

@@ -1167,15 +1167,8 @@ METHOD(task_t, build_r, status_t,
 							"%s.half_open_timeout", HALF_OPEN_IKE_SA_TIMEOUT,
 							lib->ns));
 	}
-	DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
+
-		 this->ike_sa->get_name(this->ike_sa),
+	this->ike_sa->set_condition(this->ike_sa, COND_AUTHENTICATED, TRUE);
-		 this->ike_sa->get_unique_id(this->ike_sa),
-		 this->ike_sa->get_my_host(this->ike_sa),
-		 this->ike_sa->get_my_id(this->ike_sa),
-		 this->ike_sa->get_other_host(this->ike_sa),
-		 this->ike_sa->get_other_id(this->ike_sa));
-	this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
-	charon->bus->ike_updown(charon->bus, this->ike_sa, TRUE);
 	return SUCCESS;
 
 peer_auth_failed:
@@ -1504,15 +1497,8 @@ METHOD(task_t, process_i, status_t,
 				      "canceling");
 		goto peer_auth_failed;
 	}
-	DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
+
-		 this->ike_sa->get_name(this->ike_sa),
+	this->ike_sa->set_condition(this->ike_sa, COND_AUTHENTICATED, TRUE);
-		 this->ike_sa->get_unique_id(this->ike_sa),
-		 this->ike_sa->get_my_host(this->ike_sa),
-		 this->ike_sa->get_my_id(this->ike_sa),
-		 this->ike_sa->get_other_host(this->ike_sa),
-		 this->ike_sa->get_other_id(this->ike_sa));
-	this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
-	charon->bus->ike_updown(charon->bus, this->ike_sa, TRUE);
 
 	if (this->redirect_to)
 	{

src/libcharon/sa/ikev2/tasks/ike_auth_lifetime.c

@@ -102,7 +102,7 @@ METHOD(task_t, build_r, status_t,
 	private_ike_auth_lifetime_t *this, message_t *message)
 {
 	if (message->get_exchange_type(message) == IKE_AUTH &&
-		this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
+		this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{
 		if (!ike_sa_can_reauthenticate(this->ike_sa))
 		{
@@ -117,7 +117,7 @@ METHOD(task_t, process_i, status_t,
 	private_ike_auth_lifetime_t *this, message_t *message)
 {
 	if (message->get_exchange_type(message) == IKE_AUTH &&
-		this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
+		this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{
 		process_payloads(this, message);
 		return SUCCESS;

src/libcharon/sa/ikev2/tasks/ike_cert_post.c

@@ -275,7 +275,7 @@ METHOD(task_t, build_r, status_t,
 	{
 		build_certs(this, message);
 	}
-	if (this->ike_sa->get_state(this->ike_sa) != IKE_ESTABLISHED)
+	if (!this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{	/* stay alive, we might have additional rounds with certs */
 		return NEED_MORE;
 	}
@@ -285,7 +285,7 @@ METHOD(task_t, build_r, status_t,
 METHOD(task_t, process_i, status_t,
 	private_ike_cert_post_t *this, message_t *message)
 {
-	if (this->ike_sa->get_state(this->ike_sa) != IKE_ESTABLISHED)
+	if (!this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{	/* stay alive, we might have additional rounds with CERTS */
 		return NEED_MORE;
 	}

src/libcharon/sa/ikev2/tasks/ike_config.c

@@ -346,7 +346,7 @@ METHOD(task_t, process_r, status_t,
 METHOD(task_t, build_r, status_t,
 	private_ike_config_t *this, message_t *message)
 {
-	if (this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
+	if (this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{	/* in last IKE_AUTH exchange */
 		enumerator_t *enumerator;
 		configuration_attribute_type_t type;
@@ -454,7 +454,7 @@ METHOD(task_t, build_r, status_t,
 METHOD(task_t, process_i, status_t,
 	private_ike_config_t *this, message_t *message)
 {
-	if (this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
+	if (this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{	/* in last IKE_AUTH exchange */
 		enumerator_t *enumerator;
 		host_t *host;

src/libcharon/sa/ikev2/tasks/ike_establish.c

@@ -0,0 +1,139 @@
+/*
+ * Copyright (C) 2022 Tobias Brunner
+ *
+ * Copyright (C) secunet Security Networks AG
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.  See <http://www.fsf.org/copyleft/gpl.txt>.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ */
+
+#include "ike_establish.h"
+
+#include <daemon.h>
+
+typedef struct private_ike_establish_t private_ike_establish_t;
+
+/**
+ * Private members.
+ */
+struct private_ike_establish_t {
+
+	/**
+	 * Public interface.
+	 */
+	ike_establish_t public;
+
+	/**
+	 * Assigned IKE_SA.
+	 */
+	ike_sa_t *ike_sa;
+};
+
+METHOD(task_t, build_i, status_t,
+	private_ike_establish_t *this, message_t *message)
+{
+	return NEED_MORE;
+}
+
+METHOD(task_t, process_r, status_t,
+	private_ike_establish_t *this, message_t *message)
+{
+	return NEED_MORE;
+}
+
+/**
+ * Mark the IKE_SA as established, notify listeners.
+ */
+static void establish(private_ike_establish_t *this)
+{
+	DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
+		 this->ike_sa->get_name(this->ike_sa),
+		 this->ike_sa->get_unique_id(this->ike_sa),
+		 this->ike_sa->get_my_host(this->ike_sa),
+		 this->ike_sa->get_my_id(this->ike_sa),
+		 this->ike_sa->get_other_host(this->ike_sa),
+		 this->ike_sa->get_other_id(this->ike_sa));
+	this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
+	charon->bus->ike_updown(charon->bus, this->ike_sa, TRUE);
+}
+
+METHOD(task_t, build_r, status_t,
+	private_ike_establish_t *this, message_t *message)
+{
+	if (message->get_exchange_type(message) == IKE_AUTH &&
+		this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
+	{
+		establish(this);
+		return SUCCESS;
+	}
+	return NEED_MORE;
+}
+
+METHOD(task_t, process_i, status_t,
+	private_ike_establish_t *this, message_t *message)
+{
+	if (message->get_exchange_type(message) == IKE_AUTH &&
+		this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
+	{
+		establish(this);
+		return SUCCESS;
+	}
+	return NEED_MORE;
+}
+
+METHOD(task_t, get_type, task_type_t,
+	private_ike_establish_t *this)
+{
+	return TASK_IKE_ESTABLISH;
+}
+
+METHOD(task_t, migrate, void,
+	private_ike_establish_t *this, ike_sa_t *ike_sa)
+{
+	this->ike_sa = ike_sa;
+}
+
+METHOD(task_t, destroy, void,
+	private_ike_establish_t *this)
+{
+	free(this);
+}
+
+/*
+ * Described in header
+ */
+ike_establish_t *ike_establish_create(ike_sa_t *ike_sa, bool initiator)
+{
+	private_ike_establish_t *this;
+
+	INIT(this,
+		.public = {
+			.task = {
+				.get_type = _get_type,
+				.migrate = _migrate,
+				.destroy = _destroy,
+			},
+		},
+		.ike_sa = ike_sa,
+	);
+
+	if (initiator)
+	{
+		this->public.task.build = _build_i;
+		this->public.task.process = _process_i;
+	}
+	else
+	{
+		this->public.task.build = _build_r;
+		this->public.task.process = _process_r;
+	}
+
+	return &this->public;
+}

src/libcharon/sa/ikev2/tasks/ike_establish.h

@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2022 Tobias Brunner
+ *
+ * Copyright (C) secunet Security Networks AG
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.  See <http://www.fsf.org/copyleft/gpl.txt>.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ */
+
+/**
+ * @defgroup ike_establish ike_establish
+ * @{ @ingroup tasks_v2
+ */
+
+#ifndef IKE_ESTABLISH_H_
+#define IKE_ESTABLISH_H_
+
+typedef struct ike_establish_t ike_establish_t;
+
+#include <library.h>
+#include <sa/ike_sa.h>
+#include <sa/task.h>
+
+/**
+ * Task of type TASK_IKE_ESTABLISH that sets the state of the IKE_SA to
+ * IKE_ESTABLISHED and triggers the ike_updown() event.
+ */
+struct ike_establish_t {
+
+	/**
+	 * Implements the task_t interface
+	 */
+	task_t task;
+};
+
+/**
+ * Create a new TASK_IKE_ESTABLISH task.
+ *
+ * @param ike_sa		IKE_SA this task works for
+ * @param initiator		TRUE if task is initiated by us
+ * @return				created task
+ */
+ike_establish_t *ike_establish_create(ike_sa_t *ike_sa, bool initiator);
+
+#endif /** IKE_ESTABLISH_H_ @}*/

src/libcharon/sa/ikev2/tasks/ike_mobike.c

@@ -474,7 +474,7 @@ METHOD(task_t, build_r, status_t,
 	   private_ike_mobike_t *this, message_t *message)
 {
 	if (message->get_exchange_type(message) == IKE_AUTH &&
-		this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
+		this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{	/* in last IKE_AUTH only */
 		if (this->ike_sa->supports_extension(this->ike_sa, EXT_MOBIKE))
 		{
@@ -503,7 +503,7 @@ METHOD(task_t, process_i, status_t,
 	   private_ike_mobike_t *this, message_t *message)
 {
 	if (message->get_exchange_type(message) == IKE_AUTH &&
-		this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
+		this->ike_sa->has_condition(this->ike_sa, COND_AUTHENTICATED))
 	{	/* in last IKE_AUTH only */
 		process_payloads(this, message);
 		return SUCCESS;

src/libcharon/sa/task.c

@@ -35,6 +35,7 @@ ENUM(task_type_names, TASK_IKE_INIT, TASK_ISAKMP_CERT_POST,
 	"IKE_DELETE",
 	"IKE_DPD",
 	"IKE_VENDOR",
+	"IKE_ESTABLISH",
 #ifdef ME
 	"IKE_ME",
 #endif /* ME */

src/libcharon/sa/task.h

@@ -70,6 +70,8 @@ enum task_type_t {
 	TASK_IKE_DPD,
 	/** Vendor ID processing */
 	TASK_IKE_VENDOR,
+	/** mark IKE_SA established and trigger the ike_updown() event */
+	TASK_IKE_ESTABLISH,
 #ifdef ME
 	/** handle ME stuff */
 	TASK_IKE_ME,

testing/tests/ikev2/reauth-early/evaltest.dat

@@ -1,6 +1,7 @@
 moon:: swanctl --list-sas --ike-id 2 --raw 2> /dev/null::rw.*version=2 state=ESTABLISHED local-host=PH_IP_MOON local-port=4500 local-id=moon.strongswan.org remote-host=PH_IP_CAROL remote-port=4500 [email protected].*encr-alg=AES_CBC encr-keysize=128 integ-alg=HMAC_SHA2_256_128 prf-alg=PRF_HMAC_SHA2_256 dh-group=CURVE_25519.*child-sas.*net.*reqid=1 state=INSTALLED mode=TUNNEL.*ESP.*encr-alg=AES_GCM_16 encr-keysize=128.*local-ts=\[10.1.0.0/16] remote-ts=\[192.168.0.100/32]::YES
 carol::swanctl --list-sas --ike-id 2 --raw 2> /dev/null::home.*version=2 state=ESTABLISHED local-host=PH_IP_CAROL local-port=4500 [email protected] remote-host=PH_IP_MOON remote-port=4500 remote-id=moon.strongswan.org initiator=yes.*encr-alg=AES_CBC encr-keysize=128 integ-alg=HMAC_SHA2_256_128 prf-alg=PRF_HMAC_SHA2_256 dh-group=CURVE_25519.*child-sas.*home.*state=INSTALLED mode=TUNNEL.*ESP.*encr-alg=AES_GCM_16 encr-keysize=128.*local-ts=\[192.168.0.100/32] remote-ts=\[10.1.0.0/16]::YES
 carol::cat /var/log/daemon.log::received AUTH_LIFETIME of 30s, scheduling reauthentication in 20s::YES
+carol::cat /var/log/daemon.log::reauthentication already scheduled in 20s::YES
 carol::ping -c 1 PH_IP_ALICE::64 bytes from PH_IP_ALICE: icmp_.eq=1::YES
 moon::tcpdump::IP carol.strongswan.org > moon.strongswan.org: ESP::YES
 moon::tcpdump::IP moon.strongswan.org > carol.strongswan.org: ESP::YES

testing/tests/ikev2/reauth-late/evaltest.dat

@@ -1,7 +1,7 @@
 moon:: swanctl --list-sas --ike-id 2 --raw 2> /dev/null::rw.*version=2 state=ESTABLISHED local-host=PH_IP_MOON local-port=4500 local-id=moon.strongswan.org remote-host=PH_IP_CAROL remote-port=4500 [email protected].*encr-alg=AES_CBC encr-keysize=128 integ-alg=HMAC_SHA2_256_128 prf-alg=PRF_HMAC_SHA2_256 dh-group=CURVE_25519.*child-sas.*net.*reqid=1 state=INSTALLED mode=TUNNEL.*ESP.*encr-alg=AES_GCM_16 encr-keysize=128.*local-ts=\[10.1.0.0/16] remote-ts=\[10.3.0.1/32]::YES
 carol::swanctl --list-sas --ike-id 2 --raw 2> /dev/null::home.*version=2 state=ESTABLISHED local-host=PH_IP_CAROL local-port=4500 [email protected] remote-host=PH_IP_MOON remote-port=4500 remote-id=moon.strongswan.org initiator=yes.*encr-alg=AES_CBC encr-keysize=128 integ-alg=HMAC_SHA2_256_128 prf-alg=PRF_HMAC_SHA2_256 dh-group=CURVE_25519.*child-sas.*home.*state=INSTALLED mode=TUNNEL.*ESP.*encr-alg=AES_GCM_16 encr-keysize=128.*local-ts=\[10.3.0.1/32] remote-ts=\[10.1.0.0/16]::YES
+carol::cat /var/log/daemon.log::received AUTH_LIFETIME of 3600s, scheduling reauthentication in 3590s::YES
 carol::cat /var/log/daemon.log::scheduling reauthentication in 20s::YES
-carol::cat /var/log/daemon.log::received AUTH_LIFETIME of 3600s, reauthentication already scheduled in 20s::YES
 carol::ping -c 1 PH_IP_ALICE::64 bytes from PH_IP_ALICE: icmp_.eq=1::YES
 moon::tcpdump::IP carol.strongswan.org > moon.strongswan.org: ESP::YES
 moon::tcpdump::IP moon.strongswan.org > carol.strongswan.org: ESP::YES