From: Tobias Brunner <tobias@strongswan.org>
Date: Wed, 13 Jul 2016 10:37:29 +0000 (+0200)
Subject: proposal: Make DH groups mandatory in IKE proposals parsed from strings
X-Git-Tag: 5.5.1rc1~8^2~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9b191d597576b8f3395df28d32cd5ca4f643cd1b;p=thirdparty%2Fstrongswan.git

proposal: Make DH groups mandatory in IKE proposals parsed from strings

References #2051.
---

diff --git a/src/libcharon/config/proposal.c b/src/libcharon/config/proposal.c
index ddbc155c30..9a50365b84 100644
--- a/src/libcharon/config/proposal.c
+++ b/src/libcharon/config/proposal.c
@@ -422,7 +422,7 @@ static const struct {
 /**
  * Checks the proposal read from a string.
  */
-static void check_proposal(private_proposal_t *this)
+static bool check_proposal(private_proposal_t *this)
 {
 	enumerator_t *e;
 	entry_t *entry;
@@ -463,6 +463,14 @@ static void check_proposal(private_proposal_t *this)
 			}
 		}
 		e->destroy(e);
+		e = create_enumerator(this, DIFFIE_HELLMAN_GROUP);
+		if (!e->enumerate(e, &alg, &ks))
+		{
+			DBG1(DBG_CFG, "a DH group is mandatory in IKE proposals");
+			e->destroy(e);
+			return FALSE;
+		}
+		e->destroy(e);
 	}
 
 	if (this->protocol == PROTO_ESP)
@@ -505,6 +513,7 @@ static void check_proposal(private_proposal_t *this)
 	}
 
 	array_compress(this->transforms);
+	return TRUE;
 }
 
 /**
@@ -1000,13 +1009,11 @@ proposal_t *proposal_create_from_string(protocol_id_t protocol, const char *algs
 	}
 	enumerator->destroy(enumerator);
 
-	if (failed)
+	if (failed || !check_proposal(this))
 	{
 		destroy(this);
 		return NULL;
 	}
 
-	check_proposal(this);
-
 	return &this->public;
 }
diff --git a/src/libcharon/tests/suites/test_proposal.c b/src/libcharon/tests/suites/test_proposal.c
index a6226f68fe..f5bf9440ad 100644
--- a/src/libcharon/tests/suites/test_proposal.c
+++ b/src/libcharon/tests/suites/test_proposal.c
@@ -18,38 +18,50 @@
 #include <config/proposal.h>
 
 static struct {
+	protocol_id_t proto;
 	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" },
+	{ PROTO_ESP, "aes128", "aes128", "aes128" },
+	{ PROTO_ESP, "aes128", "aes256", NULL },
+	{ PROTO_ESP, "aes128-aes256", "aes256-aes128", "aes128" },
+	{ PROTO_ESP, "aes256-aes128", "aes128-aes256", "aes256" },
+	{ PROTO_ESP, "aes128-aes256-sha1-sha256", "aes256-aes128-sha256-sha1", "aes128-sha1" },
+	{ PROTO_ESP, "aes256-aes128-sha256-sha1", "aes128-aes256-sha1-sha256", "aes256-sha256" },
+	{ PROTO_ESP, "aes128-sha256-modp3072", "aes128-sha256", NULL },
+	{ PROTO_ESP, "aes128-sha256", "aes128-sha256-modp3072", NULL },
+	{ PROTO_ESP, "aes128-sha256-modp3072", "aes128-sha256-modpnone", NULL },
+	{ PROTO_ESP, "aes128-sha256-modpnone", "aes128-sha256-modp3072", NULL },
+	{ PROTO_ESP, "aes128-sha256-modp3072-modpnone", "aes128-sha256", "aes128-sha256" },
+	{ PROTO_ESP, "aes128-sha256", "aes128-sha256-modp3072-modpnone", "aes128-sha256" },
+	{ PROTO_ESP, "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072" },
+	{ PROTO_ESP, "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone" },
+	{ PROTO_IKE, "aes128", NULL, NULL },
+	{ PROTO_IKE, "aes128-sha256", NULL, NULL },
+	{ PROTO_IKE, "aes128-sha256-modpnone", NULL, NULL },
+	{ PROTO_IKE, "aes128-sha256-modp3072", "aes128-sha256-modp3072", "aes128-sha256-modp3072" },
+	{ PROTO_IKE, "aes128-sha256-modp3072", "aes128-sha256-modp3072-modpnone", "aes128-sha256-modp3072" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-modpnone", "aes128-sha256-modp3072", "aes128-sha256-modp3072" },
 };
 
 START_TEST(test_select)
 {
 	proposal_t *self, *other, *selected, *expected;
 
-	self = proposal_create_from_string(PROTO_ESP,
+	self = proposal_create_from_string(select_data[_i].proto,
 									   select_data[_i].self);
-	other = proposal_create_from_string(PROTO_ESP,
+	if (!select_data[_i].other)
+	{
+		ck_assert(!self);
+		return;
+	}
+	other = proposal_create_from_string(select_data[_i].proto,
 										select_data[_i].other);
 	selected = self->select(self, other, FALSE);
 	if (select_data[_i].expected)
 	{
-		expected = proposal_create_from_string(PROTO_ESP,
+		expected = proposal_create_from_string(select_data[_i].proto,
 											   select_data[_i].expected);
 		ck_assert(selected);
 		ck_assert_msg(expected->equals(expected, selected), "proposal %P does "