From 86d20b0b40066590f5e26d1f9aca21cc0cba97e1 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 15 Jun 2015 11:46:33 +0200 Subject: [PATCH] ike-rekey: Reset IKE_SA on the bus after destroying new IKE_SA 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. --- src/libcharon/sa/ikev2/tasks/ike_rekey.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_rekey.c b/src/libcharon/sa/ikev2/tasks/ike_rekey.c index 1855517..1dfdc05 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/ike_rekey.c @@ -116,7 +116,6 @@ static void establish_new(private_ike_rekey_t *this) lib->processor->queue_job(lib->processor, job); } this->new_sa = NULL; - /* set threads active IKE_SA after checkin */ 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, 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; establish_new(other); return SUCCESS; } } - /* set threads active IKE_SA after checkin */ charon->bus->set_sa(charon->bus, this->ike_sa); } @@ -372,9 +369,13 @@ METHOD(ike_rekey_t, collide, void, 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) { 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); } + cur_sa = charon->bus->get_sa(charon->bus); DESTROY_IF(this->new_sa); + charon->bus->set_sa(charon->bus, cur_sa); DESTROY_IF(this->collision); +} +METHOD(task_t, migrate, void, + private_ike_rekey_t *this, ike_sa_t *ike_sa) +{ + cleanup(); this->collision = NULL; this->ike_sa = ike_sa; this->new_sa = NULL; @@ -396,16 +404,7 @@ METHOD(task_t, migrate, void, METHOD(task_t, destroy, void, private_ike_rekey_t *this) { - if (this->ike_init) - { - 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); + cleanup(); free(this); } -- 2.4.6