Hello Martin, please consider the patch attached for inclusion into upstream-charon. It fixes a possible segfault during a collision in case the colliding ike_rekey-job does not build an ike_init due to half-open children.
Thomas
>From 243ecead57b48c943629111747adc8b9b9d5bf9b Mon Sep 17 00:00:00 2001 From: Thomas Egerer <[email protected]> Date: Tue, 24 Aug 2010 14:55:47 +0200 Subject: [PATCH] Check if colliding rekey actually created an IKE_INIT In some cases (especially if a child is half-open) the colliding rekey-job might not have created the ike_init member. If so, the nonce check fails with SIGSEGV. Signed-off-by: Thomas Egerer <[email protected]> --- src/libcharon/sa/tasks/ike_rekey.c | 79 +++++++++++++++++++----------------- 1 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/libcharon/sa/tasks/ike_rekey.c b/src/libcharon/sa/tasks/ike_rekey.c index a2275e7..de23532 100644 --- a/src/libcharon/sa/tasks/ike_rekey.c +++ b/src/libcharon/sa/tasks/ike_rekey.c @@ -241,51 +241,56 @@ static status_t process_i(private_ike_rekey_t *this, message_t *message) 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) - { - /* peer should delete this SA. Add a timeout just in case. */ - job_t *job = (job_t*)delete_ike_sa_job_create( - other->new_sa->get_id(other->new_sa), TRUE); - charon->scheduler->schedule_job(charon->scheduler, job, 10); - DBG1(DBG_IKE, "IKE_SA rekey collision won, deleting rekeyed IKE_SA"); - charon->ike_sa_manager->checkin(charon->ike_sa_manager, other->new_sa); - other->new_sa = NULL; - } - else + /* ike_init can be NULL, if child_sa is half-open */ + if (other->ike_init) { - 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); - if (this->new_sa->delete(this->new_sa) == DESTROY_ME) + host_t *host; + chunk_t this_nonce, other_nonce; + + 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) { - charon->ike_sa_manager->checkin_and_destroy( - charon->ike_sa_manager, this->new_sa); + /* peer should delete this SA. Add a timeout just in case. */ + job_t *job = (job_t*)delete_ike_sa_job_create( + other->new_sa->get_id(other->new_sa), TRUE); + charon->scheduler->schedule_job(charon->scheduler, job, 10); + DBG1(DBG_IKE, "IKE_SA rekey collision won, deleting rekeyed IKE_SA"); + charon->ike_sa_manager->checkin(charon->ike_sa_manager, other->new_sa); + other->new_sa = NULL; } else { - charon->ike_sa_manager->checkin( - charon->ike_sa_manager, this->new_sa); + 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); + if (this->new_sa->delete(this->new_sa) == DESTROY_ME) + { + charon->ike_sa_manager->checkin_and_destroy( + charon->ike_sa_manager, this->new_sa); + } + else + { + 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); + /* inherit to other->new_sa in destroy() */ + this->new_sa = other->new_sa; + other->new_sa = NULL; + return SUCCESS; } - /* set threads active IKE_SA after checkin */ - charon->bus->set_sa(charon->bus, this->ike_sa); - /* inherit to other->new_sa in destroy() */ - this->new_sa = other->new_sa; - other->new_sa = NULL; - return SUCCESS; } /* set threads active IKE_SA after checkin */ charon->bus->set_sa(charon->bus, this->ike_sa); -- 1.7.1
_______________________________________________ Dev mailing list [email protected] https://lists.strongswan.org/mailman/listinfo/dev
