ike: Reset local SPI if retrying to connect in state IKE_CONNECTING

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.

diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 045858792da156622229cddc4c97924ff8c55b6f..823cf2579ad3d0a566daa8f7eadc2eba086ef660 100644 (file)
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -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);
                                }

diff --git a/src/libcharon/sa/ike_sa.h b/src/libcharon/sa/ike_sa.h
index c8ba2fd2a3aff7a0a52b9f4699da8fafb7490e3b..4d8af299dbdb50f7a96d6b648e6e0149b3312da5 100644 (file)
--- a/src/libcharon/sa/ike_sa.h
+++ b/src/libcharon/sa/ike_sa.h
@@ -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.

diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c
index 9a207ac6b1da52fbfda8bb944b7f317a1a0c22c3..f974b9f58dd41626f48de664a0a2473c59497175 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_init.c
+++ b/src/libcharon/sa/ikev2/tasks/ike_init.c
@@ -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++;