Attachment 91963 Details for Bug 73558 – enumerate: Preload and respect blacklist across all tokens

[patch] enumerate: Preload and respect blacklist across all tokens

enumerate-Preload-and-respect-blacklist-across-all.patch (text/plain), 12.17 KB, created by Stef Walter on 2014-01-13 17:18:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stef Walter

Created: 2014-01-13 17:18:00 UTC

Size: 12.17 KB

patch

obsolete

>From 635c22f4518200c7e106cdf507a4c89072f8b6ca Mon Sep 17 00:00:00 2001
>From: Stef Walter <stef@thewalter.net>
>Date: Mon, 13 Jan 2014 18:09:20 +0100
>Subject: [PATCH] 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
>---
> trust/enumerate.c            | 196 ++++++++++++++++++++++++++++++++-----------
> trust/enumerate.h            |   3 +
> trust/tests/test-enumerate.c |  39 ++++++++-
> 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);
> }
>-- 
>1.8.4.2

Actions: View | Splinter Review

Attachments on bug 73558: 91961 | 91962 | 91963

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.