strongswan/0001-restore-registration-algorithm-order.bug897512.patch

From 76ad8a6f4c83c999b9eb6d1a3506b1a8e593307e Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 20 Jun 2014 16:22:15 +0200
Subject: [PATCH] Merge branch 'algorithm-order'
Upstream: yes
References: bsc#897512

Restores the behavior we had before 2e22333fb (except for RNGs), that is,
algorithms are stored in the registration order again.  Which is not optimal
as we must rely on plugins to register them in a sensible order, but ordering
them by identifier definitely caused weaker algorithms to be proposed first
in the default proposal, which was even worse.
---
 src/libstrongswan/crypto/crypto_factory.c          |  18 +-
 src/libstrongswan/tests/Makefile.am                |   1 +
 .../tests/suites/test_crypto_factory.c             | 312 +++++++++++++++++++++
 src/libstrongswan/tests/tests.h                    |   1 +
 4 files changed, 327 insertions(+), 5 deletions(-)
 create mode 100644 src/libstrongswan/tests/suites/test_crypto_factory.c

diff --git a/src/libstrongswan/crypto/crypto_factory.c b/src/libstrongswan/crypto/crypto_factory.c
index 6dea30e..96fbc0d 100644
--- a/src/libstrongswan/crypto/crypto_factory.c
+++ b/src/libstrongswan/crypto/crypto_factory.c
@@ -392,10 +392,10 @@ METHOD(crypto_factory_t, create_dh, diffie_hellman_t*,
 /**
  * Insert an algorithm entry to a list
  *
- * Entries are sorted by algorithm identifier (which is important for RNGs)
- * while maintaining the order in which algorithms were added, unless they were
+ * Entries maintain the order in which algorithms were added, unless they were
  * benchmarked and speed is provided, which then is used to order entries of
  * the same algorithm.
+ * An exception are RNG entries, which are sorted by algorithm identifier.
  */
 static void add_entry(private_crypto_factory_t *this, linked_list_t *list,
 					  int algo, const char *plugin_name,
@@ -403,6 +403,7 @@ static void add_entry(private_crypto_factory_t *this, linked_list_t *list,
 {
 	enumerator_t *enumerator;
 	entry_t *entry, *current;
+	bool sort = (list == this->rngs), found = FALSE;
 
 	INIT(entry,
 		.algo = algo,
@@ -415,12 +416,19 @@ static void add_entry(private_crypto_factory_t *this, linked_list_t *list,
 	enumerator = list->create_enumerator(list);
 	while (enumerator->enumerate(enumerator, &current))
 	{
-		if (current->algo > algo)
+		if (sort && current->algo > algo)
 		{
 			break;
 		}
-		else if (current->algo == algo && speed &&
-				 current->speed < speed)
+		else if (current->algo == algo)
+		{
+			if (speed > current->speed)
+			{
+				break;
+			}
+			found = TRUE;
+		}
+		else if (found)
 		{
 			break;
 		}
diff --git a/src/libstrongswan/tests/Makefile.am b/src/libstrongswan/tests/Makefile.am
index 331a548..0bdf2b3 100644
--- a/src/libstrongswan/tests/Makefile.am
+++ b/src/libstrongswan/tests/Makefile.am
@@ -42,6 +42,7 @@ tests_SOURCES = tests.h tests.c \
   suites/test_host.c \
   suites/test_hasher.c \
   suites/test_crypter.c \
+  suites/test_crypto_factory.c \
   suites/test_pen.c \
   suites/test_asn1.c \
   suites/test_asn1_parser.c \
diff --git a/src/libstrongswan/tests/suites/test_crypto_factory.c b/src/libstrongswan/tests/suites/test_crypto_factory.c
new file mode 100644
index 0000000..94f45da
--- /dev/null
+++ b/src/libstrongswan/tests/suites/test_crypto_factory.c
@@ -0,0 +1,312 @@
+/*
+ * Copyright (C) 2014 Tobias Brunner
+ * 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 <crypto/crypto_factory.h>
+
+static rng_t *rng_create(rng_quality_t quality)
+{
+	rng_quality_t *q = malloc_thing(rng_quality_t);
+	*q = quality;
+	return (rng_t*)q;
+}
+
+static rng_t *rng_create_weak(rng_quality_t quality)
+{
+	ck_assert(quality == RNG_WEAK);
+	return rng_create(RNG_WEAK);
+}
+
+static rng_t *rng_create_strong(rng_quality_t quality)
+{
+	ck_assert(quality <= RNG_STRONG);
+	return rng_create(RNG_STRONG);
+}
+
+static rng_t *rng_create_true(rng_quality_t quality)
+{
+	ck_assert(quality <= RNG_TRUE);
+	return rng_create(RNG_TRUE);
+}
+
+static rng_t *rng_create_true_second(rng_quality_t quality)
+{
+	fail("should never be called");
+	return rng_create(RNG_TRUE);
+}
+
+static rng_quality_t rng_weak = RNG_WEAK;
+static rng_quality_t rng_strong = RNG_STRONG;
+static rng_quality_t rng_true = RNG_TRUE;
+
+static struct {
+	rng_quality_t *exp_weak;
+	rng_quality_t *exp_strong;
+	rng_quality_t *exp_true;
+	struct {
+		rng_quality_t *q;
+		rng_constructor_t create;
+	} data[4];
+} rng_data[] = {
+	{ NULL, NULL, NULL, {
+		{ NULL, NULL }
+	}},
+	{ &rng_weak, NULL, NULL, {
+		{ &rng_weak, rng_create_weak },
+		{ NULL, NULL }
+	}},
+	{ &rng_strong, &rng_strong, NULL, {
+		{ &rng_strong, rng_create_strong },
+		{ NULL, NULL }
+	}},
+	{ &rng_true, &rng_true, &rng_true, {
+		{ &rng_true, rng_create_true },
+		{ NULL, NULL }
+	}},
+	{ &rng_true, &rng_true, &rng_true, {
+		{ &rng_true, rng_create_true },
+		{ &rng_true, rng_create_true_second },
+		{ NULL, NULL }
+	}},
+	{ &rng_weak, &rng_true, &rng_true, {
+		{ &rng_weak, rng_create_weak },
+		{ &rng_true, rng_create_true },
+		{ NULL, NULL }
+	}},
+	{ &rng_weak, &rng_strong, &rng_true, {
+		{ &rng_true, rng_create_true },
+		{ &rng_strong, rng_create_strong },
+		{ &rng_weak, rng_create_weak },
+		{ NULL, NULL }
+	}},
+	{ &rng_weak, &rng_strong, &rng_true, {
+		{ &rng_weak, rng_create_weak },
+		{ &rng_strong, rng_create_strong },
+		{ &rng_true, rng_create_true },
+		{ NULL, NULL }
+	}},
+};
+
+static void verify_rng(crypto_factory_t *factory, rng_quality_t request,
+					   rng_quality_t *expected)
+{
+	rng_quality_t *res;
+
+	res = (rng_quality_t*)factory->create_rng(factory, request);
+	if (!expected)
+	{
+		ck_assert(!res);
+	}
+	else
+	{
+		ck_assert(res);
+		ck_assert_int_eq(*expected, *res);
+		free(res);
+	}
+}
+
+START_TEST(test_create_rng)
+{
+	crypto_factory_t *factory;
+	int i;
+
+	factory = crypto_factory_create();
+	for (i = 0; rng_data[_i].data[i].q; i++)
+	{
+		ck_assert(factory->add_rng(factory, *rng_data[_i].data[i].q, "test",
+								   rng_data[_i].data[i].create));
+	}
+	verify_rng(factory, RNG_WEAK, rng_data[_i].exp_weak);
+	verify_rng(factory, RNG_STRONG, rng_data[_i].exp_strong);
+	verify_rng(factory, RNG_TRUE, rng_data[_i].exp_true);
+	for (i = 0; rng_data[_i].data[i].q; i++)
+	{
+		factory->remove_rng(factory, rng_data[_i].data[i].create);
+	}
+	factory->destroy(factory);
+}
+END_TEST
+
+static diffie_hellman_t *dh_create(char *plugin)
+{
+	return (diffie_hellman_t*)plugin;
+}
+
+static diffie_hellman_t *dh_create_modp1024(diffie_hellman_group_t group, ...)
+{
+	ck_assert(group == MODP_1024_BIT);
+	return dh_create("plugin1");
+}
+
+static diffie_hellman_t *dh_create_modp1024_second(diffie_hellman_group_t group,
+												   ...)
+{
+	ck_assert(group == MODP_1024_BIT);
+	return dh_create("plugin2");
+}
+
+static diffie_hellman_t *dh_create_modp2048(diffie_hellman_group_t group, ...)
+{
+	ck_assert(group == MODP_2048_BIT);
+	return dh_create("plugin1");
+}
+
+static diffie_hellman_t *dh_create_modp2048_second(diffie_hellman_group_t group,
+												   ...)
+{
+	ck_assert(group == MODP_2048_BIT);
+	return dh_create("plugin2");
+}
+
+static struct {
+	char *exp1024;
+	char *exp2048;
+	struct {
+		diffie_hellman_group_t g;
+		dh_constructor_t create;
+		char *plugin;
+	} data[4];
+} dh_data[] = {
+	{ NULL, NULL, {
+		{ MODP_NONE, NULL, NULL }
+	}},
+	{ "plugin1", NULL, {
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_NONE, NULL, NULL }
+	}},
+	{ "plugin1", NULL, {
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_1024_BIT, dh_create_modp1024_second, "plugin2" },
+		{ MODP_NONE, NULL, NULL }
+	}},
+	{ "plugin2", NULL, {
+		{ MODP_1024_BIT, dh_create_modp1024_second, "plugin2" },
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_NONE, NULL, NULL }
+	}},
+	{ "plugin1", "plugin1", {
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_2048_BIT, dh_create_modp2048, "plugin1" },
+		{ MODP_NONE, NULL }
+	}},
+	{ "plugin1", "plugin1", {
+		{ MODP_2048_BIT, dh_create_modp2048, "plugin1" },
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_NONE, NULL }
+	}},
+	{ "plugin1", "plugin1", {
+		{ MODP_2048_BIT, dh_create_modp2048, "plugin1" },
+		{ MODP_2048_BIT, dh_create_modp2048_second, "plugin2" },
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_NONE, NULL }
+	}},
+	{ "plugin1", "plugin2", {
+		{ MODP_2048_BIT, dh_create_modp2048_second, "plugin2" },
+		{ MODP_2048_BIT, dh_create_modp2048, "plugin1" },
+		{ MODP_1024_BIT, dh_create_modp1024, "plugin1" },
+		{ MODP_NONE, NULL }
+	}},
+};
+
+static void verify_dh(crypto_factory_t *factory, diffie_hellman_group_t request,
+					  char *expected)
+{
+	char *plugin;
+
+	plugin = (char*)factory->create_dh(factory, request);
+	if (!expected)
+	{
+		ck_assert(!plugin);
+	}
+	else
+	{
+		ck_assert(plugin);
+		ck_assert_str_eq(expected, plugin);
+	}
+}
+
+START_TEST(test_create_dh)
+{
+	enumerator_t *enumerator;
+	crypto_factory_t *factory;
+	diffie_hellman_group_t group;
+	char *plugin;
+	int i, len = 0;
+
+
+	factory = crypto_factory_create();
+	for (i = 0; dh_data[_i].data[i].g != MODP_NONE; i++)
+	{
+		ck_assert(factory->add_dh(factory, dh_data[_i].data[i].g,
+								  dh_data[_i].data[i].plugin,
+								  dh_data[_i].data[i].create));
+	}
+	verify_dh(factory, MODP_1024_BIT, dh_data[_i].exp1024);
+	verify_dh(factory, MODP_2048_BIT, dh_data[_i].exp2048);
+
+	len = countof(dh_data[_i].data);
+	enumerator = factory->create_dh_enumerator(factory);
+	for (i = 0; enumerator->enumerate(enumerator, &group, &plugin) && i < len;)
+	{
+		ck_assert_int_eq(dh_data[_i].data[i].g, group);
+		while (dh_data[_i].data[i].g == group)
+		{	/* skip other entries by the same group */
+			i++;
+		}
+		switch (group)
+		{
+			case MODP_1024_BIT:
+				ck_assert(dh_data[_i].exp1024);
+				ck_assert_str_eq(dh_data[_i].exp1024, plugin);
+				break;
+			case MODP_2048_BIT:
+				ck_assert(dh_data[_i].exp2048);
+				ck_assert_str_eq(dh_data[_i].exp2048, plugin);
+				break;
+			default:
+				fail("unexpected DH group");
+				break;
+		}
+	}
+	ck_assert(!enumerator->enumerate(enumerator));
+	ck_assert_int_eq(dh_data[_i].data[i].g, MODP_NONE);
+	enumerator->destroy(enumerator);
+
+	for (i = 0; dh_data[_i].data[i].g != MODP_NONE; i++)
+	{
+		factory->remove_dh(factory, dh_data[_i].data[i].create);
+	}
+	factory->destroy(factory);
+}
+END_TEST
+
+Suite *crypto_factory_suite_create()
+{
+	Suite *s;
+	TCase *tc;
+
+	s = suite_create("crypto-factory");
+
+	tc = tcase_create("create_rng");
+	tcase_add_loop_test(tc, test_create_rng, 0, countof(rng_data));
+	suite_add_tcase(s, tc);
+
+	tc = tcase_create("create_dh");
+	tcase_add_loop_test(tc, test_create_dh, 0, countof(dh_data));
+	suite_add_tcase(s, tc);
+
+	return s;
+}
diff --git a/src/libstrongswan/tests/tests.h b/src/libstrongswan/tests/tests.h
index 82a5137..ab0f642 100644
--- a/src/libstrongswan/tests/tests.h
+++ b/src/libstrongswan/tests/tests.h
@@ -35,6 +35,7 @@ TEST_SUITE(host_suite_create)
 TEST_SUITE(printf_suite_create)
 TEST_SUITE(hasher_suite_create)
 TEST_SUITE(crypter_suite_create)
+TEST_SUITE(crypto_factory_suite_create)
 TEST_SUITE(pen_suite_create)
 TEST_SUITE(asn1_suite_create)
 TEST_SUITE(asn1_parser_suite_create)
-- 
2.1.2