]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
ikev2: Use hashes to detect retransmits
authorTobias Brunner <tobias@strongswan.org>
Mon, 23 Jul 2018 15:49:15 +0000 (17:49 +0200)
committerTobias Brunner <tobias@strongswan.org>
Wed, 29 Jun 2022 08:28:50 +0000 (10:28 +0200)
This way we avoid parsing messages with unexpected message IDs, which
might not even be possible if we don't have the keys anymore.  However,
the next commit should avoid the latter and this way we avoid deriving
keys for retransmits or unexpected messages.

This also changes how retransmits for fragmented messages are triggered.
Previously, we waited for all fragments and reconstructed the message
before retransmitting the response.  Now we only track the first
fragment and if we receive a retransmit of it respond immediately
without waiting for other fragments (which are now ignored).  This is in
compliance with RFC 7383, section 2.6.1, and can avoid issues if there
are lots of fragments.

src/libcharon/sa/ikev2/task_manager_v2.c

index 62707cb4653a35fb13db37602524cfb22f1a84a4..2e29f027d4ad37c6712f2c67f44132ed40b43d68 100644 (file)
@@ -41,6 +41,7 @@
 #include <sa/ikev2/tasks/child_create.h>
 #include <sa/ikev2/tasks/child_rekey.h>
 #include <sa/ikev2/tasks/child_delete.h>
+#include <encoding/payloads/encrypted_fragment_payload.h>
 #include <encoding/payloads/delete_payload.h>
 #include <encoding/payloads/unknown_payload.h>
 #include <processing/jobs/retransmit_job.h>
@@ -79,14 +80,24 @@ struct private_task_manager_t {
                uint32_t mid;
 
                /**
-                * packet(s) for retransmission
+                * Helper to defragment the request
+                */
+               message_t *defrag;
+
+               /**
+                * Hash of the current message, or its first fragment
+                */
+               uint8_t hash[HASH_SIZE_SHA1];
+
+               /**
+                * Packet(s) for retransmissions (mid-1)
                 */
                array_t *packets;
 
                /**
-                * Helper to defragment the request
+                * Hash of the previously received message, or its first fragment
                 */
-               message_t *defrag;
+               uint8_t prev_hash[HASH_SIZE_SHA1];
 
        } responding;
 
@@ -1388,27 +1399,46 @@ METHOD(task_manager_t, get_mid, uint32_t,
        return initiate ? this->initiating.mid : this->responding.mid;
 }
 
+/**
+ * Hash the given message with SHA-1
+ */
+static bool hash_message(message_t *msg, uint8_t hash[HASH_SIZE_SHA1])
+{
+       hasher_t *hasher;
+
+       hasher = lib->crypto->create_hasher(lib->crypto, HASH_SHA1);
+       if (!hasher ||
+               !hasher->get_hash(hasher, msg->get_packet_data(msg), hash))
+       {
+               DESTROY_IF(hasher);
+               return FALSE;
+       }
+       hasher->destroy(hasher);
+       return TRUE;
+}
+
 /**
  * Handle the given IKE fragment, if it is one.
  *
  * Returns SUCCESS if the message is not a fragment, and NEED_MORE if it was
  * handled properly.  Error states are returned if the fragment was invalid or
- * the reassembled message could not have been processed properly.
+ * the reassembled message could not be processed properly.
  */
 static status_t handle_fragment(private_task_manager_t *this,
                                                                message_t **defrag, message_t *msg)
 {
-       message_t *reassembled;
+       encrypted_fragment_payload_t *fragment;
        status_t status;
 
-       if (*defrag && (*defrag)->get_message_id(*defrag) < msg->get_message_id(msg))
-       {
-               /* clear fragments of an incompletely received retransmitted message */
-               (*defrag)->destroy(*defrag);
-               *defrag = NULL;
-       }
-       if (!msg->get_payload(msg, PLV2_FRAGMENT))
+       fragment = (encrypted_fragment_payload_t*)msg->get_payload(msg,
+                                                                                                                          PLV2_FRAGMENT);
+       if (!fragment)
        {
+               /* ignore reassembled messages, we collected their fragments below */
+               if (msg != *defrag)
+               {
+                       hash_message(msg, this->responding.hash);
+               }
                return SUCCESS;
        }
        if (!*defrag)
@@ -1420,18 +1450,26 @@ static status_t handle_fragment(private_task_manager_t *this,
                }
        }
        status = (*defrag)->add_fragment(*defrag, msg);
+
+       if (status == NEED_MORE || status == SUCCESS)
+       {
+               /* to detect retransmissions we only hash the first fragment */
+               if (fragment->get_fragment_number(fragment) == 1)
+               {
+                       hash_message(msg, this->responding.hash);
+               }
+       }
        if (status == SUCCESS)
        {
                /* reinject the reassembled message */
-               reassembled = *defrag;
-               *defrag = NULL;
-               status = this->ike_sa->process_message(this->ike_sa, reassembled);
+               status = this->ike_sa->process_message(this->ike_sa, *defrag);
                if (status == SUCCESS)
                {
                        /* avoid processing the last fragment */
                        status = NEED_MORE;
                }
-               reassembled->destroy(reassembled);
+               (*defrag)->destroy(*defrag);
+               *defrag = NULL;
        }
        return status;
 }
@@ -1598,47 +1636,6 @@ static status_t parse_message(private_task_manager_t *this, message_t *msg)
        return status;
 }
 
-/**
- * Check if a message with message ID 0 looks like it is used to synchronize
- * the message IDs.
- */
-static bool looks_like_mid_sync(private_task_manager_t *this, message_t *msg,
-                                                               bool strict)
-{
-       enumerator_t *enumerator;
-       notify_payload_t *notify;
-       payload_t *payload;
-       bool found = FALSE, other = FALSE;
-
-       if (msg->get_exchange_type(msg) == INFORMATIONAL)
-       {
-               enumerator = msg->create_payload_enumerator(msg);
-               while (enumerator->enumerate(enumerator, &payload))
-               {
-                       if (payload->get_type(payload) == PLV2_NOTIFY)
-                       {
-                               notify = (notify_payload_t*)payload;
-                               switch (notify->get_notify_type(notify))
-                               {
-                                       case IKEV2_MESSAGE_ID_SYNC:
-                                       case IPSEC_REPLAY_COUNTER_SYNC:
-                                               found = TRUE;
-                                               continue;
-                                       default:
-                                               break;
-                               }
-                       }
-                       if (strict)
-                       {
-                               other = TRUE;
-                               break;
-                       }
-               }
-               enumerator->destroy(enumerator);
-       }
-       return found && !other;
-}
-
 /**
  * Check whether we should reject the given request message
  */
@@ -1692,22 +1689,90 @@ static inline bool reject_request(private_task_manager_t *this,
        }
        return reject;
 }
+
 /**
  * Check if a message with message ID 0 looks like it is used to synchronize
- * the message IDs and we are prepared to process it.
+ * the message IDs.
  *
- * Note: This is not called if the responder never sent a message before (i.e.
- * we expect MID 0).
+ * Call this after checking the message with is_potential_mid_sync() first.
  */
 static bool is_mid_sync(private_task_manager_t *this, message_t *msg)
 {
-       if (this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED &&
-               this->ike_sa->supports_extension(this->ike_sa,
-                                                                                EXT_IKE_MESSAGE_ID_SYNC))
+       enumerator_t *enumerator;
+       notify_payload_t *notify;
+       payload_t *payload;
+       bool found = FALSE, other = FALSE;
+
+       enumerator = msg->create_payload_enumerator(msg);
+       while (enumerator->enumerate(enumerator, &payload))
        {
-               return looks_like_mid_sync(this, msg, TRUE);
+               if (payload->get_type(payload) == PLV2_NOTIFY)
+               {
+                       notify = (notify_payload_t*)payload;
+                       switch (notify->get_notify_type(notify))
+                       {
+                               case IKEV2_MESSAGE_ID_SYNC:
+                               case IPSEC_REPLAY_COUNTER_SYNC:
+                                       found = TRUE;
+                                       continue;
+                               default:
+                                       break;
+                       }
+               }
+               other = TRUE;
+               break;
        }
-       return FALSE;
+       enumerator->destroy(enumerator);
+       return found && !other;
+}
+
+/**
+ * Check if a message with message ID 0 looks like it could potentially be used
+ * to synchronize the message IDs and if we are prepared to process it.
+ *
+ * This may be called before the message body is parsed.
+ */
+static bool is_potential_mid_sync(private_task_manager_t *this, message_t *msg)
+{
+       return msg->get_exchange_type(msg) == INFORMATIONAL &&
+                  this->ike_sa->get_state(this->ike_sa) == IKE_ESTABLISHED &&
+                  this->ike_sa->supports_extension(this->ike_sa,
+                                                                                       EXT_IKE_MESSAGE_ID_SYNC);
+}
+
+/**
+ * Check if the given message is a retransmitted request
+ */
+static status_t is_retransmit(private_task_manager_t *this, message_t *msg)
+{
+       uint8_t hash[HASH_SIZE_SHA1];
+       uint32_t mid;
+
+       mid = msg->get_message_id(msg);
+
+       if (mid == this->responding.mid)
+       {
+               return NEED_MORE;
+       }
+
+       if (mid == this->responding.mid - 1 &&
+               array_count(this->responding.packets))
+       {
+               if (!hash_message(msg, hash))
+               {
+                       DBG1(DBG_IKE, "failed to hash message, ignored");
+                       return FAILED;
+               }
+               if (memeq_const(hash, this->responding.prev_hash, sizeof(hash)))
+               {
+                       return ALREADY_DONE;
+               }
+       }
+       /* this includes fragments with fragment number > 1 (we only hash the first
+        * fragment), for which RFC 7383, section 2.6.1 explicitly does not allow
+        * retransmitting responses. we don't parse/verify these messages to check,
+        * we just ignore them. also included are MID sync messages with MID 0 */
+       return INVALID_ARG;
 }
 
 METHOD(task_manager_t, process_message, status_t,
@@ -1715,27 +1780,85 @@ METHOD(task_manager_t, process_message, status_t,
 {
        host_t *me, *other;
        status_t status;
-       uint32_t mid;
+       uint32_t mid, *expected_mid = NULL;
        bool schedule_delete_job = FALSE;
 
+       me = msg->get_destination(msg);
+       other = msg->get_source(msg);
+       mid = msg->get_message_id(msg);
+
        charon->bus->message(charon->bus, msg, TRUE, FALSE);
-       status = parse_message(this, msg);
-       if (status != SUCCESS)
+
+       if (msg->get_request(msg))
        {
-               return status;
+               bool potential_mid_sync = FALSE;
+
+               switch (is_retransmit(this, msg))
+               {
+                       case ALREADY_DONE:
+                               DBG1(DBG_IKE, "received retransmit of request with ID %d, "
+                                        "retransmitting response", mid);
+                               this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+                                                                                       time_monotonic(NULL));
+                               charon->bus->alert(charon->bus, ALERT_RETRANSMIT_RECEIVE, msg);
+                               send_packets(this, this->responding.packets,
+                                                        msg->get_destination(msg), msg->get_source(msg));
+                               return SUCCESS;
+                       case INVALID_ARG:
+                               if (mid == 0 && is_potential_mid_sync(this, msg))
+                               {
+                                       potential_mid_sync = TRUE;
+                               }
+                               else
+                               {
+                                       expected_mid = &this->responding.mid;
+                                       break;
+                               }
+                               /* check if it's actually an MID sync message */
+                       case NEED_MORE:
+                               status = parse_message(this, msg);
+                               if (potential_mid_sync && status == SUCCESS &&
+                                       !is_mid_sync(this, msg))
+                               {
+                                       expected_mid = &this->responding.mid;
+                               }
+                               break;
+                       case FAILED:
+                       default:
+                               return FAILED;
+               }
+       }
+       else
+       {
+               if (mid == this->initiating.mid)
+               {
+                       status = parse_message(this, msg);
+               }
+               else
+               {
+                       expected_mid = &this->initiating.mid;
+               }
        }
 
-       me = msg->get_destination(msg);
-       other = msg->get_source(msg);
+       if (expected_mid)
+       {
+               DBG1(DBG_IKE, "received message ID %d, expected %d, ignored",
+                        mid, *expected_mid);
+               return SUCCESS;
+       }
+       else if (status != SUCCESS)
+       {
+               return status;
+       }
 
        /* if this IKE_SA is virgin, we check for a config */
-       if (this->ike_sa->get_ike_cfg(this->ike_sa) == NULL)
+       if (!this->ike_sa->get_ike_cfg(this->ike_sa))
        {
                ike_cfg_t *ike_cfg;
 
                ike_cfg = charon->backends->get_ike_cfg(charon->backends,
                                                                                                me, other, IKEV2);
-               if (ike_cfg == NULL)
+               if (!ike_cfg)
                {
                        /* no config found for these hosts, destroy */
                        DBG1(DBG_IKE, "no IKE config found for %H...%H, sending %N",
@@ -1750,123 +1873,88 @@ METHOD(task_manager_t, process_message, status_t,
                schedule_delete_job = TRUE;
        }
 
-       mid = msg->get_message_id(msg);
        if (msg->get_request(msg))
        {
-               if (mid == this->responding.mid || (mid == 0 && is_mid_sync(this, msg)))
+               /* special handling for requests happens in is_retransmit()
+                * reject initial messages if not received in specific states,
+                * after rekeying we only expect a DELETE in an INFORMATIONAL */
+               if (reject_request(this, msg))
                {
-                       if (reject_request(this, msg))
-                       {
-                               return FAILED;
-                       }
-                       if (!this->ike_sa->supports_extension(this->ike_sa, EXT_MOBIKE))
-                       {       /* only do implicit updates without MOBIKE, and only force
-                                * updates for IKE_AUTH (ports might change due to NAT-T) */
-                               this->ike_sa->update_hosts(this->ike_sa, me, other,
-                                                                                  mid == 1 ? UPDATE_HOSTS_FORCE_ADDRS : 0);
-                       }
-                       status = handle_fragment(this, &this->responding.defrag, msg);
-                       if (status != SUCCESS)
-                       {
-                               if (status == NEED_MORE)
-                               {
-                                       this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-                                                                                               time_monotonic(NULL));
-                               }
-                               return status;
-                       }
-                       charon->bus->message(charon->bus, msg, TRUE, TRUE);
-                       if (msg->get_exchange_type(msg) == EXCHANGE_TYPE_UNDEFINED)
-                       {       /* ignore messages altered to EXCHANGE_TYPE_UNDEFINED */
-                               return SUCCESS;
-                       }
-                       switch (process_request(this, msg))
-                       {
-                               case SUCCESS:
-                                       this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-                                                                                               time_monotonic(NULL));
-                                       this->responding.mid++;
-                                       break;
-                               case NEED_MORE:
-                                       break;
-                               default:
-                                       flush(this);
-                                       return DESTROY_ME;
-                       }
+                       return FAILED;
+               }
+               if (!this->ike_sa->supports_extension(this->ike_sa, EXT_MOBIKE))
+               {       /* only do implicit updates without MOBIKE, and only force
+                        * updates for IKE_AUTH (ports might change due to NAT-T) */
+                       this->ike_sa->update_hosts(this->ike_sa, me, other,
+                                                                          mid == 1 ? UPDATE_HOSTS_FORCE_ADDRS : 0);
                }
-               else if ((mid == this->responding.mid - 1) &&
-                                array_count(this->responding.packets) &&
-                                !(mid == 0 && looks_like_mid_sync(this, msg, FALSE)))
+               status = handle_fragment(this, &this->responding.defrag, msg);
+               if (status != SUCCESS)
                {
-                       status = handle_fragment(this, &this->responding.defrag, msg);
-                       if (status != SUCCESS)
+                       if (status == NEED_MORE)
                        {
-                               if (status == NEED_MORE)
-                               {
-                                       this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-                                                                                               time_monotonic(NULL));
-                               }
-                               return status;
+                               this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+                                                                                       time_monotonic(NULL));
                        }
-                       DBG1(DBG_IKE, "received retransmit of request with ID %d, "
-                                "retransmitting response", mid);
-                       this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-                                                                               time_monotonic(NULL));
-                       charon->bus->alert(charon->bus, ALERT_RETRANSMIT_RECEIVE, msg);
-                       send_packets(this, this->responding.packets,
-                                                msg->get_destination(msg), msg->get_source(msg));
+                       return status;
                }
-               else
+               charon->bus->message(charon->bus, msg, TRUE, TRUE);
+               if (msg->get_exchange_type(msg) == EXCHANGE_TYPE_UNDEFINED)
+               {       /* ignore messages altered to EXCHANGE_TYPE_UNDEFINED */
+                       return SUCCESS;
+               }
+               switch (process_request(this, msg))
                {
-                       DBG1(DBG_IKE, "received message ID %d, expected %d, ignored",
-                                mid, this->responding.mid);
+                       case SUCCESS:
+                               this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+                                                                                       time_monotonic(NULL));
+                               this->responding.mid++;
+                               memcpy(this->responding.prev_hash, this->responding.hash,
+                                          sizeof(this->responding.prev_hash));
+                               break;
+                       case NEED_MORE:
+                               break;
+                       default:
+                               flush(this);
+                               return DESTROY_ME;
                }
        }
        else
        {
-               if (mid == this->initiating.mid)
-               {
-                       if (this->ike_sa->get_state(this->ike_sa) == IKE_CREATED ||
-                               this->ike_sa->get_state(this->ike_sa) == IKE_CONNECTING ||
-                               msg->get_exchange_type(msg) != IKE_SA_INIT)
-                       {       /* only do updates based on verified messages (or initial ones) */
-                               if (!this->ike_sa->supports_extension(this->ike_sa, EXT_MOBIKE))
-                               {       /* only do implicit updates without MOBIKE, we force an
-                                        * update of the local address on IKE_SA_INIT as we might
-                                        * not know it yet, but never for the remote address */
-                                       this->ike_sa->update_hosts(this->ike_sa, me, other,
-                                                                                          mid == 0 ? UPDATE_HOSTS_FORCE_LOCAL : 0);
-                               }
-                       }
-                       status = handle_fragment(this, &this->initiating.defrag, msg);
-                       if (status != SUCCESS)
-                       {
-                               if (status == NEED_MORE)
-                               {
-                                       this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-                                                                                               time_monotonic(NULL));
-                               }
-                               return status;
-                       }
-                       charon->bus->message(charon->bus, msg, TRUE, TRUE);
-                       if (msg->get_exchange_type(msg) == EXCHANGE_TYPE_UNDEFINED)
-                       {       /* ignore messages altered to EXCHANGE_TYPE_UNDEFINED */
-                               return SUCCESS;
+               if (this->ike_sa->get_state(this->ike_sa) == IKE_CREATED ||
+                       this->ike_sa->get_state(this->ike_sa) == IKE_CONNECTING ||
+                       msg->get_exchange_type(msg) != IKE_SA_INIT)
+               {       /* only do updates based on verified messages (or initial ones) */
+                       if (!this->ike_sa->supports_extension(this->ike_sa, EXT_MOBIKE))
+                       {       /* only do implicit updates without MOBIKE, we force an
+                                * update of the local address on IKE_SA_INIT as we might
+                                * not know it yet, but never for the remote address */
+                               this->ike_sa->update_hosts(this->ike_sa, me, other,
+                                                                                  mid == 0 ? UPDATE_HOSTS_FORCE_LOCAL : 0);
                        }
-                       if (process_response(this, msg) != SUCCESS)
+               }
+               status = handle_fragment(this, &this->initiating.defrag, msg);
+               if (status != SUCCESS)
+               {
+                       if (status == NEED_MORE)
                        {
-                               flush(this);
-                               return DESTROY_ME;
+                               this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+                                                                                       time_monotonic(NULL));
                        }
-                       this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
-                                                                               time_monotonic(NULL));
+                       return status;
                }
-               else
-               {
-                       DBG1(DBG_IKE, "received message ID %d, expected %d, ignored",
-                                mid, this->initiating.mid);
+               charon->bus->message(charon->bus, msg, TRUE, TRUE);
+               if (msg->get_exchange_type(msg) == EXCHANGE_TYPE_UNDEFINED)
+               {       /* ignore messages altered to EXCHANGE_TYPE_UNDEFINED */
                        return SUCCESS;
                }
+               if (process_response(this, msg) != SUCCESS)
+               {
+                       flush(this);
+                       return DESTROY_ME;
+               }
+               this->ike_sa->set_statistic(this->ike_sa, STAT_INBOUND,
+                                                                       time_monotonic(NULL));
        }
 
        if (schedule_delete_job)