3 files changed, 317 insertions, 20 deletions

diff --git a/src/lib/libcrypto/x509/x509_verify.c b/src/lib/libcrypto/x509/x509_verify.c
index 630c9f9b5a..f6959d1f3a 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.55 2022/04/12 10:42:35 tb Exp $ */
+/* $OpenBSD: x509_verify.c,v 1.56 2022/06/25 20:01:43 beck Exp $ */
 /*
  * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org>
  *
@@ -32,8 +32,10 @@
 
 static int x509_verify_cert_valid(struct x509_verify_ctx *ctx, X509 *cert,
     struct x509_verify_chain *current_chain);
+static int x509_verify_cert_hostname(struct x509_verify_ctx *ctx, X509 *cert,
+    char *name);
 static void x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
-    struct x509_verify_chain *current_chain, int full_chain);
+    struct x509_verify_chain *current_chain, int full_chain, char *name);
 static int x509_verify_cert_error(struct x509_verify_ctx *ctx, X509 *cert,
     size_t depth, int error, int ok);
 static void x509_verify_chain_free(struct x509_verify_chain *chain);
@@ -233,7 +235,7 @@ x509_verify_ctx_clear(struct x509_verify_ctx *ctx)
         x509_verify_ctx_reset(ctx);
         sk_X509_pop_free(ctx->intermediates, X509_free);
         free(ctx->chains);
-        memset(ctx, 0, sizeof(*ctx));
+
 }
 
 static int
@@ -453,10 +455,11 @@ x509_verify_ctx_validate_legacy_chain(struct x509_verify_ctx *ctx,
 /* Add a validated chain to our list of valid chains */
 static int
 x509_verify_ctx_add_chain(struct x509_verify_ctx *ctx,
-    struct x509_verify_chain *chain)
+    struct x509_verify_chain *chain, char *name)
 {
         size_t depth;
         X509 *last = x509_verify_chain_last(chain);
+        X509 *leaf = x509_verify_chain_leaf(chain);
 
         depth = sk_X509_num(chain->certs);
         if (depth > 0)
@@ -488,6 +491,13 @@ x509_verify_ctx_add_chain(struct x509_verify_ctx *ctx,
                 return x509_verify_cert_error(ctx, last, depth,
                     X509_V_ERR_OUT_OF_MEM, 0);
         }
+
+        if (!x509_verify_cert_valid(ctx, leaf, NULL))
+                return 0;
+
+        if (!x509_verify_cert_hostname(ctx, leaf, name))
+                return 0;
+
         ctx->chains_count++;
         ctx->error = X509_V_OK;
         ctx->error_depth = depth;
@@ -539,7 +549,7 @@ x509_verify_parent_signature(X509 *parent, X509 *child, int *error)
 static int
 x509_verify_consider_candidate(struct x509_verify_ctx *ctx, X509 *cert,
     int is_root_cert, X509 *candidate, struct x509_verify_chain *current_chain,
-    int full_chain)
+    int full_chain, char *name)
 {
         int depth = sk_X509_num(current_chain->certs);
         struct x509_verify_chain *new_chain;
@@ -590,14 +600,14 @@ x509_verify_consider_candidate(struct x509_verify_ctx *ctx, X509 *cert,
                         x509_verify_chain_free(new_chain);
                         return 0;
                 }
-                if (!x509_verify_ctx_add_chain(ctx, new_chain)) {
+                if (!x509_verify_ctx_add_chain(ctx, new_chain, name)) {
                         x509_verify_chain_free(new_chain);
                         return 0;
                 }
                 goto done;
         }
 
-        x509_verify_build_chains(ctx, candidate, new_chain, full_chain);
+        x509_verify_build_chains(ctx, candidate, new_chain, full_chain, name);
 
  done:
         x509_verify_chain_free(new_chain);
@@ -621,7 +631,7 @@ x509_verify_cert_error(struct x509_verify_ctx *ctx, X509 *cert, size_t depth,
 
 static void
 x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
-    struct x509_verify_chain *current_chain, int full_chain)
+    struct x509_verify_chain *current_chain, int full_chain, char *name)
 {
         X509 *candidate;
         int i, depth, count, ret, is_root;
@@ -679,7 +689,7 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
                                     x509_verify_cert_self_signed(candidate);
                                 x509_verify_consider_candidate(ctx, cert,
                                     is_root, candidate, current_chain,
-                                    full_chain);
+                                    full_chain, name);
                         }
                         X509_free(candidate);
                 }
@@ -692,7 +702,7 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
                                     x509_verify_cert_self_signed(candidate);
                                 x509_verify_consider_candidate(ctx, cert,
                                     is_root, candidate, current_chain,
-                                    full_chain);
+                                    full_chain, name);
                         }
                 }
         }
@@ -704,7 +714,7 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert,
                         if (x509_verify_potential_parent(ctx, candidate, cert)) {
                                 x509_verify_consider_candidate(ctx, cert,
                                     0, candidate, current_chain,
-                                    full_chain);
+                                    full_chain, name);
                         }
                 }
         }
@@ -1116,16 +1126,18 @@ x509_verify(struct x509_verify_ctx *ctx, X509 *leaf, char *name)
                 ctx->xsc->current_cert = leaf;
         }
 
-        if (!x509_verify_cert_valid(ctx, leaf, NULL))
-                goto err;
-
-        if (!x509_verify_cert_hostname(ctx, leaf, name))
-                goto err;
-
         if ((current_chain = x509_verify_chain_new()) == NULL) {
                 ctx->error = X509_V_ERR_OUT_OF_MEM;
                 goto err;
         }
+
+        /*
+         * Add the leaf to the chain and try to build chains from it.
+         * Note that unlike Go's verifier, we have not yet checked
+         * anything about the leaf, This is intentional, so that we
+         * report failures in chain building before we report problems
+         * with the leaf.
+         */
         if (!x509_verify_chain_append(current_chain, leaf, &ctx->error)) {
                 x509_verify_chain_free(current_chain);
                 goto err;
@@ -1133,13 +1145,14 @@ x509_verify(struct x509_verify_ctx *ctx, X509 *leaf, char *name)
         do {
                 retry_chain_build = 0;
                 if (x509_verify_ctx_cert_is_root(ctx, leaf, full_chain)) {
-                        if (!x509_verify_ctx_add_chain(ctx, current_chain)) {
+                        if (!x509_verify_ctx_add_chain(ctx, current_chain,
+                            name)) {
                                 x509_verify_chain_free(current_chain);
                                 goto err;
                         }
                 } else {
                         x509_verify_build_chains(ctx, leaf, current_chain,
-                            full_chain);
+                            full_chain, name);
                         if (full_chain && ctx->chains_count == 0) {
                                 /*
                                  * Save the error state from the xsc
diff --git a/src/regress/lib/libcrypto/x509/Makefile b/src/regress/lib/libcrypto/x509/Makefile
index 919bef31d1..ca66df19cd 100644
--- a/src/regress/lib/libcrypto/x509/Makefile
+++ b/src/regress/lib/libcrypto/x509/Makefile
@@ -1,6 +1,7 @@
-#       $OpenBSD: Makefile,v 1.12 2022/06/02 12:08:41 tb Exp $
+#       $OpenBSD: Makefile,v 1.13 2022/06/25 20:01:43 beck Exp $
 
 PROGS = constraints verify x509attribute x509name x509req_ext callback
+PROGS += expirecallback
 LDADD = -lcrypto
 DPADD = ${LIBCRYPTO}
 
@@ -18,6 +19,7 @@ REGRESS_TARGETS += regress-x509attribute
 REGRESS_TARGETS += regress-x509name
 REGRESS_TARGETS += regress-x509req_ext
 REGRESS_TARGETS += regress-callback
+REGRESS_TARGETS += regress-expirecallback
 
 CLEANFILES +=   x509name.result callbackout
 
@@ -49,4 +51,7 @@ regress-callback: callback
         ./callback ${.CURDIR}/../certs
         perl ${.CURDIR}/callback.pl callback.out
 
+regress-expirecallback: expirecallback
+        ./expirecallback ${.CURDIR}/../certs
+
 .include <bsd.regress.mk>
diff --git a/src/regress/lib/libcrypto/x509/expirecallback.c b/src/regress/lib/libcrypto/x509/expirecallback.c
new file mode 100644
index 0000000000..3da49708ff
--- /dev/null
+++ b/src/regress/lib/libcrypto/x509/expirecallback.c
@@ -0,0 +1,279 @@
+/* $OpenBSD: expirecallback.c,v 1.1 2022/06/25 20:01:43 beck Exp $ */
+/*
+ * Copyright (c) 2020 Joel Sing <jsing@openbsd.org>
+ * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <err.h>
+#include <string.h>
+
+#include <openssl/bio.h>
+#include <openssl/err.h>
+#include <openssl/pem.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/x509_verify.h>
+
+#define MODE_MODERN_VFY         0
+#define MODE_MODERN_VFY_DIR     1
+#define MODE_LEGACY_VFY         2
+#define MODE_VERIFY             3
+
+static int verbose = 1;
+
+static int
+passwd_cb(char *buf, int size, int rwflag, void *u)
+{
+        memset(buf, 0, size);
+        return (0);
+}
+
+static int
+certs_from_file(const char *filename, STACK_OF(X509) **certs)
+{
+        STACK_OF(X509_INFO) *xis = NULL;
+        STACK_OF(X509) *xs = NULL;
+        BIO *bio = NULL;
+        X509 *x;
+        int i;
+
+        if ((xs = sk_X509_new_null()) == NULL)
+                errx(1, "failed to create X509 stack");
+        if ((bio = BIO_new_file(filename, "r")) == NULL) {
+                ERR_print_errors_fp(stderr);
+                errx(1, "failed to create bio");
+        }
+        if ((xis = PEM_X509_INFO_read_bio(bio, NULL, passwd_cb, NULL)) == NULL)
+                errx(1, "failed to read PEM");
+
+        for (i = 0; i < sk_X509_INFO_num(xis); i++) {
+                if ((x = sk_X509_INFO_value(xis, i)->x509) == NULL)
+                        continue;
+                if (!sk_X509_push(xs, x))
+                        errx(1, "failed to push X509");
+                X509_up_ref(x);
+        }
+
+        *certs = xs;
+        xs = NULL;
+
+        sk_X509_INFO_pop_free(xis, X509_INFO_free);
+        sk_X509_pop_free(xs, X509_free);
+        BIO_free(bio);
+
+        return 1;
+}
+
+static int
+verify_cert_cb(int ok, X509_STORE_CTX *xsc)
+{
+        X509 *current_cert;
+        int verify_err;
+
+        current_cert = X509_STORE_CTX_get_current_cert(xsc);
+        if (current_cert != NULL) {
+                X509_NAME_print_ex_fp(stderr,
+                    X509_get_subject_name(current_cert), 0,
+                    XN_FLAG_ONELINE);
+                fprintf(stderr, "\n");
+        }
+
+        verify_err = X509_STORE_CTX_get_error(xsc);
+        if (verify_err != X509_V_OK) {
+                if (verify_err == X509_V_ERR_CERT_HAS_EXPIRED)
+                        fprintf(stderr, "IGNORING ");
+                fprintf(stderr, "verify error at depth %d: %s\n",
+                    X509_STORE_CTX_get_error_depth(xsc),
+                    X509_verify_cert_error_string(verify_err));
+        }
+
+        /*
+         * Ignore expired certs, in the way people are told to do it
+         * by OpenSSL
+         */
+
+        if (verify_err == X509_V_ERR_CERT_HAS_EXPIRED)
+                return 1;
+
+        return ok;
+}
+
+static void
+verify_cert(const char *roots_dir, const char *roots_file,
+    const char *bundle_file, int *chains, int mode)
+{
+        STACK_OF(X509) *roots = NULL, *bundle = NULL;
+        time_t future = 2000000000; /* May 17 2033 */
+        X509_STORE_CTX *xsc = NULL;
+        X509_STORE *store = NULL;
+        int verify_err, use_dir;
+        X509 *leaf = NULL;
+
+        *chains = 0;
+        use_dir = (mode == MODE_MODERN_VFY_DIR);
+
+        if (!use_dir && !certs_from_file(roots_file, &roots))
+                errx(1, "failed to load roots from '%s'", roots_file);
+        if (!certs_from_file(bundle_file, &bundle))
+                errx(1, "failed to load bundle from '%s'", bundle_file);
+        if (sk_X509_num(bundle) < 1)
+                errx(1, "not enough certs in bundle");
+        leaf = sk_X509_shift(bundle);
+
+        if ((xsc = X509_STORE_CTX_new()) == NULL)
+                errx(1, "X509_STORE_CTX");
+        if (use_dir && (store = X509_STORE_new()) == NULL)
+                errx(1, "X509_STORE");
+        if (!X509_STORE_CTX_init(xsc, store, leaf, bundle)) {
+                ERR_print_errors_fp(stderr);
+                errx(1, "failed to init store context");
+        }
+
+        /*
+         * Set the time int the future to exercise the expired cert
+         * callback
+         */
+        X509_STORE_CTX_set_time(xsc, 0, future);
+
+        if (use_dir) {
+                if (!X509_STORE_load_locations(store, NULL, roots_dir))
+                        errx(1, "failed to set by_dir directory of %s", roots_dir);
+        }
+        if (mode == MODE_LEGACY_VFY)
+                X509_STORE_CTX_set_flags(xsc, X509_V_FLAG_LEGACY_VERIFY);
+        else
+                X509_VERIFY_PARAM_clear_flags(X509_STORE_CTX_get0_param(xsc),
+                    X509_V_FLAG_LEGACY_VERIFY);
+
+        if (verbose)
+                X509_STORE_CTX_set_verify_cb(xsc, verify_cert_cb);
+        if (!use_dir)
+                X509_STORE_CTX_set0_trusted_stack(xsc, roots);
+        if (X509_verify_cert(xsc) == 1) {
+                *chains = 1; /* XXX */
+                goto done;
+        }
+
+        verify_err = X509_STORE_CTX_get_error(xsc);
+        if (verify_err == 0)
+                errx(1, "Error unset on failure!\n");
+
+        fprintf(stderr, "failed to verify at %d: %s\n",
+            X509_STORE_CTX_get_error_depth(xsc),
+            X509_verify_cert_error_string(verify_err));
+
+ done:
+        sk_X509_pop_free(roots, X509_free);
+        sk_X509_pop_free(bundle, X509_free);
+        X509_STORE_free(store);
+        X509_STORE_CTX_free(xsc);
+        X509_free(leaf);
+}
+
+struct verify_cert_test {
+        const char *id;
+        int want_chains;
+        int failing;
+};
+
+struct verify_cert_test verify_cert_tests[] = {
+        {
+                .id = "1a",
+                .want_chains = 1,
+        },
+        {
+                .id = "2a",
+                .want_chains = 1,
+                .failing = 1,
+        },
+        {
+                .id = "2b",
+                .want_chains = 0,
+        },
+        {
+                .id = "2c",
+                .want_chains = 1,
+                .failing = 1,
+        },
+};
+
+#define N_VERIFY_CERT_TESTS \
+    (sizeof(verify_cert_tests) / sizeof(*verify_cert_tests))
+
+static int
+verify_cert_test(const char *certs_path, int mode)
+{
+        char *roots_file, *bundle_file, *roots_dir;
+        struct verify_cert_test *vct;
+        int failed = 0;
+        int chains;
+        size_t i;
+
+        for (i = 0; i < N_VERIFY_CERT_TESTS; i++) {
+                vct = &verify_cert_tests[i];
+
+                if (asprintf(&roots_file, "%s/%s/roots.pem", certs_path,
+                    vct->id) == -1)
+                        errx(1, "asprintf");
+                if (asprintf(&bundle_file, "%s/%s/bundle.pem", certs_path,
+                    vct->id) == -1)
+                        errx(1, "asprintf");
+                if (asprintf(&roots_dir, "./%s/roots", vct->id) == -1)
+                        errx(1, "asprintf");
+
+                fprintf(stderr, "== Test %zu (%s)\n", i, vct->id);
+                verify_cert(roots_dir, roots_file, bundle_file, &chains, mode);
+                if ((mode == MODE_VERIFY && chains == vct->want_chains) ||
+                    (chains == 0 && vct->want_chains == 0) ||
+                    (chains == 1 && vct->want_chains > 0)) {
+                        fprintf(stderr, "INFO: Succeeded with %d chains%s\n",
+                            chains, vct->failing ? " (legacy failure)" : "");
+                        if (mode == MODE_LEGACY_VFY && vct->failing)
+                                failed |= 1;
+                } else {
+                        fprintf(stderr, "FAIL: Failed with %d chains%s\n",
+                            chains, vct->failing ? " (legacy failure)" : "");
+                        if (!vct->failing)
+                                failed |= 1;
+                }
+                fprintf(stderr, "\n");
+
+                free(roots_file);
+                free(bundle_file);
+                free(roots_dir);
+        }
+
+        return failed;
+}
+
+int
+main(int argc, char **argv)
+{
+        int failed = 0;
+
+        if (argc != 2) {
+                fprintf(stderr, "usage: %s <certs_path>\n", argv[0]);
+                exit(1);
+        }
+
+        fprintf(stderr, "\n\nTesting legacy x509_vfy\n");
+        failed |= verify_cert_test(argv[1], MODE_LEGACY_VFY);
+        fprintf(stderr, "\n\nTesting modern x509_vfy\n");
+        failed |= verify_cert_test(argv[1], MODE_MODERN_VFY);
+        fprintf(stderr, "\n\nTesting modern x509_vfy by_dir\n");
+        failed |= verify_cert_test(argv[1], MODE_MODERN_VFY_DIR);
+
+        return (failed);
+}