From 10d8ec22a81b906974b9ddc26f92c35523c5284c Mon Sep 17 00:00:00 2001
From: tb <>
Date: Thu, 21 Apr 2022 04:48:12 +0000
Subject: Avoid expensive RFC 3779 checks during cert verification

X509v3_{addr,asid}_is_canonical() check that the ipAddrBlocks and
autonomousSysIds extension conform to RFC 3779. These checks are not
cheap. Certs containing non-conformant extensions should not be
considered valid, so mark them with EXFLAG_INVALID while caching the
extension information in x509v3_cache_extensions(). This way the
expensive check while walking the chains during X509_verify_cert() is
replaced with a cheap check of the extension flags. This avoids a lot
of superfluous work when validating numerous certs with similar chains
against the same roots as is done in rpki-client.

Issue noticed and fix suggested by claudio
ok claudio inoguchi jsing
---
 src/lib/libcrypto/x509/x509_addr.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

(limited to 'src/lib/libcrypto/x509/x509_addr.c')

diff --git a/src/lib/libcrypto/x509/x509_addr.c b/src/lib/libcrypto/x509/x509_addr.c
index 035353826b..d33d4f2f8e 100644
--- a/src/lib/libcrypto/x509/x509_addr.c
+++ b/src/lib/libcrypto/x509/x509_addr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: x509_addr.c,v 1.78 2022/03/16 11:44:36 tb Exp $ */
+/*	$OpenBSD: x509_addr.c,v 1.79 2022/04/21 04:48:12 tb Exp $ */
 /*
  * Contributed to the OpenSSL Project by the American Registry for
  * Internet Numbers ("ARIN").
@@ -1780,11 +1780,11 @@ addr_validate_path_internal(X509_STORE_CTX *ctx, STACK_OF(X509) *chain,
 	if (ext == NULL) {
 		depth = 0;
 		cert = sk_X509_value(chain, depth);
+		if ((X509_get_extension_flags(cert) & EXFLAG_INVALID) != 0)
+			goto done;
 		if ((ext = cert->rfc3779_addr) == NULL)
 			goto done;
-	}
-
-	if (!X509v3_addr_is_canonical(ext)) {
+	} else if (!X509v3_addr_is_canonical(ext)) {
 		if ((ret = verify_error(ctx, cert,
 		    X509_V_ERR_INVALID_EXTENSION, depth)) == 0)
 			goto done;
@@ -1806,6 +1806,12 @@ addr_validate_path_internal(X509_STORE_CTX *ctx, STACK_OF(X509) *chain,
 	for (depth++; depth < sk_X509_num(chain); depth++) {
 		cert = sk_X509_value(chain, depth);
 
+		if ((X509_get_extension_flags(cert) & EXFLAG_INVALID) != 0) {
+			if ((ret = verify_error(ctx, cert,
+			    X509_V_ERR_INVALID_EXTENSION, depth)) == 0)
+				goto done;
+		}
+
 		if ((parent = cert->rfc3779_addr) == NULL) {
 			for (i = 0; i < sk_IPAddressFamily_num(child); i++) {
 				child_af = sk_IPAddressFamily_value(child, i);
@@ -1822,12 +1828,6 @@ addr_validate_path_internal(X509_STORE_CTX *ctx, STACK_OF(X509) *chain,
 			continue;
 		}
 
-		if (!X509v3_addr_is_canonical(parent)) {
-			if ((ret = verify_error(ctx, cert,
-			    X509_V_ERR_INVALID_EXTENSION, depth)) == 0)
-				goto done;
-		}
-
 		/*
 		 * Check that the child's resources are covered by the parent.
 		 * Each covered resource is replaced with the parent's resource
-- 
cgit v1.2.3-55-g6feb