From 9de3140dbf67de5b52431e2f1d8e0b2a37e86219 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 6 Aug 2018 17:01:20 +0200 Subject: [PATCH] ikev1: Increase DPD sequence number only after receiving a response We don't retransmit DPD requests like we do requests for proper exchanges, so increasing the number with each sent DPD could result in the peer's state getting out of sync if DPDs are lost. Because according to RFC 3706, DPDs with an unexpected sequence number SHOULD be rejected (it does mention the possibility of maintaining a window of acceptable numbers, but we currently don't implement that). We partially ignore such messages (i.e. we don't update the expected sequence number and the inbound message stats, so we might send a DPD when none is required). However, we always send a response, so a peer won't really notice this (it also ensures a reply for "retransmits" caused by this change, i.e. multiple DPDs with the same number - hopefully, other implementations behave similarly when receiving such messages). Fixes #2714. --- src/libcharon/sa/ikev1/task_manager_v1.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c index 3f1a97bc4..5f6c3bbe8 100644 --- a/src/libcharon/sa/ikev1/task_manager_v1.c +++ b/src/libcharon/sa/ikev1/task_manager_v1.c @@ -921,15 +921,16 @@ static bool process_dpd(private_task_manager_t *this, message_t *message) } else /* DPD_R_U_THERE_ACK */ { - if (seq == this->dpd_send - 1) + if (seq == this->dpd_send) { + this->dpd_send++; 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); + "(expected %u), ignored", seq, this->dpd_send); } } return TRUE; @@ -1844,7 +1845,7 @@ METHOD(task_manager_t, queue_dpd, void, uint32_t t, retransmit; queue_task(this, (task_t*)isakmp_dpd_create(this->ike_sa, DPD_R_U_THERE, - this->dpd_send++)); + this->dpd_send)); peer_cfg = this->ike_sa->get_peer_cfg(this->ike_sa); /* compute timeout in milliseconds */