While studying why X509_check_ca() is the ugly thing it is, tb@
suggested x509v3_cache_extensions() might benefit from a wrapper to
avoid duplication of locking and checking the stupid EXFLAG_INVALID
flag. x509v3_cache_extensions() isn't a public function anyway.

Passes regress & rpki-client.

OK?

Kind regards,

Job

Index: x509/x509_internal.h
===================================================================
RCS file: /cvs/src/lib/libcrypto/x509/x509_internal.h,v
retrieving revision 1.23
diff -u -p -r1.23 x509_internal.h
--- x509/x509_internal.h        26 Nov 2022 16:08:54 -0000      1.23
+++ x509/x509_internal.h        20 Jan 2023 19:59:56 -0000
@@ -94,7 +94,8 @@ int x509_vfy_check_policy(X509_STORE_CTX
 int x509_vfy_check_trust(X509_STORE_CTX *ctx);
 int x509_vfy_check_chain_extensions(X509_STORE_CTX *ctx);
 int x509_vfy_callback_indicate_completion(X509_STORE_CTX *ctx);
-void x509v3_cache_extensions(X509 *x);
+void x509v3_cache_extensions_internal(X509 *x);
+int x509v3_cache_extensions(X509 *x);
 X509 *x509_vfy_lookup_cert_match(X509_STORE_CTX *ctx, X509 *x);
 
 time_t x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notafter);
Index: x509/x509_purp.c
===================================================================
RCS file: /cvs/src/lib/libcrypto/x509/x509_purp.c,v
retrieving revision 1.18
diff -u -p -r1.18 x509_purp.c
--- x509/x509_purp.c    26 Nov 2022 16:08:55 -0000      1.18
+++ x509/x509_purp.c    20 Jan 2023 19:59:57 -0000
@@ -76,7 +76,8 @@
 #define ns_reject(x, usage) \
        (((x)->ex_flags & EXFLAG_NSCERT) && !((x)->ex_nscert & (usage)))
 
-void x509v3_cache_extensions(X509 *x);
+void x509v3_cache_extensions_internal(X509 *x);
+int x509v3_cache_extensions(X509 *x);
 
 static int check_ssl_ca(const X509 *x);
 static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
@@ -132,12 +133,10 @@ X509_check_purpose(X509 *x, int id, int 
        const X509_PURPOSE *pt;
 
        if (!(x->ex_flags & EXFLAG_SET)) {
-               CRYPTO_w_lock(CRYPTO_LOCK_X509);
-               x509v3_cache_extensions(x);
-               CRYPTO_w_unlock(CRYPTO_LOCK_X509);
-               if (x->ex_flags & EXFLAG_INVALID)
+               if (!x509v3_cache_extensions(x))
                        return -1;
        }
+
        if (id == -1)
                return 1;
        idx = X509_PURPOSE_get_by_id(id);
@@ -450,7 +449,7 @@ setup_crldp(X509 *x)
 }
 
 void
-x509v3_cache_extensions(X509 *x)
+x509v3_cache_extensions_internal(X509 *x)
 {
        BASIC_CONSTRAINTS *bs;
        PROXY_CERT_INFO_EXTENSION *pci;
@@ -640,6 +639,19 @@ x509v3_cache_extensions(X509 *x)
        x->ex_flags |= EXFLAG_SET;
 }
 
+/*
+ * 1 is success, 0 is failure.
+ */
+int
+x509v3_cache_extensions(X509 *x)
+{
+       CRYPTO_w_lock(CRYPTO_LOCK_X509);
+       x509v3_cache_extensions_internal(x);
+       CRYPTO_w_unlock(CRYPTO_LOCK_X509);
+
+       return ((x->ex_flags & EXFLAG_INVALID) == 0);
+}
+
 /* CA checks common to all purposes
  * return codes:
  * 0 not a CA
@@ -680,11 +692,8 @@ check_ca(const X509 *x)
 int
 X509_check_ca(X509 *x)
 {
-       if (!(x->ex_flags & EXFLAG_SET)) {
-               CRYPTO_w_lock(CRYPTO_LOCK_X509);
+       if (!(x->ex_flags & EXFLAG_SET))
                x509v3_cache_extensions(x);
-               CRYPTO_w_unlock(CRYPTO_LOCK_X509);
-       }
 
        return check_ca(x);
 }
@@ -896,19 +905,13 @@ X509_check_issued(X509 *issuer, X509 *su
            X509_get_issuer_name(subject)))
                return X509_V_ERR_SUBJECT_ISSUER_MISMATCH;
        if (!(issuer->ex_flags & EXFLAG_SET)) {
-               CRYPTO_w_lock(CRYPTO_LOCK_X509);
-               x509v3_cache_extensions(issuer);
-               CRYPTO_w_unlock(CRYPTO_LOCK_X509);
+               if (!x509v3_cache_extensions(issuer))
+                       return X509_V_ERR_UNSPECIFIED;
        }
-       if (issuer->ex_flags & EXFLAG_INVALID)
-               return X509_V_ERR_UNSPECIFIED;
        if (!(subject->ex_flags & EXFLAG_SET)) {
-               CRYPTO_w_lock(CRYPTO_LOCK_X509);
-               x509v3_cache_extensions(subject);
-               CRYPTO_w_unlock(CRYPTO_LOCK_X509);
+               if (!x509v3_cache_extensions(subject))
+                       return X509_V_ERR_UNSPECIFIED;
        }
-       if (subject->ex_flags & EXFLAG_INVALID)
-               return X509_V_ERR_UNSPECIFIED;
 
        if (subject->akid) {
                int ret = X509_check_akid(issuer, subject->akid);
Index: x509/x509_verify.c
===================================================================
RCS file: /cvs/src/lib/libcrypto/x509/x509_verify.c,v
retrieving revision 1.62
diff -u -p -r1.62 x509_verify.c
--- x509/x509_verify.c  17 Jan 2023 23:49:28 -0000      1.62
+++ x509/x509_verify.c  20 Jan 2023 19:59:57 -0000
@@ -242,12 +242,9 @@ static int
 x509_verify_cert_cache_extensions(X509 *cert)
 {
        if (!(cert->ex_flags & EXFLAG_SET)) {
-               CRYPTO_w_lock(CRYPTO_LOCK_X509);
-               x509v3_cache_extensions(cert);
-               CRYPTO_w_unlock(CRYPTO_LOCK_X509);
+               if (!x509v3_cache_extensions(cert))
+                       return 0;
        }
-       if (cert->ex_flags & EXFLAG_INVALID)
-               return 0;
 
        return (cert->ex_flags & EXFLAG_SET);
 }

Reply via email to