From c11d5933fc08935e07f8109a186cf5f8c5ef17a2 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Thu, 9 Oct 2014 08:15:29 +0200 Subject: [PATCH] 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 --- trust/builder.c | 55 ++++++++++++++++++++++++++++++++++++++++++---------- trust/parser.c | 37 ++++++++++------------------------- trust/test-builder.c | 2 +- trust/test-parser.c | 2 -- trust/test-trust.c | 2 ++ trust/x509.c | 32 +++++++++++++++++++++++++----- trust/x509.h | 7 ++++++- 7 files changed, 91 insertions(+), 46 deletions(-) diff --git a/trust/builder.c b/trust/builder.c index 4c62fac..6e8a1e4 100644 --- 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 7f523e9..7b569d9 100644 --- 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 bf1eed1..5f4b823 100644 --- 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 201ed81..b5c2525 100644 --- 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 20306e0..802007d 100644 --- 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 b93d26c..3b4fb2d 100644 --- 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 af91c28..45fa628 100644 --- 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, -- 1.9.3