ike-init: Switch to an alternative config if proposals don't match

This way we don't rely on the order of equally matching configs as
heavily anymore (which is actually tricky in vici) and this also doesn't
require repeating weak algorithms in all configs that might potentially be
selected if there are some clients that require them.

There is currently no ordering, so an explicitly configured exactly matching
proposal isn't a better match than e.g. the default proposal that also
contains the proposed algorithms.
This commit is contained in:
Tobias Brunner
2018-06-28 18:46:42 +02:00
parent da288a07aa
commit 054ee5e7c0
2 changed files with 77 additions and 31 deletions
+1
View File
@@ -674,6 +674,7 @@ METHOD(ike_sa_t, get_ike_cfg, ike_cfg_t*,
METHOD(ike_sa_t, set_ike_cfg, void, METHOD(ike_sa_t, set_ike_cfg, void,
private_ike_sa_t *this, ike_cfg_t *ike_cfg) private_ike_sa_t *this, ike_cfg_t *ike_cfg)
{ {
DESTROY_IF(this->ike_cfg);
ike_cfg->get_ref(ike_cfg); ike_cfg->get_ref(ike_cfg);
this->ike_cfg = ike_cfg; this->ike_cfg = ike_cfg;
} }
+76 -31
View File
@@ -54,11 +54,6 @@ struct private_ike_init_t {
*/ */
bool initiator; bool initiator;
/**
* IKE config to establish
*/
ike_cfg_t *config;
/** /**
* diffie hellman group to use * diffie hellman group to use
*/ */
@@ -286,14 +281,15 @@ static bool build_payloads(private_ike_init_t *this, message_t *message)
ike_sa_id_t *id; ike_sa_id_t *id;
proposal_t *proposal; proposal_t *proposal;
enumerator_t *enumerator; enumerator_t *enumerator;
ike_cfg_t *ike_cfg;
id = this->ike_sa->get_id(this->ike_sa); id = this->ike_sa->get_id(this->ike_sa);
this->config = this->ike_sa->get_ike_cfg(this->ike_sa); ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
if (this->initiator) if (this->initiator)
{ {
proposal_list = this->config->get_proposals(this->config); proposal_list = ike_cfg->get_proposals(ike_cfg);
other_dh_groups = linked_list_create(); other_dh_groups = linked_list_create();
enumerator = proposal_list->create_enumerator(proposal_list); enumerator = proposal_list->create_enumerator(proposal_list);
while (enumerator->enumerate(enumerator, (void**)&proposal)) while (enumerator->enumerate(enumerator, (void**)&proposal))
@@ -357,7 +353,7 @@ static bool build_payloads(private_ike_init_t *this, message_t *message)
/* negotiate fragmentation if we are not rekeying */ /* negotiate fragmentation if we are not rekeying */
if (!this->old_sa && if (!this->old_sa &&
this->config->fragmentation(this->config) != FRAGMENTATION_NO) ike_cfg->fragmentation(ike_cfg) != FRAGMENTATION_NO)
{ {
if (this->initiator || if (this->initiator ||
this->ike_sa->supports_extension(this->ike_sa, this->ike_sa->supports_extension(this->ike_sa,
@@ -403,6 +399,68 @@ static bool build_payloads(private_ike_init_t *this, message_t *message)
return TRUE; return TRUE;
} }
/**
* Process the SA payload and select a proposal
*/
static void process_sa_payload(private_ike_init_t *this, message_t *message,
sa_payload_t *sa_payload)
{
ike_cfg_t *ike_cfg, *cfg, *alt_cfg = NULL;
enumerator_t *enumerator;
linked_list_t *proposal_list;
host_t *me, *other;
bool private, prefer_configured;
ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
proposal_list = sa_payload->get_proposals(sa_payload);
private = this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN);
prefer_configured = lib->settings->get_bool(lib->settings,
"%s.prefer_configured_proposals", TRUE, lib->ns);
this->proposal = ike_cfg->select_proposal(ike_cfg, proposal_list, private,
prefer_configured);
if (!this->proposal)
{
if (!this->initiator && !this->old_sa)
{
me = message->get_destination(message);
other = message->get_source(message);
enumerator = charon->backends->create_ike_cfg_enumerator(
charon->backends, me, other, IKEV2);
while (enumerator->enumerate(enumerator, &cfg))
{
if (ike_cfg == cfg)
{ /* already tried and failed */
continue;
}
DBG1(DBG_IKE, "no matching proposal found, trying alternative "
"config");
this->proposal = cfg->select_proposal(cfg, proposal_list,
private, prefer_configured);
if (this->proposal)
{
alt_cfg = cfg->get_ref(cfg);
break;
}
}
enumerator->destroy(enumerator);
}
if (alt_cfg)
{
this->ike_sa->set_ike_cfg(this->ike_sa, alt_cfg);
alt_cfg->destroy(alt_cfg);
}
else
{
charon->bus->alert(charon->bus, ALERT_PROPOSAL_MISMATCH_IKE,
proposal_list);
}
}
proposal_list->destroy_offset(proposal_list,
offsetof(proposal_t, destroy));
}
/** /**
* Read payloads from message * Read payloads from message
*/ */
@@ -419,24 +477,7 @@ static void process_payloads(private_ike_init_t *this, message_t *message)
{ {
case PLV2_SECURITY_ASSOCIATION: case PLV2_SECURITY_ASSOCIATION:
{ {
sa_payload_t *sa_payload = (sa_payload_t*)payload; process_sa_payload(this, message, (sa_payload_t*)payload);
linked_list_t *proposal_list;
bool private, prefer_configured;
proposal_list = sa_payload->get_proposals(sa_payload);
private = this->ike_sa->supports_extension(this->ike_sa,
EXT_STRONGSWAN);
prefer_configured = lib->settings->get_bool(lib->settings,
"%s.prefer_configured_proposals", TRUE, lib->ns);
this->proposal = this->config->select_proposal(this->config,
proposal_list, private, prefer_configured);
if (!this->proposal)
{
charon->bus->alert(charon->bus, ALERT_PROPOSAL_MISMATCH_IKE,
proposal_list);
}
proposal_list->destroy_offset(proposal_list,
offsetof(proposal_t, destroy));
break; break;
} }
case PLV2_KEY_EXCHANGE: case PLV2_KEY_EXCHANGE:
@@ -533,7 +574,10 @@ static void process_payloads(private_ike_init_t *this, message_t *message)
METHOD(task_t, build_i, status_t, METHOD(task_t, build_i, status_t,
private_ike_init_t *this, message_t *message) private_ike_init_t *this, message_t *message)
{ {
this->config = this->ike_sa->get_ike_cfg(this->ike_sa); ike_cfg_t *ike_cfg;
ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
DBG0(DBG_IKE, "initiating IKE_SA %s[%d] to %H", DBG0(DBG_IKE, "initiating IKE_SA %s[%d] to %H",
this->ike_sa->get_name(this->ike_sa), this->ike_sa->get_name(this->ike_sa),
this->ike_sa->get_unique_id(this->ike_sa), this->ike_sa->get_unique_id(this->ike_sa),
@@ -563,12 +607,12 @@ METHOD(task_t, build_i, status_t,
} }
else else
{ /* this shouldn't happen, but let's be safe */ { /* this shouldn't happen, but let's be safe */
this->dh_group = this->config->get_dh_group(this->config); this->dh_group = ike_cfg->get_dh_group(ike_cfg);
} }
} }
else else
{ {
this->dh_group = this->config->get_dh_group(this->config); this->dh_group = ike_cfg->get_dh_group(ike_cfg);
} }
this->dh = this->keymat->keymat.create_dh(&this->keymat->keymat, this->dh = this->keymat->keymat.create_dh(&this->keymat->keymat,
this->dh_group); this->dh_group);
@@ -627,7 +671,6 @@ METHOD(task_t, build_i, status_t,
METHOD(task_t, process_r, status_t, METHOD(task_t, process_r, status_t,
private_ike_init_t *this, message_t *message) private_ike_init_t *this, message_t *message)
{ {
this->config = this->ike_sa->get_ike_cfg(this->ike_sa);
DBG0(DBG_IKE, "%H is initiating an IKE_SA", message->get_source(message)); DBG0(DBG_IKE, "%H is initiating an IKE_SA", message->get_source(message));
this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING); this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
@@ -770,12 +813,14 @@ METHOD(task_t, build_r, status_t,
*/ */
static void raise_alerts(private_ike_init_t *this, notify_type_t type) static void raise_alerts(private_ike_init_t *this, notify_type_t type)
{ {
ike_cfg_t *ike_cfg;
linked_list_t *list; linked_list_t *list;
switch (type) switch (type)
{ {
case NO_PROPOSAL_CHOSEN: case NO_PROPOSAL_CHOSEN:
list = this->config->get_proposals(this->config); ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
list = ike_cfg->get_proposals(ike_cfg);
charon->bus->alert(charon->bus, ALERT_PROPOSAL_MISMATCH_IKE, list); charon->bus->alert(charon->bus, ALERT_PROPOSAL_MISMATCH_IKE, list);
list->destroy_offset(list, offsetof(proposal_t, destroy)); list->destroy_offset(list, offsetof(proposal_t, destroy));
break; break;