proposal: Handle MODP_NONE in both directions when selecting proposals

src/libcharon/config/proposal.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2014 Tobias Brunner
+ * Copyright (C) 2008-2016 Tobias Brunner
  * Copyright (C) 2006-2010 Martin Willi
  * Copyright (C) 2013-2015 Andreas Steffen
  * Hochschule fuer Technik Rapperswil
@@ -210,7 +210,7 @@ static bool select_algo(private_proposal_t *this, proposal_t *other,
 
 	e1 = create_enumerator(this, type);
 	e2 = other->create_enumerator(other, type);
-	if (!e1->enumerate(e1, NULL, NULL))
+	if (!e1->enumerate(e1, &alg1, NULL))
 	{
 		if (!e2->enumerate(e2, &alg2, NULL))
 		{
@@ -219,12 +219,23 @@ static bool select_algo(private_proposal_t *this, proposal_t *other,
 		else if (optional)
 		{
 			do
-			{	/* if the other peer proposes NONE, we accept the proposal */
+			{	/* if NONE is proposed, we accept the proposal */
 				found = !alg2;
 			}
 			while (!found && e2->enumerate(e2, &alg2, NULL));
 		}
 	}
+	else if (!e2->enumerate(e2, NULL, NULL))
+	{
+		if (optional)
+		{
+			do
+			{	/* if NONE is proposed, we accept the proposal */
+				found = !alg1;
+			}
+			while (!found && e1->enumerate(e1, &alg1, NULL));
+		}
+	}
 
 	e1->destroy(e1);
 	e1 = create_enumerator(this, type);
@@ -244,7 +255,6 @@ static bool select_algo(private_proposal_t *this, proposal_t *other,
 						 "but peer implementation is unknown, skipped");
 					continue;
 				}
-				/* ok, we have an algorithm */
 				selected->add_algorithm(selected, type, alg1, ks1);
 				found = TRUE;
 				break;
@@ -288,9 +298,7 @@ METHOD(proposal_t, select_proposal, proposal_t*,
 	}
 
 	DBG2(DBG_CFG, "  proposal matches");
-
 	selected->set_spi(selected, other->get_spi(other));
-
 	return selected;
 }
 

src/libcharon/tests/Makefile.am

@@ -3,6 +3,7 @@ TESTS = libcharon_tests exchange_tests
 check_PROGRAMS = $(TESTS)
 
 libcharon_tests_SOURCES = \
+  suites/test_proposal.c \
   suites/test_ike_cfg.c \
   suites/test_mem_pool.c \
   suites/test_message_chapoly.c \

src/libcharon/tests/libcharon_tests.h

@@ -24,6 +24,7 @@
  * @ingroup libcharon-tests
  */
 
+TEST_SUITE(proposal_suite_create)
 TEST_SUITE(ike_cfg_suite_create)
 TEST_SUITE(mem_pool_suite_create)
 TEST_SUITE_DEPEND(message_chapoly_suite_create, AEAD, ENCR_CHACHA20_POLY1305, 32)

src/libcharon/tests/suites/test_proposal.c

@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2016 Tobias Brunner
+ * 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
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.  See <http://www.fsf.org/copyleft/gpl.txt>.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ */
+
+#include "test_suite.h"
+
+#include <config/proposal.h>
+
+static struct {
+	char *self;
+	char *other;
+	char *expected;
+} select_data[] = {
+	{ "aes128", "aes128", "aes128" },
+	{ "aes128", "aes256", NULL },
+	{ "aes128-aes256", "aes256-aes128", "aes128" },
+	{ "aes256-aes128", "aes128-aes256", "aes256" },
+	{ "aes128-aes256-sha1-sha256", "aes256-aes128-sha256-sha1", "aes128-sha1" },
+	{ "aes256-aes128-sha256-sha1", "aes128-aes256-sha1-sha256", "aes256-sha256" },
+	{ "aes128-sha256-modp3072", "aes128-sha256", NULL },
+	{ "aes128-sha256", "aes128-sha256-modp3072", NULL },
+	{ "aes128-sha256-modp3072", "aes128-sha256-modpnone", NULL },
+	{ "aes128-sha256-modpnone", "aes128-sha256-modp3072", NULL },
+	{ "aes128-sha256-modp3072-modpnone", "aes128-sha256", "aes128-sha256" },
+	{ "aes128-sha256", "aes128-sha256-modp3072-modpnone", "aes128-sha256" },
+	{ "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072" },
+	{ "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone" },
+};
+
+START_TEST(test_select)
+{
+	proposal_t *self, *other, *selected, *expected;
+
+	self = proposal_create_from_string(PROTO_ESP,
+									   select_data[_i].self);
+	other = proposal_create_from_string(PROTO_ESP,
+										select_data[_i].other);
+	selected = self->select(self, other, FALSE);
+	if (select_data[_i].expected)
+	{
+		expected = proposal_create_from_string(PROTO_ESP,
+											   select_data[_i].expected);
+		ck_assert(selected);
+		ck_assert_msg(expected->equals(expected, selected), "proposal %P does "
+					  "not match expected %P", selected, expected);
+		expected->destroy(expected);
+	}
+	else
+	{
+		ck_assert(!selected);
+	}
+	DESTROY_IF(selected);
+	other->destroy(other);
+	self->destroy(self);
+}
+END_TEST
+
+Suite *proposal_suite_create()
+{
+	Suite *s;
+	TCase *tc;
+
+	s = suite_create("proposal");
+
+	tc = tcase_create("select");
+	tcase_add_loop_test(tc, test_select, 0, countof(select_data));
+	suite_add_tcase(s, tc);
+
+	return s;
+}