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@

3 files changed, 33 insertions, 78 deletions

diff --git a/src/lib/libcrypto/x509/x509_internal.h b/src/lib/libcrypto/x509/x509_internal.h
index f6ce78346e..7d3250d063 100644
--- a/src/lib/libcrypto/x509/x509_internal.h
+++ b/src/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/src/lib/libcrypto/x509/x509_verify.c b/src/lib/libcrypto/x509/x509_verify.c
index 3176e110ba..68dd2863a7 100644
--- a/src/lib/libcrypto/x509/x509_verify.c
+++ b/src/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/src/lib/libcrypto/x509/x509_vfy.c b/src/lib/libcrypto/x509/x509_vfy.c
index 233c95c408..a161b330ae 100644
--- a/src/lib/libcrypto/x509/x509_vfy.c
+++ b/src/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);
         }