enumerate: Preload and respect blacklist across all tokens

This fixes an issue where a blacklist in one token wasn't properly
skipping anchors being extracted with extract-compat

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

3 files changed, 186 insertions, 52 deletions

diff --git a/trust/enumerate.c b/trust/enumerate.c
index 3025c6a..679b635 100644
--- a/trust/enumerate.c
+++ b/trust/enumerate.c
@@ -177,47 +177,7 @@ extract_purposes (p11_enumerate *ex)
 }
 
 static bool
-check_blacklisted (P11KitIter *iter,
-                   CK_ATTRIBUTE *cert)
-{
-	CK_OBJECT_HANDLE dummy;
-	CK_FUNCTION_LIST *module;
-	CK_SESSION_HANDLE session;
-	CK_BBOOL distrusted = CK_TRUE;
-	CK_ULONG have;
-	CK_RV rv;
-
-	CK_ATTRIBUTE match[] = {
-		{ CKA_VALUE, cert->pValue, cert->ulValueLen },
-		{ CKA_X_DISTRUSTED, &distrusted, sizeof (distrusted) },
-	};
-
-	module = p11_kit_iter_get_module (iter);
-	session = p11_kit_iter_get_session (iter);
-
-	rv = (module->C_FindObjectsInit) (session, match, 2);
-	if (rv == CKR_OK) {
-		rv = (module->C_FindObjects) (session, &dummy, 1, &have);
-		(module->C_FindObjectsFinal) (session);
-	}
-
-	if (rv != CKR_OK) {
-		p11_message ("couldn't check if certificate is on blacklist");
-		return true;
-	}
-
-	if (have == 0) {
-		p11_debug ("anchor is not on blacklist");
-		return false;
-	} else {
-		p11_debug ("anchor is on blacklist");
-		return true;
-	}
-}
-
-static bool
-check_trust_flags (p11_enumerate *ex,
-                   CK_ATTRIBUTE *cert)
+check_trust_flags (p11_enumerate *ex)
 {
 	CK_BBOOL trusted;
 	CK_BBOOL distrusted;
@@ -227,15 +187,18 @@ check_trust_flags (p11_enumerate *ex,
 	if (!(ex->flags & (P11_ENUMERATE_ANCHORS | P11_ENUMERATE_BLACKLIST)))
 		return true;
 
-	if (p11_attrs_find_bool (ex->attrs, CKA_TRUSTED, &trusted) &&
-	    trusted && !check_blacklisted (ex->iter, cert)) {
-		flags |= P11_ENUMERATE_ANCHORS;
-	}
+	/* Is this a blacklisted directly? */
+	if (p11_attrs_find_bool (ex->attrs, CKA_X_DISTRUSTED, &distrusted) && distrusted)
+		flags = P11_ENUMERATE_BLACKLIST;
 
-	if (p11_attrs_find_bool (ex->attrs, CKA_X_DISTRUSTED, &distrusted) &&
-	    distrusted) {
-		flags |= P11_ENUMERATE_BLACKLIST;
-	}
+	/* Is it blacklisted elsewhere? then prevent it from being an anchor */
+	else if (p11_dict_get (ex->blacklist_public_key, ex->attrs) ||
+	         p11_dict_get (ex->blacklist_issuer_serial, ex->attrs))
+		flags = 0;
+
+	/* Otherwise it might be an anchor? */
+	else if (p11_attrs_find_bool (ex->attrs, CKA_TRUSTED, &trusted) && trusted)
+		flags = P11_ENUMERATE_ANCHORS;
 
 	/* Any of the flags can match */
 	if (flags & ex->flags)
@@ -281,7 +244,7 @@ extract_certificate (p11_enumerate *ex)
 			return false;
 	}
 
-	if (!check_trust_flags (ex, attr)) {
+	if (!check_trust_flags (ex)) {
 		p11_debug ("skipping certificate that doesn't match trust flags");
 		return false;
 	}
@@ -319,6 +282,7 @@ extract_info (p11_enumerate *ex)
 		{ CKA_VALUE, },
 		{ CKA_SUBJECT, },
 		{ CKA_ISSUER, },
+		{ CKA_SERIAL_NUMBER, },
 		{ CKA_TRUSTED, },
 		{ CKA_CERTIFICATE_CATEGORY },
 		{ CKA_X_DISTRUSTED },
@@ -414,6 +378,116 @@ on_iterate_load_filter (p11_kit_iter *iter,
 	return CKR_OK;
 }
 
+/*
+ * Various skip lookup tables, used for blacklists and collapsing
+ * duplicate entries.
+ *
+ * The dict hash/lookup callbacks are special cased
+ * so we can just pass in full attribute lists for lookup and only match
+ * the attributes we're interested in.
+ *
+ * Note that both p11_attr_hash and p11_attr_equal are NULL safe.
+ */
+
+static bool
+public_key_equal (const void *one,
+                  const void *two)
+{
+	return p11_attr_equal (p11_attrs_find_valid ((CK_ATTRIBUTE *)one, CKA_X_PUBLIC_KEY_INFO),
+	                       p11_attrs_find_valid ((CK_ATTRIBUTE *)two, CKA_X_PUBLIC_KEY_INFO));
+}
+
+static unsigned int
+public_key_hash (const void *data)
+{
+	return p11_attr_hash (p11_attrs_find_valid ((CK_ATTRIBUTE *)data, CKA_X_PUBLIC_KEY_INFO));
+}
+
+static bool
+issuer_serial_equal (const void *one,
+                     const void *two)
+{
+	return p11_attr_equal (p11_attrs_find_valid ((CK_ATTRIBUTE *)one, CKA_ISSUER),
+	                       p11_attrs_find_valid ((CK_ATTRIBUTE *)two, CKA_ISSUER)) &&
+	       p11_attr_equal (p11_attrs_find_valid ((CK_ATTRIBUTE *)one, CKA_SERIAL_NUMBER),
+	                       p11_attrs_find_valid ((CK_ATTRIBUTE *)two, CKA_SERIAL_NUMBER));
+}
+
+static unsigned int
+issuer_serial_hash (const void *data)
+{
+	return p11_attr_hash (p11_attrs_find_valid ((CK_ATTRIBUTE *)data, CKA_ISSUER)) ^
+	       p11_attr_hash (p11_attrs_find_valid ((CK_ATTRIBUTE *)data, CKA_SERIAL_NUMBER));
+}
+
+static bool
+blacklist_load (p11_enumerate *ex)
+{
+	p11_kit_iter *iter;
+	CK_BBOOL distrusted = CK_TRUE;
+	CK_RV rv = CKR_OK;
+	CK_ATTRIBUTE *attrs;
+	CK_ATTRIBUTE *key;
+	CK_ATTRIBUTE *serial;
+	CK_ATTRIBUTE *issuer;
+	CK_ATTRIBUTE *public_key;
+
+	CK_ATTRIBUTE match[] = {
+		{ CKA_X_DISTRUSTED, &distrusted, sizeof (distrusted) },
+	};
+
+	CK_ATTRIBUTE template[] = {
+		{ CKA_SERIAL_NUMBER, },
+		{ CKA_X_PUBLIC_KEY_INFO, },
+		{ CKA_ISSUER, },
+	};
+
+	iter = p11_kit_iter_new (ex->uri, 0);
+	p11_kit_iter_add_filter (iter, match, 1);
+	p11_kit_iter_begin (iter, ex->modules);
+
+	attrs = p11_attrs_buildn (NULL, template, 3);
+
+	while ((rv = p11_kit_iter_next (iter)) == CKR_OK) {
+
+		/*
+		 * Fail "safe" in that first failure doesn't cause ignoring
+		 * the remainder of the blacklist.
+		 */
+		rv = p11_kit_iter_load_attributes (iter, attrs, 3);
+		if (rv != CKR_OK) {
+			p11_message ("couldn't load blacklist: %s", p11_kit_strerror (rv));
+			continue;
+		}
+
+		/* A blacklisted item with an issuer and serial number */
+		issuer = p11_attrs_find_valid (attrs, CKA_ISSUER);
+		serial = p11_attrs_find_valid (attrs, CKA_SERIAL_NUMBER);
+		if (issuer != NULL && serial != NULL) {
+			key = p11_attrs_build (NULL, issuer, serial, NULL);
+			if (!key || !p11_dict_set (ex->blacklist_issuer_serial, key, "x"))
+				return_val_if_reached (false);
+		}
+
+		/* A blacklisted item with a public key */
+		public_key = p11_attrs_find_valid (attrs, CKA_X_PUBLIC_KEY_INFO);
+		if (public_key != NULL) {
+			key = p11_attrs_build (NULL, public_key, NULL);
+			if (!public_key || !p11_dict_set (ex->blacklist_public_key, key, "x"))
+				return_val_if_reached (false);
+		}
+	}
+
+	p11_attrs_free (attrs);
+	p11_kit_iter_free (iter);
+
+	if (rv == CKR_CANCEL)
+		return true;
+
+	p11_message ("couldn't load blacklist: %s", p11_kit_strerror (rv));
+	return false;
+}
+
 void
 p11_enumerate_init (p11_enumerate *ex)
 {
@@ -424,6 +498,14 @@ p11_enumerate_init (p11_enumerate *ex)
 	ex->iter = p11_kit_iter_new (NULL, 0);
 	return_if_fail (ex->iter != NULL);
 
+	ex->blacklist_public_key = p11_dict_new (public_key_hash, public_key_equal,
+	                                         p11_attrs_free, NULL);
+	return_if_fail (ex->blacklist_public_key);
+
+	ex->blacklist_issuer_serial = p11_dict_new (issuer_serial_hash, issuer_serial_equal,
+	                                            p11_attrs_free, NULL);
+	return_if_fail (ex->blacklist_issuer_serial);
+
 	p11_kit_iter_add_callback (ex->iter, on_iterate_load_filter, ex, NULL);
 }
 
@@ -437,6 +519,10 @@ p11_enumerate_cleanup (p11_enumerate *ex)
 
 	p11_dict_free (ex->already_seen);
 	ex->already_seen = NULL;
+	p11_dict_free (ex->blacklist_public_key);
+	ex->blacklist_public_key = NULL;
+	p11_dict_free (ex->blacklist_issuer_serial);
+	ex->blacklist_issuer_serial = NULL;
 
 	p11_dict_free (ex->asn1_defs);
 	ex->asn1_defs = NULL;
@@ -593,6 +679,16 @@ p11_enumerate_ready (p11_enumerate *ex,
 	if (ex->modules[0] == NULL)
 		p11_message ("no modules containing trust policy are registered");
 
+	/*
+	 * If loading anchors, then the caller expects that the blacklist is
+	 * "applied" and any anchors on the blacklist are taken out. This is
+	 * for compatibility with software that does not support blacklists.
+	 */
+	if (ex->flags & P11_ENUMERATE_ANCHORS) {
+		if (!blacklist_load (ex))
+			return false;
+	}
+
 	p11_kit_iter_begin (ex->iter, ex->modules);
 	return true;
 }
diff --git a/trust/enumerate.h b/trust/enumerate.h
index 8b1e7e4..d49bf16 100644
--- a/trust/enumerate.h
+++ b/trust/enumerate.h
@@ -61,6 +61,9 @@ typedef struct {
 	int num_filters;
 	int flags;
 
+	p11_dict *blacklist_issuer_serial;
+	p11_dict *blacklist_public_key;
+
 	/*
 	 * Stuff below is parsed info for the current iteration.
 	 * Currently this information is generally all relevant
diff --git a/trust/tests/test-enumerate.c b/trust/tests/test-enumerate.c
index 1cd9b84..75d3f16 100644
--- a/trust/tests/test-enumerate.c
+++ b/trust/tests/test-enumerate.c
@@ -178,6 +178,7 @@ teardown (void *unused)
 }
 
 static CK_OBJECT_CLASS certificate_class = CKO_CERTIFICATE;
+static CK_OBJECT_CLASS public_key_class = CKO_PUBLIC_KEY;
 static CK_OBJECT_CLASS extension_class = CKO_X_CERTIFICATE_EXTENSION;
 static CK_CERTIFICATE_TYPE x509_type = CKC_X_509;
 static CK_BBOOL truev = CK_TRUE;
@@ -188,6 +189,8 @@ static CK_ATTRIBUTE cacert3_trusted[] = {
 	{ CKA_CERTIFICATE_TYPE, &x509_type, sizeof (x509_type) },
 	{ CKA_LABEL, "Cacert3 Here", 11 },
 	{ CKA_SUBJECT, (void *)test_cacert3_ca_subject, sizeof (test_cacert3_ca_subject) },
+	{ CKA_ISSUER, (void *)test_cacert3_ca_issuer, sizeof (test_cacert3_ca_issuer) },
+	{ CKA_SERIAL_NUMBER, (void *)test_cacert3_ca_serial, sizeof (test_cacert3_ca_serial) },
 	{ CKA_X_PUBLIC_KEY_INFO, (void *)test_cacert3_ca_public_key, sizeof (test_cacert3_ca_public_key) },
 	{ CKA_TRUSTED, &truev, sizeof (truev) },
 	{ CKA_ID, "ID1", 3 },
@@ -200,6 +203,15 @@ static CK_ATTRIBUTE cacert3_distrusted[] = {
 	{ CKA_CERTIFICATE_TYPE, &x509_type, sizeof (x509_type) },
 	{ CKA_LABEL, "Another CaCert", 11 },
 	{ CKA_SUBJECT, (void *)test_cacert3_ca_subject, sizeof (test_cacert3_ca_subject) },
+	{ CKA_ISSUER, (void *)test_cacert3_ca_issuer, sizeof (test_cacert3_ca_issuer) },
+	{ CKA_SERIAL_NUMBER, (void *)test_cacert3_ca_serial, sizeof (test_cacert3_ca_serial) },
+	{ CKA_X_DISTRUSTED, &truev, sizeof (truev) },
+	{ CKA_INVALID },
+};
+
+static CK_ATTRIBUTE cacert3_distrusted_by_key[] = {
+	{ CKA_CLASS, &public_key_class, sizeof (public_key_class) },
+	{ CKA_X_PUBLIC_KEY_INFO, (void *)test_cacert3_ca_public_key, sizeof (test_cacert3_ca_public_key) },
 	{ CKA_X_DISTRUSTED, &truev, sizeof (truev) },
 	{ CKA_INVALID },
 };
@@ -454,9 +466,10 @@ test_distrust_match (void)
 }
 
 static void
-test_anytrust_match (void)
+test_override_by_issuer_serial (void)
 {
 	CK_ATTRIBUTE certificate = { CKA_CLASS, &certificate_class, sizeof (certificate_class) };
+	CK_BBOOL distrusted = CK_FALSE;
 	CK_RV rv;
 
 	mock_module_add_object (MOCK_SLOT_ONE_ID, cacert3_trusted);
@@ -469,6 +482,27 @@ test_anytrust_match (void)
 	rv = p11_kit_iter_next (test.ex.iter);
 	assert_num_eq (CKR_OK, rv);
 
+	assert (p11_attrs_find_bool (test.ex.attrs, CKA_X_DISTRUSTED, &distrusted));
+	assert_num_eq (CK_TRUE, distrusted);
+
+	rv = p11_kit_iter_next (test.ex.iter);
+	assert_num_eq (CKR_CANCEL, rv);
+}
+
+static void
+test_override_by_public_key (void)
+{
+	CK_ATTRIBUTE certificate = { CKA_CLASS, &certificate_class, sizeof (certificate_class) };
+	CK_RV rv;
+
+	mock_module_add_object (MOCK_SLOT_ONE_ID, cacert3_trusted);
+	mock_module_add_object (MOCK_SLOT_ONE_ID, cacert3_distrusted_by_key);
+
+	test.ex.flags =  P11_ENUMERATE_ANCHORS | P11_ENUMERATE_BLACKLIST;
+	p11_kit_iter_add_filter (test.ex.iter, &certificate, 1);
+	p11_enumerate_ready (&test.ex, NULL);
+
+	/* No results returned, because distrust is not a cert */
 	rv = p11_kit_iter_next (test.ex.iter);
 	assert_num_eq (CKR_CANCEL, rv);
 }
@@ -495,7 +529,8 @@ main (int argc,
 	p11_test (test_duplicate_distrusted, "/extract/test-duplicate-distrusted");
 	p11_test (test_trusted_match, "/extract/test_trusted_match");
 	p11_test (test_distrust_match, "/extract/test_distrust_match");
-	p11_test (test_anytrust_match, "/extract/test_anytrust_match");
+	p11_test (test_override_by_issuer_serial, "/extract/override-by-issuer-and-serial");
+	p11_test (test_override_by_public_key, "/extract/override-by-public-key");
 
 	return p11_test_run (argc, argv);
 }