proposal: Copy SPI and proposal number from correct proposal in select()
authorTobias Brunner <tobias@strongswan.org>
Thu, 15 Dec 2016 17:22:11 +0000 (18:22 +0100)
committerTobias Brunner <tobias@strongswan.org>
Mon, 6 Feb 2017 10:14:31 +0000 (11:14 +0100)
If charon.prefer_configured_proposals is disabled select() is called on
the received proposal. This incorrectly set the SPI to 0 as the
configured proposal has no SPI set.

Fixes #2190.

src/libcharon/config/child_cfg.c
src/libcharon/config/ike_cfg.c
src/libcharon/config/proposal.c
src/libcharon/config/proposal.h
src/libcharon/tests/suites/test_proposal.c

index 6a9c342..d35c20e 100644 (file)
@@ -249,7 +249,7 @@ METHOD(child_cfg_t, select_proposal, proposal_t*,
                        {
                                proposal->strip_dh(proposal, MODP_NONE);
                        }
-                       selected = proposal->select(proposal, match, private);
+                       selected = proposal->select(proposal, match, prefer_self, private);
                        if (selected)
                        {
                                DBG2(DBG_CFG, "received proposals: %#P", proposals);
index 7d52ac8..8375680 100644 (file)
@@ -339,7 +339,7 @@ METHOD(ike_cfg_t, select_proposal, proposal_t*,
                }
                while (match_enum->enumerate(match_enum, (void**)&match))
                {
-                       selected = proposal->select(proposal, match, private);
+                       selected = proposal->select(proposal, match, prefer_self, private);
                        if (selected)
                        {
                                DBG2(DBG_CFG, "received proposals: %#P", proposals);
index e1305ce..a2dc113 100644 (file)
@@ -273,7 +273,8 @@ static bool select_algo(private_proposal_t *this, proposal_t *other,
 }
 
 METHOD(proposal_t, select_proposal, proposal_t*,
-       private_proposal_t *this, proposal_t *other, bool private)
+       private_proposal_t *this, proposal_t *other, bool other_remote,
+       bool private)
 {
        proposal_t *selected;
 
@@ -285,7 +286,17 @@ METHOD(proposal_t, select_proposal, proposal_t*,
                return NULL;
        }
 
-       selected = proposal_create(this->protocol, other->get_number(other));
+       if (other_remote)
+       {
+               selected = proposal_create(this->protocol, other->get_number(other));
+               selected->set_spi(selected, other->get_spi(other));
+       }
+       else
+       {
+               selected = proposal_create(this->protocol, this->number);
+               selected->set_spi(selected, this->spi);
+
+       }
 
        if (!select_algo(this, other, selected, ENCRYPTION_ALGORITHM, private) ||
                !select_algo(this, other, selected, PSEUDO_RANDOM_FUNCTION, private) ||
@@ -298,7 +309,6 @@ METHOD(proposal_t, select_proposal, proposal_t*,
        }
 
        DBG2(DBG_CFG, "  proposal matches");
-       selected->set_spi(selected, other->get_spi(other));
        return selected;
 }
 
index f9f2778..2bdf345 100644 (file)
@@ -1,6 +1,7 @@
 /*
+ * Copyright (C) 2009-2016 Tobias Brunner
  * Copyright (C) 2006 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
@@ -124,10 +125,14 @@ struct proposal_t {
         * in common, a resulting proposal of this kind is created.
         *
         * @param other                 proposal to compare against
+        * @param other_remote  whether other is the remote proposal from which to
+        *                                              copy SPI and proposal number to the result,
+        *                                              otherwise copy from this proposal
         * @param private               accepts algorithms allocated in a private range
         * @return                              selected proposal, NULL if proposals don't match
         */
-       proposal_t *(*select) (proposal_t *this, proposal_t *other, bool private);
+       proposal_t *(*select)(proposal_t *this, proposal_t *other,
+                                                 bool other_remote, bool private);
 
        /**
         * Get the protocol ID of the proposal.
index 19f4cd1..f159179 100644 (file)
@@ -108,7 +108,7 @@ START_TEST(test_select)
                                                                           select_data[_i].self);
        other = proposal_create_from_string(select_data[_i].proto,
                                                                                select_data[_i].other);
-       selected = self->select(self, other, FALSE);
+       selected = self->select(self, other, TRUE, FALSE);
        if (select_data[_i].expected)
        {
                expected = proposal_create_from_string(select_data[_i].proto,
@@ -128,6 +128,29 @@ START_TEST(test_select)
 }
 END_TEST
 
+START_TEST(test_select_spi)
+{
+       proposal_t *self, *other, *selected;
+
+       self = proposal_create_from_string(PROTO_ESP, "aes128-sha256-modp3072");
+       other = proposal_create_from_string(PROTO_ESP, "aes128-sha256-modp3072");
+       other->set_spi(other, 0x12345678);
+
+       selected = self->select(self, other, TRUE, FALSE);
+       ck_assert(selected);
+       ck_assert_int_eq(selected->get_spi(selected), other->get_spi(other));
+       selected->destroy(selected);
+
+       selected = self->select(self, other, FALSE, FALSE);
+       ck_assert(selected);
+       ck_assert_int_eq(selected->get_spi(selected), self->get_spi(self));
+       selected->destroy(selected);
+
+       other->destroy(other);
+       self->destroy(self);
+}
+END_TEST
+
 Suite *proposal_suite_create()
 {
        Suite *s;
@@ -141,6 +164,7 @@ Suite *proposal_suite_create()
 
        tc = tcase_create("select");
        tcase_add_loop_test(tc, test_select, 0, countof(select_data));
+       tcase_add_test(tc, test_select_spi);
        suite_add_tcase(s, tc);
 
        return s;