From 22f13dcecdd7c788cf5cbf9533d2fb7a1f8317f5 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 15 Dec 2016 18:22:11 +0100 Subject: [PATCH] proposal: Copy SPI and proposal number from correct proposal in select() 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 | 2 +- src/libcharon/config/ike_cfg.c | 2 +- src/libcharon/config/proposal.c | 16 ++++++++++--- src/libcharon/config/proposal.h | 9 ++++++-- src/libcharon/tests/suites/test_proposal.c | 26 +++++++++++++++++++++- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/libcharon/config/child_cfg.c b/src/libcharon/config/child_cfg.c index 6a9c342f44..d35c20ec57 100644 --- a/src/libcharon/config/child_cfg.c +++ b/src/libcharon/config/child_cfg.c @@ -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); diff --git a/src/libcharon/config/ike_cfg.c b/src/libcharon/config/ike_cfg.c index 7d52ac88f7..8375680af8 100644 --- a/src/libcharon/config/ike_cfg.c +++ b/src/libcharon/config/ike_cfg.c @@ -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); diff --git a/src/libcharon/config/proposal.c b/src/libcharon/config/proposal.c index e1305ce910..a2dc113a5c 100644 --- a/src/libcharon/config/proposal.c +++ b/src/libcharon/config/proposal.c @@ -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; } diff --git a/src/libcharon/config/proposal.h b/src/libcharon/config/proposal.h index f9f277820b..2bdf3454fb 100644 --- a/src/libcharon/config/proposal.h +++ b/src/libcharon/config/proposal.h @@ -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. diff --git a/src/libcharon/tests/suites/test_proposal.c b/src/libcharon/tests/suites/test_proposal.c index 19f4cd1e19..f1591794a1 100644 --- a/src/libcharon/tests/suites/test_proposal.c +++ b/src/libcharon/tests/suites/test_proposal.c @@ -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; -- 2.47.2