Refactor the legacy chain validation from the chain adding code into its

own function, in preparation for subesquent change. No functional change.

ok tb@

Author: beck

src/lib/libcrypto/x509/x509_verify.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_verify.c,v 1.39 2021/07/12 15:12:38 beck Exp $ */
+/* $OpenBSD: x509_verify.c,v 1.40 2021/08/18 15:10:46 beck Exp $ */
 /*
  * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org>
  *
@@ -307,6 +307,71 @@ x509_verify_ctx_restore_xsc_error(struct x509_verify_ctx *ctx)
 	return 1;
 }
 
+/* Perform legacy style validation of a chain */
+static int
+x509_verify_ctx_validate_legacy_chain(struct x509_verify_ctx *ctx,
+    struct x509_verify_chain *chain, size_t depth)
+{
+	int ret = 0;
+
+	if (ctx->xsc == NULL)
+		return 1;
+
+	/*
+	 * If we have a legacy xsc, choose a validated chain, and
+	 * apply the extensions, revocation, and policy checks just
+	 * like the legacy code did. We do this here instead of as
+	 * building the chains to more easily support the callback and
+	 * the bewildering array of VERIFY_PARAM knobs that are there
+	 * for the fiddling.
+	 */
+
+	/* These may be set in one of the following calls. */
+	ctx->xsc->error = X509_V_OK;
+	ctx->xsc->error_depth = 0;
+
+	if (!x509_verify_ctx_set_xsc_chain(ctx, chain, 0, 1))
+		goto err;
+
+	/*
+	 * XXX currently this duplicates some work done in chain
+	 * build, but we keep it here until we have feature parity
+	 */
+	if (!x509_vfy_check_chain_extensions(ctx->xsc))
+		goto err;
+
+	if (!x509_constraints_chain(ctx->xsc->chain,
+		&ctx->xsc->error, &ctx->xsc->error_depth)) {
+		X509 *cert = sk_X509_value(ctx->xsc->chain, depth);
+		if (!x509_verify_cert_error(ctx, cert,
+			ctx->xsc->error_depth, ctx->xsc->error, 0))
+			goto err;
+	}
+
+	if (!x509_vfy_check_revocation(ctx->xsc))
+		goto err;
+
+	if (!x509_vfy_check_policy(ctx->xsc))
+		goto err;
+
+	ret = 1;
+
+ err:
+	/*
+	 * The above checks may have set ctx->xsc->error and
+	 * ctx->xsc->error_depth - save these for later on.
+	 */
+	if (ctx->xsc->error != X509_V_OK) {
+		if (ctx->xsc->error_depth < 0 ||
+		    ctx->xsc->error_depth >= X509_VERIFY_MAX_CHAIN_CERTS)
+			return 0;
+		chain->cert_errors[ctx->xsc->error_depth] =
+		    ctx->xsc->error;
+	}
+
+	return ret;
+}
+
 /* Add a validated chain to our list of valid chains */
 static int
 x509_verify_ctx_add_chain(struct x509_verify_ctx *ctx,
@@ -328,59 +393,12 @@ x509_verify_ctx_add_chain(struct x509_verify_ctx *ctx,
 	    X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY)
 		chain->cert_errors[depth] = X509_V_OK;
 
-	/*
-	 * If we have a legacy xsc, choose a validated chain,
-	 * and apply the extensions, revocation, and policy checks
-	 * just like the legacy code did. We do this here instead
-	 * of as building the chains to more easily support the
-	 * callback and the bewildering array of VERIFY_PARAM
-	 * knobs that are there for the fiddling.
-	 */
-	if (ctx->xsc != NULL) {
-		/* These may be set in one of the following calls. */
-		ctx->xsc->error = X509_V_OK;
-		ctx->xsc->error_depth = 0;
-
-		if (!x509_verify_ctx_set_xsc_chain(ctx, chain, 0, 1))
-			return 0;
-
-		/*
-		 * XXX currently this duplicates some work done
-		 * in chain build, but we keep it here until
-		 * we have feature parity
-		 */
-		if (!x509_vfy_check_chain_extensions(ctx->xsc))
-			return 0;
-
-		if (!x509_constraints_chain(ctx->xsc->chain,
-		    &ctx->xsc->error, &ctx->xsc->error_depth)) {
-			X509 *cert = sk_X509_value(ctx->xsc->chain, depth);
-			if (!x509_verify_cert_error(ctx, cert,
-			    ctx->xsc->error_depth, ctx->xsc->error, 0))
-				return 0;
-		}
-
-		if (!x509_vfy_check_revocation(ctx->xsc))
-			return 0;
-
-		if (!x509_vfy_check_policy(ctx->xsc))
-			return 0;
+	if (!x509_verify_ctx_validate_legacy_chain(ctx, chain, depth))
+		return 0;
 
-		/*
-		 * The above checks may have set ctx->xsc->error and
-		 * ctx->xsc->error_depth - save these for later on.
-		 */
-		if (ctx->xsc->error != X509_V_OK) {
-			if (ctx->xsc->error_depth < 0 ||
-			    ctx->xsc->error_depth >= X509_VERIFY_MAX_CHAIN_CERTS)
-				return 0;
-			chain->cert_errors[ctx->xsc->error_depth] =
-			    ctx->xsc->error;
-		}
-	}
 	/*
-	 * no xsc means we are being called from the non-legacy API,
-	 * extensions and purpose are dealt with as the chain is built.
+	 * In the non-legacy code, extensions and purpose are dealt
+	 * with as the chain is built.
 	 *
 	 * The non-legacy api returns multiple chains but does not do
 	 * any revocation checking (it must be done by the caller on