Fix ossl_x509v3_cache_extensions(): EXFLAG_NO_FINGERPRINT should not be an error

This allows reverting the recent workaround on cmp_ctx_test regarding X509_new()

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/16043)

diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 177e2b589324962dc3a97986527eccad556bb331..fa3a8b1ebf495f492d9da15920d8db7373e82f4a 100644 (file)
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -419,12 +419,12 @@ int ossl_x509v3_cache_extensions(X509 *x)
         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))
         x->ex_flags |= EXFLAG_NO_FINGERPRINT;
 
-    ERR_set_mark();
-
     /* V1 should mean no extensions ... */
     if (X509_get_version(x) == X509_VERSION_1)
         x->ex_flags |= EXFLAG_V1;
@@ -575,8 +575,6 @@ int ossl_x509v3_cache_extensions(X509 *x)
     res = setup_crldp(x);
     if (res == 0)
         x->ex_flags |= EXFLAG_INVALID;
-    else if (res < 0)
-        goto err;
 
 #ifndef OPENSSL_NO_RFC3779
     x->rfc3779_addr = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, &i, NULL);
@@ -629,17 +627,13 @@ int ossl_x509v3_cache_extensions(X509 *x)
      */
 #endif
     ERR_pop_to_mark();
-    if ((x->ex_flags & (EXFLAG_INVALID | EXFLAG_NO_FINGERPRINT)) == 0) {
+
+    if ((x->ex_flags & EXFLAG_INVALID) == 0) {
         CRYPTO_THREAD_unlock(x->lock);
         return 1;
     }
-    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 */
     CRYPTO_THREAD_unlock(x->lock);
+    ERR_raise(ERR_LIB_X509, X509V3_R_INVALID_CERTIFICATE);
     return 0;
 }
 

diff --git a/test/cmp_ctx_test.c b/test/cmp_ctx_test.c
index e4f80d93fc430df5bb4e155cfa60b1f3006f609a..b18a83c60f79b24e01ed6f898352a01a37f96552 100644 (file)
--- a/test/cmp_ctx_test.c
+++ b/test/cmp_ctx_test.c
@@ -13,11 +13,6 @@
 
 #include <openssl/x509_vfy.h>
 
-static X509 *test_cert;
-
-/* Avoid using X509_new() via the generic macros below. */
-#define X509_new() X509_dup(test_cert)
-
 typedef struct test_fixture {
     const char *test_case_name;
     OSSL_CMP_CTX *ctx;
@@ -47,7 +42,7 @@ static OSSL_CMP_CTX_TEST_FIXTURE *set_up(const char *const test_case_name)
 static STACK_OF(X509) *sk_X509_new_1(void)
 {
     STACK_OF(X509) *sk = sk_X509_new_null();
-    X509 *x = X509_dup(test_cert);
+    X509 *x = X509_new();
 
     if (x == NULL || !sk_X509_push(sk, x)) {
         sk_X509_free(sk);
@@ -67,18 +62,19 @@ static int execute_CTX_reinit_test(OSSL_CMP_CTX_TEST_FIXTURE *fixture)
     OSSL_CMP_CTX *ctx = fixture->ctx;
     ASN1_OCTET_STRING *bytes = NULL;
     STACK_OF(X509) *certs = NULL;
+    X509 *cert = X509_new();
     int res = 0;
 
     /* set non-default values in all relevant fields */
     ctx->status = 1;
     ctx->failInfoCode = 1;
     if (!ossl_cmp_ctx_set0_statusString(ctx, sk_ASN1_UTF8STRING_new_null())
-            || !ossl_cmp_ctx_set0_newCert(ctx, X509_dup(test_cert))
+            || !ossl_cmp_ctx_set0_newCert(ctx, X509_new())
             || !TEST_ptr(certs = sk_X509_new_1())
             || !ossl_cmp_ctx_set1_newChain(ctx, certs)
             || !ossl_cmp_ctx_set1_caPubs(ctx, certs)
             || !ossl_cmp_ctx_set1_extraCertsIn(ctx, certs)
-            || !ossl_cmp_ctx_set1_validatedSrvCert(ctx, test_cert)
+            || !ossl_cmp_ctx_set1_validatedSrvCert(ctx, cert)
             || !TEST_ptr(bytes = ASN1_OCTET_STRING_new())
             || !OSSL_CMP_CTX_set1_transactionID(ctx, bytes)
             || !OSSL_CMP_CTX_set1_senderNonce(ctx, bytes)
@@ -106,6 +102,7 @@ static int execute_CTX_reinit_test(OSSL_CMP_CTX_TEST_FIXTURE *fixture)
     res = 1;
 
  err:
+    X509_free(cert);
     sk_X509_pop_X509_free(certs);
     ASN1_OCTET_STRING_free(bytes);
     return res;
@@ -671,7 +668,7 @@ static int execute_CTX_##PUSHN##_##ELEM(OSSL_CMP_CTX_TEST_FIXTURE *fixture) \
     } \
     \
     if (!(*push_fn)(ctx, val2)) { \
-        TEST_error("pushting second value failed"); \
+        TEST_error("pushing second value failed"); \
         res = 0; \
     } \
     if (PUSHN == 0) \
@@ -792,17 +789,11 @@ DEFINE_SET_TEST(ossl_cmp, ctx, 1, 1, recipNonce, ASN1_OCTET_STRING)
 
 int setup_tests(void)
 {
-    char *cert_file;
-
     if (!test_skip_common_options()) {
         TEST_error("Error parsing test options\n");
         return 0;
     }
 
-    if (!TEST_ptr(cert_file = test_get_argument(0))
-        || !TEST_ptr(test_cert = load_cert_pem(cert_file, NULL)))
-        return 0;
-
     /* OSSL_CMP_CTX_new() is tested by set_up() */
     /* OSSL_CMP_CTX_free() is tested by tear_down() */
     ADD_TEST(test_CTX_reinit);

diff --git a/test/recipes/65-test_cmp_ctx.t b/test/recipes/65-test_cmp_ctx.t
index d3476736408b992698ca7cb3e7b3f9ab03816fd1..780e025151b4b8947fe67e18c9fdf2ad4b577e02 100644 (file)
--- a/test/recipes/65-test_cmp_ctx.t
+++ b/test/recipes/65-test_cmp_ctx.t
@@ -21,4 +21,4 @@ plan skip_all => "This test is not supported in a no-cmp build"
 
 plan tests => 1;
 
-ok(run(test(["cmp_ctx_test", srctop_file("test", "certs", "ee-cert.pem")])));
+ok(run(test(["cmp_ctx_test"])));