Check certificate extensions in trusted certificates.

Historically the standards let the implementation decide to
either check or ignore the certificate properties of trust anchors.
You could either use them simply as a source of a public key which
was trusted for everything, or you were also permitted to check the
certificate properties and fully enforce them. Hooray for freedumb.

OpenSSL changed to checking these with :
commit 0daccd4dc1f1ac62181738a91714f35472e50f3c
Author: Viktor Dukhovni <openssl-users@dukhovni.org>
Date:   Thu Jan 28 03:01:45 2016 -0500

BoringSSL currently does not check them, as it also inherited
the previous OpenSSL behaviour. It will change to check them in
the future.
(https://bugs.chromium.org/p/boringssl/issues/detail?id=533)

diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h
index beafd36..9e80b2d 100644 (file)
--- a/lib/libcrypto/x509/x509_internal.h
+++ b/lib/libcrypto/x509/x509_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_internal.h,v 1.20 2022/11/11 12:02:34 beck Exp $ */
+/* $OpenBSD: x509_internal.h,v 1.21 2022/11/13 18:37:32 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -134,6 +134,7 @@ int x509_constraints_check(struct x509_constraints_names *names,
     struct x509_constraints_names *excluded, int *error);
 int x509_constraints_chain(STACK_OF(X509) *chain, int *error,
     int *depth);
+int x509_check_trust_no_compat(X509 *x, int id, int flags);
 void x509_verify_cert_info_populate(X509 *cert);
 int x509_vfy_check_security_level(X509_STORE_CTX *ctx);
 

diff --git a/lib/libcrypto/x509/x509_trs.c b/lib/libcrypto/x509/x509_trs.c
index a967edf..23eca49 100644 (file)
--- a/lib/libcrypto/x509/x509_trs.c
+++ b/lib/libcrypto/x509/x509_trs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_trs.c,v 1.26 2022/11/10 16:52:19 beck Exp $ */
+/* $OpenBSD: x509_trs.c,v 1.27 2022/11/13 18:37:32 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -110,8 +110,8 @@ int
        return oldtrust;
 }
 
-int
-X509_check_trust(X509 *x, int id, int flags)
+static int
+X509_check_trust_internal(X509 *x, int id, int flags, int compat)
 {
        X509_TRUST *pt;
        int idx;
@@ -132,7 +132,7 @@ X509_check_trust(X509 *x, int id, int flags)
                rv = obj_trust(NID_anyExtendedKeyUsage, x, 0);
                if (rv != X509_TRUST_UNTRUSTED)
                        return rv;
-               return trust_compat(NULL, x, 0);
+               return compat && trust_compat(NULL, x, 0);
        }
        idx = X509_TRUST_get_by_id(id);
        if (idx == -1)
@@ -141,6 +141,18 @@ X509_check_trust(X509 *x, int id, int flags)
        return pt->check_trust(pt, x, flags);
 }
 
+int
+X509_check_trust(X509 *x, int id, int flags)
+{
+       return X509_check_trust_internal(x, id, flags, /*compat =*/1);
+}
+
+int
+x509_check_trust_no_compat(X509 *x, int id, int flags)
+{
+       return X509_check_trust_internal(x, id, flags, /*compat =*/0);
+}
+
 int
 X509_TRUST_get_count(void)
 {

diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c
index fb87877..11bf3d9 100644 (file)
--- a/lib/libcrypto/x509/x509_vfy.c
+++ b/lib/libcrypto/x509/x509_vfy.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vfy.c,v 1.103 2022/08/31 07:15:31 tb Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.104 2022/11/13 18:37:32 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -724,6 +724,43 @@ get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x)
                return 0;
 }
 
+/*
+ * X509_check_purpose is special.
+ * 0 is bad, 1 is good, values > 1 are maybe good for web pki necromancy
+ * and certificates that were checked into software unit tests years ago
+ * that nobody knows how to change. (Netscape Server Gated Crypto Forever!)
+ */
+#define PURPOSE_GOOD(x) (x == 1)
+#define PURPOSE_BAD(x) (x == 0)
+static int
+check_purpose(X509_STORE_CTX *ctx, X509 *x, int purpose, int depth,
+    int must_be_ca)
+{
+       int purpose_check, trust;
+
+       purpose_check = X509_check_purpose(x, purpose, must_be_ca > 0);
+       trust = X509_TRUST_UNTRUSTED;
+
+       /*
+        * For trusted certificates we want to see whether any auxiliary trust
+        * settings for the desired purpose override the purpose constraints
+        * from the certificate EKU.
+        */
+       if (depth >= ctx->num_untrusted && purpose == ctx->param->purpose)
+               trust = x509_check_trust_no_compat(x, ctx->param->trust, 0);
+
+       /* XXX STRICT should really be the default */
+       if (trust != X509_TRUST_REJECTED && !PURPOSE_BAD(purpose_check)) {
+               return PURPOSE_GOOD(purpose_check) ||
+                   (ctx->param->flags & X509_V_FLAG_X509_STRICT) == 0;
+       }
+
+       ctx->error = X509_V_ERR_INVALID_PURPOSE;
+       ctx->error_depth = depth;
+       ctx->current_cert = x;
+       return ctx->verify_cb(0, ctx);
+}
+
 /* Check a certificate chains extensions for consistency
  * with the supplied purpose
  */
@@ -740,6 +777,7 @@ x509_vfy_check_chain_extensions(X509_STORE_CTX *ctx)
        int proxy_path_length = 0;
        int purpose;
        int allow_proxy_certs;
+       size_t chain_len;
 
        cb = ctx->verify_cb;
 
@@ -763,8 +801,8 @@ x509_vfy_check_chain_extensions(X509_STORE_CTX *ctx)
                purpose = ctx->param->purpose;
        }
 
-       /* Check all untrusted certificates */
-       for (i = 0; i < ctx->num_untrusted; i++) {
+       chain_len = sk_X509_num(ctx->chain);
+       for (i = 0; i < chain_len; i++) {
                int ret;
                x = sk_X509_value(ctx->chain, i);
                if (!(ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) &&
@@ -818,6 +856,11 @@ x509_vfy_check_chain_extensions(X509_STORE_CTX *ctx)
                        if (!ok)
                                goto end;
                }
+               if (purpose > 0) {
+                       ok = check_purpose(ctx, x, purpose, i, must_be_ca);
+                       if (!ok)
+                               goto end;
+               }
                if (ctx->param->purpose > 0) {
                        ret = X509_check_purpose(x, purpose, must_be_ca > 0);
                        if ((ret == 0) ||