From: tb <tb@openbsd.org>
Date: Sat, 8 Jun 2024 13:28:35 +0000 (+0000)
Subject: Add a x509_cache_extensions() helper
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e891962d45dc3df273cac7428d6c43204f2989d1;p=openbsd

Add a x509_cache_extensions() helper

This is a simple wrapper around X509_check_policy(cert, -1, 0) that
doesn't need an explanatory comment in the caller.

The reason for having to do this is that various OpenSSL API calls rely
on having extension information cached. As an unsurprising consequence of
OpenSSL's characteristic API misdesign these calls can't report errors,
so they call the extension caching without error checking and the result
is that they may report nonsense.

To work around this, cache the extensions up front so a second call can't
fail and thus API calls such as X509_check_ca(), X509_get_key_usage() and
X509_cmp() work reliably.

ok job
---

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 0faf997c454..ae162652626 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cert.c,v 1.141 2024/06/07 08:36:54 tb Exp $ */
+/*	$OpenBSD: cert.c,v 1.142 2024/06/08 13:28:35 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -744,6 +744,9 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
 	if (!cert_check_subject_and_issuer(fn, x))
 		goto out;
 
+	if (!x509_cache_extensions(x, fn))
+		goto out;
+
 	if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
 		warnx("%s: RFC 6487 section 4.8.4: KU must be digitalSignature",
 		    fn);
@@ -827,11 +830,8 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		goto out;
 	}
 
-	/* Cache X509v3 extensions, see X509_check_ca(3). */
-	if (X509_check_purpose(x, -1, -1) <= 0) {
-		warnx("%s: could not cache X509v3 extensions", fn);
+	if (!x509_cache_extensions(x, fn))
 		goto out;
-	}
 
 	if (X509_get_version(x) != 2) {
 		warnx("%s: RFC 6487 4.1: X.509 version must be v3", fn);
diff --git a/usr.sbin/rpki-client/cms.c b/usr.sbin/rpki-client/cms.c
index 5ef647e1542..0394680e5c0 100644
--- a/usr.sbin/rpki-client/cms.c
+++ b/usr.sbin/rpki-client/cms.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cms.c,v 1.45 2024/05/24 12:57:20 tb Exp $ */
+/*	$OpenBSD: cms.c,v 1.46 2024/06/08 13:28:35 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -324,11 +324,8 @@ cms_parse_validate_internal(X509 **xp, const char *fn, const unsigned char *der,
 		goto out;
 	}
 
-	/* Cache X509v3 extensions, see X509_check_ca(3). */
-	if (X509_check_purpose(*xp, -1, -1) <= 0) {
-		warnx("%s: could not cache X509v3 extensions", fn);
+	if (!x509_cache_extensions(*xp, fn))
 		goto out;
-	}
 
 	if (!x509_get_notafter(*xp, fn, &notafter))
 		goto out;
diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index 601d7d2893e..446eaf12982 100644
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: extern.h,v 1.221 2024/06/04 04:17:18 tb Exp $ */
+/*	$OpenBSD: extern.h,v 1.222 2024/06/08 13:28:35 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -901,6 +901,7 @@ struct ibuf	*io_buf_recvfd(int, struct ibuf **);
 /* X509 helpers. */
 
 void		 x509_init_oid(void);
+int		 x509_cache_extensions(X509 *, const char *);
 int		 x509_get_aia(X509 *, const char *, char **);
 int		 x509_get_aki(X509 *, const char *, char **);
 int		 x509_get_sia(X509 *, const char *, char **);
diff --git a/usr.sbin/rpki-client/x509.c b/usr.sbin/rpki-client/x509.c
index dc9ffcb7ca5..3c0517c89ab 100644
--- a/usr.sbin/rpki-client/x509.c
+++ b/usr.sbin/rpki-client/x509.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: x509.c,v 1.94 2024/06/07 08:36:54 tb Exp $ */
+/*	$OpenBSD: x509.c,v 1.95 2024/06/08 13:28:35 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
@@ -132,6 +132,26 @@ x509_init_oid(void)
 	}
 }
 
+/*
+ * A number of critical OpenSSL API functions can't properly indicate failure
+ * and are unreliable if the extensions aren't already cached. An old trick is
+ * to cache the extensions using an error-checked call to X509_check_purpose()
+ * with a purpose of -1. This way functions such as X509_check_ca(), X509_cmp(),
+ * X509_get_key_usage(), X509_get_extended_key_usage() won't lie.
+ *
+ * Should be called right after deserialization and is essentially free to call
+ * multiple times.
+ */
+int
+x509_cache_extensions(X509 *x509, const char *fn)
+{
+	if (X509_check_purpose(x509, -1, 0) <= 0) {
+		warnx("%s: could not cache X509v3 extensions", fn);
+		return 0;
+	}
+	return 1;
+}
+
 /*
  * Parse X509v3 authority key identifier (AKI), RFC 6487 sec. 4.8.3.
  * Returns the AKI or NULL if it could not be parsed.