trust: Use a SHA-1 hash of subjectPublicKeyInfo as CKA_ID by default

This is what's recommended by the spec, and allows stapled extensions
to hang off a predictable CKA_ID.

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

diff --git a/common/x509.c b/common/x509.c
index f86d2b3498ad4d1c33e78322bc820b51eeb1874b..ae1c8101964b7ea5ccb0f4c5e918a9b56f79afe9 100644 (file)
--- a/common/x509.c
+++ b/common/x509.c
@@ -36,6 +36,7 @@
 
 #include "asn1.h"
 #define P11_DEBUG_FLAG P11_DEBUG_TRUST
+#include "checksum.h"
 #include "debug.h"
 #include "oid.h"
 #include "utf8.h"
@@ -103,6 +104,27 @@ p11_x509_find_extension (node_asn *cert,
        return NULL;
 }
 
+bool
+p11_x509_calc_keyid (node_asn *cert,
+                     const unsigned char *der,
+                     size_t der_len,
+                     unsigned char *keyid)
+{
+       int start, end;
+       int ret;
+
+       return_val_if_fail (cert != NULL, NULL);
+       return_val_if_fail (der != NULL, NULL);
+       return_val_if_fail (keyid != NULL, NULL);
+
+       ret = asn1_der_decoding_startEnd (cert, der, der_len, "tbsCertificate.subjectPublicKeyInfo", &start, &end);
+       return_val_if_fail (ret == ASN1_SUCCESS, false);
+       return_val_if_fail (end >= start, false);
+
+       p11_checksum_sha1 (keyid, (der + start), (end - start) + 1, NULL);
+       return true;
+}
+
 bool
 p11_x509_parse_basic_constraints (p11_dict *asn1_defs,
                                   const unsigned char *ext_der,

diff --git a/common/x509.h b/common/x509.h
index cbfc574f74f3d601be738c6f90fd8de9f86e953c..af91c286df0906abe44c3514c28f9e7e350384c2 100644 (file)
--- a/common/x509.h
+++ b/common/x509.h
@@ -46,6 +46,11 @@ unsigned char *  p11_x509_find_extension            (node_asn *cert,
                                                      size_t der_len,
                                                      size_t *ext_len);
 
+bool             p11_x509_calc_keyid                (node_asn *cert,
+                                                     const unsigned char *der,
+                                                     size_t der_len,
+                                                     unsigned char *keyid);
+
 bool             p11_x509_parse_basic_constraints   (p11_dict *asn1_defs,
                                                      const unsigned char *ext_der,
                                                      size_t ext_len,

diff --git a/trust/builder.c b/trust/builder.c
index 4397b9b4ce8ee833e396e3876bb4179891dbff3d..33221571920fe48ce8bd5b9f2e7472cbe9cff6b8 100644 (file)
--- a/trust/builder.c
+++ b/trust/builder.c
@@ -124,7 +124,7 @@ lookup_extension (p11_builder *builder,
 
        /* Look for a stapled certificate extension */
        id = p11_attrs_find (cert, CKA_ID);
-       if (id != NULL) {
+       if (id != NULL && id->ulValueLen > 0) {
                match[0].pValue = id->pValue;
                match[0].ulValueLen = id->ulValueLen;
 
@@ -417,6 +417,7 @@ certificate_value_attrs (CK_ATTRIBUTE *attrs,
                          const unsigned char *der,
                          size_t der_len)
 {
+       unsigned char checksum[P11_CHECKSUM_SHA1_LENGTH];
        CK_BBOOL falsev = CK_FALSE;
        CK_ULONG zero = 0UL;
        CK_BYTE checkv[3];
@@ -427,7 +428,7 @@ certificate_value_attrs (CK_ATTRIBUTE *attrs,
        CK_ATTRIBUTE trusted = { CKA_TRUSTED, &falsev, sizeof (falsev) };
        CK_ATTRIBUTE distrusted = { CKA_X_DISTRUSTED, &falsev, sizeof (falsev) };
        CK_ATTRIBUTE url = { CKA_URL, "", 0 };
-       CK_ATTRIBUTE hash_of_subject_public_key = { CKA_HASH_OF_SUBJECT_PUBLIC_KEY, "", 0 };
+       CK_ATTRIBUTE hash_of_subject_public_key = { CKA_HASH_OF_SUBJECT_PUBLIC_KEY, checksum, sizeof (checksum) };
        CK_ATTRIBUTE hash_of_issuer_public_key = { CKA_HASH_OF_ISSUER_PUBLIC_KEY, "", 0 };
        CK_ATTRIBUTE java_midp_security_domain = { CKA_JAVA_MIDP_SECURITY_DOMAIN, &zero, sizeof (zero) };
        CK_ATTRIBUTE check_value = { CKA_CHECK_VALUE, &checkv, sizeof (checkv) };
@@ -437,6 +438,7 @@ certificate_value_attrs (CK_ATTRIBUTE *attrs,
        CK_ATTRIBUTE issuer = { CKA_ISSUER, "", 0 };
        CK_ATTRIBUTE serial_number = { CKA_SERIAL_NUMBER, "", 0 };
        CK_ATTRIBUTE label = { CKA_LABEL };
+       CK_ATTRIBUTE id = { CKA_ID, checksum, sizeof (checksum) };
 
        return_val_if_fail (attrs != NULL, NULL);
 
@@ -455,6 +457,11 @@ certificate_value_attrs (CK_ATTRIBUTE *attrs,
                subject.type = CKA_INVALID;
        calc_element (node, der, der_len, "tbsCertificate.serialNumber", &serial_number);
 
+       if (!node || !p11_x509_calc_keyid (node, der, der_len, checksum)) {
+               hash_of_subject_public_key.ulValueLen = 0;
+               id.type = CKA_INVALID;
+       }
+
        if (node) {
                labelv = p11_x509_lookup_dn_name (node, "tbsCertificate.subject",
                                                  der, der_len, P11_OID_CN);
@@ -475,11 +482,12 @@ certificate_value_attrs (CK_ATTRIBUTE *attrs,
 
        attrs = p11_attrs_build (attrs, &trusted, &distrusted, &url, &hash_of_issuer_public_key,
                                 &hash_of_subject_public_key, &java_midp_security_domain,
-                                &check_value, &start_date, &end_date,
+                                &check_value, &start_date, &end_date, &id,
                                 &subject, &issuer, &serial_number, &label,
                                 NULL);
        return_val_if_fail (attrs != NULL, NULL);
 
+       free (labelv);
        return attrs;
 }
 
@@ -1405,7 +1413,7 @@ replace_compat_for_cert (p11_builder *builder,
 
        value = p11_attrs_find (attrs, CKA_VALUE);
        id = p11_attrs_find (attrs, CKA_ID);
-       if (value == NULL || id == NULL)
+       if (value == NULL || id == NULL || id->ulValueLen == 0)
                return;
 
        /*
@@ -1439,7 +1447,7 @@ replace_compat_for_ext (p11_builder *builder,
        int i;
 
        id = p11_attrs_find (attrs, CKA_ID);
-       if (id == NULL)
+       if (id == NULL || id->ulValueLen == 0)
                return;
 
        handles = lookup_related (index, CKO_CERTIFICATE, id);
@@ -1470,7 +1478,7 @@ update_related_category (p11_builder *builder,
        };
 
        id = p11_attrs_find (attrs, CKA_ID);
-       if (id == NULL)
+       if (id == NULL || id->ulValueLen == 0)
                return;
 
        /* Find all other objects with this handle */

diff --git a/trust/parser.c b/trust/parser.c
index 978d30eb1f756062e9544a61e094e9bd4dd4f074..0ab9468a41c4c8e454a43d095e898138e13be466 100644 (file)
--- a/trust/parser.c
+++ b/trust/parser.c
@@ -146,17 +146,9 @@ sink_object (p11_parser *parser,
                p11_message ("couldn't load file into objects: %s", parser->basename);
 }
 
-static void
-id_generate (p11_parser *parser,
-             CK_BYTE *vid)
-{
-       CK_ULONG val = p11_module_next_id ();
-       p11_checksum_sha1 (vid, &val, sizeof (val), NULL);
-}
-
 static CK_ATTRIBUTE *
 certificate_attrs (p11_parser *parser,
-                   CK_BYTE *idv,
+                   CK_ATTRIBUTE *id,
                    const unsigned char *der,
                    size_t der_len)
 {
@@ -168,9 +160,8 @@ certificate_attrs (p11_parser *parser,
        CK_ATTRIBUTE klass = { CKA_CLASS, &klassv, sizeof (klassv) };
        CK_ATTRIBUTE certificate_type = { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) };
        CK_ATTRIBUTE value = { CKA_VALUE, (void *)der, der_len };
-       CK_ATTRIBUTE id = { CKA_ID, idv, ID_LENGTH };
 
-       return p11_attrs_build (NULL, &klass, &modifiable, &certificate_type, &value, &id, NULL);
+       return p11_attrs_build (NULL, &klass, &modifiable, &certificate_type, &value, id, NULL);
 }
 
 static int
@@ -179,6 +170,7 @@ parse_der_x509_certificate (p11_parser *parser,
                             size_t length)
 {
        CK_BYTE idv[ID_LENGTH];
+       CK_ATTRIBUTE id = { CKA_ID, idv, sizeof (idv) };
        CK_ATTRIBUTE *attrs;
        CK_ATTRIBUTE *value;
        node_asn *cert;
@@ -188,9 +180,10 @@ parse_der_x509_certificate (p11_parser *parser,
                return P11_PARSE_UNRECOGNIZED;
 
        /* The CKA_ID links related objects */
-       id_generate (parser, idv);
+       if (!p11_x509_calc_keyid (cert, data, length, idv))
+               id.type = CKA_INVALID;
 
-       attrs = certificate_attrs (parser, idv, data, length);
+       attrs = certificate_attrs (parser, &id, data, length);
        return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
 
        value = p11_attrs_find (attrs, CKA_VALUE);
@@ -204,7 +197,7 @@ parse_der_x509_certificate (p11_parser *parser,
 
 static CK_ATTRIBUTE *
 extension_attrs (p11_parser *parser,
-                 CK_BYTE *idv,
+                 CK_ATTRIBUTE *id,
                  const unsigned char *oid_der,
                  CK_BBOOL vcritical,
                  const unsigned char *ext_der,
@@ -218,17 +211,16 @@ extension_attrs (p11_parser *parser,
        CK_ATTRIBUTE critical = { CKA_X_CRITICAL, &vcritical, sizeof (vcritical) };
        CK_ATTRIBUTE oid = { CKA_OBJECT_ID, (void *)oid_der, p11_oid_length (oid_der) };
        CK_ATTRIBUTE value = { CKA_VALUE, (void *)ext_der, ext_len };
-       CK_ATTRIBUTE id = { CKA_ID, idv, ID_LENGTH };
 
        if (ext_der == NULL)
                value.type = CKA_INVALID;
 
-       return p11_attrs_build (NULL, &id, &klass, &modifiable, &oid, &critical, &value, NULL);
+       return p11_attrs_build (NULL, id, &klass, &modifiable, &oid, &critical, &value, NULL);
 }
 
 static CK_ATTRIBUTE *
 stapled_attrs (p11_parser *parser,
-               CK_BYTE *idv,
+               CK_ATTRIBUTE *id,
                const unsigned char *oid,
                CK_BBOOL critical,
                node_asn *ext)
@@ -237,7 +229,7 @@ stapled_attrs (p11_parser *parser,
        unsigned char *der;
        size_t len;
 
-       attrs = extension_attrs (parser, idv, oid, critical, NULL, 0);
+       attrs = extension_attrs (parser, id, oid, critical, NULL, 0);
        return_val_if_fail (attrs != NULL, NULL);
 
        der = p11_asn1_encode (ext, &len);
@@ -285,7 +277,7 @@ load_seq_of_oid_str (node_asn *node,
 
 static CK_ATTRIBUTE *
 stapled_eku_attrs (p11_parser *parser,
-                   CK_BYTE *idv,
+                   CK_ATTRIBUTE *id,
                    const unsigned char *oid,
                    CK_BBOOL critical,
                    p11_dict *oid_strs)
@@ -333,7 +325,7 @@ stapled_eku_attrs (p11_parser *parser,
        }
 
 
-       attrs = stapled_attrs (parser, idv, oid, critical, dest);
+       attrs = stapled_attrs (parser, id, oid, critical, dest);
        asn1_delete_structure (&dest);
 
        return attrs;
@@ -342,7 +334,7 @@ stapled_eku_attrs (p11_parser *parser,
 static CK_ATTRIBUTE *
 build_openssl_extensions (p11_parser *parser,
                           CK_ATTRIBUTE *cert,
-                          CK_BYTE *idv,
+                          CK_ATTRIBUTE *id,
                           node_asn *aux,
                           const unsigned char *aux_der,
                           size_t aux_len)
@@ -395,7 +387,7 @@ build_openssl_extensions (p11_parser *parser,
         */
 
        if (trust) {
-               attrs = stapled_eku_attrs (parser, idv, P11_OID_EXTENDED_KEY_USAGE, CK_TRUE, trust);
+               attrs = stapled_eku_attrs (parser, id, P11_OID_EXTENDED_KEY_USAGE, CK_TRUE, trust);
                return_val_if_fail (attrs != NULL, NULL);
                sink_object (parser, attrs);
        }
@@ -409,7 +401,7 @@ build_openssl_extensions (p11_parser *parser,
         */
 
        if (reject && p11_dict_size (reject) > 0) {
-               attrs = stapled_eku_attrs (parser, idv, P11_OID_OPENSSL_REJECT, CK_FALSE, reject);
+               attrs = stapled_eku_attrs (parser, id, P11_OID_OPENSSL_REJECT, CK_FALSE, reject);
                return_val_if_fail (attrs != NULL, NULL);
                sink_object (parser, attrs);
        }
@@ -455,7 +447,7 @@ build_openssl_extensions (p11_parser *parser,
        return_val_if_fail (ret == ASN1_SUCCESS || ret == ASN1_ELEMENT_NOT_FOUND, NULL);
 
        if (ret == ASN1_SUCCESS) {
-               attrs = extension_attrs (parser, idv, P11_OID_SUBJECT_KEY_IDENTIFIER, CK_FALSE,
+               attrs = extension_attrs (parser, id, P11_OID_SUBJECT_KEY_IDENTIFIER, CK_FALSE,
                                         aux_der + start, (end - start) + 1);
                return_val_if_fail (attrs != NULL, NULL);
                sink_object (parser, attrs);
@@ -472,6 +464,7 @@ parse_openssl_trusted_certificate (p11_parser *parser,
 {
        CK_ATTRIBUTE *attrs;
        CK_BYTE idv[ID_LENGTH];
+       CK_ATTRIBUTE id = { CKA_ID, idv, sizeof (idv) };
        CK_ATTRIBUTE *value;
        char *label = NULL;
        node_asn *cert;
@@ -502,9 +495,10 @@ parse_openssl_trusted_certificate (p11_parser *parser,
        }
 
        /* The CKA_ID links related objects */
-       id_generate (parser, idv);
+       if (!p11_x509_calc_keyid (cert, data, cert_len, idv))
+               id.type = CKA_INVALID;
 
-       attrs = certificate_attrs (parser, idv, data, cert_len);
+       attrs = certificate_attrs (parser, &id, data, cert_len);
        return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
 
        /* Cache the parsed certificate ASN.1 for later use by the builder */
@@ -526,7 +520,7 @@ parse_openssl_trusted_certificate (p11_parser *parser,
                return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
        }
 
-       attrs = build_openssl_extensions (parser, attrs, idv, aux,
+       attrs = build_openssl_extensions (parser, attrs, &id, aux,
                                          data + cert_len, length - cert_len);
        return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
 

diff --git a/trust/tests/files/verisign-v1.der b/trust/tests/files/verisign-v1.der
new file mode 100644 (file)
index 0000000..bcd5ebb
Binary files /dev/null and b/trust/tests/files/verisign-v1.der differ

diff --git a/trust/tests/test-builder.c b/trust/tests/test-builder.c
index 8ffab88bca9447f9f3dcef689088054400ed91f7..f8797067242760685da0e0cd32e500351bc08e52 100644 (file)
--- a/trust/tests/test-builder.c
+++ b/trust/tests/test-builder.c
@@ -152,6 +152,21 @@ test_build_certificate (CuTest *cu)
                { CKA_INVALID },
        };
 
+       CK_ATTRIBUTE expected[] = {
+               { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) },
+               { CKA_CERTIFICATE_CATEGORY, &certificate_authority, sizeof (certificate_authority) },
+               { CKA_VALUE, (void *)test_cacert3_ca_der, sizeof (test_cacert3_ca_der) },
+               { CKA_CHECK_VALUE, "\xad\x7c\x3f", 3 },
+               { CKA_START_DATE, "20110523", 8 },
+               { CKA_END_DATE, "20210520", 8, },
+               { 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_LABEL, "the label", 9 },
+               { CKA_ID, "\xf0""a\xd8?\x95\x8fMx\xb1G\xb3\x13""9\x97\x8e\xa9\xc2Q\xba\x9b", 20},
+               { CKA_INVALID },
+       };
+
        CK_ATTRIBUTE *attrs;
        CK_ATTRIBUTE *merge;
        CK_RV rv;
@@ -163,7 +178,7 @@ test_build_certificate (CuTest *cu)
        rv = p11_builder_build (test.builder, test.index, &attrs, merge);
        CuAssertIntEquals (cu, CKR_OK, rv);
 
-       test_check_cacert3_ca (cu, attrs, "the label");
+       test_check_attrs (cu, expected, attrs);
        p11_attrs_free (attrs);
 
        teardown (cu);
@@ -1406,6 +1421,7 @@ test_changed_without_id (CuTest *cu)
                { CKA_CERTIFICATE_CATEGORY, &certificate_authority, sizeof (certificate_authority) },
                { CKA_VALUE, (void *)test_cacert3_ca_der, sizeof (test_cacert3_ca_der) },
                { CKA_TRUSTED, &truev, sizeof (truev) },
+               { CKA_ID, NULL, 0, },
                { CKA_INVALID },
        };
 

diff --git a/trust/tests/test-module.c b/trust/tests/test-module.c
index 9c633f0b38e4e8b4baa09603c3c7bdfa07fba250..772dc8a809ba03145ee0100cbfb53e91098b5cb3 100644 (file)
--- a/trust/tests/test-module.c
+++ b/trust/tests/test-module.c
@@ -82,7 +82,7 @@ setup (CuTest *cu)
        CuAssertTrue (cu, rv == CKR_OK);
 
        memset (&args, 0, sizeof (args));
-       paths = SRCDIR "/input:" SRCDIR "/files/cacert-ca.der:" SRCDIR "/files/testing-server.der";
+       paths = SRCDIR "/input:" SRCDIR "/files/self-signed-with-ku.der:" SRCDIR "/files/thawte.pem";
        if (asprintf (&arguments, "paths='%s'", paths) < 0)
                CuAssertTrue (cu, false && "not reached");
        args.pReserved = arguments;
@@ -154,8 +154,8 @@ test_get_slot_info (CuTest *cu)
        /* These are the paths passed in in setup() */
        const char *paths[] = {
                SRCDIR "/input",
-               SRCDIR "/files/cacert-ca.der",
-               SRCDIR "/files/testing-server.der"
+               SRCDIR "/files/self-signed-with-ku.der",
+               SRCDIR "/files/thawte.pem"
        };
 
        setup (cu);
@@ -191,8 +191,8 @@ test_get_token_info (CuTest *cu)
        /* These are the paths passed in in setup() */
        const char *labels[] = {
                "input",
-               "cacert-ca.der",
-               "testing-server.der"
+               "self-signed-with-ku.der",
+               "thawte.pem"
        };
 
        setup (cu);

diff --git a/trust/tests/test-parser.c b/trust/tests/test-parser.c
index 3ad89da7a6be4798daa0521fcfee3edd3b4770a7..a63d7a51dcc243fb3fdd98e486f2630b6a503db8 100644 (file)
--- a/trust/tests/test-parser.c
+++ b/trust/tests/test-parser.c
@@ -339,6 +339,36 @@ test_parse_anchor (CuTest *cu)
        teardown (cu);
 }
 
+static void
+test_parse_thawte (CuTest *cu)
+{
+       CK_ATTRIBUTE *cert;
+       int ret;
+
+       CK_ATTRIBUTE expected[] = {
+               { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) },
+               { CKA_CLASS, &certificate, sizeof (certificate) },
+               { CKA_MODIFIABLE, &falsev, sizeof (falsev) },
+               { CKA_TRUSTED, &falsev, sizeof (falsev) },
+               { CKA_X_DISTRUSTED, &falsev, sizeof (falsev) },
+               { CKA_INVALID },
+       };
+
+       setup (cu);
+
+       ret = p11_parse_file (test.parser, SRCDIR "/files/thawte.pem",
+                             P11_PARSE_FLAG_NONE);
+       CuAssertIntEquals (cu, P11_PARSE_SUCCESS, ret);
+
+       /* Should have gotten certificate  */
+       CuAssertIntEquals (cu, 1, p11_index_size (test.index));
+
+       cert = parsed_attrs (certificate_match);
+       test_check_attrs (cu, expected, cert);
+
+       teardown (cu);
+}
+
 /* TODO: A certificate that uses generalTime needs testing */
 
 static void
@@ -393,6 +423,7 @@ main (void)
        SUITE_ADD_TEST (suite, test_parse_openssl_trusted);
        SUITE_ADD_TEST (suite, test_parse_openssl_distrusted);
        SUITE_ADD_TEST (suite, test_parse_anchor);
+       SUITE_ADD_TEST (suite, test_parse_thawte);
        SUITE_ADD_TEST (suite, test_parse_invalid_file);
        SUITE_ADD_TEST (suite, test_parse_unrecognized);