From: beck <beck@openbsd.org>
Date: Sat, 28 Aug 2021 15:22:42 +0000 (+0000)
Subject: Get rid of historical code to extract the roots in the legacy case.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=99d404eba6729661ec083f2f0a2d23289d5616e6;p=openbsd

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 f6ce78346eb..7d3250d0632 100644
--- 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 3176e110bab..68dd2863a71 100644
--- 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 233c95c4086..a161b330aec 100644
--- 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);
 	}