From 054ee5e7c0c5f5334f9de1be041b29af3e4161aa Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 29 May 2018 17:04:12 +0200 Subject: [PATCH] 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. --- src/libcharon/sa/ike_sa.c | 1 + src/libcharon/sa/ikev2/tasks/ike_init.c | 107 +++++++++++++++++------- 2 files changed, 77 insertions(+), 31 deletions(-) diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c index f39fed6f06..a4ad866d3a 100644 --- a/src/libcharon/sa/ike_sa.c +++ b/src/libcharon/sa/ike_sa.c @@ -674,6 +674,7 @@ METHOD(ike_sa_t, get_ike_cfg, ike_cfg_t*, METHOD(ike_sa_t, set_ike_cfg, void, private_ike_sa_t *this, ike_cfg_t *ike_cfg) { + DESTROY_IF(this->ike_cfg); ike_cfg->get_ref(ike_cfg); this->ike_cfg = ike_cfg; } diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 3d73d728b7..09744a7004 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -54,11 +54,6 @@ struct private_ike_init_t { */ bool initiator; - /** - * IKE config to establish - */ - ike_cfg_t *config; - /** * 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; proposal_t *proposal; enumerator_t *enumerator; + ike_cfg_t *ike_cfg; 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) { - proposal_list = this->config->get_proposals(this->config); + proposal_list = ike_cfg->get_proposals(ike_cfg); other_dh_groups = linked_list_create(); enumerator = proposal_list->create_enumerator(proposal_list); 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 */ if (!this->old_sa && - this->config->fragmentation(this->config) != FRAGMENTATION_NO) + ike_cfg->fragmentation(ike_cfg) != FRAGMENTATION_NO) { if (this->initiator || 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; } +/** + * 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 */ @@ -419,24 +477,7 @@ static void process_payloads(private_ike_init_t *this, message_t *message) { case PLV2_SECURITY_ASSOCIATION: { - sa_payload_t *sa_payload = (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)); + process_sa_payload(this, message, (sa_payload_t*)payload); break; } 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, 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", this->ike_sa->get_name(this->ike_sa), this->ike_sa->get_unique_id(this->ike_sa), @@ -563,12 +607,12 @@ METHOD(task_t, build_i, status_t, } else { /* 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 { - 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_group); @@ -627,7 +671,6 @@ METHOD(task_t, build_i, status_t, METHOD(task_t, process_r, status_t, 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)); 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) { + ike_cfg_t *ike_cfg; linked_list_t *list; switch (type) { 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); list->destroy_offset(list, offsetof(proposal_t, destroy)); break; -- 2.47.2