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

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

ok tb@

1 files changed, 70 insertions, 52 deletions

diff --git a/src/lib/libcrypto/x509/x509_verify.c b/src/lib/libcrypto/x509/x509_verify.c
index 18d395d273..dd053ad812 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.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