Moved IKEv1 DPD processing to task manager, fix sequence issues
authorMartin Willi <martin@revosec.ch>
Tue, 15 May 2012 14:10:47 +0000 (16:10 +0200)
committerMartin Willi <martin@revosec.ch>
Tue, 15 May 2012 15:00:12 +0000 (17:00 +0200)
src/libcharon/sa/ikev1/task_manager_v1.c
src/libcharon/sa/ikev1/tasks/isakmp_dpd.c
src/libcharon/sa/ikev1/tasks/isakmp_dpd.h

index 2e29267..7e56fb7 100755 (executable)
@@ -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
index e470e90..a3395a0 100755 (executable)
@@ -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;
index 688d19f..13bd34e 100755 (executable)
@@ -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_ @}*/