p11-kit: New priority option and change trust-policy option

 * Sort loaded modules appropriately using the 'priority' option. This
   allows us to have a predictable order for callers, when callers
   iterate through modules.
 * Modules default to having an 'priority' option of '0'.
 * If modules have the same order value, then sort by name.
 * The above assumes the role of ordering trust-policy sources.
 * Change the trust-policy option to a boolean
 * Some of this code will be rearranged when the managed branch
   is merged.

https://bugs.freedesktop.org/show_bug.cgi?id=61978

15 files changed, 136 insertions, 54 deletions

diff --git a/doc/manual/p11-kit-trust.xml b/doc/manual/p11-kit-trust.xml
index 06f168e..198d5db 100644
--- a/doc/manual/p11-kit-trust.xml
+++ b/doc/manual/p11-kit-trust.xml
@@ -107,11 +107,12 @@ $ pkg-config --variable p11_trust_paths p11-kit-1
 		<listitem><para>Disable loading trust policy information
 		from this module by adding a file to <literal>/etc/pkcs11/modules</literal>
 		called <literal>p11-kit-trust.module</literal> containing a
-		<literal>trust-policy:</literal> line.</para></listitem>
+		<literal>trust-policy: no</literal> line.</para></listitem>
+
 		<listitem><para>Disable this module completely by
 		adding a file to <literal>/etc/pkcs11/modules</literal>
 		called <literal>p11-kit-trust.module</literal> containing a
-		<literal>enable-in:</literal> line.</para></listitem>
+		<literal>enable-in:</literal> line (without a value).</para></listitem>
 	</itemizedlist>
 
 </section>
diff --git a/doc/manual/pkcs11.conf.xml b/doc/manual/pkcs11.conf.xml
index 3146f60..5ff0863 100644
--- a/doc/manual/pkcs11.conf.xml
+++ b/doc/manual/pkcs11.conf.xml
@@ -128,13 +128,23 @@ x-custom : text
 		</listitem>
 	</varlistentry>
 	<varlistentry>
+		<term><option>priority:</option></term>
+		<listitem>
+			<para>The value should be an integer. When lists of modules are
+			returned to a caller of p11-kit, modules with a higher number are sorted
+			first. When applications search modules for for certificates, keys and
+			trust policy information, this setting will affect what find
+			first.</para>
+			<para>This argument is optional, and defaults to zero. Modules
+			with the same <option>priority</option> option will be sorted
+			alphabetically.</para>
+		</listitem>
+	</varlistentry>
+	<varlistentry>
 		<term><option>trust-policy:</option></term>
 		<listitem>
-			<para>If this setting is present then this module is used to load
-			trust policy information such as certificate anchors and black lists.
-			The value should be an integer. Modules with a lower number are loaded
-			first. Trust policy information in modules loaded later overrides
-			those loaded first.</para>
+			<para>Set to <literal>yes</literal> to use use this module as a source
+			of trust policy information such as certificate anchors and black lists.</para>
 		</listitem>
 	</varlistentry>
 	</variablelist>
diff --git a/p11-kit/modules.c b/p11-kit/modules.c
index eaa1564..7648167 100644
--- a/p11-kit/modules.c
+++ b/p11-kit/modules.c
@@ -896,6 +896,51 @@ p11_kit_finalize_registered (void)
 	return rv;
 }
 
+static int
+compar_priority (const void *one,
+                 const void *two)
+{
+	CK_FUNCTION_LIST_PTR f1 = *((CK_FUNCTION_LIST_PTR *)one);
+	CK_FUNCTION_LIST_PTR f2 = *((CK_FUNCTION_LIST_PTR *)two);
+	Module *m1, *m2;
+	const char *v1, *v2;
+	int o1, o2;
+
+	m1 = p11_dict_get (gl.modules, f1);
+	m2 = p11_dict_get (gl.modules, f2);
+	assert (m1 != NULL && m2 != NULL);
+
+	v1 = p11_dict_get (m1->config, "priority");
+	v2 = p11_dict_get (m2->config, "priority");
+
+	o1 = atoi (v1 ? v1 : "0");
+	o2 = atoi (v2 ? v2 : "0");
+
+	/* Priority is in descending order, highest first */
+	if (o1 != o2)
+		return o1 > o2 ? -1 : 1;
+
+	/*
+	 * Otherwise use the names alphabetically in ascending order. This
+	 * is really just to provide consistency between various loads of
+	 * the configuration.
+	 */
+	if (m1->name == m2->name)
+		return 0;
+	if (!m1->name)
+		return -1;
+	if (!m2->name)
+		return 1;
+	return strcmp (m1->name, m2->name);
+}
+
+static void
+sort_modules_by_priority (CK_FUNCTION_LIST_PTR *modules,
+                          int count)
+{
+	qsort (modules, count, sizeof (CK_FUNCTION_LIST_PTR), compar_priority);
+}
+
 CK_FUNCTION_LIST_PTR_PTR
 _p11_kit_registered_modules_unlocked (void)
 {
@@ -927,6 +972,8 @@ _p11_kit_registered_modules_unlocked (void)
 				result[i++] = mod->funcs;
 			}
 		}
+
+		sort_modules_by_priority (result, i);
 	}
 
 	return result;
diff --git a/p11-kit/tests/files/package-modules/four.module b/p11-kit/tests/files/package-modules/four.module
index 6eace3c..545c285 100644
--- a/p11-kit/tests/files/package-modules/four.module
+++ b/p11-kit/tests/files/package-modules/four.module
@@ -1,3 +1,4 @@
 
 module: mock-four.so
 disable-in: test-disable, test-other
+priority: 4
\ No newline at end of file
diff --git a/p11-kit/tests/files/package-modules/win32/four.module b/p11-kit/tests/files/package-modules/win32/four.module
index 7fd1540..6dc87c9 100644
--- a/p11-kit/tests/files/package-modules/win32/four.module
+++ b/p11-kit/tests/files/package-modules/win32/four.module
@@ -1,3 +1,4 @@
 
 module: mock-four.dll
 disable-in: test-disable, test-other
+priority: 4
\ No newline at end of file
diff --git a/p11-kit/tests/files/system-modules/two-duplicate.module b/p11-kit/tests/files/system-modules/two-duplicate.module
index 907aa75..756af69 100644
--- a/p11-kit/tests/files/system-modules/two-duplicate.module
+++ b/p11-kit/tests/files/system-modules/two-duplicate.module
@@ -1,3 +1,4 @@
 
 # This is a duplicate of the 'two' module
 module: mock-two.so
+# no priority, use name
\ No newline at end of file
diff --git a/p11-kit/tests/files/system-modules/two.badname b/p11-kit/tests/files/system-modules/two.badname
index 0d41cac..eec3af0 100644
--- a/p11-kit/tests/files/system-modules/two.badname
+++ b/p11-kit/tests/files/system-modules/two.badname
@@ -3,3 +3,4 @@
 
 module: mock-two.so
 setting: system2
+# no priority, use name
\ No newline at end of file
diff --git a/p11-kit/tests/files/system-modules/win32/one.module b/p11-kit/tests/files/system-modules/win32/one.module
index 5f80304..d153ce5 100644
--- a/p11-kit/tests/files/system-modules/win32/one.module
+++ b/p11-kit/tests/files/system-modules/win32/one.module
@@ -1,3 +1,4 @@
 
 module: mock-one.dll
-setting: system1
\ No newline at end of file
+setting: system1
+# no order, use name
\ No newline at end of file
diff --git a/p11-kit/tests/files/system-modules/win32/two-duplicate.module b/p11-kit/tests/files/system-modules/win32/two-duplicate.module
index e80c9e8..54ef1cc 100644
--- a/p11-kit/tests/files/system-modules/win32/two-duplicate.module
+++ b/p11-kit/tests/files/system-modules/win32/two-duplicate.module
@@ -1,3 +1,4 @@
 
 # This is a duplicate of the 'two' module
 module: mock-two.dll
+# no order, use name
\ No newline at end of file
diff --git a/p11-kit/tests/files/system-modules/win32/two.badname b/p11-kit/tests/files/system-modules/win32/two.badname
index ae44b83..af63cf9 100644
--- a/p11-kit/tests/files/system-modules/win32/two.badname
+++ b/p11-kit/tests/files/system-modules/win32/two.badname
@@ -3,3 +3,4 @@
 
 module: mock-two.dll
 setting: system2
+# no order, use name
\ No newline at end of file
diff --git a/p11-kit/tests/files/user-modules/three.module b/p11-kit/tests/files/user-modules/three.module
index 00caab5..3a2366d 100644
--- a/p11-kit/tests/files/user-modules/three.module
+++ b/p11-kit/tests/files/user-modules/three.module
@@ -2,4 +2,5 @@
 module: mock-three.so
 setting: user3
 
-enable-in: test-enable
\ No newline at end of file
+enable-in: test-enable
+priority: 3
\ No newline at end of file
diff --git a/p11-kit/tests/files/user-modules/win32/three.module b/p11-kit/tests/files/user-modules/win32/three.module
index 58f883d..30a3b63 100644
--- a/p11-kit/tests/files/user-modules/win32/three.module
+++ b/p11-kit/tests/files/user-modules/win32/three.module
@@ -2,4 +2,5 @@
 module: mock-three.dll
 setting: user3
 
-enable-in: test-enable
\ No newline at end of file
+enable-in: test-enable
+priority: 3
\ No newline at end of file
diff --git a/p11-kit/tests/test-modules.c b/p11-kit/tests/test-modules.c
index eb8d952..5bdbaa4 100644
--- a/p11-kit/tests/test-modules.c
+++ b/p11-kit/tests/test-modules.c
@@ -219,6 +219,47 @@ test_enable (CuTest *tc)
 	p11_kit_set_progname (NULL);
 }
 
+static void
+test_priority (CuTest *tc)
+{
+	CK_FUNCTION_LIST_PTR_PTR modules;
+	char *name;
+	int i;
+
+	/*
+	 * The expected order.
+	 * - four is marked with a priority of 4, the highest therefore first
+	 * - three is marked with a priority of 3, next highest
+	 * - one and two do not have priority marked, so they default to zero
+	 *   and fallback to sorting alphabetically. 'o' comes before 't'
+	 */
+
+	const char *expected[] = { "four", "three", "one", "two.badname" };
+
+	/* This enables module three */
+	p11_kit_set_progname ("test-enable");
+
+	modules = initialize_and_get_modules (tc);
+
+	/* The loaded modules should not contain duplicates */
+	for (i = 0; modules[i] != NULL; i++) {
+		name = p11_kit_registered_module_to_name (modules[i]);
+		CuAssertPtrNotNull (tc, name);
+
+		/* Either one of these can be loaded, as this is a duplicate module */
+		if (strcmp (name, "two-duplicate") == 0) {
+			free (name);
+			name = strdup ("two.badname");
+		}
+
+		CuAssertStrEquals (tc, expected[i], name);
+		free (name);
+	}
+
+	CuAssertIntEquals (tc, 4, i);
+	finalize_and_free_modules (tc, modules);
+}
+
 int
 main (void)
 {
@@ -233,6 +274,7 @@ main (void)
 	SUITE_ADD_TEST (suite, test_disable);
 	SUITE_ADD_TEST (suite, test_disable_later);
 	SUITE_ADD_TEST (suite, test_enable);
+	SUITE_ADD_TEST (suite, test_priority);
 
 	p11_kit_be_quiet ();
 
diff --git a/tools/extract.c b/tools/extract.c
index 40a3911..fe5ba15 100644
--- a/tools/extract.c
+++ b/tools/extract.c
@@ -203,28 +203,12 @@ format_argument (const char *optarg,
 	return true;
 }
 
-static int
-compar_longs (const void *v1,
-              const void *v2)
-{
-	const long *o1 = v1;
-	const long *o2 = v2;
-	return (int)(o1 - o2);
-}
-
 static void
 limit_modules_if_necessary (CK_FUNCTION_LIST_PTR *modules,
                             CK_ATTRIBUTE *match)
 {
-	long policy;
 	char *string;
 	int i, out;
-	char *endptr;
-
-	struct {
-		long policy;
-		CK_FUNCTION_LIST_PTR module;
-	} *order;
 
 	/*
 	 * We only "believe" the CKA_TRUSTED and CKA_X_DISTRUSTED attributes
@@ -241,35 +225,16 @@ limit_modules_if_necessary (CK_FUNCTION_LIST_PTR *modules,
 	if (out == 0)
 		return;
 
-	order = malloc (sizeof (*order) * out);
-	return_if_fail (order != NULL);
-
+	/* TODO: This logic will move once we merge our p11-kit managed code */
 	for (i = 0, out = 0; modules[i] != NULL; i++) {
 		string = p11_kit_registered_option (modules[i], "trust-policy");
-		if (string) {
-			policy = strtol (string, &endptr, 10);
-			if (!endptr || endptr[0] != '\0' || policy > INT16_MAX || policy < INT16_MIN) {
-				p11_message ("skipping module with invalid 'trust-policy' setting: %s", string);
-
-			} else {
-				order[out].module = modules[i];
-				order[out].policy = policy;
-				out++;
-			}
-
-			free (string);
-		}
+		if (string && strcmp (string, "yes") == 0)
+			modules[out++] = modules[i];
+		else if (string && strcmp (string, "no") != 0)
+			p11_message ("skipping module with invalid 'trust-policy' setting: %s", string);
+		free (string);
 	}
 
-	/* Our compare function compares the first member of Order */
-	qsort (order, out, sizeof (*order), compar_longs);
-
-	for (i = 0; i < out; i++)
-		modules[i] = order[i].module;
-	modules[i] = NULL;
-
-	free (order);
-
 	if (out == 0)
 		p11_message ("no modules containing trust policy are registered");
 }
diff --git a/trust/p11-kit-trust.module b/trust/p11-kit-trust.module
index 1a6e94b..a55932a 100644
--- a/trust/p11-kit-trust.module
+++ b/trust/p11-kit-trust.module
@@ -1,9 +1,17 @@
+# See pkcs11.conf(5) to understand this file
 
 # This is a module config for the 'included' p11-kit trust module
 module: p11-kit-trust.so
 
-# The order in which this is loaded in the trust policy
-trust-policy: 1
+# This setting affects the order that trust policy and other information
+# is looked up when going across various modules. Other trust policy modules
+# need to specify the priority where they slot into things.
+priority: 1
 
-# This is for drop-in compatibilty with glib-networking and gcr
+# Mark this module as a viable source of trust policy information
+trust-policy: yes
+
+# This is for drop-in compatibilty with glib-networking and gcr. Those
+# projects used this non-standard attribute to denote slots to use to
+# retrieve trust information.
 x-trust-lookup: pkcs11:library-description=PKCS%2311%20Kit%20Trust%20Module