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

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

ok tb@

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