From: tb <tb@openbsd.org>
Date: Mon, 8 Jan 2024 10:06:50 +0000 (+0000)
Subject: Disable X509_STORE_CTX_purpose_inherit()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=208c5b2a90852687e27138d76eedc7ed4919aa4e;p=openbsd

Disable X509_STORE_CTX_purpose_inherit()

Nothing uses this function, except two internal callers. So split its guts
temporarily into a helper function and disable the gross general case.
The internal helper can be simplified by observing that def_purpose == 0:

Overriding 0 by 0 doesn't do anything, so drop that bit. Rename ptmp into
purp, and inline X509_PURPOSE_get_by_id(), i.e., make appropriate checks and
subtract X509_PURPOSE_MIN. The fallback to X509_PURPOSE_get_by_id(0) will
always fail since X509_PURPOSE_MIN == 1. So ditch that call. In particular,
X509_STORE_CTX_set_purpose(ctx, X509_PURPOSE_ANY) fails in current because
of this. That's nonsense. So remove the purp->trust == X509_TRUST_DEFAULT
check as only change of behavior. This matches what OpenSSL do nowadays.
They now set def_purpose = purpose if purpose != 0 and def_purpose == 0,
so in all real-world uses of this function they will just fetch the same
purpose again and do not check for default trust the second time around.
Finally, X509_TRUST_get_by_id() is only used to ensure that a non-zero (or
overridden) trust is between X509_TRUST_MIN and X509_TRUST_MAX. So expand
that into its explicit form.

ok jsing
---

diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c
index 92aa9dfc5b2..3d6b68afee5 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.136 2024/01/07 18:15:42 tb Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.137 2024/01/08 10:06:50 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2177,35 +2177,35 @@ int
 X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose,
     int purpose, int trust)
 {
-	int idx;
+	X509error(ERR_R_DISABLED);
+	return 0;
+}
+LCRYPTO_ALIAS(X509_STORE_CTX_purpose_inherit);
 
-	/* If purpose not set use default */
-	if (purpose == 0)
-		purpose = def_purpose;
+static int
+x509_vfy_purpose_inherit(X509_STORE_CTX *ctx, int purpose, int trust)
+{
 	/* If we have a purpose then check it is valid */
 	if (purpose != 0) {
-		X509_PURPOSE *ptmp;
-		idx = X509_PURPOSE_get_by_id(purpose);
-		if (idx == -1) {
+		const X509_PURPOSE *purp;
+		int purpose_idx;
+
+		if (purpose < X509_PURPOSE_MIN || purpose > X509_TRUST_MAX) {
 			X509error(X509_R_UNKNOWN_PURPOSE_ID);
 			return 0;
 		}
-		ptmp = X509_PURPOSE_get0(idx);
-		if (ptmp->trust == X509_TRUST_DEFAULT) {
-			idx = X509_PURPOSE_get_by_id(def_purpose);
-			if (idx == -1) {
-				X509error(X509_R_UNKNOWN_PURPOSE_ID);
-				return 0;
-			}
-			ptmp = X509_PURPOSE_get0(idx);
+		purpose_idx = purpose - X509_PURPOSE_MIN;
+		if ((purp = X509_PURPOSE_get0(purpose_idx)) == NULL) {
+			X509error(X509_R_UNKNOWN_PURPOSE_ID);
+			return 0;
 		}
-		/* If trust not set then get from purpose default */
+
+		/* If trust is unset, use the purpose's trust. */
 		if (trust == 0)
-			trust = ptmp->trust;
+			trust = purp->trust;
 	}
 	if (trust != 0) {
-		idx = X509_TRUST_get_by_id(trust);
-		if (idx == -1) {
+		if (trust < X509_TRUST_MIN || trust > X509_TRUST_MAX) {
 			X509error(X509_R_UNKNOWN_TRUST_ID);
 			return 0;
 		}
@@ -2218,19 +2218,18 @@ X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose,
 
 	return 1;
 }
-LCRYPTO_ALIAS(X509_STORE_CTX_purpose_inherit);
 
 int
 X509_STORE_CTX_set_purpose(X509_STORE_CTX *ctx, int purpose)
 {
-	return X509_STORE_CTX_purpose_inherit(ctx, 0, purpose, 0);
+	return x509_vfy_purpose_inherit(ctx, purpose, 0);
 }
 LCRYPTO_ALIAS(X509_STORE_CTX_set_purpose);
 
 int
 X509_STORE_CTX_set_trust(X509_STORE_CTX *ctx, int trust)
 {
-	return X509_STORE_CTX_purpose_inherit(ctx, 0, 0, trust);
+	return x509_vfy_purpose_inherit(ctx, 0, trust);
 }
 LCRYPTO_ALIAS(X509_STORE_CTX_set_trust);