Simplified DPD handling by using a task for a single message only
authorMartin Willi <martin@revosec.ch>
Tue, 10 Jan 2012 16:21:52 +0000 (17:21 +0100)
committerMartin Willi <martin@revosec.ch>
Tue, 20 Mar 2012 16:31:35 +0000 (17:31 +0100)
src/libcharon/sa/ikev1/task_manager_v1.c
src/libcharon/sa/ikev1/tasks/aggressive_mode.c
src/libcharon/sa/ikev1/tasks/informational.c
src/libcharon/sa/ikev1/tasks/informational.h
src/libcharon/sa/ikev1/tasks/isakmp_dpd.c
src/libcharon/sa/ikev1/tasks/isakmp_dpd.h
src/libcharon/sa/ikev1/tasks/main_mode.c
src/libcharon/sa/ikev1/tasks/quick_mode.c

index c07df41..ec5eee1 100755 (executable)
@@ -203,12 +203,12 @@ struct private_task_manager_t {
        /**
         * Sequence number for sending DPD requests
         */
-       u_int32_t dpd_send_seqnr;
+       u_int32_t dpd_send;
 
        /**
         * Sequence number for received DPD requests
         */
-       u_int32_t dpd_rec_seqnr;
+       u_int32_t dpd_recv;
 };
 
 /**
@@ -736,7 +736,8 @@ static status_t process_request(private_task_manager_t *this,
        enumerator_t *enumerator;
        task_t *task = NULL;
        bool send_response = FALSE;
-       bool informational = 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)
@@ -779,9 +780,27 @@ static status_t process_request(private_task_manager_t *this,
                                this->passive_tasks->insert_last(this->passive_tasks, task);
                                break;
                        case INFORMATIONAL_V1:
-                               task = (task_t *)informational_create(this->ike_sa, NULL, this->dpd_rec_seqnr);
+                               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++);
+                               }
+                               else if (message->get_notify(message, DPD_R_U_THERE_ACK))
+                               {
+                                       task = (task_t *)isakmp_dpd_create(this->ike_sa, TRUE,
+                                                                                                          this->dpd_send - 1);
+                               }
+                               else
+                               {
+                                       task = (task_t *)informational_create(this->ike_sa, NULL);
+                               }
                                this->passive_tasks->insert_first(this->passive_tasks, task);
-                               informational = TRUE;
                                break;
                        case TRANSACTION:
                                if (this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED)
@@ -812,15 +831,6 @@ static status_t process_request(private_task_manager_t *this,
                        case NEED_MORE:
                                /* processed, but task needs at least another call to build() */
                                send_response = TRUE;
-                               if (informational && !this->dpd_rec_seqnr)
-                               {
-                                       /* Update the received DPD sequence number if it the first received one */
-                                       if (task->get_type(task) == TASK_ISAKMP_DPD)
-                                       {
-                                               isakmp_dpd_t *isakmp_dpd = (isakmp_dpd_t *)task;
-                                               this->dpd_rec_seqnr = isakmp_dpd->get_dpd_seqnr(isakmp_dpd);
-                                       }
-                               }
                                continue;
                        case ALREADY_DONE:
                                send_response = FALSE;
@@ -987,7 +997,6 @@ METHOD(task_manager_t, process_message, status_t,
        u_int32_t hash, mid, i;
        host_t *me, *other;
        status_t status;
-       bool dpd_response = FALSE;
 
        /* TODO-IKEv1: update hosts more selectively */
        me = msg->get_destination(msg);
@@ -1015,27 +1024,7 @@ METHOD(task_manager_t, process_message, status_t,
                }
        }
 
-       /* DPD Acks are not sent with a same message ID as the request.*/
-       if (msg->get_exchange_type(msg) == INFORMATIONAL_V1 &&
-               this->active_tasks->get_count(this->active_tasks))
-       {
-               enumerator_t *enumerator;
-               task_t *task;
-               /* In case of ongoing DPD request, let the DPD task handle all information exchanges. */
-               enumerator = this->active_tasks->create_enumerator(this->active_tasks);
-               while (enumerator->enumerate(enumerator, (void**)&task))
-               {
-                       if (task->get_type(task) == TASK_ISAKMP_DPD)
-                       {
-                               dpd_response = TRUE;
-                               break;
-                       }
-               }
-               enumerator->destroy(enumerator);
-       }
-
-
-       if ((mid && mid == this->initiating.mid) || dpd_response ||
+       if ((mid && mid == this->initiating.mid) ||
                (this->initiating.mid == 0 &&
                 msg->get_exchange_type(msg) == this->initiating.type &&
                 this->active_tasks->get_count(this->active_tasks)))
@@ -1332,8 +1321,8 @@ METHOD(task_manager_t, queue_child_delete, void,
 METHOD(task_manager_t, queue_dpd, void,
        private_task_manager_t *this)
 {
-       queue_task(this, (task_t*)isakmp_dpd_create(this->ike_sa, NULL,
-                                                                                               this->dpd_send_seqnr++));
+       queue_task(this, (task_t*)isakmp_dpd_create(this->ike_sa, TRUE,
+                                                                                               this->dpd_send++));
 }
 
 METHOD(task_manager_t, adopt_tasks, void,
@@ -1483,9 +1472,9 @@ task_manager_v1_t *task_manager_v1_create(ike_sa_t *ike_sa)
                return NULL;
        }
 
-       this->rng->get_bytes(this->rng, sizeof(this->dpd_send_seqnr),
-                                                (void*)&this->dpd_send_seqnr);
-       this->dpd_send_seqnr &= 0x7FFFFFFF;
+       this->rng->get_bytes(this->rng, sizeof(this->dpd_send),
+                                                (void*)&this->dpd_send);
+       this->dpd_send &= 0x7FFFFFFF;
 
        return &this->public;
 }
index df0c94b..1fe36a9 100755 (executable)
@@ -164,7 +164,7 @@ static status_t send_notify(private_aggressive_mode_t *this, notify_type_t type)
        notify->set_spi_data(notify, spi);
 
        this->ike_sa->queue_task(this->ike_sa,
-                                               (task_t*)informational_create(this->ike_sa, notify, 0));
+                                               (task_t*)informational_create(this->ike_sa, notify));
        /* cancel all active/passive tasks in favour of informational */
        return ALREADY_DONE;
 }
index 2af641e..4e53e5c 100755 (executable)
@@ -18,7 +18,6 @@
 #include <daemon.h>
 #include <sa/ikev1/tasks/isakmp_delete.h>
 #include <sa/ikev1/tasks/quick_delete.h>
-#include <sa/ikev1/tasks/isakmp_dpd.h>
 
 #include <encoding/payloads/delete_payload.h>
 
@@ -48,16 +47,6 @@ struct private_informational_t {
         * Delete subtask
         */
        task_t *del;
-
-       /**
-        * DPD subtask
-        */
-       task_t *dpd;
-
-       /**
-        * DPD sequence number
-        */
-       u_int32_t dpd_seqnr;
 };
 
 METHOD(task_t, build_i, status_t,
@@ -92,15 +81,6 @@ METHOD(task_t, process_r, status_t,
                                        this->ike_sa->set_condition(this->ike_sa,
                                                                                                COND_INIT_CONTACT_SEEN, TRUE);
                                }
-                               else if (type == DPD_R_U_THERE)
-                               {
-                                       DBG3(DBG_IKE, "received DPD request");
-                                       this->dpd = (task_t*)isakmp_dpd_create(this->ike_sa, notify, this->dpd_seqnr);
-                               }
-                               else if (type == DPD_R_U_THERE_ACK)
-                               {
-                                       DBG3(DBG_IKE, "received DPD request ack");
-                               }
                                else if (type < 16384)
                                {
                                        DBG1(DBG_IKE, "received %N error notify",
@@ -144,11 +124,6 @@ METHOD(task_t, process_r, status_t,
        {
                return this->del->process(this->del, message);
        }
-
-       if (this->dpd && status == SUCCESS)
-       {
-               return this->dpd->process(this->dpd, message);
-       }
        return status;
 }
 
@@ -159,13 +134,6 @@ METHOD(task_t, build_r, status_t,
        {
                return this->del->build(this->del, message);
        }
-
-       if (this->dpd)
-       {
-               status_t status = this->dpd->build(this->dpd, message);
-               this->dpd->destroy(this->dpd);
-               return status;
-       }
        return FAILED;
 }
 
@@ -198,7 +166,7 @@ METHOD(task_t, destroy, void,
 /*
  * Described in header.
  */
-informational_t *informational_create(ike_sa_t *ike_sa, notify_payload_t *notify, u_int32_t dpd_seqnr)
+informational_t *informational_create(ike_sa_t *ike_sa, notify_payload_t *notify)
 {
        private_informational_t *this;
 
@@ -212,7 +180,6 @@ informational_t *informational_create(ike_sa_t *ike_sa, notify_payload_t *notify
                },
                .ike_sa = ike_sa,
                .notify = notify,
-               .dpd_seqnr = dpd_seqnr,
        );
 
        if (notify)
index 59b8110..26d8d51 100755 (executable)
@@ -19,7 +19,7 @@
  */
 
 #ifndef INFORMATIONAL_H_
-#define informational_H_
+#define INFORMATIONAL_H_
 
 typedef struct informational_t informational_t;
 
@@ -44,9 +44,8 @@ struct informational_t {
  *
  * @param ike_sa               IKE_SA this task works for
  * @param notify               notify to send as initiator, NULL if responder
- * @param dpd_seqnr    DPD sequence number, incoming or outgoing
  * @return                             task to handle by the task_manager
  */
-informational_t *informational_create(ike_sa_t *ike_sa, notify_payload_t *notify, u_int32_t dpd_seqnr);
+informational_t *informational_create(ike_sa_t *ike_sa, notify_payload_t *notify);
 
 #endif /** INFORMATIONAL_H_ @}*/
index ec0a1ed..e2ed17d 100755 (executable)
@@ -1,14 +1,22 @@
-#include "isakmp_dpd.h"
+/*
+ * Copyright (C) 2011 Martin Willi
+ * Copyright (C) 2011 revosec 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 <encoding/payloads/notify_payload.h>
-#include <sa/ikev1/tasks/informational.h>
+#include "isakmp_dpd.h"
 
 #include <daemon.h>
-
-#ifdef SLIPSTREAM
-/* Should be the last include */
-#include <ikev2_mem.h>
-#endif /* SLIPSTREAM */
+#include <encoding/payloads/notify_payload.h>
 
 typedef struct private_isakmp_dpd_t private_isakmp_dpd_t;
 
@@ -28,9 +36,9 @@ struct private_isakmp_dpd_t {
        u_int32_t seqnr;
 
        /**
-        * Notify payload, only provided for requests.
+        * DPD initiator?
         */
-       notify_payload_t *notify;
+       bool initiator;
 
        /**
         * IKE SA we are serving.
@@ -38,42 +46,19 @@ struct private_isakmp_dpd_t {
        ike_sa_t *ike_sa;
 };
 
-/**
- * Get DPD sequence number from notify payload.
- */
-static bool get_seqnr(notify_payload_t *notify, u_int32_t *seqnr)
-{
-       chunk_t chunk = notify->get_notification_data(notify);
-
-       if( chunk.ptr && chunk.len == 4)
-       {
-               u_int32_t seqnr_read = *((u_int32_t*)chunk.ptr);
-
-               *seqnr = ntohl(seqnr_read);
-
-               return TRUE;
-       }
-
-       DBG1(DBG_IKE, "no DPD seqnr received");
-
-       return FALSE;
-}
-
-/**
- * Add notify payload to message.
- */
-static void add_notify(private_isakmp_dpd_t *this, message_t *message, notify_type_t type)
+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, 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);
@@ -84,156 +69,53 @@ static void add_notify(private_isakmp_dpd_t *this, message_t *message, notify_ty
        notify->set_notification_data(notify, chunk_from_thing(seqnr));
 
        message->add_payload(message, (payload_t*)notify);
-}
-
-METHOD(isakmp_dpd_t, get_dpd_seqnr, u_int32_t,
-       private_isakmp_dpd_t *this)
-{
-       return this->seqnr;
-}
-
-METHOD(task_t, build_i, status_t,
-       private_isakmp_dpd_t *this, message_t *message)
-{
-       add_notify(this, message, DPD_R_U_THERE);
-
-       return NEED_MORE;
-}
-
-METHOD(task_t, build_r, status_t,
-                        private_isakmp_dpd_t *this, message_t *message)
-{
-       add_notify(this, message, DPD_R_U_THERE_ACK);
 
        return SUCCESS;
 }
 
-METHOD(task_t, process_i, status_t,
+METHOD(task_t, process, status_t,
        private_isakmp_dpd_t *this, message_t *message)
 {
-       enumerator_t *enumerator;
        notify_payload_t *notify;
        notify_type_t type;
-       payload_t *payload;
-       task_t *info_task = NULL;
+       u_int32_t seqnr = 0;
+       chunk_t chunk;
 
-       enumerator = message->create_payload_enumerator(message);
-       while (enumerator->enumerate(enumerator, &payload))
+       type = this->initiator ? DPD_R_U_THERE_ACK : DPD_R_U_THERE;
+       notify = message->get_notify(message, type);
+       if (notify)
        {
-               switch (payload->get_type(payload))
+               chunk = notify->get_notification_data(notify);
+               if (chunk.len == 4)
                {
-                       case NOTIFY_V1:
-                               notify = (notify_payload_t*)payload;
-                               type = notify->get_notify_type(notify);
-
-                               if (type == DPD_R_U_THERE_ACK)
-                               {
-                                       u_int32_t seqnr;
-
-                                       if (!get_seqnr(notify, &seqnr))
-                                       {
-                                               return FAILED;
-                                       }
-
-                                       if (this->seqnr != seqnr)
-                                       {
-                                               DBG1(DBG_IKE, "received DPD Ack with unexpected seqnr (%u) expect (%u)",seqnr,this->seqnr);
-                                               return SUCCESS;
-                                       }
-
-                                       DBG4(DBG_IKE, "received DPD Ack with seqnr (%u)",seqnr);
-
-                                       return SUCCESS;
-
-                               }
-                               else if (type == DPD_R_U_THERE)
-                               {
-                                       u_int32_t expected = this->seqnr + 1;
-
-                                       if (!get_seqnr(notify, &this->seqnr))
-                                       {
-                                               return FAILED;
-                                       }
-
-                                       if (expected != 1 && this->seqnr != expected)
-                                       {
-                                               DBG1(DBG_IKE, "received DPD request with unexpected seqnr (%u) expect (%u)",
-                                                       this->seqnr,expected);
-                                               return SUCCESS;
-                                       }
-
-                                       DBG4(DBG_IKE, "received DPD request with seqnr %u",this->seqnr);
-
-                                       this->public.task.build = _build_r;
-                                       return NEED_MORE;
+                       seqnr = untoh32(chunk.ptr);
+                       if (seqnr == this->seqnr)
+                       {
+                               if (!this->initiator)
+                               {       /* queue DPD_ACK */
+                                       this->ike_sa->queue_task(this->ike_sa,
+                                                               &isakmp_dpd_create(this->ike_sa, FALSE,
+                                                                                                  this->seqnr)->task);
                                }
-                               else
-                               {
-                                       info_task = (task_t*)informational_create(this->ike_sa, NULL, 0);
-                               }
-                               continue;
-
-                       default:
-                               continue;
+                               return SUCCESS;
+                       }
                }
-               break;
-       }
-       enumerator->destroy(enumerator);
-
-       if (info_task)
-       {
-               status_t status = info_task->process(info_task, message);
-               /* Assuming that the informational task will not need to send other replies than dpd */
-               info_task->destroy(info_task);
-               return status;
        }
-
+       DBG1(DBG_IKE, "received invalid DPD sequence number %u (expected %u), "
+                "ignored", seqnr, this->seqnr);
        return SUCCESS;
 }
 
-METHOD(task_t, process_r, status_t,
-                        private_isakmp_dpd_t *this, message_t *message)
-{
-       u_int32_t expected = this->seqnr + 1;
-
-       if (this->notify)
-       {
-               if (!get_seqnr(this->notify, &this->seqnr))
-               {
-                       return FAILED;
-               }
-
-               if (expected != 1 && this->seqnr != expected)
-               {
-                       DBG1(DBG_IKE, "received DPD request with unexpected seqnr (%u) expect (%u)",
-                               this->seqnr,expected);
-                       return SUCCESS;
-               }
-
-               DBG4(DBG_IKE, "DPD request received with seqnr %u",this->seqnr);
-       }
-       else
-       {
-               DBG1(DBG_IKE, "no notify provided");
-               return FAILED;
-       }
-       return NEED_MORE;
-}
-
-
 METHOD(task_t, get_type, task_type_t,
        private_isakmp_dpd_t *this)
 {
        return TASK_ISAKMP_DPD;
 }
 
-
 METHOD(task_t, migrate, void,
        private_isakmp_dpd_t *this, ike_sa_t *ike_sa)
 {
        this->ike_sa = ike_sa;
-       this->seqnr = 0;
-
 }
 
 METHOD(task_t, destroy, void,
@@ -245,7 +127,8 @@ METHOD(task_t, destroy, void,
 /*
  * Described in header.
  */
-isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, notify_payload_t *notify, u_int32_t seqnr)
+isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, bool initiator,
+                                                               u_int32_t seqnr)
 {
        private_isakmp_dpd_t *this;
 
@@ -253,26 +136,16 @@ isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, notify_payload_t *notify, u_in
                .public = {
                        .task = {
                                .get_type = _get_type,
+                               .build = _build,
+                               .process = _process,
                                .migrate = _migrate,
                                .destroy = _destroy,
                        },
-                       .get_dpd_seqnr = _get_dpd_seqnr,
                },
-               .notify = notify,
                .ike_sa = ike_sa,
                .seqnr = seqnr,
+               .initiator = initiator,
        );
 
-       if (!notify)
-       {
-               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;
 }
index 5d940b8..688d19f 100755 (executable)
@@ -1,5 +1,25 @@
+/*
+ * Copyright (C) 2012 Martin Willi
+ * Copyright (C) 2012 revosec 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 isakmp_dpd isakmp_dpd
+ * @{ @ingroup tasks
+ */
+
 #ifndef ISAKMP_DPD_H_
-#define ISAPMP_DPD_H_
+#define ISAKMP_DPD_H_
 
 typedef struct isakmp_dpd_t isakmp_dpd_t;
 
@@ -8,9 +28,7 @@ typedef struct isakmp_dpd_t isakmp_dpd_t;
 #include <sa/task.h>
 
 /**
- * Task of type isakmp_dpd, detects dead peers.
- *
- *
+ * IKEv1 dead peer detection task.
  */
 struct isakmp_dpd_t {
 
@@ -18,21 +36,17 @@ struct isakmp_dpd_t {
         * Implements the task_t interface
         */
        task_t task;
-
-       /**
-        * Get the received dpd seqnr.
-        *
-        * @return                              protocol ID
-        */
-       u_int32_t (*get_dpd_seqnr) (isakmp_dpd_t *dpd_task);
 };
 
 /**
- * Create a new isakmp_dpd task.
+ * Create a new ISAKMP_DPD task.
  *
- * @param initiator            TRUE if task is the original initiator
- * @return                             isakmp_dpd task to handle by the task_manager
+ * @param ike_sa               associated IKE_SA
+ * @param initiator            TRUE if DPD initiator
+ * @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, notify_payload_t *notify, u_int32_t seqnr);
+isakmp_dpd_t *isakmp_dpd_create(ike_sa_t *ike_sa, bool initiator,
+                                                               u_int32_t seqnr);
 
-#endif /** ISAKMP_DPD_H_ @}*/
\ No newline at end of file
+#endif /** ISAKMP_DPD_H_ @}*/
index f1e3290..ba1a9ad 100755 (executable)
@@ -176,7 +176,7 @@ static status_t send_notify(private_main_mode_t *this, notify_type_t type)
        notify->set_spi_data(notify, spi);
 
        this->ike_sa->queue_task(this->ike_sa,
-                                               (task_t*)informational_create(this->ike_sa, notify, 0));
+                                               (task_t*)informational_create(this->ike_sa, notify));
        /* cancel all active/passive tasks in favour of informational */
        return ALREADY_DONE;
 }
index b27b00d..dedeab1 100755 (executable)
@@ -539,7 +539,7 @@ static status_t send_notify(private_quick_mode_t *this, notify_type_t type)
        notify->set_spi(notify, this->spi_i);
 
        this->ike_sa->queue_task(this->ike_sa,
-                                               (task_t*)informational_create(this->ike_sa, notify, 0));
+                                               (task_t*)informational_create(this->ike_sa, notify));
        /* cancel all active/passive tasks in favour of informational */
        return ALREADY_DONE;
 }