diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index a3673e63fa..d9ce52faa4 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -387,6 +387,7 @@ static int check_sig_alg_match(const EVP_PKEY *pkey, const X509 *subject) /* * Cache info on various X.509v3 extensions and further derived information, * e.g., if cert 'x' is self-issued, in x->ex_flags and other internal fields. + * x->sha1_hash is filled in, or else EXFLAG_NO_FINGERPRINT is set in x->flags. * X509_SIG_INFO_VALID is set in x->flags if x->siginf was filled successfully. * Set EXFLAG_INVALID and return 0 in case the certificate is invalid. */ @@ -411,15 +412,12 @@ int x509v3_cache_extensions(X509 *x) CRYPTO_THREAD_unlock(x->lock); return (x->ex_flags & EXFLAG_INVALID) == 0; } - ERR_set_mark(); /* Cache the SHA1 digest of the cert */ if (!X509_digest(x, EVP_sha1(), x->sha1_hash, NULL)) - /* - * Note that the cert is marked invalid also on internal malloc failure - * or on failure of EVP_MD_fetch(), potentially called by X509_digest(). - */ - x->ex_flags |= EXFLAG_INVALID; + x->ex_flags |= EXFLAG_NO_FINGERPRINT; + + ERR_set_mark(); /* V1 should mean no extensions ... */ if (X509_get_version(x) == 0) @@ -625,11 +623,13 @@ int x509v3_cache_extensions(X509 *x) */ #endif ERR_pop_to_mark(); - if ((x->ex_flags & EXFLAG_INVALID) == 0) { + if ((x->ex_flags & (EXFLAG_INVALID | EXFLAG_NO_FINGERPRINT)) == 0) { CRYPTO_THREAD_unlock(x->lock); return 1; } - ERR_raise(ERR_LIB_X509, X509V3_R_INVALID_CERTIFICATE); + if ((x->ex_flags & EXFLAG_INVALID) != 0) + ERR_raise(ERR_LIB_X509, X509V3_R_INVALID_CERTIFICATE); + /* If computing sha1_hash failed the error queue already reflects this. */ err: x->ex_flags |= EXFLAG_SET; /* indicate that cert has been processed */ diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c index 1231fb4be1..d18d1e2b67 100644 --- a/crypto/x509/x509_cmp.c +++ b/crypto/x509/x509_cmp.c @@ -81,7 +81,13 @@ int X509_CRL_cmp(const X509_CRL *a, const X509_CRL *b) int X509_CRL_match(const X509_CRL *a, const X509_CRL *b) { - int rv = memcmp(a->sha1_hash, b->sha1_hash, 20); + int rv; + + if ((a->flags & EXFLAG_NO_FINGERPRINT) == 0 + && (b->flags & EXFLAG_NO_FINGERPRINT) == 0) + rv = memcmp(a->sha1_hash, b->sha1_hash, SHA_DIGEST_LENGTH); + else + return -2; return rv < 0 ? -1 : rv > 0; } @@ -140,19 +146,21 @@ unsigned long X509_subject_name_hash_old(X509 *x) */ int X509_cmp(const X509 *a, const X509 *b) { - int rv; + int rv = 0; if (a == b) /* for efficiency */ return 0; - /* ensure hash is valid */ - if (X509_check_purpose((X509 *)a, -1, 0) != 1) - return -2; - if (X509_check_purpose((X509 *)b, -1, 0) != 1) - return -2; - rv = memcmp(a->sha1_hash, b->sha1_hash, SHA_DIGEST_LENGTH); + /* attempt to compute cert hash */ + (void)X509_check_purpose((X509 *)a, -1, 0); + (void)X509_check_purpose((X509 *)b, -1, 0); + + if ((a->ex_flags & EXFLAG_NO_FINGERPRINT) == 0 + && (b->ex_flags & EXFLAG_NO_FINGERPRINT) == 0) + rv = memcmp(a->sha1_hash, b->sha1_hash, SHA_DIGEST_LENGTH); if (rv != 0) return rv < 0 ? -1 : 1; + /* Check for match against stored encoding too */ if (!a->cert_info.enc.modified && !b->cert_info.enc.modified) { if (a->cert_info.enc.len < b->cert_info.enc.len) diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index eb730bb24d..00d45ea809 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -702,7 +702,7 @@ X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, if (!X509_cmp(obj->data.x509, x->data.x509)) return obj; } else if (x->type == X509_LU_CRL) { - if (!X509_CRL_match(obj->data.crl, x->data.crl)) + if (X509_CRL_match(obj->data.crl, x->data.crl) == 0) return obj; } else return obj; diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c index 680a1cf48c..9d9079f7f5 100644 --- a/crypto/x509/x_all.c +++ b/crypto/x509/x_all.c @@ -392,7 +392,7 @@ int X509_digest(const X509 *cert, const EVP_MD *md, unsigned char *data, unsigned int *len) { if (EVP_MD_is_a(md, SN_sha1) && (cert->ex_flags & EXFLAG_SET) != 0 - && (cert->ex_flags & EXFLAG_INVALID) == 0) { + && (cert->ex_flags & EXFLAG_NO_FINGERPRINT) == 0) { /* Asking for SHA1 and we already computed it. */ if (len != NULL) *len = sizeof(cert->sha1_hash); @@ -436,7 +436,7 @@ int X509_CRL_digest(const X509_CRL *data, const EVP_MD *type, unsigned char *md, unsigned int *len) { if (type == EVP_sha1() && (data->flags & EXFLAG_SET) != 0 - && (data->flags & EXFLAG_INVALID) == 0) { + && (data->flags & EXFLAG_NO_FINGERPRINT) == 0) { /* Asking for SHA1; always computed in CRL d2i. */ if (len != NULL) *len = sizeof(data->sha1_hash); diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c index 164d425ab2..ef54a9a3cd 100644 --- a/crypto/x509/x_crl.c +++ b/crypto/x509/x_crl.c @@ -147,7 +147,7 @@ static int crl_set_issuers(X509_CRL *crl) /* * The X509_CRL structure needs a bit of customisation. Cache some extensions - * and hash of the whole CRL. + * and hash of the whole CRL or set EXFLAG_NO_FINGERPRINT if this fails. */ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, void *exarg) @@ -185,7 +185,7 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, case ASN1_OP_D2I_POST: if (!X509_CRL_digest(crl, EVP_sha1(), crl->sha1_hash, NULL)) - crl->flags |= EXFLAG_INVALID; + crl->flags |= EXFLAG_NO_FINGERPRINT; crl->idp = X509_CRL_get_ext_d2i(crl, NID_issuing_distribution_point, &i, NULL); diff --git a/doc/internal/man3/x509v3_cache_extensions.pod b/doc/internal/man3/x509v3_cache_extensions.pod index 418a19738c..cd00942333 100644 --- a/doc/internal/man3/x509v3_cache_extensions.pod +++ b/doc/internal/man3/x509v3_cache_extensions.pod @@ -17,7 +17,8 @@ This function processes any X509v3 extensions present in an X509 object I and caches the result of that processing as well as further derived info, for instance whether the certificate is self-issued or has version X.509v1. It computes the SHA1 digest of the certificate using the default library context -and property query string and stores the result in x->sha1_hash. +and property query string and stores the result in x->sha1_hash, +or on failure sets B in x->flags. It sets B in x->flags if x->siginf was filled successfully, which may not be possible if a referenced algorithm is unknown or not available. Many OpenSSL functions that use an X509 object call this function implicitly. diff --git a/doc/man3/X509_cmp.pod b/doc/man3/X509_cmp.pod index 1e6a166e65..777d055ad8 100644 --- a/doc/man3/X509_cmp.pod +++ b/doc/man3/X509_cmp.pod @@ -55,7 +55,8 @@ The B comparison functions return B<-1>, B<0>, or B<1> if object I is found to be less than, to match, or be greater than object I, respectively. X509_NAME_cmp(), X509_issuer_and_serial_cmp(), X509_issuer_name_cmp(), -X509_subject_name_cmp() and X509_CRL_cmp() may return B<-2> to indicate an error. +X509_subject_name_cmp(), X509_CRL_cmp(), and X509_CRL_match() +may return B<-2> to indicate an error. =head1 NOTES diff --git a/doc/man3/X509_get_extension_flags.pod b/doc/man3/X509_get_extension_flags.pod index 3f09939e52..cac43d716e 100644 --- a/doc/man3/X509_get_extension_flags.pod +++ b/doc/man3/X509_get_extension_flags.pod @@ -78,12 +78,17 @@ The certificate contains an unhandled critical extension. =item B -Some certificate extension values are invalid or inconsistent. The -certificate should be rejected. +Some certificate extension values are invalid or inconsistent. +The certificate should be rejected. This bit may also be raised after an out-of-memory error while processing the X509 object, so it may not be related to the processed ASN1 object itself. +=item B + +Failed to compute the internal SHA1 hash value of the certificate or CRL. +This may be due to malloc failure or because no SHA1 implementation was found. + =item B The NID_certificate_policies certificate extension is invalid or diff --git a/include/openssl/x509v3.h.in b/include/openssl/x509v3.h.in index 7234aa2c62..dad8694ffa 100644 --- a/include/openssl/x509v3.h.in +++ b/include/openssl/x509v3.h.in @@ -406,6 +406,7 @@ struct ISSUING_DIST_POINT_st { # define EXFLAG_AKID_CRITICAL 0x20000 # define EXFLAG_SKID_CRITICAL 0x40000 # define EXFLAG_SAN_CRITICAL 0x80000 +# define EXFLAG_NO_FINGERPRINT 0x100000 # define KU_DIGITAL_SIGNATURE 0x0080 # define KU_NON_REPUDIATION 0x0040 diff --git a/test/certs/invalid-cert.pem b/test/certs/invalid-cert.pem new file mode 100644 index 0000000000..a8951305a3 --- /dev/null +++ b/test/certs/invalid-cert.pem @@ -0,0 +1,19 @@ +-----BEGIN TRUSTED CERTIFICATE----- +MIIDJTCCAg2gAwIBAgIUEUSW5o7qpgNCWyXic9Fc9tCLS0gwDQYJKoZIhvcNAQEL +BQAwEzERMA8GA1UEAwwIUGVyc29TaW0wHhcNMjAxMjE2MDY1NjM5WhcNMzAxMjE2 +MDY1NjM5WjATMREwDwYDVQQDDAhQZXJzb1NpbTCCASIwDQYJKoZIhvcNAQEBBQAD +ggEPADCCAQoCggEBAMsgRKnnZbQtG9bB9Hn+CoOOsanmnRELSlGq521qi/eBgs2w +SdHYM6rsJFwY89RvINLGeUZh/pu7c+ODtTafAWE3JkynG01d2Zrvp1V1r97+FGyD +f+b1hAggxBy70bTRyr1gAoKQTAm74U/1lj13EpWz7zshgXJ/Pn/hUyTmpNW+fTRE +xaifN0jkl5tZUURGA6w3+BRhVDQtt92vLihqUGaEFpL8yqqFnN44AoQ5+lgMafWi +UyYMHcK75ZB8WWklq8zjRP3xC1h56k01rT6KJO6i+BxMcADerYsn5qTlcUiKcpRU +b6RzLvCUwj91t1aX6npDI3BzSP+wBUUANBfuHEMCAwEAAaNxMG8wFwYDVR0OBBA8 +yBBnvz1Zt6pHm2GwBaRyMBcGA1UdIwQQPMgQZ789WbeqR5thsAWkcjAPBgNVHRMB +Af8EBTADAQH/MAsGA1UdDwQEAwIChDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDQYJKoZIhvcNAQELBQADggEBAIEzVbttOUc7kK4aY+74TANFZK/qtBQ7 +94a/P30TGWSRUq2HnDsR8Vo4z8xm5oKeC+SIi6NGzviWYquuzpJ7idcbr0MIuSyD ++Vg6n1sG64DxWNdGO9lR5c4mWFdIajShczS2+4QIRB/lFZCf7GhPMtIcbP1o9ckY +2vyv5ZAEU9Z5n0PY+abrKsj0XyvJwdycEsUTywa36fuv6hP3UboLtvK6naXLMrTj +WtSA6PXjHy7h8h0NC8XLk64mc0lcRC4WM+xJ/C+NHglpmBqBxnStpnZykMZYD1Vy +JJ1wNc+Y3e2uMBDxZviH3dIPIgqP1Vpi2TWfqr3DTBNCRf4dl/wwNU8= +-----END TRUSTED CERTIFICATE----- diff --git a/test/recipes/80-test_x509aux.t b/test/recipes/80-test_x509aux.t index 8debd80fc1..327f861fe1 100644 --- a/test/recipes/80-test_x509aux.t +++ b/test/recipes/80-test_x509aux.t @@ -14,14 +14,17 @@ use OpenSSL::Test::Utils; setup("test_x509aux"); +my @path = qw(test certs); + plan skip_all => "test_dane uses ec which is not supported by this OpenSSL build" if disabled("ec"); plan tests => 1; # The number of tests being performed ok(run(test(["x509aux", - srctop_file("test", "certs", "roots.pem"), - srctop_file("test", "certs", "root+anyEKU.pem"), - srctop_file("test", "certs", "root-anyEKU.pem"), - srctop_file("test", "certs", "root-cert.pem")] - )), "x509aux tests"); + srctop_file(@path, "roots.pem"), + srctop_file(@path, "root+anyEKU.pem"), + srctop_file(@path, "root-anyEKU.pem"), + srctop_file(@path, "root-cert.pem"), + srctop_file(@path, "invalid-cert.pem"), + ])), "x509aux tests"); diff --git a/test/x509aux.c b/test/x509aux.c index bd8a781bdb..d170cf7e9e 100644 --- a/test/x509aux.c +++ b/test/x509aux.c @@ -30,17 +30,16 @@ static int test_certs(int num) typedef int (*i2d_X509_t)(const X509 *, unsigned char **); int err = 0; BIO *fp = BIO_new_file(test_get_argument(num), "r"); - X509 *reuse = NULL; if (!TEST_ptr(fp)) return 0; for (c = 0; !err && PEM_read_bio(fp, &name, &header, &data, &len); ++c) { const int trusted = (strcmp(name, PEM_STRING_X509_TRUSTED) == 0); - d2i_X509_t d2i = trusted ? d2i_X509_AUX : d2i_X509; i2d_X509_t i2d = trusted ? i2d_X509_AUX : i2d_X509; X509 *cert = NULL; + X509 *reuse = NULL; const unsigned char *p = data; unsigned char *buf = NULL; unsigned char *bufp; @@ -93,9 +92,15 @@ static int test_certs(int num) goto next; } p = buf; - reuse = d2i(&reuse, &p, enclen); - if (reuse == NULL || X509_cmp (reuse, cert)) { - TEST_error("X509_cmp does not work with %s", name); + reuse = d2i(NULL, &p, enclen); + if (reuse == NULL) { + TEST_error("second d2i call failed for %s", name); + err = 1; + goto next; + } + err = X509_cmp(reuse, cert); + if (err != 0) { + TEST_error("X509_cmp for %s resulted in %d", name, err); err = 1; goto next; } @@ -141,13 +146,13 @@ static int test_certs(int num) */ next: X509_free(cert); + X509_free(reuse); OPENSSL_free(buf); OPENSSL_free(name); OPENSSL_free(header); OPENSSL_free(data); } BIO_free(fp); - X509_free(reuse); if (ERR_GET_REASON(ERR_peek_last_error()) == PEM_R_NO_START_LINE) { /* Reached end of PEM file */