Get rid of historical code to extract the roots in the legacy case.

Due to the need to support by_dir, we use the get_issuer stuff when running
in x509_vfy compatibility mode amyway - so just use it any time we are
doing that. Removes a bunch of yukky stuff and a "Don't Look Ethel"

ok tb@ jsing@

diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h
index f6ce783..7d3250d 100644 (file)
--- a/lib/libcrypto/x509/x509_internal.h
+++ b/lib/libcrypto/x509/x509_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_internal.h,v 1.10 2021/08/28 07:49:00 beck Exp $ */
+/* $OpenBSD: x509_internal.h,v 1.11 2021/08/28 15:22:42 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -96,8 +96,7 @@ X509 *x509_vfy_lookup_cert_match(X509_STORE_CTX *ctx, X509 *x);
 int x509_verify_asn1_time_to_tm(const ASN1_TIME *atime, struct tm *tm,
     int notafter);
 
-struct x509_verify_ctx *x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc,
-    STACK_OF(X509) *roots);
+struct x509_verify_ctx *x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc);
 
 void x509_constraints_name_clear(struct x509_constraints_name *name);
 int x509_constraints_names_add(struct x509_constraints_names *names,

diff --git a/lib/libcrypto/x509/x509_verify.c b/lib/libcrypto/x509/x509_verify.c
index 3176e11..68dd286 100644 (file)
--- a/lib/libcrypto/x509/x509_verify.c
+++ b/lib/libcrypto/x509/x509_verify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_verify.c,v 1.43 2021/08/28 07:49:00 beck Exp $ */
+/* $OpenBSD: x509_verify.c,v 1.44 2021/08/28 15:22:42 beck Exp $ */
 /*
  * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org>
  *
@@ -213,13 +213,6 @@ x509_verify_ctx_cert_is_root(struct x509_verify_ctx *ctx, X509 *cert,
        if (!x509_verify_cert_cache_extensions(cert))
                return 0;
 
-       /* Check the provided roots */
-       for (i = 0; i < sk_X509_num(ctx->roots); i++) {
-               if (X509_cmp(sk_X509_value(ctx->roots, i), cert) == 0)
-                       return !full_chain ||
-                           x509_verify_cert_self_signed(cert);
-       }
-
        /* Check by lookup if we have a legacy xsc */
        if (ctx->xsc != NULL) {
                if ((match = x509_vfy_lookup_cert_match(ctx->xsc,
@@ -228,6 +221,13 @@ x509_verify_ctx_cert_is_root(struct x509_verify_ctx *ctx, X509 *cert,
                        return !full_chain ||
                            x509_verify_cert_self_signed(cert);
                }
+       } else {
+               /* Check the provided roots */
+               for (i = 0; i < sk_X509_num(ctx->roots); i++) {
+                       if (X509_cmp(sk_X509_value(ctx->roots, i), cert) == 0)
+                               return !full_chain ||
+                                   x509_verify_cert_self_signed(cert);
+               }
        }
 
        return 0;
@@ -611,17 +611,6 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
                            X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN;
        }
 
-       /* Check to see if we have a trusted root issuer. */
-       for (i = 0; i < sk_X509_num(ctx->roots); i++) {
-               candidate = sk_X509_value(ctx->roots, i);
-               if (x509_verify_potential_parent(ctx, candidate, cert)) {
-                       is_root = !full_chain ||
-                           x509_verify_cert_self_signed(candidate);
-                       x509_verify_consider_candidate(ctx, cert,
-                           cert_md, is_root, candidate, current_chain,
-                           full_chain);
-               }
-       }
        /* Check for legacy mode roots */
        if (ctx->xsc != NULL) {
                if ((ret = ctx->xsc->get_issuer(&candidate, ctx->xsc, cert)) < 0) {
@@ -639,6 +628,18 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
                        }
                        X509_free(candidate);
                }
+       } else {
+               /* Check to see if we have a trusted root issuer. */
+               for (i = 0; i < sk_X509_num(ctx->roots); i++) {
+                       candidate = sk_X509_value(ctx->roots, i);
+                       if (x509_verify_potential_parent(ctx, candidate, cert)) {
+                               is_root = !full_chain ||
+                                   x509_verify_cert_self_signed(candidate);
+                               x509_verify_consider_candidate(ctx, cert,
+                                   cert_md, is_root, candidate, current_chain,
+                                   full_chain);
+                       }
+               }
        }
 
        /* Check intermediates after checking roots */
@@ -933,7 +934,7 @@ x509_verify_cert_valid(struct x509_verify_ctx *ctx, X509 *cert,
 }
 
 struct x509_verify_ctx *
-x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc, STACK_OF(X509) *roots)
+x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc)
 {
        struct x509_verify_ctx *ctx;
        size_t max_depth;
@@ -941,7 +942,7 @@ x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc, STACK_OF(X509) *roots)
        if (xsc == NULL)
                return NULL;
 
-       if ((ctx = x509_verify_ctx_new(roots)) == NULL)
+       if ((ctx = x509_verify_ctx_new(NULL)) == NULL)
                return NULL;
 
        ctx->xsc = xsc;
@@ -969,14 +970,16 @@ x509_verify_ctx_new(STACK_OF(X509) *roots)
 {
        struct x509_verify_ctx *ctx;
 
-       if (roots == NULL)
-               return NULL;
-
        if ((ctx = calloc(1, sizeof(struct x509_verify_ctx))) == NULL)
                return NULL;
 
-       if ((ctx->roots = X509_chain_up_ref(roots)) == NULL)
-               goto err;
+       if (roots != NULL) {
+               if  ((ctx->roots = X509_chain_up_ref(roots)) == NULL)
+                       goto err;
+       } else {
+               if ((ctx->roots = sk_X509_new_null()) == NULL)
+                       goto err;
+       }
 
        ctx->max_depth = X509_VERIFY_MAX_CHAIN_CERTS;
        ctx->max_chains = X509_VERIFY_MAX_CHAINS;

diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c
index 233c95c..a161b33 100644 (file)
--- a/lib/libcrypto/x509/x509_vfy.c
+++ b/lib/libcrypto/x509/x509_vfy.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vfy.c,v 1.87 2021/08/19 03:44:00 beck Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.88 2021/08/28 15:22:42 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -634,54 +634,7 @@ X509_verify_cert(X509_STORE_CTX *ctx)
 
        /* Use the modern multi-chain verifier from x509_verify_cert */
 
-       /* Find our trusted roots */
-       ctx->error = X509_V_ERR_OUT_OF_MEM;
-
-       if (ctx->get_issuer == get_issuer_sk) {
-               /*
-                * We are using the trusted stack method. so
-                * the roots are in the aptly named "ctx->other_ctx"
-                * pointer. (It could have been called "al")
-                */
-               if ((roots = X509_chain_up_ref(ctx->other_ctx)) == NULL)
-                       return -1;
-       } else {
-               /*
-                * We have a X509_STORE and need to pull out the roots.
-                * Don't look Ethel...
-                */
-               STACK_OF(X509_OBJECT) *objs;
-               size_t i, good = 1;
-
-               if ((roots = sk_X509_new_null()) == NULL)
-                       return -1;
-
-               CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
-               if ((objs = X509_STORE_get0_objects(ctx->ctx)) == NULL)
-                       good = 0;
-               for (i = 0; good && i < sk_X509_OBJECT_num(objs); i++) {
-                       X509_OBJECT *obj;
-                       X509 *root;
-                       obj = sk_X509_OBJECT_value(objs, i);
-                       if (obj->type != X509_LU_X509)
-                               continue;
-                       root = obj->data.x509;
-                       if (X509_up_ref(root) == 0)
-                               good = 0;
-                       if (sk_X509_push(roots, root) == 0) {
-                               X509_free(root);
-                               good = 0;
-                       }
-               }
-               CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
-
-               if (!good) {
-                       sk_X509_pop_free(roots, X509_free);
-                       return -1;
-               }
-       }
-
-       if ((vctx = x509_verify_ctx_new_from_xsc(ctx, roots)) != NULL) {
+       if ((vctx = x509_verify_ctx_new_from_xsc(ctx)) != NULL) {
                ctx->error = X509_V_OK; /* Initialize to OK */
                chain_count = x509_verify(vctx, NULL, NULL);
        }