summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStef Walter <stefw@gnome.org>2013-03-20 15:53:43 +0100
committerStef Walter <stefw@gnome.org>2013-03-20 16:22:35 +0100
commit0ecabc858dd6c1c2055f53202a01251e2ad7d2c2 (patch)
tree69184001695208399944f3fc5c9cd9b4fd692e98
parente075585ef1cffc988894b4efbf3d14d5e55dcdcc (diff)
downloadp11-kit-0ecabc858dd6c1c2055f53202a01251e2ad7d2c2.tar.gz
trust: Predictable behavior with duplicate certificates in token
If duplicate certificates are present in a token, we warn about this, and don't really recommend it. However we have predictable behavior where blacklist is prefered to anchor is preferred to unknown trust. https://bugs.freedesktop.org/show_bug.cgi?id=62548
-rw-r--r--trust/parser.c94
-rw-r--r--trust/tests/test-parser.c127
-rw-r--r--trust/token.c19
3 files changed, 224 insertions, 16 deletions
diff --git a/trust/parser.c b/trust/parser.c
index 91fd5e8..7ea879a 100644
--- a/trust/parser.c
+++ b/trust/parser.c
@@ -130,20 +130,112 @@ populate_trust (p11_parser *parser,
return p11_attrs_build (attrs, &trusted, &distrust, NULL);
}
+static bool
+lookup_cert_duplicate (p11_index *index,
+ CK_ATTRIBUTE *attrs,
+ CK_OBJECT_HANDLE *handle,
+ CK_ATTRIBUTE **dupl)
+{
+ CK_OBJECT_CLASS klass = CKO_CERTIFICATE;
+ CK_ATTRIBUTE *value;
+
+ CK_ATTRIBUTE match[] = {
+ { CKA_VALUE, },
+ { CKA_CLASS, &klass, sizeof (klass) },
+ { CKA_INVALID },
+ };
+
+ /*
+ * TODO: This will need to be adapted when we support reload on
+ * the fly, but for now since we only load once, we can assume
+ * that any certs already present in the index are duplicates.
+ */
+
+ value = p11_attrs_find_valid (attrs, CKA_VALUE);
+ if (value != NULL) {
+ memcpy (match, value, sizeof (CK_ATTRIBUTE));
+ *handle = p11_index_find (index, match, -1);
+ if (*handle != 0) {
+ *dupl = p11_index_lookup (index, *handle);
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static char *
+pull_cert_label (CK_ATTRIBUTE *attrs)
+{
+ char *label;
+ size_t len;
+
+ label = p11_attrs_find_value (attrs, CKA_LABEL, &len);
+ if (label)
+ label = strndup (label, len);
+
+ return label;
+}
+
+static int
+calc_cert_priority (CK_ATTRIBUTE *attrs)
+{
+ CK_BBOOL boolv;
+
+ enum {
+ PRI_UNKNOWN,
+ PRI_TRUSTED,
+ PRI_DISTRUST
+ };
+
+ if (p11_attrs_find_bool (attrs, CKA_X_DISTRUSTED, &boolv) && boolv)
+ return PRI_DISTRUST;
+ else if (p11_attrs_find_bool (attrs, CKA_TRUSTED, &boolv) && boolv)
+ return PRI_TRUSTED;
+
+ return PRI_UNKNOWN;
+}
+
static void
sink_object (p11_parser *parser,
CK_ATTRIBUTE *attrs)
{
+ CK_OBJECT_HANDLE handle;
CK_OBJECT_CLASS klass;
+ CK_ATTRIBUTE *dupl;
+ char *label;
CK_RV rv;
+ /* By default not replacing anything */
+ handle = 0;
+
if (p11_attrs_find_ulong (attrs, CKA_CLASS, &klass) &&
klass == CKO_CERTIFICATE) {
attrs = populate_trust (parser, attrs);
return_if_fail (attrs != NULL);
+
+ if (lookup_cert_duplicate (parser->index, attrs, &handle, &dupl)) {
+
+ /* This is not a good place to be for a well configured system */
+ label = pull_cert_label (dupl);
+ p11_message ("duplicate '%s' certificate found in: %s",
+ label ? label : "?", parser->basename);
+ free (label);
+
+ /*
+ * Nevertheless we provide predictable behavior about what
+ * overrides what. If we have a lower or equal priority
+ * to what's there, then just go away, otherwise replace.
+ */
+ if (calc_cert_priority (attrs) <= calc_cert_priority (dupl)) {
+ p11_attrs_free (attrs);
+ return;
+ }
+ }
}
- rv = p11_index_take (parser->index, attrs, NULL);
+ /* If handle is zero, this just adds */
+ rv = p11_index_replace (parser->index, handle, attrs);
if (rv != CKR_OK)
p11_message ("couldn't load file into objects: %s", parser->basename);
}
diff --git a/trust/tests/test-parser.c b/trust/tests/test-parser.c
index 4c182a0..94cfc49 100644
--- a/trust/tests/test-parser.c
+++ b/trust/tests/test-parser.c
@@ -437,6 +437,131 @@ test_parse_unrecognized (CuTest *cu)
teardown (cu);
}
+static void
+test_duplicate (CuTest *cu)
+{
+ CK_ATTRIBUTE cacert3[] = {
+ { CKA_CLASS, &certificate, sizeof (certificate) },
+ { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) },
+ { CKA_VALUE, (void *)test_cacert3_ca_der, sizeof (test_cacert3_ca_der) },
+ { CKA_MODIFIABLE, &falsev, sizeof (falsev) },
+ { CKA_TRUSTED, &falsev, sizeof (falsev) },
+ { CKA_X_DISTRUSTED, &falsev, sizeof (falsev) },
+ { CKA_INVALID },
+ };
+
+ CK_OBJECT_HANDLE *handles;
+ CK_ATTRIBUTE *cert;
+ int ret;
+
+ setup (cu);
+
+ ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der", 0);
+ CuAssertIntEquals (cu, P11_PARSE_SUCCESS, ret);
+
+ p11_message_quiet ();
+
+ /* This shouldn't be added, should print a message */
+ ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der", 0);
+ CuAssertIntEquals (cu, P11_PARSE_SUCCESS, ret);
+
+ CuAssertTrue (cu, strstr (p11_message_last (), "duplicate") != NULL);
+
+ p11_message_loud ();
+
+ /* Should only be one certificate since the above two are identical */
+ handles = p11_index_find_all (test.index, cacert3, 2);
+ CuAssertPtrNotNull (cu, handles);
+ CuAssertTrue (cu, handles[0] != 0);
+ CuAssertTrue (cu, handles[1] == 0);
+
+ cert = p11_index_lookup (test.index, handles[0]);
+ test_check_attrs (cu, cacert3, cert);
+
+ free (handles);
+ teardown (cu);
+}
+
+static void
+test_duplicate_priority (CuTest *cu)
+{
+ CK_ATTRIBUTE cacert3[] = {
+ { CKA_CLASS, &certificate, sizeof (certificate) },
+ { CKA_VALUE, (void *)test_cacert3_ca_der, sizeof (test_cacert3_ca_der) },
+ { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) },
+ { CKA_MODIFIABLE, &falsev, sizeof (falsev) },
+ { CKA_INVALID },
+ };
+
+ CK_ATTRIBUTE trusted[] = {
+ { CKA_CLASS, &certificate, sizeof (certificate) },
+ { CKA_VALUE, (void *)test_cacert3_ca_der, sizeof (test_cacert3_ca_der) },
+ { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) },
+ { CKA_TRUSTED, &truev, sizeof (truev) },
+ { CKA_X_DISTRUSTED, &falsev, sizeof (falsev) },
+ { CKA_INVALID },
+ };
+
+ CK_ATTRIBUTE distrust[] = {
+ { CKA_CLASS, &certificate, sizeof (certificate) },
+ { CKA_VALUE, (void *)test_cacert3_ca_der, sizeof (test_cacert3_ca_der) },
+ { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) },
+ { CKA_TRUSTED, &falsev, sizeof (falsev) },
+ { CKA_X_DISTRUSTED, &truev, sizeof (truev) },
+ { CKA_INVALID },
+ };
+
+ CK_OBJECT_HANDLE *handles;
+ CK_ATTRIBUTE *cert;
+ int ret;
+
+ setup (cu);
+
+ ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der", 0);
+ CuAssertIntEquals (cu, P11_PARSE_SUCCESS, ret);
+
+ p11_message_quiet ();
+
+ /* This shouldn't be added, should print a message */
+ ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der",
+ P11_PARSE_FLAG_ANCHOR);
+ CuAssertIntEquals (cu, P11_PARSE_SUCCESS, ret);
+
+ CuAssertTrue (cu, strstr (p11_message_last (), "duplicate") != NULL);
+
+ p11_message_loud ();
+
+ /* We should now find the trusted certificate */
+ handles = p11_index_find_all (test.index, cacert3, 2);
+ CuAssertPtrNotNull (cu, handles);
+ CuAssertTrue (cu, handles[0] != 0);
+ CuAssertTrue (cu, handles[1] == 0);
+ cert = p11_index_lookup (test.index, handles[0]);
+ test_check_attrs (cu, trusted, cert);
+ free (handles);
+
+ /* Now add a distrutsed one, this should override the trusted */
+
+ p11_message_quiet ();
+
+ ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der",
+ P11_PARSE_FLAG_BLACKLIST);
+ CuAssertIntEquals (cu, P11_PARSE_SUCCESS, ret);
+
+ p11_message_loud ();
+
+ /* We should now find the distrusted certificate */
+ handles = p11_index_find_all (test.index, cacert3, 2);
+ CuAssertPtrNotNull (cu, handles);
+ CuAssertTrue (cu, handles[0] != 0);
+ CuAssertTrue (cu, handles[1] == 0);
+ cert = p11_index_lookup (test.index, handles[0]);
+ test_check_attrs (cu, distrust, cert);
+ free (handles);
+
+ teardown (cu);
+}
+
int
main (void)
{
@@ -457,6 +582,8 @@ main (void)
SUITE_ADD_TEST (suite, test_parse_thawte);
SUITE_ADD_TEST (suite, test_parse_invalid_file);
SUITE_ADD_TEST (suite, test_parse_unrecognized);
+ SUITE_ADD_TEST (suite, test_duplicate);
+ SUITE_ADD_TEST (suite, test_duplicate_priority);
CuSuiteRun (suite);
CuSuiteSummary (suite, output);
diff --git a/trust/token.c b/trust/token.c
index e0c2089..b562788 100644
--- a/trust/token.c
+++ b/trust/token.c
@@ -56,8 +56,6 @@
#include <stdlib.h>
#include <string.h>
-#define ELEMS(x) (sizeof (x) / sizeof (x[0]))
-
struct _p11_token {
p11_parser *parser;
p11_index *index;
@@ -68,18 +66,6 @@ struct _p11_token {
int loaded;
};
-static void
-on_parser_object (CK_ATTRIBUTE *attrs,
- void *user_data)
-{
- p11_token *token = user_data;
-
- return_if_fail (attrs != NULL);
-
- if (p11_index_take (token->index, attrs, NULL) != CKR_OK)
- return_if_reached ();
-}
-
static int
loader_load_file (p11_token *token,
const char *filename,
@@ -211,6 +197,7 @@ load_builtin_objects (p11_token *token)
CK_OBJECT_CLASS builtin = CKO_NSS_BUILTIN_ROOT_LIST;
CK_BBOOL vtrue = CK_TRUE;
CK_BBOOL vfalse = CK_FALSE;
+ CK_RV rv;
const char *trust_anchor_roots = "Trust Anchor Roots";
CK_ATTRIBUTE builtin_root_list[] = {
@@ -219,10 +206,12 @@ load_builtin_objects (p11_token *token)
{ CKA_PRIVATE, &vfalse, sizeof (vfalse) },
{ CKA_MODIFIABLE, &vfalse, sizeof (vfalse) },
{ CKA_LABEL, (void *)trust_anchor_roots, strlen (trust_anchor_roots) },
+ { CKA_INVALID },
};
p11_index_batch (token->index);
- on_parser_object (p11_attrs_buildn (NULL, builtin_root_list, ELEMS (builtin_root_list)), token);
+ rv = p11_index_take (token->index, p11_attrs_dup (builtin_root_list), NULL);
+ return_val_if_fail (rv == CKR_OK, 0);
p11_index_finish (token->index);
return 1;
}