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.
This commit is contained in:
Tobias Brunner 2017-08-29 09:06:55 +02:00
parent eaedcf8c00
commit c353996191
3 changed files with 19 additions and 11 deletions

View 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);
}

View 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.

View 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++;