From: Martin Willi Date: Wed, 21 Mar 2007 09:25:36 +0000 (-0000) Subject: handling of CHILD_SA rekeying collisions X-Git-Tag: 4.1.0~9 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=195ada0b4b162f7dc9704521c6c6b4a01d288891;p=thirdparty%2Fstrongswan.git handling of CHILD_SA rekeying collisions --- diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c index c40ec6e51b..9073bed7a5 100644 --- a/src/charon/sa/task_manager.c +++ b/src/charon/sa/task_manager.c @@ -433,6 +433,56 @@ static status_t process_response(private_task_manager_t *this, return build_request(this); } +/** + * handle exchange collisions + */ +static void handle_collisions(private_task_manager_t *this, task_t *task) +{ + iterator_t *iterator; + task_t *active; + task_type_t type; + + type = task->get_type(task); + + /* check if we have initiated rekeying ourself */ + if (this->initiating.type != CREATE_CHILD_SA || + (type != IKE_REKEY && type != CHILD_REKEY && type != CHILD_DELETE)) + { + task->destroy(task); + return; + } + + /* find an exchange collision, and notify these tasks */ + iterator = this->active_tasks->create_iterator(this->active_tasks, TRUE); + while (iterator->iterate(iterator, (void**)&active)) + { + switch (active->get_type(active)) + { + case IKE_REKEY: + if (type == IKE_REKEY) + { + //ike_rekey_t *rekey = (ike_rekey_t*)active; + //rekey->collide(rekey, (ike_rekey_t*)task); + break; + } + continue; + case CHILD_REKEY: + /* TODO: check if it is the SAME child we are talking about! */ + if (type == CHILD_REKEY || type == CHILD_DELETE) + { + child_rekey_t *rekey = (child_rekey_t*)active; + rekey->collide(rekey, task); + break; + } + continue; + default: + continue; + } + break; + } + iterator->destroy(iterator); +} + /** * build a response depending on the "passive" task list */ @@ -458,8 +508,7 @@ static status_t build_response(private_task_manager_t *this, case SUCCESS: /* task completed, remove it */ iterator->remove(iterator); - task->destroy(task); - break; + handle_collisions(this, task); case NEED_MORE: /* processed, but task needs another exchange */ break; diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c index 9526146a8c..7fa3c60815 100644 --- a/src/charon/sa/tasks/child_create.c +++ b/src/charon/sa/tasks/child_create.c @@ -102,6 +102,11 @@ struct private_child_create_t { * CHILD_SA which gets established */ child_sa_t *child_sa; + + /** + * successfully established the CHILD? + */ + bool established; }; /** @@ -306,7 +311,7 @@ static status_t select_and_install(private_child_create_t *this) /* add to IKE_SA, and remove from task */ this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); this->ike_sa->add_child_sa(this->ike_sa, this->child_sa); - this->child_sa = NULL; + this->established = TRUE; return SUCCESS; } @@ -594,7 +599,6 @@ static status_t process_i(private_child_create_t *this, message_t *message) { iterator_t *iterator; payload_t *payload; - status_t status; switch (message->get_exchange_type(message)) { @@ -648,10 +652,10 @@ static status_t process_i(private_child_create_t *this, message_t *message) process_payloads(this, message); - status = select_and_install(this); - - SIG(CHILD_UP_SUCCESS, "established CHILD_SA successfully"); - + if (select_and_install(this) == SUCCESS) + { + SIG(CHILD_UP_SUCCESS, "established CHILD_SA successfully"); + } return SUCCESS; } @@ -671,6 +675,18 @@ static void use_reqid(private_child_create_t *this, u_int32_t reqid) this->reqid = reqid; } +/** + * Implementation of child_create_t.get_child + */ +static child_sa_t* get_child(private_child_create_t *this) +{ + if (this->established) + { + return this->child_sa; + } + return NULL; +} + /** * Implementation of task_t.migrate */ @@ -700,6 +716,7 @@ static void migrate(private_child_create_t *this, ike_sa_t *ike_sa) this->child_sa = NULL; this->mode = MODE_TUNNEL; this->reqid = 0; + this->established = FALSE; } /** @@ -717,7 +734,10 @@ static void destroy(private_child_create_t *this) { this->tsi->destroy_offset(this->tsi, offsetof(traffic_selector_t, destroy)); } - DESTROY_IF(this->child_sa); + if (!this->established) + { + DESTROY_IF(this->child_sa); + } DESTROY_IF(this->proposal); if (this->proposals) { @@ -735,6 +755,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.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; @@ -764,6 +785,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa, policy_t *policy) this->child_sa = NULL; this->mode = MODE_TUNNEL; this->reqid = 0; + this->established = FALSE; return &this->public; } diff --git a/src/charon/sa/tasks/child_create.h b/src/charon/sa/tasks/child_create.h index 1644865b99..f3f1a633f0 100644 --- a/src/charon/sa/tasks/child_create.h +++ b/src/charon/sa/tasks/child_create.h @@ -58,6 +58,16 @@ struct child_create_t { * @param reqid reqid to use */ void (*use_reqid) (child_create_t *this, u_int32_t reqid); + + /** + * @brief Get the CHILD_SA established by this task. + * + * This call returns a child only when it has been established successfully. + * + * @param this calling object + * @return child_sa + */ + child_sa_t* (*get_child) (child_create_t *this); }; /** diff --git a/src/charon/sa/tasks/child_delete.c b/src/charon/sa/tasks/child_delete.c index 4fb7214454..30270f27e5 100644 --- a/src/charon/sa/tasks/child_delete.c +++ b/src/charon/sa/tasks/child_delete.c @@ -128,6 +128,8 @@ static void process_payloads(private_child_delete_t *this, message_t *message) "but no such SA", protocol_id_names, protocol, ntohl(*spi)); continue; } + DBG2(DBG_IKE, "received DELETE for %N CHILD_SA with SPI 0x%x", + protocol_id_names, protocol, ntohl(*spi)); switch (child_sa->get_state(child_sa)) { diff --git a/src/charon/sa/tasks/child_rekey.c b/src/charon/sa/tasks/child_rekey.c index 660a9c15fa..fddfa598ac 100644 --- a/src/charon/sa/tasks/child_rekey.c +++ b/src/charon/sa/tasks/child_rekey.c @@ -63,46 +63,40 @@ struct private_child_rekey_t { child_sa_t *child_sa; /** - * redundandt CHILD_SA created simultaneously + * colliding task, may be delete or rekey */ - child_sa_t *simultaneous; + task_t *collision; /** * the lowest nonce compared so far */ chunk_t nonce; - - /** - * TRUE if we have the lower nonce - */ - bool winner; }; /** - * get the nonce from a message, return TRUE if it was lower than this->nonce + * get the nonce from a message, IF it is lower than a previous one */ -static bool get_nonce(private_child_rekey_t *this, message_t *message) +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 == NULL) - { - return FALSE; - } - nonce = payload->get_nonce(payload); + 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); - return FALSE; + 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; + } } - - chunk_free(&this->nonce); - this->nonce = nonce; - return TRUE; } /** @@ -192,6 +186,7 @@ static status_t build_r(private_child_rekey_t *this, message_t *message) if (this->child_sa == NULL || this->child_sa->get_state(this->child_sa) == CHILD_DELETING) { + DBG1(DBG_IKE, "unable to rekey, CHILD_SA not found"); message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty); return SUCCESS; } @@ -210,11 +205,6 @@ static status_t build_r(private_child_rekey_t *this, message_t *message) } get_nonce(this, message); - - if (this->child_sa->get_state(this->child_sa) == CHILD_REKEYING) - { - /* TODO: handle simultaneous rekeying */ - } this->child_sa->set_state(this->child_sa, CHILD_REKEYING); @@ -228,19 +218,53 @@ static status_t process_i(private_child_rekey_t *this, message_t *message) { protocol_id_t protocol; u_int32_t spi; + child_sa_t *to_delete; this->child_create->task.process(&this->child_create->task, message); if (message->get_payload(message, SECURITY_ASSOCIATION) == NULL) { - /* establishing new child failed, fallback */ - this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); - /* TODO: rescedule rekeying */ + /* establishing new child failed, reuse old. but not when we + * recieved a delete in the meantime */ + if (!(this->collision && + this->collision->get_type(this->collision) == CHILD_DELETE)) + { + this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); + /* TODO: rescedule rekeying */ + } return SUCCESS; } - /* TODO: delete the redundant CHILD_SA, instead of the rekeyed */ - spi = this->child_sa->get_spi(this->child_sa, TRUE); - protocol = this->child_sa->get_protocol(this->child_sa); + get_nonce(this, message); + + to_delete = this->child_sa; + + /* check for rekey collisions */ + if (this->collision && + this->collision->get_type(this->collision) == CHILD_REKEY) + { + private_child_rekey_t *other = (private_child_rekey_t*)this->collision; + + /* 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, "CHILD_SA rekey collision won, deleting rekeyed child"); + } + else + { + DBG1(DBG_IKE, "CHILD_SA rekey collision lost, deleting redundant child"); + to_delete = this->child_create->get_child(this->child_create); + if (to_delete == NULL) + { + /* ooops, should not happen, fallback */ + to_delete = this->child_sa; + } + } + } + + spi = to_delete->get_spi(to_delete, TRUE); + protocol = to_delete->get_protocol(to_delete); if (this->ike_sa->delete_child_sa(this->ike_sa, protocol, spi) != SUCCESS) { return FAILED; @@ -256,6 +280,15 @@ static task_type_t get_type(private_child_rekey_t *this) return CHILD_REKEY; } +/** + * Implementation of child_rekey_t.collide + */ +static void collide(private_child_rekey_t *this, task_t *other) +{ + DESTROY_IF(this->collision); + this->collision = other; +} + /** * Implementation of task_t.migrate */ @@ -265,8 +298,7 @@ static void migrate(private_child_rekey_t *this, ike_sa_t *ike_sa) chunk_free(&this->nonce); this->ike_sa = ike_sa; - this->winner = TRUE; - this->simultaneous = NULL; + this->collision = NULL; } /** @@ -276,6 +308,7 @@ 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); } @@ -287,6 +320,7 @@ child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, child_sa_t *child_sa) private_child_rekey_t *this = malloc_thing(private_child_rekey_t); policy_t *policy; + this->public.collide = (void (*)(child_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; @@ -309,8 +343,7 @@ 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->winner = TRUE; - this->simultaneous = NULL; + this->collision = NULL; return &this->public; } diff --git a/src/charon/sa/tasks/child_rekey.h b/src/charon/sa/tasks/child_rekey.h index 77297b13d2..3515f0c3fd 100644 --- a/src/charon/sa/tasks/child_rekey.h +++ b/src/charon/sa/tasks/child_rekey.h @@ -44,6 +44,18 @@ struct child_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)(child_rekey_t* this, task_t *other); }; /**