trust: Certificate CKA_ID is SubjectKeyIdentifier if possible

The PKCS#11 spec states that the CKA_ID should match the
SubjectKeyIdentifier if such an extension is present.

We delay the filling of CKA_ID until the builder phase of populating
attributes which allows us to have more control over how this works.

Note that we don't make CKA_ID reflect SubjectKeyIdentifier *attached*
extensions. The CKA_ID isn't supposed to change after object creation.
Making it dependent on attached extensions would be making promises
we cannot keep, since attached extensions can be added/removed at any
time.

This also means the CKA_ID of attached extensions and certificates
won't necessarily match up, but that was never promised, and not how
attached extensions should be matched to their certificate anyway.

Based on a patch and research done by David Woodhouse.

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

diff --git a/trust/builder.c b/trust/builder.c
index 4c62fac683b0d9bb46d349a181410534c68dd9f7..6e8a1e435bad38d25d9abca52b5111dc555fe7ce 100644 (file)
--- a/trust/builder.c
+++ b/trust/builder.c
@@ -610,13 +610,18 @@ calc_certificate_category (p11_builder *builder,
 }
 
 static CK_ATTRIBUTE *
-certificate_value_attrs (CK_ATTRIBUTE *attrs,
+certificate_value_attrs (p11_builder *builder,
+                         CK_ATTRIBUTE *attrs,
                          node_asn *node,
                          const unsigned char *der,
                          size_t der_len,
                          CK_ATTRIBUTE *public_key)
 {
        unsigned char checksum[P11_DIGEST_SHA1_LEN];
+       unsigned char *keyid = NULL;
+       size_t keyid_len;
+       unsigned char *ext = NULL;
+       size_t ext_len;
        CK_BBOOL falsev = CK_FALSE;
        CK_ULONG zero = 0UL;
        CK_BYTE checkv[3];
@@ -637,7 +642,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) };
+       CK_ATTRIBUTE id = { CKA_ID, NULL, 0 };
 
        return_val_if_fail (attrs != NULL, NULL);
 
@@ -660,9 +665,23 @@ 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)) {
+       /* Try to build a keyid from an extension */
+       if (node) {
+               ext = p11_x509_find_extension (node, P11_OID_SUBJECT_KEY_IDENTIFIER, der, der_len, &ext_len);
+               if (ext) {
+                       keyid = p11_x509_parse_subject_key_identifier (builder->asn1_defs, ext,
+                                                                      ext_len, &keyid_len);
+                       id.pValue = keyid;
+                       id.ulValueLen = keyid_len;
+               }
+       }
+
+       if (!node || !p11_x509_hash_subject_public_key (node, der, der_len, checksum))
                hash_of_subject_public_key.ulValueLen = 0;
-               id.type = CKA_INVALID;
+
+       if (id.pValue == NULL) {
+               id.pValue = hash_of_subject_public_key.pValue;
+               id.ulValueLen = hash_of_subject_public_key.ulValueLen;
        }
 
        if (node) {
@@ -690,6 +709,8 @@ certificate_value_attrs (CK_ATTRIBUTE *attrs,
                                 NULL);
        return_val_if_fail (attrs != NULL, NULL);
 
+       free (ext);
+       free (keyid);
        free (labelv);
        return attrs;
 }
@@ -716,7 +737,7 @@ certificate_populate (p11_builder *builder,
        if (der != NULL)
                node = decode_or_get_asn1 (builder, "PKIX1.Certificate", der, der_len);
 
-       attrs = certificate_value_attrs (attrs, node, der, der_len, &public_key);
+       attrs = certificate_value_attrs (builder, attrs, node, der, der_len, &public_key);
        return_val_if_fail (attrs != NULL, NULL);
 
        if (!calc_certificate_category (builder, index, cert, &public_key, &categoryv))
@@ -794,8 +815,11 @@ extension_populate (p11_builder *builder,
                     p11_index *index,
                     CK_ATTRIBUTE *extension)
 {
-       CK_ATTRIBUTE object_id = { CKA_OBJECT_ID };
+       unsigned char checksum[P11_DIGEST_SHA1_LEN];
+       CK_ATTRIBUTE object_id = { CKA_INVALID };
+       CK_ATTRIBUTE id = { CKA_INVALID };
        CK_ATTRIBUTE *attrs = NULL;
+
        void *der;
        size_t len;
        node_asn *asn;
@@ -803,6 +827,16 @@ extension_populate (p11_builder *builder,
        attrs = common_populate (builder, index, extension);
        return_val_if_fail (attrs != NULL, NULL);
 
+       if (!p11_attrs_find_valid (attrs, CKA_ID)) {
+               der = p11_attrs_find_value (extension, CKA_PUBLIC_KEY_INFO, &len);
+               return_val_if_fail (der != NULL, NULL);
+
+               p11_digest_sha1 (checksum, der, len, NULL);
+               id.pValue = checksum;
+               id.ulValueLen = sizeof (checksum);
+               id.type = CKA_ID;
+       }
+
        /* Pull the object id out of the extension if not present */
        if (!p11_attrs_find_valid (attrs, CKA_OBJECT_ID)) {
                der = p11_attrs_find_value (extension, CKA_VALUE, &len);
@@ -811,12 +845,13 @@ extension_populate (p11_builder *builder,
                asn = decode_or_get_asn1 (builder, "PKIX1.Extension", der, len);
                return_val_if_fail (asn != NULL, NULL);
 
-               if (calc_element (asn, der, len, "extnID", &object_id)) {
-                       attrs = p11_attrs_build (attrs, &object_id, NULL);
-                       return_val_if_fail (attrs != NULL, NULL);
-               }
+               if (calc_element (asn, der, len, "extnID", &object_id))
+                       object_id.type = CKA_OBJECT_ID;
        }
 
+       attrs = p11_attrs_build (attrs, &object_id, &id, NULL);
+       return_val_if_fail (attrs != NULL, NULL);
+
        return attrs;
 }
 

diff --git a/trust/parser.c b/trust/parser.c
index 7f523e9cf27086197b8867acce8620c90fbd02c8..7b569d91641337c01c329d48904adaccd045741e 100644 (file)
--- a/trust/parser.c
+++ b/trust/parser.c
@@ -152,7 +152,6 @@ sink_object (p11_parser *parser,
 
 static CK_ATTRIBUTE *
 certificate_attrs (p11_parser *parser,
-                   CK_ATTRIBUTE *id,
                    const unsigned char *der,
                    size_t der_len)
 {
@@ -165,7 +164,7 @@ certificate_attrs (p11_parser *parser,
        CK_ATTRIBUTE certificate_type = { CKA_CERTIFICATE_TYPE, &x509, sizeof (x509) };
        CK_ATTRIBUTE value = { CKA_VALUE, (void *)der, der_len };
 
-       return p11_attrs_build (NULL, &klass, &modifiable, &certificate_type, &value, id, NULL);
+       return p11_attrs_build (NULL, &klass, &modifiable, &certificate_type, &value, NULL);
 }
 
 int
@@ -174,8 +173,6 @@ p11_parser_format_x509 (p11_parser *parser,
                         size_t length)
 {
        char message[ASN1_MAX_ERROR_DESCRIPTION_SIZE];
-       CK_BYTE idv[ID_LENGTH];
-       CK_ATTRIBUTE id = { CKA_ID, idv, sizeof (idv) };
        CK_ATTRIBUTE *attrs;
        CK_ATTRIBUTE *value;
        node_asn *cert;
@@ -184,11 +181,7 @@ p11_parser_format_x509 (p11_parser *parser,
        if (cert == NULL)
                return P11_PARSE_UNRECOGNIZED;
 
-       /* The CKA_ID links related objects */
-       if (!p11_x509_calc_keyid (cert, data, length, idv))
-               id.type = CKA_INVALID;
-
-       attrs = certificate_attrs (parser, &id, data, length);
+       attrs = certificate_attrs (parser, data, length);
        return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
 
        value = p11_attrs_find_valid (attrs, CKA_VALUE);
@@ -202,7 +195,6 @@ p11_parser_format_x509 (p11_parser *parser,
 
 static CK_ATTRIBUTE *
 extension_attrs (p11_parser *parser,
-                 CK_ATTRIBUTE *id,
                  CK_ATTRIBUTE *public_key_info,
                  const char *oid_str,
                  const unsigned char *oid_der,
@@ -223,7 +215,7 @@ extension_attrs (p11_parser *parser,
        size_t len;
        int ret;
 
-       attrs = p11_attrs_build (NULL, id, public_key_info, &klass, &modifiable, &oid, NULL);
+       attrs = p11_attrs_build (NULL, public_key_info, &klass, &modifiable, &oid, NULL);
        return_val_if_fail (attrs != NULL, NULL);
 
        dest = p11_asn1_create (parser->asn1_defs, "PKIX1.Extension");
@@ -252,7 +244,6 @@ extension_attrs (p11_parser *parser,
 
 static CK_ATTRIBUTE *
 attached_attrs (p11_parser *parser,
-                CK_ATTRIBUTE *id,
                 CK_ATTRIBUTE *public_key_info,
                 const char *oid_str,
                 const unsigned char *oid_der,
@@ -266,7 +257,7 @@ attached_attrs (p11_parser *parser,
        der = p11_asn1_encode (ext, &len);
        return_val_if_fail (der != NULL, NULL);
 
-       attrs = extension_attrs (parser, id, public_key_info, oid_str, oid_der,
+       attrs = extension_attrs (parser, public_key_info, oid_str, oid_der,
                                 critical, der, len);
        return_val_if_fail (attrs != NULL, NULL);
 
@@ -303,7 +294,6 @@ load_seq_of_oid_str (node_asn *node,
 
 static CK_ATTRIBUTE *
 attached_eku_attrs (p11_parser *parser,
-                    CK_ATTRIBUTE *id,
                     CK_ATTRIBUTE *public_key_info,
                     const char *oid_str,
                     const unsigned char *oid_der,
@@ -353,7 +343,7 @@ attached_eku_attrs (p11_parser *parser,
        }
 
 
-       attrs = attached_attrs (parser, id, public_key_info, oid_str, oid_der, critical, dest);
+       attrs = attached_attrs (parser, public_key_info, oid_str, oid_der, critical, dest);
        asn1_delete_structure (&dest);
 
        return attrs;
@@ -362,7 +352,6 @@ attached_eku_attrs (p11_parser *parser,
 static CK_ATTRIBUTE *
 build_openssl_extensions (p11_parser *parser,
                           CK_ATTRIBUTE *cert,
-                          CK_ATTRIBUTE *id,
                           CK_ATTRIBUTE *public_key_info,
                           node_asn *aux,
                           const unsigned char *aux_der,
@@ -416,7 +405,7 @@ build_openssl_extensions (p11_parser *parser,
         */
 
        if (trust) {
-               attrs = attached_eku_attrs (parser, id, public_key_info,
+               attrs = attached_eku_attrs (parser, public_key_info,
                                            P11_OID_EXTENDED_KEY_USAGE_STR,
                                            P11_OID_EXTENDED_KEY_USAGE,
                                            true, trust);
@@ -433,7 +422,7 @@ build_openssl_extensions (p11_parser *parser,
         */
 
        if (reject && p11_dict_size (reject) > 0) {
-               attrs = attached_eku_attrs (parser, id, public_key_info,
+               attrs = attached_eku_attrs (parser, public_key_info,
                                            P11_OID_OPENSSL_REJECT_STR,
                                            P11_OID_OPENSSL_REJECT,
                                            false, reject);
@@ -482,7 +471,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, id, public_key_info,
+               attrs = extension_attrs (parser, public_key_info,
                                         P11_OID_SUBJECT_KEY_IDENTIFIER_STR,
                                         P11_OID_SUBJECT_KEY_IDENTIFIER,
                                         false, aux_der + start, (end - start) + 1);
@@ -501,8 +490,6 @@ parse_openssl_trusted_certificate (p11_parser *parser,
 {
        char message[ASN1_MAX_ERROR_DESCRIPTION_SIZE];
        CK_ATTRIBUTE *attrs;
-       CK_BYTE idv[ID_LENGTH];
-       CK_ATTRIBUTE id = { CKA_ID, idv, sizeof (idv) };
        CK_ATTRIBUTE public_key_info = { CKA_PUBLIC_KEY_INFO };
        CK_ATTRIBUTE *value;
        char *label = NULL;
@@ -539,11 +526,7 @@ parse_openssl_trusted_certificate (p11_parser *parser,
                }
        }
 
-       /* The CKA_ID links related objects */
-       if (!p11_x509_calc_keyid (cert, data, cert_len, idv))
-               id.type = CKA_INVALID;
-
-       attrs = certificate_attrs (parser, &id, data, cert_len);
+       attrs = certificate_attrs (parser, data, cert_len);
        return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
 
        /* Cache the parsed certificate ASN.1 for later use by the builder */
@@ -570,7 +553,7 @@ parse_openssl_trusted_certificate (p11_parser *parser,
                        return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
                }
 
-               attrs = build_openssl_extensions (parser, attrs, &id, &public_key_info, aux,
+               attrs = build_openssl_extensions (parser, attrs, &public_key_info, aux,
                                                  data + cert_len, length - cert_len);
                return_val_if_fail (attrs != NULL, P11_PARSE_FAILURE);
        }

diff --git a/trust/test-builder.c b/trust/test-builder.c
index bf1eed1eeb73defb565b55702327eacdbfdd37cc..5f4b8239d13176c46aa0976c47a0e14b30c628bc 100644 (file)
--- a/trust/test-builder.c
+++ b/trust/test-builder.c
@@ -160,7 +160,7 @@ test_build_certificate (void)
                { 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_ID, "u\xa8q`L\x88\x13\xf0x\xd9\x89w\xb5m\xc5\x89\xdf\xbc\xb1z", 20},
                { CKA_INVALID },
        };
 

diff --git a/trust/test-parser.c b/trust/test-parser.c
index 201ed81d965839febc6e354c52728f25c7810f3a..b5c25254c6d534b7d69698bf881bb66b9a5e98ac 100644 (file)
--- a/trust/test-parser.c
+++ b/trust/test-parser.c
@@ -247,7 +247,6 @@ test_parse_openssl_trusted (void)
                assert_ptr_not_null (object);
 
                test_check_attrs (expected[i], object);
-               test_check_id (cert, object);
        }
 }
 
@@ -329,7 +328,6 @@ test_parse_openssl_distrusted (void)
                assert_ptr_not_null (object);
 
                test_check_attrs (expected[i], object);
-               test_check_id (cert, object);
        }
 }
 

diff --git a/trust/test-trust.c b/trust/test-trust.c
index 20306e0c5c2cfe9702a164d1838e1e5e73f764fd..802007d087f510d7dc6eb6d78aa1a3ba0108e480 100644 (file)
--- a/trust/test-trust.c
+++ b/trust/test-trust.c
@@ -131,6 +131,8 @@ test_check_attrs_msg (const char *file,
        CK_OBJECT_CLASS klass;
        CK_ATTRIBUTE *attr;
 
+       assert (expected != NULL);
+
        if (!p11_attrs_find_ulong (expected, CKA_CLASS, &klass))
                klass = CKA_INVALID;
 

diff --git a/trust/x509.c b/trust/x509.c
index b93d26c6c07f680dd9fd599f69c60dc75682a606..3b4fb2d3fb455cb37fcea259ff8c85114965c7b8 100644 (file)
--- a/trust/x509.c
+++ b/trust/x509.c
@@ -92,10 +92,10 @@ p11_x509_find_extension (node_asn *cert,
 }
 
 bool
-p11_x509_calc_keyid (node_asn *cert,
-                     const unsigned char *der,
-                     size_t der_len,
-                     unsigned char *keyid)
+p11_x509_hash_subject_public_key (node_asn *cert,
+                                  const unsigned char *der,
+                                  size_t der_len,
+                                  unsigned char *keyid)
 {
        int start, end;
        size_t len;
@@ -103,7 +103,6 @@ p11_x509_calc_keyid (node_asn *cert,
 
        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);
@@ -114,6 +113,29 @@ p11_x509_calc_keyid (node_asn *cert,
        return true;
 }
 
+unsigned char *
+p11_x509_parse_subject_key_identifier  (p11_dict *asn1_defs,
+                                        const unsigned char *ext_der,
+                                        size_t ext_len,
+                                        size_t *keyid_len)
+{
+       unsigned char *keyid;
+       node_asn *ext;
+
+       return_val_if_fail (keyid_len != NULL, false);
+
+       ext = p11_asn1_decode (asn1_defs, "PKIX1.SubjectKeyIdentifier", ext_der, ext_len, NULL);
+       if (ext == NULL)
+               return NULL;
+
+       keyid = p11_asn1_read (ext, "", keyid_len);
+       return_val_if_fail (keyid != NULL, NULL);
+
+       asn1_delete_structure (&ext);
+
+       return keyid;
+}
+
 bool
 p11_x509_parse_basic_constraints (p11_dict *asn1_defs,
                                   const unsigned char *ext_der,

diff --git a/trust/x509.h b/trust/x509.h
index af91c286df0906abe44c3514c28f9e7e350384c2..45fa628044120a5969b408f14b6355ea5e4f97be 100644 (file)
--- a/trust/x509.h
+++ b/trust/x509.h
@@ -46,7 +46,7 @@ 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,
+bool             p11_x509_hash_subject_public_key   (node_asn *cert,
                                                      const unsigned char *der,
                                                      size_t der_len,
                                                      unsigned char *keyid);
@@ -65,6 +65,11 @@ p11_array *      p11_x509_parse_extended_key_usage  (p11_dict *asn1_defs,
                                                      const unsigned char *ext_der,
                                                      size_t ext_len);
 
+unsigned char *  p11_x509_parse_subject_key_identifier  (p11_dict *asn1_defs,
+                                                         const unsigned char *ext_der,
+                                                         size_t ext_len,
+                                                         size_t *keyid_len);
+
 char *           p11_x509_parse_dn_name             (p11_dict *asn_defs,
                                                      const unsigned char *der,
                                                      size_t der_len,