From: Tobias Brunner Date: Thu, 5 Sep 2019 15:29:00 +0000 (+0200) Subject: proposal: Add selection flags to clone() method X-Git-Tag: 5.8.2dr2~19^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a2cb2c9cc8619ee487283ab4b20dcedf8a91a996;p=thirdparty%2Fstrongswan.git proposal: Add selection flags to clone() method This avoids having to call strip_dh() in child_cfg_t::get_proposals(). It also inverts the ALLOW_PRIVATE flag (i.e. makes it SKIP_PRIVATE) so nothing has to be supplied to clone complete proposals. --- diff --git a/src/libcharon/config/child_cfg.c b/src/libcharon/config/child_cfg.c index fcc0efc25a..e0aee8b6a5 100644 --- a/src/libcharon/config/child_cfg.c +++ b/src/libcharon/config/child_cfg.c @@ -209,16 +209,18 @@ METHOD(child_cfg_t, get_proposals, linked_list_t*, { enumerator_t *enumerator; proposal_t *current; + proposal_selection_flag_t flags = 0; linked_list_t *proposals = linked_list_create(); + if (strip_dh) + { + flags |= PROPOSAL_SKIP_DH; + } + enumerator = this->proposals->create_enumerator(this->proposals); while (enumerator->enumerate(enumerator, ¤t)) { - current = current->clone(current); - if (strip_dh) - { - current->strip_dh(current, MODP_NONE); - } + current = current->clone(current, flags); if (proposals->find_first(proposals, match_proposal, NULL, current)) { current->destroy(current); diff --git a/src/libcharon/config/ike_cfg.c b/src/libcharon/config/ike_cfg.c index c9971882df..79a344e450 100644 --- a/src/libcharon/config/ike_cfg.c +++ b/src/libcharon/config/ike_cfg.c @@ -310,7 +310,7 @@ METHOD(ike_cfg_t, get_proposals, linked_list_t*, enumerator = this->proposals->create_enumerator(this->proposals); while (enumerator->enumerate(enumerator, ¤t)) { - current = current->clone(current); + current = current->clone(current, 0); proposals->insert_last(proposals, current); } enumerator->destroy(enumerator); @@ -330,7 +330,7 @@ METHOD(ike_cfg_t, has_proposal, bool, while (enumerator->enumerate(enumerator, &proposal)) { if (proposal->matches(proposal, match, - private ? PROPOSAL_ALLOW_PRIVATE : 0)) + private ? 0 : PROPOSAL_SKIP_PRIVATE)) { enumerator->destroy(enumerator); return TRUE; diff --git a/src/libcharon/plugins/load_tester/load_tester_config.c b/src/libcharon/plugins/load_tester/load_tester_config.c index 6fb6673751..77c9630ca5 100644 --- a/src/libcharon/plugins/load_tester/load_tester_config.c +++ b/src/libcharon/plugins/load_tester/load_tester_config.c @@ -749,7 +749,7 @@ static peer_cfg_t* generate_config(private_load_tester_config_t *this, uint num) ike.local_port = charon->socket->get_port(charon->socket, FALSE); } ike_cfg = ike_cfg_create(&ike); - ike_cfg->add_proposal(ike_cfg, this->proposal->clone(this->proposal)); + ike_cfg->add_proposal(ike_cfg, this->proposal->clone(this->proposal, 0)); peer_cfg = peer_cfg_create("load-test", ike_cfg, &peer); if (this->vip) @@ -784,7 +784,7 @@ static peer_cfg_t* generate_config(private_load_tester_config_t *this, uint num) } child_cfg = child_cfg_create("load-test", &child); - child_cfg->add_proposal(child_cfg, this->esp->clone(this->esp)); + child_cfg->add_proposal(child_cfg, this->esp->clone(this->esp, 0)); if (num) { /* initiator */ diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index fc60b413f1..b11236b8f9 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -429,7 +429,7 @@ METHOD(child_sa_t, get_proposal, proposal_t*, METHOD(child_sa_t, set_proposal, void, private_child_sa_t *this, proposal_t *proposal) { - this->proposal = proposal->clone(proposal); + this->proposal = proposal->clone(proposal, 0); } METHOD(child_sa_t, create_ts_enumerator, enumerator_t*, diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c index 4e60ed8e23..d6cc4e5a75 100644 --- a/src/libcharon/sa/ike_sa.c +++ b/src/libcharon/sa/ike_sa.c @@ -610,7 +610,7 @@ METHOD(ike_sa_t, set_proposal, void, private_ike_sa_t *this, proposal_t *proposal) { DESTROY_IF(this->proposal); - this->proposal = proposal->clone(proposal); + this->proposal = proposal->clone(proposal, 0); } METHOD(ike_sa_t, set_message_id, void, diff --git a/src/libcharon/sa/ikev1/tasks/aggressive_mode.c b/src/libcharon/sa/ikev1/tasks/aggressive_mode.c index 6ad4a0f4b4..94c3b76821 100644 --- a/src/libcharon/sa/ikev1/tasks/aggressive_mode.c +++ b/src/libcharon/sa/ikev1/tasks/aggressive_mode.c @@ -374,7 +374,7 @@ METHOD(task_t, process_r, status_t, id_payload_t *id_payload; identification_t *id; linked_list_t *list; - proposal_selection_flag_t flags = 0; + proposal_selection_flag_t flags = PROPOSAL_SKIP_PRIVATE; uint16_t group; this->ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa); diff --git a/src/libcharon/sa/ikev1/tasks/main_mode.c b/src/libcharon/sa/ikev1/tasks/main_mode.c index 2678a1b4a3..43848ad1a4 100644 --- a/src/libcharon/sa/ikev1/tasks/main_mode.c +++ b/src/libcharon/sa/ikev1/tasks/main_mode.c @@ -386,9 +386,9 @@ METHOD(task_t, process_r, status_t, } list = sa_payload->get_proposals(sa_payload); - if (this->ike_sa->supports_extension(this->ike_sa , EXT_STRONGSWAN)) + if (!this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) { - flags |= PROPOSAL_ALLOW_PRIVATE; + flags |= PROPOSAL_SKIP_PRIVATE; } if (!lib->settings->get_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns)) @@ -640,9 +640,9 @@ METHOD(task_t, process_i, status_t, return send_notify(this, INVALID_PAYLOAD_TYPE); } list = sa_payload->get_proposals(sa_payload); - if (this->ike_sa->supports_extension(this->ike_sa , EXT_STRONGSWAN)) + if (!this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) { - flags |= PROPOSAL_ALLOW_PRIVATE; + flags |= PROPOSAL_SKIP_PRIVATE; } this->proposal = this->ike_cfg->select_proposal(this->ike_cfg, list, flags); diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 00c9edd711..9ded2dd539 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -1132,9 +1132,9 @@ METHOD(task_t, process_r, status_t, DESTROY_IF(list); list = sa_payload->get_proposals(sa_payload); } - if (this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) + if (!this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) { - flags |= PROPOSAL_ALLOW_PRIVATE; + flags |= PROPOSAL_SKIP_PRIVATE; } if (!lib->settings->get_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns)) @@ -1370,9 +1370,9 @@ METHOD(task_t, process_i, status_t, DESTROY_IF(list); list = sa_payload->get_proposals(sa_payload); } - if (this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) + if (!this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) { - flags |= PROPOSAL_ALLOW_PRIVATE; + flags |= PROPOSAL_SKIP_PRIVATE; } this->proposal = this->config->select_proposal(this->config, list, flags); diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 533cf82c0f..e98c1dbcc4 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -564,9 +564,9 @@ static status_t select_and_install(private_child_create_t *this, { flags |= PROPOSAL_SKIP_DH; } - if (this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) + if (!this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) { - flags |= PROPOSAL_ALLOW_PRIVATE; + flags |= PROPOSAL_SKIP_PRIVATE; } if (!lib->settings->get_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns)) diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 143ae7bcf5..d15b5b107a 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -458,9 +458,9 @@ static void process_sa_payload(private_ike_init_t *this, message_t *message, ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa); proposal_list = sa_payload->get_proposals(sa_payload); - if (this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) + if (!this->ike_sa->supports_extension(this->ike_sa, EXT_STRONGSWAN)) { - flags |= PROPOSAL_ALLOW_PRIVATE; + flags |= PROPOSAL_SKIP_PRIVATE; } if (!lib->settings->get_bool(lib->settings, "%s.prefer_configured_proposals", TRUE, lib->ns)) diff --git a/src/libstrongswan/crypto/proposal/proposal.c b/src/libstrongswan/crypto/proposal/proposal.c index c880020de8..3b05b3b530 100644 --- a/src/libstrongswan/crypto/proposal/proposal.c +++ b/src/libstrongswan/crypto/proposal/proposal.c @@ -390,7 +390,7 @@ static bool select_algo(private_proposal_t *this, proposal_t *other, { if (alg1 == alg2 && ks1 == ks2) { - if (!(flags & PROPOSAL_ALLOW_PRIVATE) && alg1 >= 1024) + if ((flags & PROPOSAL_SKIP_PRIVATE) && alg1 >= 1024) { if (log) { @@ -604,25 +604,27 @@ METHOD(proposal_t, equals, bool, } METHOD(proposal_t, clone_, proposal_t*, - private_proposal_t *this) + private_proposal_t *this, proposal_selection_flag_t flags) { private_proposal_t *clone; enumerator_t *enumerator; entry_t *entry; - transform_type_t *type; clone = (private_proposal_t*)proposal_create(this->protocol, 0); enumerator = array_create_enumerator(this->transforms); while (enumerator->enumerate(enumerator, &entry)) { + if (entry->alg >= 1024 && (flags & PROPOSAL_SKIP_PRIVATE)) + { + continue; + } + if (entry->type == DIFFIE_HELLMAN_GROUP && (flags & PROPOSAL_SKIP_DH)) + { + continue; + } array_insert(clone->transforms, ARRAY_TAIL, entry); - } - enumerator->destroy(enumerator); - enumerator = array_create_enumerator(this->types); - while (enumerator->enumerate(enumerator, &type)) - { - array_insert(clone->types, ARRAY_TAIL, type); + add_type(clone->types, entry->type); } enumerator->destroy(enumerator); diff --git a/src/libstrongswan/crypto/proposal/proposal.h b/src/libstrongswan/crypto/proposal/proposal.h index 8e952dd4f0..f05669e1c7 100644 --- a/src/libstrongswan/crypto/proposal/proposal.h +++ b/src/libstrongswan/crypto/proposal/proposal.h @@ -56,10 +56,10 @@ extern enum_name_t *protocol_id_names; * Flags for selecting proposals */ enum proposal_selection_flag_t { - /** Accept algorithms from a private range. */ - PROPOSAL_ALLOW_PRIVATE = (1<<0), /** Whether to prefer configured (default) or supplied proposals. */ - PROPOSAL_PREFER_SUPPLIED = (1<<1), + PROPOSAL_PREFER_SUPPLIED = (1<<0), + /** Whether to skip and ignore algorithms from a private range. */ + PROPOSAL_SKIP_PRIVATE = (1<<1), /** Whether to skip and ignore diffie hellman groups. */ PROPOSAL_SKIP_DH = (1<<2), }; @@ -207,9 +207,10 @@ struct proposal_t { /** * Clone a proposal. * + * @param flags flags to consider during cloning * @return clone of proposal */ - proposal_t *(*clone) (proposal_t *this); + proposal_t *(*clone)(proposal_t *this, proposal_selection_flag_t flags); /** * Destroys the proposal object. diff --git a/src/libstrongswan/tests/suites/test_proposal.c b/src/libstrongswan/tests/suites/test_proposal.c index 68116d92cd..c323119ea7 100644 --- a/src/libstrongswan/tests/suites/test_proposal.c +++ b/src/libstrongswan/tests/suites/test_proposal.c @@ -410,6 +410,44 @@ START_TEST(test_chacha20_poly1305_key_length) } END_TEST +static struct { + protocol_id_t proto; + char *orig; + char *expected; + proposal_selection_flag_t flags; +} clone_data[] = { + { PROTO_ESP, "aes128", "aes128" }, + { PROTO_ESP, "aes128-serpent", "aes128-serpent" }, + { PROTO_ESP, "aes128-serpent", "aes128", PROPOSAL_SKIP_PRIVATE }, + { PROTO_ESP, "aes128-sha256-modp3072", "aes128-sha256-modp3072" }, + { PROTO_ESP, "aes128-sha256-modp3072", "aes128-sha256", PROPOSAL_SKIP_DH }, + { PROTO_ESP, "aes128-serpent-modp3072", "aes128-serpent", + PROPOSAL_SKIP_DH }, + { PROTO_ESP, "aes128-serpent-modp3072", "aes128", + PROPOSAL_SKIP_PRIVATE | PROPOSAL_SKIP_DH }, +}; + +START_TEST(test_clone) +{ + proposal_t *orig, *result, *expected; + + orig = proposal_create_from_string(clone_data[_i].proto, + clone_data[_i].orig); + orig->set_spi(orig, 0x12345678); + + result = orig->clone(orig, clone_data[_i].flags); + + expected = proposal_create_from_string(clone_data[_i].proto, + clone_data[_i].expected); + ck_assert_msg(expected->equals(expected, result), "proposal %P does " + "not match expected %P", result, expected); + ck_assert_int_eq(orig->get_spi(orig), result->get_spi(result)); + + expected->destroy(expected); + result->destroy(result); + orig->destroy(orig); +} +END_TEST Suite *proposal_suite_create() { @@ -454,5 +492,9 @@ Suite *proposal_suite_create() tcase_add_test(tc, test_chacha20_poly1305_key_length); suite_add_tcase(s, tc); + tc = tcase_create("clone"); + tcase_add_loop_test(tc, test_clone, 0, countof(clone_data)); + suite_add_tcase(s, tc); + return s; }