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

Move establish/inherit of rekeyed IKE_SAs to delete messages

Browse Source 
Having the inherit() function delayed to the IKE_SA establish procedure
was problematic. The task destroy function was never a good place and
results in locking/cleanup problems. After establishing the SA, it
should be really checked in ASAP to avoid any triggered DPD checks
to get lost.
This commit is contained in:
Martin Willi
2011-03-15 15:20:09 +01:00
parent f42156a8c8
commit 3ced6b51e4
5 changed files with 54 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libcharon/sa/tasks/child_rekey.c
+2
View File
@@ -412,6 +412,8 @@ static void collide(private_child_rekey_t *this, task_t *other)
other->destroy(other);
return;
}
DBG1(DBG_IKE, "detected %N collision with %N", task_type_names, CHILD_REKEY,
task_type_names, other->get_type(other));
DESTROY_IF(this->collision);
this->collision = other;
}
src/libcharon/sa/tasks/ike_rekey.c
+38 -48
View File
@@ -67,9 +67,35 @@ struct private_ike_rekey_t {
task_t *collision;
};
/**
* Establish the new replacement IKE_SA
*/
static void establish_new(private_ike_rekey_t *this)
{
if (this->new_sa)
{
this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
DBG0(DBG_IKE, "IKE_SA %s[%d] rekeyed between %H[%Y]...%H[%Y]",
this->new_sa->get_name(this->new_sa),
this->new_sa->get_unique_id(this->new_sa),
this->ike_sa->get_my_host(this->ike_sa),
this->ike_sa->get_my_id(this->ike_sa),
this->ike_sa->get_other_host(this->ike_sa),
this->ike_sa->get_other_id(this->ike_sa));
this->new_sa->inherit(this->new_sa, this->ike_sa);
charon->bus->ike_rekey(charon->bus, this->ike_sa, this->new_sa);
charon->ike_sa_manager->checkin(charon->ike_sa_manager, this->new_sa);
this->new_sa = NULL;
/* set threads active IKE_SA after checkin */
charon->bus->set_sa(charon->bus, this->ike_sa);
}
}
METHOD(task_t, process_r_delete, status_t,
private_ike_rekey_t *this, message_t *message)
{
establish_new(this);
return this->ike_delete->task.process(&this->ike_delete->task, message);
}
@@ -176,14 +202,6 @@ METHOD(task_t, build_r, status_t,
}
this->ike_sa->set_state(this->ike_sa, IKE_REKEYING);
this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
this->new_sa->get_name(this->new_sa),
this->new_sa->get_unique_id(this->new_sa),
this->ike_sa->get_my_host(this->ike_sa),
this->ike_sa->get_my_id(this->ike_sa),
this->ike_sa->get_other_host(this->ike_sa),
this->ike_sa->get_other_id(this->ike_sa));
/* rekeying successful, delete the IKE_SA using a subtask */
this->ike_delete = ike_delete_create(this->ike_sa, FALSE);
@@ -233,15 +251,6 @@ METHOD(task_t, process_i, status_t,
break;
}
this->new_sa->set_state(this->new_sa, IKE_ESTABLISHED);
DBG0(DBG_IKE, "IKE_SA %s[%d] established between %H[%Y]...%H[%Y]",
this->new_sa->get_name(this->new_sa),
this->new_sa->get_unique_id(this->new_sa),
this->ike_sa->get_my_host(this->ike_sa),
this->ike_sa->get_my_id(this->ike_sa),
this->ike_sa->get_other_host(this->ike_sa),
this->ike_sa->get_other_id(this->ike_sa));
/* check for collisions */
if (this->collision &&
this->collision->get_type(this->collision) == IKE_REKEY)
@@ -280,21 +289,20 @@ METHOD(task_t, process_i, status_t,
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);
this->new_sa->set_state(this->new_sa, IKE_REKEYING);
if (this->new_sa->delete(this->new_sa) == DESTROY_ME)
{
charon->ike_sa_manager->checkin_and_destroy(
charon->ike_sa_manager, this->new_sa);
this->new_sa->destroy(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);
}
/* 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;
this->new_sa = NULL;
establish_new(other);
return SUCCESS;
}
}
@@ -302,6 +310,8 @@ METHOD(task_t, process_i, status_t,
charon->bus->set_sa(charon->bus, this->ike_sa);
}
establish_new(this);
/* rekeying successful, delete the IKE_SA using a subtask */
this->ike_delete = ike_delete_create(this->ike_sa, TRUE);
this->public.task.build = _build_i_delete;
@@ -319,6 +329,8 @@ METHOD(task_t, get_type, task_type_t,
METHOD(ike_rekey_t, collide, void,
private_ike_rekey_t* this, task_t *other)
{
DBG1(DBG_IKE, "detected %N collision with %N", task_type_names, IKE_REKEY,
task_type_names, other->get_type(other));
DESTROY_IF(this->collision);
this->collision = other;
}
@@ -334,13 +346,7 @@ METHOD(task_t, migrate, void,
{
this->ike_delete->task.destroy(&this->ike_delete->task);
}
if (this->new_sa)
{
charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
this->new_sa);
/* set threads active IKE_SA after checkin */
charon->bus->set_sa(charon->bus, this->ike_sa);
}
DESTROY_IF(this->new_sa);
DESTROY_IF(this->collision);
this->collision = NULL;
@@ -353,23 +359,6 @@ METHOD(task_t, migrate, void,
METHOD(task_t, destroy, void,
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) != DESTROY_ME)
{
/* invoke hook if rekeying was successful */
charon->bus->ike_rekey(charon->bus, this->ike_sa, this->new_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);
}
/* set threads active IKE_SA after checkin */
charon->bus->set_sa(charon->bus, this->ike_sa);
}
if (this->ike_init)
{
this->ike_init->task.destroy(&this->ike_init->task);
@@ -378,6 +367,7 @@ METHOD(task_t, destroy, void,
{
this->ike_delete->task.destroy(&this->ike_delete->task);
}
DESTROY_IF(this->new_sa);
DESTROY_IF(this->collision);
free(this);
}