From 18242f62c27bf829fd8ea8bbfc08e517fa9a2965 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 10:26:36 +0100 Subject: [PATCH 1/9] ike-init: Move creation of DH instance after INVALID_KE_PAYLOAD to build_i() This way we get proper error handling if the DH group the peer requested is not actually supported for some reason (otherwise we'd just retry to initiate with the configured group and get back another notify). --- src/libcharon/sa/ikev2/tasks/ike_init.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index d75d21715..635c32c0d 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -544,6 +544,18 @@ METHOD(task_t, build_i, status_t, return FAILED; } } + else if (this->dh->get_dh_group(this->dh) != this->dh_group) + { /* reset DH instance if group changed (INVALID_KE_PAYLOAD) */ + this->dh->destroy(this->dh); + this->dh = this->keymat->keymat.create_dh(&this->keymat->keymat, + this->dh_group); + if (!this->dh) + { + DBG1(DBG_IKE, "requested DH group %N not supported", + diffie_hellman_group_names, this->dh_group); + return FAILED; + } + } /* generate nonce only when we are trying the first time */ if (this->my_nonce.ptr == NULL) @@ -929,12 +941,6 @@ METHOD(task_t, migrate, void, this->keymat = (keymat_v2_t*)ike_sa->get_keymat(ike_sa); this->proposal = NULL; this->dh_failed = FALSE; - if (this->dh && this->dh->get_dh_group(this->dh) != this->dh_group) - { /* reset DH value only if group changed (INVALID_KE_PAYLOAD) */ - this->dh->destroy(this->dh); - this->dh = this->keymat->keymat.create_dh(&this->keymat->keymat, - this->dh_group); - } } METHOD(task_t, destroy, void, From 727615ee05f063d16a0319db32cd82567edf0cea Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 10:29:35 +0100 Subject: [PATCH 2/9] ike-init: Reuse the DH group of the previous IKE_SA when rekeying --- src/libcharon/sa/ikev2/tasks/ike_init.c | 27 +++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 635c32c0d..dae9a4dc7 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -1,8 +1,8 @@ /* - * Copyright (C) 2008-2015 Tobias Brunner + * Copyright (C) 2008-2018 Tobias Brunner * Copyright (C) 2005-2008 Martin Willi * Copyright (C) 2005 Jan Hutter - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -531,10 +531,29 @@ METHOD(task_t, build_i, status_t, return FAILED; } - /* if the DH group is set via use_dh_group(), we already have a DH object */ + /* if we are retrying after an INVALID_KE_PAYLOAD we already have one */ if (!this->dh) { - this->dh_group = this->config->get_dh_group(this->config); + if (this->old_sa) + { /* reuse the DH group we used for the old IKE_SA when rekeying */ + proposal_t *proposal; + uint16_t dh_group; + + proposal = this->old_sa->get_proposal(this->old_sa); + if (proposal->get_algorithm(proposal, DIFFIE_HELLMAN_GROUP, + &dh_group, NULL)) + { + this->dh_group = dh_group; + } + else + { /* this shouldn't happen, but let's be safe */ + this->dh_group = this->config->get_dh_group(this->config); + } + } + else + { + this->dh_group = this->config->get_dh_group(this->config); + } this->dh = this->keymat->keymat.create_dh(&this->keymat->keymat, this->dh_group); if (!this->dh) From f8e53003aa9f613c920ca7f42a36eeb988ad1852 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 10:43:17 +0100 Subject: [PATCH 3/9] child-create: Add an option to set the DH group to be used --- src/libcharon/sa/ikev2/tasks/child_create.c | 13 ++++++++++--- src/libcharon/sa/ikev2/tasks/child_create.h | 12 +++++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 4d4d72e0b..17c55736a 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2017 Tobias Brunner + * Copyright (C) 2008-2018 Tobias Brunner * Copyright (C) 2005-2008 Martin Willi * Copyright (C) 2005 Jan Hutter * HSR Hochschule fuer Technik Rapperswil @@ -1006,8 +1006,8 @@ METHOD(task_t, build_i, status_t, chunk_empty); return SUCCESS; } - if (!this->retry) - { + if (!this->retry && this->dh_group == MODP_NONE) + { /* during a rekeying the group might already be set */ this->dh_group = this->config->get_dh_group(this->config); } break; @@ -1615,6 +1615,12 @@ METHOD(child_create_t, use_marks, void, this->mark_out = out; } +METHOD(child_create_t, use_dh_group, void, + private_child_create_t *this, diffie_hellman_group_t dh_group) +{ + this->dh_group = dh_group; +} + METHOD(child_create_t, get_child, child_sa_t*, private_child_create_t *this) { @@ -1736,6 +1742,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa, .get_lower_nonce = _get_lower_nonce, .use_reqid = _use_reqid, .use_marks = _use_marks, + .use_dh_group = _use_dh_group, .task = { .get_type = _get_type, .migrate = _migrate, diff --git a/src/libcharon/sa/ikev2/tasks/child_create.h b/src/libcharon/sa/ikev2/tasks/child_create.h index f48d7b0a9..59fc6d2d9 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.h +++ b/src/libcharon/sa/ikev2/tasks/child_create.h @@ -1,6 +1,7 @@ /* + * Copyright (C) 2018 Tobias Brunner * Copyright (C) 2007 Martin Willi - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -59,6 +60,15 @@ struct child_create_t { */ void (*use_marks)(child_create_t *this, u_int in, u_int out); + /** + * Initially propose a specific DH group to override configuration. + * + * This is used during rekeying to prefer the previously negotiated group. + * + * @param dh_group DH group to use + */ + void (*use_dh_group)(child_create_t *this, diffie_hellman_group_t dh_group); + /** * Get the lower of the two nonces, used for rekey collisions. * From 27b0bd91d486c0438c5b02de320f6dc313612421 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 10:48:21 +0100 Subject: [PATCH 4/9] child-rekey: Use previously negotiated DH group when rekeying CHILD_SAs For the CHILD_SA created with the IKE_SA the group won't be set in the proposal, so we will use the first one configure just as if the SA was created new with a CREATE_CHILD_SA exchange. I guess we could theoretically try to use the DH group negotiated for IKE but then this would get a lot more complicated as we'd have to check if that group is actually contained in any of the CHILD_SA's configured proposals. --- src/libcharon/sa/ikev2/tasks/child_rekey.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index b67e9b80f..f90056658 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2017 Tobias Brunner + * Copyright (C) 2009-2018 Tobias Brunner * Copyright (C) 2005-2007 Martin Willi * Copyright (C) 2005 Jan Hutter * HSR Hochschule fuer Technik Rapperswil @@ -190,8 +190,18 @@ METHOD(task_t, build_i, status_t, /* our CHILD_CREATE task does the hard work for us */ if (!this->child_create) { + proposal_t *proposal; + uint16_t dh_group; + this->child_create = child_create_create(this->ike_sa, config->get_ref(config), TRUE, NULL, NULL); + + proposal = this->child_sa->get_proposal(this->child_sa); + if (proposal->get_algorithm(proposal, DIFFIE_HELLMAN_GROUP, + &dh_group, NULL)) + { /* reuse the DH group negotiated previously */ + this->child_create->use_dh_group(this->child_create, dh_group); + } } reqid = this->child_sa->get_reqid(this->child_sa); this->child_create->use_reqid(this->child_create, reqid); From 576d9b907c4cde0d4b11daf382c6ea625b45d619 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 10:59:25 +0100 Subject: [PATCH 5/9] ike-init: Make DH group reuse optional to test INVALID_KE_PAYLOAD handling This is currently not an issue for CHILD_SA rekeying tests as these only check rekeyings of the CHILD_SA created with the IKE_SA, i.e. there is no previous DH group to reuse. --- src/libcharon/sa/ikev2/tasks/ike_init.c | 3 ++- src/libcharon/tests/suites/test_ike_rekey.c | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index dae9a4dc7..10225df74 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -534,7 +534,8 @@ METHOD(task_t, build_i, status_t, /* if we are retrying after an INVALID_KE_PAYLOAD we already have one */ if (!this->dh) { - if (this->old_sa) + if (this->old_sa && lib->settings->get_bool(lib->settings, + "%s.prefer_previous_dh_group", TRUE, lib->ns)) { /* reuse the DH group we used for the old IKE_SA when rekeying */ proposal_t *proposal; uint16_t dh_group; diff --git a/src/libcharon/tests/suites/test_ike_rekey.c b/src/libcharon/tests/suites/test_ike_rekey.c index ba39657a4..e22a0c288 100644 --- a/src/libcharon/tests/suites/test_ike_rekey.c +++ b/src/libcharon/tests/suites/test_ike_rekey.c @@ -138,6 +138,8 @@ START_TEST(test_regular_ke_invalid) lib->settings->set_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns); + lib->settings->set_bool(lib->settings, "%s.prefer_previous_dh_group", + FALSE, lib->ns); initiate_rekey(a); @@ -382,6 +384,8 @@ START_TEST(test_collision_ke_invalid) lib->settings->set_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns); + lib->settings->set_bool(lib->settings, "%s.prefer_previous_dh_group", + FALSE, lib->ns); /* Six nonces and SPIs are needed (SPI 1 and 2 are used for the initial * IKE_SA): @@ -591,6 +595,8 @@ START_TEST(test_collision_ke_invalid_delayed_retry) lib->settings->set_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns); + lib->settings->set_bool(lib->settings, "%s.prefer_previous_dh_group", + FALSE, lib->ns); /* Five nonces and SPIs are needed (SPI 1 and 2 are used for the initial * IKE_SA): From 97ad041e6e0c4296f711b14ad53ea12fc2a80039 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 11:11:38 +0100 Subject: [PATCH 6/9] unit-tests: Make sure we reuse the DH group during CHILD_SA rekeying --- src/libcharon/tests/suites/test_child_rekey.c | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/libcharon/tests/suites/test_child_rekey.c b/src/libcharon/tests/suites/test_child_rekey.c index ac169723f..44d004ab7 100644 --- a/src/libcharon/tests/suites/test_child_rekey.c +++ b/src/libcharon/tests/suites/test_child_rekey.c @@ -231,6 +231,61 @@ START_TEST(test_regular_ke_invalid) /* child_updown */ assert_hook(); + /* because the DH group should get reused another rekeying should complete + * without additional exchange */ + initiate_rekey(a, 5); + /* this should never get called as this results in a successful rekeying */ + assert_hook_not_called(child_updown); + + /* CREATE_CHILD_SA { N(REKEY_SA), SA, Ni, [KEi,] TSi, TSr } --> */ + assert_hook_called(child_rekey); + assert_notify(IN, REKEY_SA); + exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(b, 6, CHILD_REKEYED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_REGISTERED); + assert_ipsec_sas_installed(b, 5, 6, 8); + assert_hook(); + + /* <-- CREATE_CHILD_SA { SA, Nr, [KEr,] TSi, TSr } */ + assert_hook_called(child_rekey); + assert_no_notify(IN, REKEY_SA); + exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 5, CHILD_DELETING, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_state(a, 7, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_ipsec_sas_installed(a, 5, 6, 7, 8); + assert_hook(); + + /* INFORMATIONAL { D } --> */ + assert_hook_not_called(child_rekey); + assert_single_payload(IN, PLV2_DELETE); + exchange_test_helper->process_message(exchange_test_helper, b, NULL); + assert_child_sa_state(b, 6, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(b, 8, CHILD_INSTALLED, CHILD_OUTBOUND_INSTALLED); + assert_child_sa_count(b, 2); + assert_ipsec_sas_installed(b, 6, 7, 8); + assert_hook(); + + /* <-- INFORMATIONAL { D } */ + assert_hook_not_called(child_rekey); + assert_single_payload(IN, PLV2_DELETE); + exchange_test_helper->process_message(exchange_test_helper, a, NULL); + assert_child_sa_state(a, 5, CHILD_DELETING, CHILD_OUTBOUND_NONE); + assert_child_sa_state(a, 7, CHILD_INSTALLED); + assert_child_sa_count(a, 2); + assert_ipsec_sas_installed(a, 5, 7, 8); + assert_hook(); + + /* simulate the execution of the scheduled jobs */ + destroy_rekeyed(a, 5); + assert_child_sa_count(a, 1); + assert_ipsec_sas_installed(a, 7, 8); + destroy_rekeyed(b, 6); + assert_child_sa_count(b, 1); + assert_ipsec_sas_installed(b, 7, 8); + + /* child_updown */ + assert_hook(); + call_ikesa(a, destroy); call_ikesa(b, destroy); } From d9c9b7b832cd5978477de0f6f15e0924e2d0927d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 11:32:39 +0100 Subject: [PATCH 7/9] proposal: Add method to move a given DH group to the front This way a responder (like strongSwan) selecting a proposal first and then checking if the KE payload matches sees the peer's preferred group first. --- src/libstrongswan/crypto/proposal/proposal.c | 35 +++++++++- src/libstrongswan/crypto/proposal/proposal.h | 13 +++- .../tests/suites/test_proposal.c | 65 ++++++++++++++++--- 3 files changed, 101 insertions(+), 12 deletions(-) diff --git a/src/libstrongswan/crypto/proposal/proposal.c b/src/libstrongswan/crypto/proposal/proposal.c index 221375f7b..7d857bff0 100644 --- a/src/libstrongswan/crypto/proposal/proposal.c +++ b/src/libstrongswan/crypto/proposal/proposal.c @@ -1,8 +1,8 @@ /* - * Copyright (C) 2008-2016 Tobias Brunner + * Copyright (C) 2008-2018 Tobias Brunner * Copyright (C) 2006-2010 Martin Willi * Copyright (C) 2013-2015 Andreas Steffen - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -171,6 +171,36 @@ METHOD(proposal_t, has_dh_group, bool, return found; } +METHOD(proposal_t, promote_dh_group, bool, + private_proposal_t *this, diffie_hellman_group_t group) +{ + enumerator_t *enumerator; + entry_t *entry; + bool found = FALSE; + + enumerator = array_create_enumerator(this->transforms); + while (enumerator->enumerate(enumerator, &entry)) + { + if (entry->type == DIFFIE_HELLMAN_GROUP && + entry->alg == group) + { + array_remove_at(this->transforms, enumerator); + found = TRUE; + } + } + enumerator->destroy(enumerator); + + if (found) + { + entry_t entry = { + .type = DIFFIE_HELLMAN_GROUP, + .alg = group, + }; + array_insert(this->transforms, ARRAY_HEAD, &entry); + } + return found; +} + METHOD(proposal_t, strip_dh, void, private_proposal_t *this, diffie_hellman_group_t keep) { @@ -716,6 +746,7 @@ proposal_t *proposal_create(protocol_id_t protocol, u_int number) .create_enumerator = _create_enumerator, .get_algorithm = _get_algorithm, .has_dh_group = _has_dh_group, + .promote_dh_group = _promote_dh_group, .strip_dh = _strip_dh, .select = _select_proposal, .get_protocol = _get_protocol, diff --git a/src/libstrongswan/crypto/proposal/proposal.h b/src/libstrongswan/crypto/proposal/proposal.h index d9a2af79f..0052674b9 100644 --- a/src/libstrongswan/crypto/proposal/proposal.h +++ b/src/libstrongswan/crypto/proposal/proposal.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2016 Tobias Brunner + * Copyright (C) 2009-2018 Tobias Brunner * Copyright (C) 2006 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -108,7 +108,16 @@ struct proposal_t { * @param group group to check for * @return TRUE if algorithm included */ - bool (*has_dh_group) (proposal_t *this, diffie_hellman_group_t group); + bool (*has_dh_group)(proposal_t *this, diffie_hellman_group_t group); + + /** + * Move the given DH group to the front of the list if it was contained in + * the proposal. + * + * @param group group to promote + * @return TRUE if algorithm included + */ + bool (*promote_dh_group)(proposal_t *this, diffie_hellman_group_t group); /** * Strip DH groups from proposal to use it without PFS. diff --git a/src/libstrongswan/tests/suites/test_proposal.c b/src/libstrongswan/tests/suites/test_proposal.c index 91766266d..1a2f97d5f 100644 --- a/src/libstrongswan/tests/suites/test_proposal.c +++ b/src/libstrongswan/tests/suites/test_proposal.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Tobias Brunner + * Copyright (C) 2016-2018 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -57,21 +57,27 @@ static struct { { PROTO_AH, "sha256-esn-noesn", "AH:HMAC_SHA2_256_128/EXT_SEQ/NO_EXT_SEQ" }, }; -START_TEST(test_create_from_string) +static void assert_proposal_eq(proposal_t *proposal, char *expected) { - proposal_t *proposal; char str[BUF_LEN]; - proposal = proposal_create_from_string(create_data[_i].proto, - create_data[_i].proposal); - if (!create_data[_i].expected) + if (!expected) { ck_assert(!proposal); return; } snprintf(str, sizeof(str), "%P", proposal); - ck_assert_str_eq(create_data[_i].expected, str); - proposal->destroy(proposal); + ck_assert_str_eq(expected, str); +} + +START_TEST(test_create_from_string) +{ + proposal_t *proposal; + + proposal = proposal_create_from_string(create_data[_i].proto, + create_data[_i].proposal); + assert_proposal_eq(proposal, create_data[_i].expected); + DESTROY_IF(proposal); } END_TEST @@ -151,6 +157,43 @@ START_TEST(test_select_spi) } END_TEST +START_TEST(test_promote_dh_group) +{ + proposal_t *proposal; + + proposal = proposal_create_from_string(PROTO_IKE, + "aes128-sha256-modp3072-ecp256"); + ck_assert(proposal->promote_dh_group(proposal, ECP_256_BIT)); + assert_proposal_eq(proposal, "IKE:AES_CBC_128/HMAC_SHA2_256_128/PRF_HMAC_SHA2_256/ECP_256/MODP_3072"); + proposal->destroy(proposal); +} +END_TEST + +START_TEST(test_promote_dh_group_already_front) +{ + proposal_t *proposal; + + proposal = proposal_create_from_string(PROTO_IKE, + "aes128-sha256-modp3072-ecp256"); + ck_assert(proposal->promote_dh_group(proposal, MODP_3072_BIT)); + assert_proposal_eq(proposal, "IKE:AES_CBC_128/HMAC_SHA2_256_128/PRF_HMAC_SHA2_256/MODP_3072/ECP_256"); + proposal->destroy(proposal); +} +END_TEST + +START_TEST(test_promote_dh_group_not_contained) +{ + proposal_t *proposal; + + proposal = proposal_create_from_string(PROTO_IKE, + "aes128-sha256-modp3072-ecp256"); + + ck_assert(!proposal->promote_dh_group(proposal, MODP_2048_BIT)); + assert_proposal_eq(proposal, "IKE:AES_CBC_128/HMAC_SHA2_256_128/PRF_HMAC_SHA2_256/MODP_3072/ECP_256"); + proposal->destroy(proposal); +} +END_TEST + Suite *proposal_suite_create() { Suite *s; @@ -167,5 +210,11 @@ Suite *proposal_suite_create() tcase_add_test(tc, test_select_spi); suite_add_tcase(s, tc); + tc = tcase_create("promote_dh_group"); + tcase_add_test(tc, test_promote_dh_group); + tcase_add_test(tc, test_promote_dh_group_already_front); + tcase_add_test(tc, test_promote_dh_group_not_contained); + suite_add_tcase(s, tc); + return s; } From ff79020cd216848f588ec741c8a240cb39dcf146 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 11:49:18 +0100 Subject: [PATCH 8/9] ike-init: Promote selected DH group and demote proposals that don't contain it --- src/libcharon/sa/ikev2/tasks/ike_init.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 10225df74..3d73d728b 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -282,7 +282,7 @@ static bool build_payloads(private_ike_init_t *this, message_t *message) sa_payload_t *sa_payload; ke_payload_t *ke_payload; nonce_payload_t *nonce_payload; - linked_list_t *proposal_list; + linked_list_t *proposal_list, *other_dh_groups; ike_sa_id_t *id; proposal_t *proposal; enumerator_t *enumerator; @@ -294,16 +294,31 @@ static bool build_payloads(private_ike_init_t *this, message_t *message) if (this->initiator) { proposal_list = this->config->get_proposals(this->config); - if (this->old_sa) + other_dh_groups = linked_list_create(); + enumerator = proposal_list->create_enumerator(proposal_list); + while (enumerator->enumerate(enumerator, (void**)&proposal)) { /* include SPI of new IKE_SA when we are rekeying */ - enumerator = proposal_list->create_enumerator(proposal_list); - while (enumerator->enumerate(enumerator, (void**)&proposal)) + if (this->old_sa) { proposal->set_spi(proposal, id->get_initiator_spi(id)); } - enumerator->destroy(enumerator); + /* move the selected DH group to the front of the proposal */ + if (!proposal->promote_dh_group(proposal, this->dh_group)) + { /* the proposal does not include the group, move to the back */ + proposal_list->remove_at(proposal_list, enumerator); + other_dh_groups->insert_last(other_dh_groups, proposal); + } } + enumerator->destroy(enumerator); + /* add proposals that don't contain the selected group */ + enumerator = other_dh_groups->create_enumerator(other_dh_groups); + while (enumerator->enumerate(enumerator, (void**)&proposal)) + { /* no need to remove from the list as we destroy it anyway*/ + proposal_list->insert_last(proposal_list, proposal); + } + enumerator->destroy(enumerator); + other_dh_groups->destroy(other_dh_groups); sa_payload = sa_payload_create_from_proposals_v2(proposal_list); proposal_list->destroy_offset(proposal_list, offsetof(proposal_t, destroy)); From 5a259ade4ecd3c3caa6c0c91f83ef1745d19d255 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 2 Feb 2018 12:04:18 +0100 Subject: [PATCH 9/9] child-create: Promote selected DH group, demote proposals that don't contain it --- src/libcharon/sa/ikev2/tasks/child_create.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 17c55736a..5161ca1ca 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -277,12 +277,13 @@ static bool ts_list_is_host(linked_list_t *list, host_t *host) } /** - * Allocate SPIs and update proposals + * Allocate SPIs and update proposals, we also promote the selected DH group */ static bool allocate_spi(private_child_create_t *this) { enumerator_t *enumerator; proposal_t *proposal; + linked_list_t *other_dh_groups; if (this->initiator) { @@ -304,12 +305,29 @@ static bool allocate_spi(private_child_create_t *this) { if (this->initiator) { + other_dh_groups = linked_list_create(); enumerator = this->proposals->create_enumerator(this->proposals); while (enumerator->enumerate(enumerator, &proposal)) { proposal->set_spi(proposal, this->my_spi); + + /* move the selected DH group to the front, if any */ + if (this->dh_group != MODP_NONE && + !proposal->promote_dh_group(proposal, this->dh_group)) + { /* proposals that don't contain the selected group are + * moved to the back */ + this->proposals->remove_at(this->proposals, enumerator); + other_dh_groups->insert_last(other_dh_groups, proposal); + } } enumerator->destroy(enumerator); + enumerator = other_dh_groups->create_enumerator(other_dh_groups); + while (enumerator->enumerate(enumerator, (void**)&proposal)) + { /* no need to remove from the list as we destroy it anyway*/ + this->proposals->insert_last(this->proposals, proposal); + } + enumerator->destroy(enumerator); + other_dh_groups->destroy(other_dh_groups); } else {