From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 16 Jul 2021 11:52:19 +0000 (+0200)
Subject: proposal: Prevent selection of duplicate key exchange methods
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9ce65dd34103a46a1f1f0391f1e853d97c400503;p=thirdparty%2Fstrongswan.git

proposal: Prevent selection of duplicate key exchange methods

All additional (and the initial) key exchanges must use a different method.
---

diff --git a/src/libstrongswan/crypto/proposal/proposal.c b/src/libstrongswan/crypto/proposal/proposal.c
index 606aa4ca37..869f08f098 100644
--- a/src/libstrongswan/crypto/proposal/proposal.c
+++ b/src/libstrongswan/crypto/proposal/proposal.c
@@ -21,6 +21,7 @@
 #include "proposal.h"
 
 #include <collections/array.h>
+#include <collections/hashtable.h>
 #include <utils/identification.h>
 
 #include <crypto/transform.h>
@@ -316,7 +317,7 @@ METHOD(proposal_t, promote_transform, bool,
  */
 static bool select_algo(private_proposal_t *this, proposal_t *other,
 						transform_type_t type, proposal_selection_flag_t flags,
-						bool log, uint16_t *alg, uint16_t *ks)
+						hashtable_t *kes, bool log, uint16_t *alg, uint16_t *ks)
 {
 	enumerator_t *e1, *e2;
 	uint16_t alg1, alg2, ks1, ks2;
@@ -359,9 +360,13 @@ static bool select_algo(private_proposal_t *this, proposal_t *other,
 
 	e1->destroy(e1);
 	e1 = create_enumerator(this, type);
-	/* compare algs, order of algs in "first" is preferred */
+	/* compare algs, order of algs in "e1" is preferred */
 	while (!found && e1->enumerate(e1, &alg1, &ks1))
 	{
+		if (is_ke_transform(type) && kes->get(kes, (void*)(uintptr_t)alg1))
+		{
+			continue;
+		}
 		e2->destroy(e2);
 		e2 = other->create_enumerator(other, type);
 		while (e2->enumerate(e2, &alg2, &ks2))
@@ -390,6 +395,23 @@ static bool select_algo(private_proposal_t *this, proposal_t *other,
 	return found;
 }
 
+/**
+ * Hash an algorithm identifier
+ */
+static u_int hash_alg(const void *key)
+{
+	uint16_t alg = (uint16_t)(uintptr_t)key;
+	return chunk_hash(chunk_from_thing(alg));
+}
+
+/**
+ * Compare two algorithm identifiers
+ */
+static bool equals_alg(const void *key, const void *other_key)
+{
+	return (uint16_t)(uintptr_t)key == (uint16_t)(uintptr_t)other_key;
+}
+
 /**
  * Select algorithms from the given proposals, if selected is given, the result
  * is stored there and errors are logged.
@@ -398,10 +420,13 @@ static bool select_algos(private_proposal_t *this, proposal_t *other,
 						 proposal_t *selected, proposal_selection_flag_t flags)
 {
 	transform_type_t type;
+	hashtable_t *kes;
 	array_t *types;
 	bool skip_integrity = FALSE;
 	int i;
 
+	kes = hashtable_create(hash_alg, equals_alg, 8);
+
 	types = merge_types(this, (private_proposal_t*)other);
 	for (i = 0; i < array_count(types); i++)
 	{
@@ -416,7 +441,8 @@ static bool select_algos(private_proposal_t *this, proposal_t *other,
 		{
 			continue;
 		}
-		if (select_algo(this, other, type, flags, selected != NULL, &alg, &ks))
+		if (select_algo(this, other, type, flags, kes, selected != NULL,
+						&alg, &ks))
 		{
 			if (alg == 0 && type != EXTENDED_SEQUENCE_NUMBERS)
 			{	/* 0 is "valid" for extended sequence numbers, for other
@@ -427,6 +453,10 @@ static bool select_algos(private_proposal_t *this, proposal_t *other,
 			{
 				selected->add_algorithm(selected, type, alg, ks);
 			}
+			if (is_ke_transform(type))
+			{
+				kes->put(kes, (void*)(uintptr_t)alg, (void*)(uintptr_t)alg);
+			}
 			if (type == ENCRYPTION_ALGORITHM &&
 				encryption_algorithm_is_aead(alg))
 			{
@@ -442,10 +472,12 @@ static bool select_algos(private_proposal_t *this, proposal_t *other,
 					 type);
 			}
 			array_destroy(types);
+			kes->destroy(kes);
 			return FALSE;
 		}
 	}
 	array_destroy(types);
+	kes->destroy(kes);
 	return TRUE;
 }
 
diff --git a/src/libstrongswan/tests/suites/test_proposal.c b/src/libstrongswan/tests/suites/test_proposal.c
index 7798bfb24a..78724bb6f1 100644
--- a/src/libstrongswan/tests/suites/test_proposal.c
+++ b/src/libstrongswan/tests/suites/test_proposal.c
@@ -143,6 +143,28 @@ static struct {
 	{ PROTO_IKE, "aes128-sha256-modp3072", "aes128-sha256-modp3072", "aes128-sha256-modp3072" },
 	{ PROTO_IKE, "aes128-sha256-modp3072", "aes128-sha256-modp3072-none", "aes128-sha256-modp3072" },
 	{ PROTO_IKE, "aes128-sha256-modp3072-none", "aes128-sha256-modp3072", "aes128-sha256-modp3072" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ke1_modp3072",
+				 "aes128-sha256-modp3072-ke1_modp3072", NULL },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ecp256-ecp384-ke1_modp3072-ke1_ecp256-ke1_ecp384-ke2_modp3072-ke2_ecp256-ke2_ecp384",
+				 "aes128-sha256-modp3072-ecp256-ecp384-ke1_modp3072-ke1_ecp256-ke1_ecp384-ke2_modp3072-ke2_ecp256-ke2_ecp384",
+				 "aes128-sha256-modp3072-ke1_ecp256-ke2_ecp384" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ke1_modp3072-ke1_none",
+				 "aes128-sha256-modp3072-ke1_modp3072-ke1_none",
+				 "aes128-sha256-modp3072" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ke1_modp3072-ke1_none-ke2_modp3072-ke2_none",
+				 "aes128-sha256-modp3072-ke1_modp3072-ke1_none-ke2_modp3072-ke2_none",
+				 "aes128-sha256-modp3072" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ke1_modp3072-ke1_ecp256",
+				 "aes128-sha256-modp3072-ke1_modp3072-ke1_ecp256",
+				 "aes128-sha256-modp3072-ke1_ecp256" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ke1_modp3072-ke1_ecp256",
+				 "aes128-sha256-modp3072-ke1_ecp256",
+				 "aes128-sha256-modp3072-ke1_ecp256" },
+	{ PROTO_IKE, "aes128-sha256-modp3072-ke1_ecp256",
+				 "aes128-sha256-modp3072-ke1_modp3072-ke1_ecp256",
+				 "aes128-sha256-modp3072-ke1_ecp256" },
+	{ PROTO_IKE, "aes128-sha256-ecp256-ke1_modp3072",
+				 "aes128-sha256-modp3072-ecp256-ke1_ecp256-ke2_ecp384", NULL },
 };
 
 START_TEST(test_select)