denozord/strongswan-ext
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ike-rekey: Reset IKE_SA on the bus after destroying new IKE_SA

Browse Source 
The destroy() method sets the IKE_SA on the bus to NULL, we reset it to
the current IKE_SA so any events and log messages that follow happen in
the correct context.

A practical example where this is problematic is a DH group mismatch,
which causes the first CREATE_CHILD_SA exchange to fail.  Because the SA
was not reset previously, the message() hook for the CREATE_CHILD_SA
response, for instance, was triggered outside the context of an IKE_SA,
that is, the ike_sa parameter was NULL, which is definitely not expected
by several plugins.

Fixes #862.
This commit is contained in:
Tobias Brunner
2015-07-27 14:44:32 +02:00
parent 5af74bcb84
commit 86d20b0b40
1 changed files with 15 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libcharon/sa/ikev2/tasks/ike_rekey.c
+15 -16
View File
@@ -116,7 +116,6 @@ static void establish_new(private_ike_rekey_t *this)
lib->processor->queue_job(lib->processor, job); lib->processor->queue_job(lib->processor, job);
} }
this->new_sa = NULL; this->new_sa = NULL;
/* set threads active IKE_SA after checkin */
charon->bus->set_sa(charon->bus, this->ike_sa); charon->bus->set_sa(charon->bus, this->ike_sa);
} }
} }
@@ -335,15 +334,13 @@ METHOD(task_t, process_i, status_t,
{ {
charon->ike_sa_manager->checkin( charon->ike_sa_manager->checkin(
charon->ike_sa_manager, this->new_sa); charon->ike_sa_manager, this->new_sa);
/* set threads active IKE_SA after checkin */
charon->bus->set_sa(charon->bus, this->ike_sa);
} }
charon->bus->set_sa(charon->bus, this->ike_sa);
this->new_sa = NULL; this->new_sa = NULL;
establish_new(other); establish_new(other);
return SUCCESS; return SUCCESS;
} }
} }
/* set threads active IKE_SA after checkin */
charon->bus->set_sa(charon->bus, this->ike_sa); charon->bus->set_sa(charon->bus, this->ike_sa);
} }
@@ -372,9 +369,13 @@ METHOD(ike_rekey_t, collide, void,
this->collision = other; this->collision = other;
} }
METHOD(task_t, migrate, void, /**
private_ike_rekey_t *this, ike_sa_t *ike_sa) * Cleanup the task
*/
static void cleanup(private_ike_rekey_t *this)
{ {
ike_sa_t *cur_sa;
if (this->ike_init) if (this->ike_init)
{ {
this->ike_init->task.destroy(&this->ike_init->task); this->ike_init->task.destroy(&this->ike_init->task);
@@ -383,9 +384,16 @@ METHOD(task_t, migrate, void,
{ {
this->ike_delete->task.destroy(&this->ike_delete->task); this->ike_delete->task.destroy(&this->ike_delete->task);
} }
cur_sa = charon->bus->get_sa(charon->bus);
DESTROY_IF(this->new_sa); DESTROY_IF(this->new_sa);
charon->bus->set_sa(charon->bus, cur_sa);
DESTROY_IF(this->collision); DESTROY_IF(this->collision);
}
METHOD(task_t, migrate, void,
private_ike_rekey_t *this, ike_sa_t *ike_sa)
{
cleanup();
this->collision = NULL; this->collision = NULL;
this->ike_sa = ike_sa; this->ike_sa = ike_sa;
this->new_sa = NULL; this->new_sa = NULL;
@@ -396,16 +404,7 @@ METHOD(task_t, migrate, void,
METHOD(task_t, destroy, void, METHOD(task_t, destroy, void,
private_ike_rekey_t *this) private_ike_rekey_t *this)
{ {
if (this->ike_init) cleanup();
{
this->ike_init->task.destroy(&this->ike_init->task);
}
if (this->ike_delete)
{
this->ike_delete->task.destroy(&this->ike_delete->task);
}
DESTROY_IF(this->new_sa);
DESTROY_IF(this->collision);
free(this); free(this);
} }