proposal: Add selection flags to clone() method
authorTobias Brunner <tobias@strongswan.org>
Thu, 5 Sep 2019 15:29:00 +0000 (17:29 +0200)
committerTobias Brunner <tobias@strongswan.org>
Thu, 24 Oct 2019 15:43:21 +0000 (17:43 +0200)
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.

13 files changed:
src/libcharon/config/child_cfg.c
src/libcharon/config/ike_cfg.c
src/libcharon/plugins/load_tester/load_tester_config.c
src/libcharon/sa/child_sa.c
src/libcharon/sa/ike_sa.c
src/libcharon/sa/ikev1/tasks/aggressive_mode.c
src/libcharon/sa/ikev1/tasks/main_mode.c
src/libcharon/sa/ikev1/tasks/quick_mode.c
src/libcharon/sa/ikev2/tasks/child_create.c
src/libcharon/sa/ikev2/tasks/ike_init.c
src/libstrongswan/crypto/proposal/proposal.c
src/libstrongswan/crypto/proposal/proposal.h
src/libstrongswan/tests/suites/test_proposal.c

index fcc0efc..e0aee8b 100644 (file)
@@ -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, &current))
        {
-               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);
index c997188..79a344e 100644 (file)
@@ -310,7 +310,7 @@ METHOD(ike_cfg_t, get_proposals, linked_list_t*,
        enumerator = this->proposals->create_enumerator(this->proposals);
        while (enumerator->enumerate(enumerator, &current))
        {
-               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;
index 6fb6673..77c9630 100644 (file)
@@ -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 */
index fc60b41..b11236b 100644 (file)
@@ -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*,
index 4e60ed8..d6cc4e5 100644 (file)
@@ -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,
index 6ad4a0f..94c3b76 100644 (file)
@@ -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);
index 2678a1b..43848ad 100644 (file)
@@ -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);
index 00c9edd..9ded2dd 100644 (file)
@@ -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);
index 533cf82..e98c1db 100644 (file)
@@ -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))
index 143ae7b..d15b5b1 100644 (file)
@@ -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))
index c880020..3b05b3b 100644 (file)
@@ -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);
 
index 8e952dd..f05669e 100644 (file)
@@ -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.
index 68116d9..c323119 100644 (file)
@@ -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;
 }