From eef085904350d98d3fbe2bb9290df4e4b320c229 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 21 Mar 2007 14:42:49 +0000 Subject: [PATCH] fixed child rekey collision implemented ike rekey collision --- src/charon/sa/task_manager.c | 6 +- src/charon/sa/tasks/child_create.c | 17 ++++++ src/charon/sa/tasks/child_create.h | 8 +++ src/charon/sa/tasks/child_rekey.c | 51 +++------------- src/charon/sa/tasks/ike_init.c | 17 ++++++ src/charon/sa/tasks/ike_init.h | 10 +++- src/charon/sa/tasks/ike_rekey.c | 96 ++++++++++++++++++++++++------ src/charon/sa/tasks/ike_rekey.h | 12 ++++ 8 files changed, 150 insertions(+), 67 deletions(-) diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c index 9073bed7a..d63c614c0 100644 --- a/src/charon/sa/task_manager.c +++ b/src/charon/sa/task_manager.c @@ -459,10 +459,10 @@ static void handle_collisions(private_task_manager_t *this, task_t *task) switch (active->get_type(active)) { case IKE_REKEY: - if (type == IKE_REKEY) + if (type == IKE_REKEY || type == IKE_DELETE) { - //ike_rekey_t *rekey = (ike_rekey_t*)active; - //rekey->collide(rekey, (ike_rekey_t*)task); + ike_rekey_t *rekey = (ike_rekey_t*)active; + rekey->collide(rekey, task); break; } continue; diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c index 7fa3c6081..60172cf20 100644 --- a/src/charon/sa/tasks/child_create.c +++ b/src/charon/sa/tasks/child_create.c @@ -687,6 +687,22 @@ static child_sa_t* get_child(private_child_create_t *this) return NULL; } +/** + * Implementation of child_create_t.get_lower_nonce + */ +static chunk_t get_lower_nonce(private_child_create_t *this) +{ + if (memcmp(this->my_nonce.ptr, this->other_nonce.ptr, + min(this->my_nonce.len, this->other_nonce.len)) < 0) + { + return this->my_nonce; + } + else + { + return this->other_nonce; + } +} + /** * Implementation of task_t.migrate */ @@ -756,6 +772,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa, policy_t *policy) private_child_create_t *this = malloc_thing(private_child_create_t); this->public.get_child = (child_sa_t*(*)(child_create_t*))get_child; + this->public.get_lower_nonce = (chunk_t(*)(child_create_t*))get_lower_nonce; this->public.use_reqid = (void(*)(child_create_t*,u_int32_t))use_reqid; this->public.task.get_type = (task_type_t(*)(task_t*))get_type; this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate; diff --git a/src/charon/sa/tasks/child_create.h b/src/charon/sa/tasks/child_create.h index f3f1a633f..7ff2fb22d 100644 --- a/src/charon/sa/tasks/child_create.h +++ b/src/charon/sa/tasks/child_create.h @@ -59,6 +59,14 @@ struct child_create_t { */ void (*use_reqid) (child_create_t *this, u_int32_t reqid); + /** + * @brief Get the lower of the two nonces, used for rekey collisions. + * + * @param this calling object + * @return lower nonce + */ + chunk_t (*get_lower_nonce) (child_create_t *this); + /** * @brief Get the CHILD_SA established by this task. * diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c index fddfa598a..3fb4a54f6 100644 --- a/src/charon/sa/tasks/child_rekey.c +++ b/src/charon/sa/tasks/child_rekey.c @@ -24,9 +24,7 @@ #include "child_rekey.h" #include -#include #include -#include #include @@ -66,39 +64,8 @@ struct private_child_rekey_t { * colliding task, may be delete or rekey */ task_t *collision; - - /** - * the lowest nonce compared so far - */ - chunk_t nonce; }; -/** - * get the nonce from a message, IF it is lower than a previous one - */ -static void get_nonce(private_child_rekey_t *this, message_t *message) -{ - nonce_payload_t *payload; - chunk_t nonce; - - payload = (nonce_payload_t*)message->get_payload(message, NONCE); - if (payload) - { - nonce = payload->get_nonce(payload); - - if (this->nonce.ptr && memcmp(nonce.ptr, this->nonce.ptr, - min(nonce.len, this->nonce.len)) > 0) - { - chunk_free(&nonce); - } - else - { - chunk_free(&this->nonce); - this->nonce = nonce; - } - } -} - /** * find a child using the REKEY_SA notify */ @@ -155,7 +122,6 @@ static status_t build_i(private_child_rekey_t *this, message_t *message) reqid = this->child_sa->get_reqid(this->child_sa); this->child_create->use_reqid(this->child_create, reqid); this->child_create->task.build(&this->child_create->task, message); - get_nonce(this, message); this->child_sa->set_state(this->child_sa, CHILD_REKEYING); @@ -169,7 +135,6 @@ static status_t process_r(private_child_rekey_t *this, message_t *message) { /* let the CHILD_CREATE task process the message */ this->child_create->task.process(&this->child_create->task, message); - get_nonce(this, message); find_child(this, message); @@ -204,8 +169,6 @@ static status_t build_r(private_child_rekey_t *this, message_t *message) return SUCCESS; } - get_nonce(this, message); - this->child_sa->set_state(this->child_sa, CHILD_REKEYING); return SUCCESS; @@ -234,20 +197,22 @@ static status_t process_i(private_child_rekey_t *this, message_t *message) return SUCCESS; } - get_nonce(this, message); - to_delete = this->child_sa; /* check for rekey collisions */ if (this->collision && this->collision->get_type(this->collision) == CHILD_REKEY) { + chunk_t this_nonce, other_nonce; private_child_rekey_t *other = (private_child_rekey_t*)this->collision; + this_nonce = this->child_create->get_lower_nonce(this->child_create); + other_nonce = other->child_create->get_lower_nonce(other->child_create); + /* if we have the lower nonce, delete rekeyed SA. If not, delete * the redundant. */ - if (memcmp(this->nonce.ptr, other->nonce.ptr, - min(this->nonce.len, other->nonce.len)) < 0) + if (memcmp(this_nonce.ptr, other_nonce.ptr, + min(this_nonce.len, other_nonce.len)) < 0) { DBG1(DBG_IKE, "CHILD_SA rekey collision won, deleting rekeyed child"); } @@ -295,7 +260,7 @@ static void collide(private_child_rekey_t *this, task_t *other) static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa) { this->child_create->task.migrate(&this->child_create->task, ike_sa); - chunk_free(&this->nonce); + DESTROY_IF(this->collision); this->ike_sa = ike_sa; this->collision = NULL; @@ -307,7 +272,6 @@ static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa) static void destroy(private_child_rekey_t *this) { this->child_create->task.destroy(&this->child_create->task); - chunk_free(&this->nonce); DESTROY_IF(this->collision); free(this); } @@ -342,7 +306,6 @@ child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa) this->ike_sa = ike_sa; this->child_sa = child_sa; - this->nonce = chunk_empty; this->collision = NULL; return &this->public; diff --git a/src/charon/sa/tasks/ike_init.c b/src/charon/sa/tasks/ike_init.c index d49209aaa..5b5ef3512 100644 --- a/src/charon/sa/tasks/ike_init.c +++ b/src/charon/sa/tasks/ike_init.c @@ -503,6 +503,22 @@ static task_type_t get_type(private_ike_init_t *this) return IKE_INIT; } +/** + * Implementation of task_t.get_type + */ +static chunk_t get_lower_nonce(private_ike_init_t *this) +{ + if (memcmp(this->my_nonce.ptr, this->other_nonce.ptr, + min(this->my_nonce.len, this->other_nonce.len)) < 0) + { + return this->my_nonce; + } + else + { + return this->other_nonce; + } +} + /** * Implementation of task_t.migrate */ @@ -537,6 +553,7 @@ ike_init_t *ike_init_create(ike_sa_t *ike_sa, bool initiator, ike_sa_t *old_sa) { private_ike_init_t *this = malloc_thing(private_ike_init_t); + this->public.get_lower_nonce = (chunk_t(*)(ike_init_t*))get_lower_nonce; this->public.task.get_type = (task_type_t(*)(task_t*))get_type; this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate; this->public.task.destroy = (void(*)(task_t*))destroy; diff --git a/src/charon/sa/tasks/ike_init.h b/src/charon/sa/tasks/ike_init.h index 290cc46e8..f60c096e8 100644 --- a/src/charon/sa/tasks/ike_init.h +++ b/src/charon/sa/tasks/ike_init.h @@ -32,7 +32,7 @@ typedef struct ike_init_t ike_init_t; /** * @brief Task of type IKE_INIT, creates an IKE_SA without authentication. * - * The authentication of is handle in the ike_auth and/or ike_auth_eap task. + * The authentication of is handle in the ike_auth task. * * @b Constructors: * - ike_init_create() @@ -45,6 +45,14 @@ struct ike_init_t { * Implements the task_t interface */ task_t task; + + /** + * @brief Get the lower of the two nonces, used for rekey collisions. + * + * @param this calling object + * @return lower nonce + */ + chunk_t (*get_lower_nonce) (ike_init_t *this); }; /** diff --git a/src/charon/sa/tasks/ike_rekey.c b/src/charon/sa/tasks/ike_rekey.c index 01698b11e..879a3ee62 100644 --- a/src/charon/sa/tasks/ike_rekey.c +++ b/src/charon/sa/tasks/ike_rekey.c @@ -24,9 +24,7 @@ #include "ike_rekey.h" #include -#include #include -#include #include #include @@ -62,6 +60,11 @@ struct private_ike_rekey_t { * the IKE_INIT task which is reused to simplify rekeying */ ike_init_t *ike_init; + + /** + * colliding task detected by the task manager + */ + task_t *collision; }; /** @@ -158,10 +161,7 @@ static status_t build_r(private_ike_rekey_t *this, message_t *message) } this->ike_sa->set_state(this->ike_sa, IKE_REKEYING); - this->new_sa->inherit(this->new_sa, this->ike_sa); this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED); - charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa); - this->new_sa = NULL; return SUCCESS; } @@ -172,23 +172,62 @@ static status_t build_r(private_ike_rekey_t *this, message_t *message) static status_t process_i(private_ike_rekey_t *this, message_t *message) { job_t *job; + ike_sa_id_t *to_delete; if (this->ike_init->task.process(&this->ike_init->task, message) == FAILED) { /* rekeying failed, fallback to old SA */ - this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED); - /* TODO: reschedule rekeying */ + if (!(this->collision && + this->collision->get_type(this->collision) == IKE_DELETE)) + { + this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED); + /* TODO: reschedule rekeying */ + } return SUCCESS; } - + this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED); - this->new_sa->inherit(this->new_sa, this->ike_sa); - charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa); - this->new_sa = NULL; + to_delete = this->ike_sa->get_id(this->ike_sa); + + /* check for collisions */ + if (this->collision && + this->collision->get_type(this->collision) == IKE_REKEY) + { + chunk_t this_nonce, other_nonce; + host_t *host; + private_ike_rekey_t *other = (private_ike_rekey_t*)this->collision; + + this_nonce = this->ike_init->get_lower_nonce(this->ike_init); + other_nonce = other->ike_init->get_lower_nonce(other->ike_init); + + /* if we have the lower nonce, delete rekeyed SA. If not, delete + * the redundant. */ + if (memcmp(this_nonce.ptr, other_nonce.ptr, + min(this_nonce.len, other_nonce.len)) < 0) + { + DBG1(DBG_IKE, "IKE_SA rekey collision won, deleting rekeyed IKE_SA"); + charon->ike_sa_manager->checkin(charon->ike_sa_manager, other->new_sa); + } + else + { + DBG1(DBG_IKE, "IKE_SA rekey collision lost, deleting redundant IKE_SA"); + /* apply host for a proper delete */ + host = this->ike_sa->get_my_host(this->ike_sa); + this->new_sa->set_my_host(this->new_sa, host->clone(host)); + host = this->ike_sa->get_other_host(this->ike_sa); + this->new_sa->set_other_host(this->new_sa, host->clone(host)); + this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED); + to_delete = this->new_sa->get_id(this->new_sa); + charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa); + /* inherit to other->new_sa in destroy() */ + this->new_sa = other->new_sa; + other->new_sa = NULL; + } + } + + job = (job_t*)delete_ike_sa_job_create(to_delete, TRUE); + charon->job_queue->add(charon->job_queue, job); - job = (job_t*)delete_ike_sa_job_create(this->ike_sa->get_id(this->ike_sa), - TRUE); - charon->job_queue->add(charon->job_queue, job); return SUCCESS; } @@ -200,6 +239,12 @@ static task_type_t get_type(private_ike_rekey_t *this) return IKE_REKEY; } +static void collide(private_ike_rekey_t* this, task_t *other) +{ + DESTROY_IF(this->collision); + this->collision = other; +} + /** * Implementation of task_t.migrate */ @@ -214,7 +259,9 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa) charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, this->new_sa); } + DESTROY_IF(this->collision); + this->collision = NULL; this->ike_sa = ike_sa; this->new_sa = NULL; this->ike_init = NULL; @@ -225,15 +272,24 @@ static void migrate(private_ike_rekey_t *this, ike_sa_t *ike_sa) */ static void destroy(private_ike_rekey_t *this) { + if (this->new_sa) + { + if (this->new_sa->get_state(this->new_sa) == IKE_ESTABLISHED) + { + this->new_sa->inherit(this->new_sa, this->ike_sa); + charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa); + } + else + { + charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, + this->new_sa); + } + } if (this->ike_init) { this->ike_init->task.destroy(&this->ike_init->task); } - if (this->new_sa) - { - charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, - this->new_sa); - } + DESTROY_IF(this->collision); free(this); } @@ -244,6 +300,7 @@ ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator) { private_ike_rekey_t *this = malloc_thing(private_ike_rekey_t); + this->public.collide = (void(*)(ike_rekey_t*,task_t*))collide; this->public.task.get_type = (task_type_t(*)(task_t*))get_type; this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate; this->public.task.destroy = (void(*)(task_t*))destroy; @@ -262,6 +319,7 @@ ike_rekey_t *ike_rekey_create(ike_sa_t *ike_sa, bool initiator) this->new_sa = NULL; this->ike_init = NULL; this->initiator = initiator; + this->collision = NULL; return &this->public; } diff --git a/src/charon/sa/tasks/ike_rekey.h b/src/charon/sa/tasks/ike_rekey.h index f1caf5758..125422efd 100644 --- a/src/charon/sa/tasks/ike_rekey.h +++ b/src/charon/sa/tasks/ike_rekey.h @@ -43,6 +43,18 @@ struct ike_rekey_t { * Implements the task_t interface */ task_t task; + + /** + * @brief Register a rekeying task which collides with this one. + * + * If two peers initiate rekeying at the same time, the collision must + * be handled gracefully. The task manager is aware of what exchanges + * are going on and notifies the outgoing task by passing the incoming. + * + * @param this task initated by us + * @param other incoming task + */ + void (*collide)(ike_rekey_t* this, task_t *other); }; /**