ikev2: Let ike/child-rekey tasks indicate if the passive task was adopted

This gives us more flexibility with tasks that return NEED_MORE (currently
none of the colliding tasks do, but that will change with multi-KE
rekeyings).  The active task has to check itself if the passive task is
done and should be removed from the task manager.
This commit is contained in:
Tobias Brunner 2020-06-12 11:24:18 +02:00 committed by Andreas Steffen
parent bbbdc8fb68
commit 350f1a81f6
5 changed files with 30 additions and 29 deletions

View File

@ -882,13 +882,15 @@ static status_t process_response(private_task_manager_t *this,
} }
/** /**
* handle exchange collisions * Handle exchange collisions, returns TRUE if the given passive task was
* adopted by the active task and the task manager lost control over it.
*/ */
static bool handle_collisions(private_task_manager_t *this, task_t *task) static bool handle_collisions(private_task_manager_t *this, task_t *task)
{ {
enumerator_t *enumerator; enumerator_t *enumerator;
task_t *active; task_t *active;
task_type_t type; task_type_t type;
bool adopted = FALSE;
type = task->get_type(task); type = task->get_type(task);
@ -906,7 +908,7 @@ static bool handle_collisions(private_task_manager_t *this, task_t *task)
if (type == TASK_IKE_REKEY || type == TASK_IKE_DELETE) if (type == TASK_IKE_REKEY || type == TASK_IKE_DELETE)
{ {
ike_rekey_t *rekey = (ike_rekey_t*)active; ike_rekey_t *rekey = (ike_rekey_t*)active;
rekey->collide(rekey, task); adopted = rekey->collide(rekey, task);
break; break;
} }
continue; continue;
@ -914,7 +916,7 @@ static bool handle_collisions(private_task_manager_t *this, task_t *task)
if (type == TASK_CHILD_REKEY || type == TASK_CHILD_DELETE) if (type == TASK_CHILD_REKEY || type == TASK_CHILD_DELETE)
{ {
child_rekey_t *rekey = (child_rekey_t*)active; child_rekey_t *rekey = (child_rekey_t*)active;
rekey->collide(rekey, task); adopted = rekey->collide(rekey, task);
break; break;
} }
continue; continue;
@ -922,11 +924,11 @@ static bool handle_collisions(private_task_manager_t *this, task_t *task)
continue; continue;
} }
enumerator->destroy(enumerator); enumerator->destroy(enumerator);
return TRUE; return adopted;
} }
enumerator->destroy(enumerator); enumerator->destroy(enumerator);
} }
return FALSE; return adopted;
} }
/** /**

View File

@ -518,7 +518,7 @@ METHOD(child_rekey_t, is_redundant, bool,
return FALSE; return FALSE;
} }
METHOD(child_rekey_t, collide, void, METHOD(child_rekey_t, collide, bool,
private_child_rekey_t *this, task_t *other) private_child_rekey_t *this, task_t *other)
{ {
/* the task manager only detects exchange collision, but not if /* the task manager only detects exchange collision, but not if
@ -530,16 +530,14 @@ METHOD(child_rekey_t, collide, void,
if (rekey->child_sa != this->child_sa) if (rekey->child_sa != this->child_sa)
{ /* not the same child => no collision */ { /* not the same child => no collision */
other->destroy(other); return FALSE;
return;
} }
/* ignore passive tasks that did not successfully create a CHILD_SA */ /* ignore passive tasks that did not successfully create a CHILD_SA */
other_child = rekey->child_create->get_child(rekey->child_create); other_child = rekey->child_create->get_child(rekey->child_create);
if (!other_child || if (!other_child ||
other_child->get_state(other_child) != CHILD_INSTALLED) other_child->get_state(other_child) != CHILD_INSTALLED)
{ {
other->destroy(other); return FALSE;
return;
} }
} }
else if (other->get_type(other) == TASK_CHILD_DELETE) else if (other->get_type(other) == TASK_CHILD_DELETE)
@ -548,26 +546,24 @@ METHOD(child_rekey_t, collide, void,
if (is_redundant(this, del->get_child(del))) if (is_redundant(this, del->get_child(del)))
{ {
this->other_child_destroyed = TRUE; this->other_child_destroyed = TRUE;
other->destroy(other); return FALSE;
return;
} }
if (del->get_child(del) != this->child_sa) if (del->get_child(del) != this->child_sa)
{ {
/* not the same child => no collision */ /* not the same child => no collision */
other->destroy(other); return FALSE;
return;
} }
} }
else else
{ {
/* any other task is not critical for collisions, ignore */ /* shouldn't happen */
other->destroy(other); return FALSE;
return;
} }
DBG1(DBG_IKE, "detected %N collision with %N", task_type_names, DBG1(DBG_IKE, "detected %N collision with %N", task_type_names,
TASK_CHILD_REKEY, task_type_names, other->get_type(other)); TASK_CHILD_REKEY, task_type_names, other->get_type(other));
DESTROY_IF(this->collision); DESTROY_IF(this->collision);
this->collision = other; this->collision = other;
return TRUE;
} }
METHOD(task_t, migrate, void, METHOD(task_t, migrate, void,

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2016 Tobias Brunner * Copyright (C) 2016-2020 Tobias Brunner
* Copyright (C) 2007 Martin Willi * Copyright (C) 2007 Martin Willi
* HSR Hochschule fuer Technik Rapperswil * HSR Hochschule fuer Technik Rapperswil
* *
@ -58,9 +58,11 @@ struct child_rekey_t {
* be handled gracefully. The task manager is aware of what exchanges * be handled gracefully. The task manager is aware of what exchanges
* are going on and notifies the active task by passing the passive. * are going on and notifies the active task by passing the passive.
* *
* @param other passive task (adopted) * @param other passive task
* @return whether the task was adopted and should be removed from
* the task manager's control
*/ */
void (*collide)(child_rekey_t* this, task_t *other); bool (*collide)(child_rekey_t* this, task_t *other);
}; };
/** /**

View File

@ -421,7 +421,7 @@ METHOD(ike_rekey_t, did_collide, bool,
return this->collision != NULL; return this->collision != NULL;
} }
METHOD(ike_rekey_t, collide, void, METHOD(ike_rekey_t, collide, bool,
private_ike_rekey_t* this, task_t *other) private_ike_rekey_t* this, task_t *other)
{ {
DBG1(DBG_IKE, "detected %N collision with %N", task_type_names, DBG1(DBG_IKE, "detected %N collision with %N", task_type_names,
@ -431,7 +431,6 @@ METHOD(ike_rekey_t, collide, void,
{ {
case TASK_IKE_DELETE: case TASK_IKE_DELETE:
conclude_undetected_collision(this); conclude_undetected_collision(this);
other->destroy(other);
break; break;
case TASK_IKE_REKEY: case TASK_IKE_REKEY:
{ {
@ -441,17 +440,17 @@ METHOD(ike_rekey_t, collide, void,
{ {
DBG1(DBG_IKE, "colliding exchange did not result in an IKE_SA, " DBG1(DBG_IKE, "colliding exchange did not result in an IKE_SA, "
"ignore"); "ignore");
other->destroy(other);
break; break;
} }
DESTROY_IF(&this->collision->public.task); DESTROY_IF(&this->collision->public.task);
this->collision = rekey; this->collision = rekey;
break; return TRUE;
} }
default: default:
/* shouldn't happen */ /* shouldn't happen */
break; break;
} }
return FALSE;
} }
/** /**

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2016 Tobias Brunner * Copyright (C) 2016-2020 Tobias Brunner
* Copyright (C) 2007 Martin Willi * Copyright (C) 2007 Martin Willi
* HSR Hochschule fuer Technik Rapperswil * HSR Hochschule fuer Technik Rapperswil
* *
@ -46,15 +46,17 @@ struct ike_rekey_t {
bool (*did_collide)(ike_rekey_t *this); bool (*did_collide)(ike_rekey_t *this);
/** /**
* Register a rekeying task which collides with this one. * Register a rekeying/delete task which collides with this one.
* *
* If two peers initiate rekeying at the same time, the collision must * If two peers initiate rekeying at the same time, the collision must
* be handled gracefully. The task manager is aware of what exchanges * be handled gracefully. The task manager is aware of what exchanges
* are going on and notifies the outgoing task by passing the incoming. * are going on and notifies the active task by passing the passive.
* *
* @param other incoming task * @param other passive task
* @return whether the task was adopted and should be removed from
* the task manager's control
*/ */
void (*collide)(ike_rekey_t* this, task_t *other); bool (*collide)(ike_rekey_t* this, task_t *other);
}; };
/** /**