ike: Reset local SPI if retrying to connect in state IKE_CONNECTING
authorTobias Brunner <tobias@strongswan.org>
Tue, 29 Aug 2017 07:06:55 +0000 (09:06 +0200)
committerTobias Brunner <tobias@strongswan.org>
Mon, 4 Sep 2017 09:16:00 +0000 (11:16 +0200)
In case we send retransmits for an IKE_SA_INIT where we propose a DH
group the responder will reject we might later receive delayed responses
that either contain INVALID_KE_PAYLOAD notifies with the group we already
use or, if we retransmitted an IKE_SA_INIT with the requested group but
then had to restart again, a KE payload with a group different from the
one we proposed.  So far we didn't change the initiator SPI when
restarting the connection, i.e. these delayed responses were processed
and might have caused fatal errors due to a failed DH negotiation or
because of the internal retry counter in the ike-init task.  Changing
the initiator SPI avoids that as we won't process the delayed responses
anymore that caused this confusion.

src/libcharon/sa/ike_sa.c
src/libcharon/sa/ike_sa.h
src/libcharon/sa/ikev2/tasks/ike_init.c

index 0458587..823cf25 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Tobias Brunner
+ * Copyright (C) 2006-2017 Tobias Brunner
  * Copyright (C) 2006 Daniel Roethlisberger
  * Copyright (C) 2005-2009 Martin Willi
  * Copyright (C) 2005 Jan Hutter
@@ -914,9 +914,15 @@ METHOD(ike_sa_t, set_state, void,
 }
 
 METHOD(ike_sa_t, reset, void,
-       private_ike_sa_t *this)
+       private_ike_sa_t *this, bool new_spi)
 {
-       /*  the responder ID is reset, as peer may choose another one */
+       /* reset the initiator SPI if requested */
+       if (new_spi)
+       {
+               charon->ike_sa_manager->new_initiator_spi(charon->ike_sa_manager,
+                                                                                                 &this->public);
+       }
+       /* the responder ID is reset, as peer may choose another one */
        if (this->ike_sa_id->is_initiator(this->ike_sa_id))
        {
                this->ike_sa_id->set_responder_spi(this->ike_sa_id, 0);
@@ -1849,7 +1855,7 @@ METHOD(ike_sa_t, reauth, status_t,
        {
                DBG0(DBG_IKE, "reinitiating IKE_SA %s[%d]",
                         get_name(this), this->unique_id);
-               reset(this);
+               reset(this, TRUE);
                return this->task_manager->initiate(this->task_manager);
        }
        /* we can't reauthenticate as responder when we use EAP or virtual IPs.
@@ -2222,7 +2228,7 @@ static bool redirect_connecting(private_ike_sa_t *this, identification_t *to)
        {
                return FALSE;
        }
-       reset(this);
+       reset(this, TRUE);
        DESTROY_IF(this->redirected_from);
        this->redirected_from = this->other_host->clone(this->other_host);
        DESTROY_IF(this->remote_host);
@@ -2351,7 +2357,7 @@ METHOD(ike_sa_t, retransmit, status_t,
                                {
                                        DBG1(DBG_IKE, "peer not responding, trying again (%d/%d)",
                                                 this->keyingtry + 1, tries);
-                                       reset(this);
+                                       reset(this, TRUE);
                                        resolve_hosts(this);
                                        return this->task_manager->initiate(this->task_manager);
                                }
index c8ba2fd..4d8af29 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Tobias Brunner
+ * Copyright (C) 2006-2017 Tobias Brunner
  * Copyright (C) 2006 Daniel Roethlisberger
  * Copyright (C) 2005-2009 Martin Willi
  * Copyright (C) 2005 Jan Hutter
@@ -1169,9 +1169,11 @@ struct ike_sa_t {
        void (*inherit_post) (ike_sa_t *this, ike_sa_t *other);
 
        /**
-        * Reset the IKE_SA, useable when initiating fails
+        * Reset the IKE_SA, useable when initiating fails.
+        *
+        * @param new_spi               TRUE to allocate a new initiator SPI
         */
-       void (*reset) (ike_sa_t *this);
+       void (*reset) (ike_sa_t *this, bool new_spi);
 
        /**
         * Destroys a ike_sa_t object.
index 9a207ac..f974b9f 100644 (file)
@@ -815,7 +815,7 @@ METHOD(task_t, process_i, status_t,
 
                                        if (this->old_sa == NULL)
                                        {       /* reset the IKE_SA if we are not rekeying */
-                                               this->ike_sa->reset(this->ike_sa);
+                                               this->ike_sa->reset(this->ike_sa, FALSE);
                                        }
 
                                        enumerator->destroy(enumerator);
@@ -833,7 +833,7 @@ METHOD(task_t, process_i, status_t,
                                {
                                        chunk_free(&this->cookie);
                                        this->cookie = chunk_clone(notify->get_notification_data(notify));
-                                       this->ike_sa->reset(this->ike_sa);
+                                       this->ike_sa->reset(this->ike_sa, FALSE);
                                        enumerator->destroy(enumerator);
                                        DBG2(DBG_IKE, "received %N notify", notify_type_names, type);
                                        this->retry++;