Moved IKEv1 DPD processing to task manager, fix sequence issues

3 files changed, 72 insertions, 73 deletions

diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c
index 2e2926761..7e56fb71c 100755
--- a/src/libcharon/sa/ikev1/task_manager_v1.c
+++ b/src/libcharon/sa/ikev1/task_manager_v1.c
@@ -720,6 +720,62 @@ static void send_notify(private_task_manager_t *this, message_t *request,
 }
 
 /**
+ * Process a DPD request/response
+ */
+static bool process_dpd(private_task_manager_t *this, message_t *message)
+{
+	notify_payload_t *notify;
+	notify_type_t type;
+	u_int32_t seq;
+	chunk_t data;
+
+	type = DPD_R_U_THERE;
+	notify = message->get_notify(message, type);
+	if (!notify)
+	{
+		type = DPD_R_U_THERE_ACK;
+		notify = message->get_notify(message, type);
+	}
+	if (!notify)
+	{
+		return FALSE;
+	}
+	data = notify->get_notification_data(notify);
+	if (data.len != 4)
+	{
+		return FALSE;
+	}
+	seq = untoh32(data.ptr);
+
+	if (type == DPD_R_U_THERE)
+	{
+		if (this->dpd_recv == 0 || seq == this->dpd_recv)
+		{	/* check sequence validity */
+			this->dpd_recv = seq + 1;
+			this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+										time_monotonic(NULL));
+		}
+		/* but respond anyway */
+		this->ike_sa->queue_task(this->ike_sa,
+				&isakmp_dpd_create(this->ike_sa, DPD_R_U_THERE_ACK, seq)->task);
+	}
+	else /* DPD_R_U_THERE_ACK */
+	{
+		if (seq == this->dpd_send - 1)
+		{
+			this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+										time_monotonic(NULL));
+		}
+		else
+		{
+			DBG1(DBG_IKE, "received invalid DPD sequence number %u "
+				 "(expected %u), ignored", seq, this->dpd_send - 1);
+		}
+	}
+	return TRUE;
+}
+
+/**
  * handle an incoming request message
  */
 static status_t process_request(private_task_manager_t *this,
@@ -728,8 +784,6 @@ static status_t process_request(private_task_manager_t *this,
 	enumerator_t *enumerator;
 	task_t *task = NULL;
 	bool send_response = FALSE, dpd = FALSE;
-	notify_payload_t *notify;
-	chunk_t data;
 
 	if (message->get_exchange_type(message) == INFORMATIONAL_V1 ||
 		this->passive_tasks->get_count(this->passive_tasks) == 0)
@@ -772,35 +826,15 @@ static status_t process_request(private_task_manager_t *this,
 				this->passive_tasks->insert_last(this->passive_tasks, task);
 				break;
 			case INFORMATIONAL_V1:
-				notify = message->get_notify(message, DPD_R_U_THERE);
-				if (notify)
-				{
-					data = notify->get_notification_data(notify);
-					if (this->dpd_recv == 0 && data.len == 4)
-					{	/* first DPD request, initialize counter */
-						this->dpd_recv = untoh32(data.ptr);
-					}
-					task = (task_t *)isakmp_dpd_create(this->ike_sa, FALSE,
-													   this->dpd_recv++);
-					dpd = TRUE;
-				}
-				else if ((notify = message->get_notify(message,
-													   DPD_R_U_THERE_ACK)))
+				if (process_dpd(this, message))
 				{
-					data = notify->get_notification_data(notify);
-					if (data.len == 4 && untoh32(data.ptr) == this->dpd_send)
-					{
-						this->dpd_send++;
-					}
-					task = (task_t *)isakmp_dpd_create(this->ike_sa, TRUE,
-													   this->dpd_send - 1);
 					dpd = TRUE;
 				}
 				else
 				{
 					task = (task_t *)informational_create(this->ike_sa, NULL);
+					this->passive_tasks->insert_first(this->passive_tasks, task);
 				}
-				this->passive_tasks->insert_first(this->passive_tasks, task);
 				break;
 			case TRANSACTION:
 				if (this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
@@ -817,11 +851,12 @@ static status_t process_request(private_task_manager_t *this,
 				return FAILED;
 		}
 	}
-	if (!dpd)
+	if (dpd)
 	{
-		this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-									time_monotonic(NULL));
+		return initiate(this);
 	}
+	this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND, time_monotonic(NULL));
+
 	/* let the tasks process the message */
 	enumerator = this->passive_tasks->create_enumerator(this->passive_tasks);
 	while (enumerator->enumerate(enumerator, (void*)&task))
@@ -856,12 +891,6 @@ static status_t process_request(private_task_manager_t *this,
 	}
 	enumerator->destroy(enumerator);
 
-	if (dpd && this->initiating.type == INFORMATIONAL_V1)
-	{	/* got a DPD reply, cancel any retransmission */
-		this->initiating.type = EXCHANGE_TYPE_UNDEFINED;
-		DESTROY_IF(this->initiating.packet);
-		this->initiating.packet = NULL;
-	}
 	if (send_response)
 	{
 		if (build_response(this, message) != SUCCESS)
@@ -1361,7 +1390,7 @@ METHOD(task_manager_t, queue_dpd, void,
 {
 	u_int32_t t = 0, retransmit;
 
-	queue_task(this, (task_t*)isakmp_dpd_create(this->ike_sa, TRUE,
+	queue_task(this, (task_t*)isakmp_dpd_create(this->ike_sa, DPD_R_U_THERE,
 												this->dpd_send++));
 
 	/* schedule DPD timeout job using the same timeout as a retransmitting
diff --git a/src/libcharon/sa/ikev1/tasks/isakmp_dpd.c b/src/libcharon/sa/ikev1/tasks/isakmp_dpd.c
index e470e90ad..a3395a043 100755
--- a/src/libcharon/sa/ikev1/tasks/isakmp_dpd.c
+++ b/src/libcharon/sa/ikev1/tasks/isakmp_dpd.c
@@ -36,9 +36,9 @@ struct private_isakmp_dpd_t {
 	u_int32_t seqnr;
 
 	/**
-	 * DPD initiator?
+	 * DPD notify type
 	 */
-	bool initiator;
+	notify_type_t type;
 
 	/**
 	 * IKE SA we are serving.
@@ -50,15 +50,13 @@ METHOD(task_t, build, status_t,
 	private_isakmp_dpd_t *this, message_t *message)
 {
 	notify_payload_t *notify;
-	notify_type_t type;
 	ike_sa_id_t *ike_sa_id;
 	u_int64_t spi_i, spi_r;
 	u_int32_t seqnr;
 	chunk_t spi;
 
-	type = this->initiator ? DPD_R_U_THERE : DPD_R_U_THERE_ACK;
 	notify = notify_payload_create_from_protocol_and_type(NOTIFY_V1,
-														  PROTO_IKE, type);
+														  PROTO_IKE, this->type);
 	seqnr = htonl(this->seqnr);
 	ike_sa_id = this->ike_sa->get_id(this->ike_sa);
 	spi_i = ike_sa_id->get_initiator_spi(ike_sa_id);
@@ -76,36 +74,8 @@ METHOD(task_t, build, status_t,
 METHOD(task_t, process, status_t,
 	private_isakmp_dpd_t *this, message_t *message)
 {
-	notify_payload_t *notify;
-	notify_type_t type;
-	u_int32_t seqnr = 0;
-	chunk_t chunk;
-
-	type = this->initiator ? DPD_R_U_THERE_ACK : DPD_R_U_THERE;
-	notify = message->get_notify(message, type);
-	if (notify)
-	{
-		chunk = notify->get_notification_data(notify);
-		if (chunk.len == 4)
-		{
-			seqnr = untoh32(chunk.ptr);
-			if (seqnr == this->seqnr)
-			{
-				this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-											time_monotonic(NULL));
-				if (!this->initiator)
-				{	/* queue DPD_ACK */
-					this->ike_sa->queue_task(this->ike_sa,
-								&isakmp_dpd_create(this->ike_sa, FALSE,
-												   this->seqnr)->task);
-				}
-				return SUCCESS;
-			}
-		}
-	}
-	DBG1(DBG_IKE, "received invalid DPD sequence number %u (expected %u), "
-		 "ignored", seqnr, this->seqnr);
-	return SUCCESS;
+	/* done in task manager */
+	return FAILED;
 }
 
 METHOD(task_t, get_type, task_type_t,
@@ -129,7 +99,7 @@ METHOD(task_t, destroy, void,
 /*
  * Described in header.
  */
-isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, bool initiator,
+isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, notify_type_t type,
 								u_int32_t seqnr)
 {
 	private_isakmp_dpd_t *this;
@@ -146,7 +116,7 @@ isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, bool initiator,
 		},
 		.ike_sa = ike_sa,
 		.seqnr = seqnr,
-		.initiator = initiator,
+		.type = type,
 	);
 
 	return &this->public;
diff --git a/src/libcharon/sa/ikev1/tasks/isakmp_dpd.h b/src/libcharon/sa/ikev1/tasks/isakmp_dpd.h
index 688d19fd8..13bd34e44 100755
--- a/src/libcharon/sa/ikev1/tasks/isakmp_dpd.h
+++ b/src/libcharon/sa/ikev1/tasks/isakmp_dpd.h
@@ -42,11 +42,11 @@ struct isakmp_dpd_t {
  * Create a new ISAKMP_DPD task.
  *
  * @param ike_sa		associated IKE_SA
- * @param initiator		TRUE if DPD initiator
+ * @param type			DPD notify to use, DPD_R_U_THERE | DPD_R_U_THERE_ACK
  * @param seqnr			DPD sequence number to use/expect
  * @return				ISAKMP_DPD task to handle by the task_manager
  */
-isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, bool initiator,
+isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, notify_type_t type,
 								u_int32_t seqnr);
 
 #endif /** ISAKMP_DPD_H_ @}*/