From 5822070f84708d84fa2bf715a3c3272c5437e707 Mon Sep 17 00:00:00 2001 From: beck Date: Thu, 17 Nov 2022 00:42:12 +0000 Subject: [PATCH] Revert "Check certificate extensions in trusted certificates" There are some possible strange side effects noticed by the openssl cms regress tests that I missed. Backing this out until I untangle it ok tb@ --- lib/libcrypto/x509/x509_internal.h | 3 +- lib/libcrypto/x509/x509_trs.c | 20 +++--------- lib/libcrypto/x509/x509_vfy.c | 49 ++---------------------------- 3 files changed, 8 insertions(+), 64 deletions(-) diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h index 9e80b2d2cf5..472b417403d 100644 --- a/lib/libcrypto/x509/x509_internal.h +++ b/lib/libcrypto/x509/x509_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_internal.h,v 1.21 2022/11/13 18:37:32 beck Exp $ */ +/* $OpenBSD: x509_internal.h,v 1.22 2022/11/17 00:42:12 beck Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -134,7 +134,6 @@ 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 b075d1b6c86..c4d371446cd 100644 --- a/lib/libcrypto/x509/x509_trs.c +++ b/lib/libcrypto/x509/x509_trs.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_trs.c,v 1.28 2022/11/14 17:48:50 beck Exp $ */ +/* $OpenBSD: x509_trs.c,v 1.29 2022/11/17 00:42:12 beck Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 1999. */ @@ -111,8 +111,8 @@ int } LCRYPTO_ALIAS(X509_TRUST_set_default) -static int -X509_check_trust_internal(X509 *x, int id, int flags, int compat) +int +X509_check_trust(X509 *x, int id, int flags) { X509_TRUST *pt; int idx; @@ -133,7 +133,7 @@ X509_check_trust_internal(X509 *x, int id, int flags, int compat) rv = obj_trust(NID_anyExtendedKeyUsage, x, 0); if (rv != X509_TRUST_UNTRUSTED) return rv; - return compat && trust_compat(NULL, x, 0); + return trust_compat(NULL, x, 0); } idx = X509_TRUST_get_by_id(id); if (idx == -1) @@ -143,18 +143,6 @@ X509_check_trust_internal(X509 *x, int id, int flags, int compat) } LCRYPTO_ALIAS(X509_check_trust) -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 09c0b8105e2..0a9965ae304 100644 --- a/lib/libcrypto/x509/x509_vfy.c +++ b/lib/libcrypto/x509/x509_vfy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.c,v 1.105 2022/11/14 17:48:50 beck Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.106 2022/11/17 00:42:12 beck Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -725,43 +725,6 @@ 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 */ @@ -778,7 +741,6 @@ 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; @@ -802,8 +764,8 @@ x509_vfy_check_chain_extensions(X509_STORE_CTX *ctx) purpose = ctx->param->purpose; } - chain_len = sk_X509_num(ctx->chain); - for (i = 0; i < chain_len; i++) { + /* Check all untrusted certificates */ + for (i = 0; i < ctx->num_untrusted; i++) { int ret; x = sk_X509_value(ctx->chain, i); if (!(ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) && @@ -857,11 +819,6 @@ 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) || -- 2.20.1