From 29af2c1eeca2fb0257e1172753b129d638472f0f Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Fri, 15 Mar 2013 16:24:27 +0100 Subject: 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 --- trust/builder.c | 20 +++++++++++----- trust/parser.c | 48 +++++++++++++++++--------------------- trust/tests/files/verisign-v1.der | Bin 0 -> 576 bytes trust/tests/test-builder.c | 18 +++++++++++++- trust/tests/test-module.c | 10 ++++---- trust/tests/test-parser.c | 31 ++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 39 deletions(-) create mode 100644 trust/tests/files/verisign-v1.der (limited to 'trust') diff --git a/trust/builder.c b/trust/builder.c index 4397b9b..3322157 100644 --- 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 978d30e..0ab9468 100644 --- 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 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 8ffab88..f879706 100644 --- 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 9c633f0..772dc8a 100644 --- 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 3ad89da..a63d7a5 100644 --- 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); -- cgit v1.1