From 76281e496c3671bb6099f1e9ed1300009eb705f9 Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Fri, 23 Jun 2023 07:40:28 +0000
Subject: [PATCH] rpki-client: check for duplicate certificate extensions

RFC 5280 disallows multiple extensions with the same OID. Since libcrypto
does not check that currently, do this by hand. This only deals with CA
certs for now, EE certs could do that similarly.

Found with BBN test corpora

ok job
---
 usr.sbin/rpki-client/cert.c | 41 +++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 387ee9d08d6..ecef2026d23 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cert.c,v 1.110 2023/06/23 07:26:21 tb Exp $ */
+/*	$OpenBSD: cert.c,v 1.111 2023/06/23 07:40:28 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -662,7 +662,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 {
 	const unsigned char	*oder;
 	int			 extsz;
-	int			 sia_present = 0;
 	size_t			 i;
 	X509			*x = NULL;
 	X509_EXTENSION		*ext = NULL;
@@ -672,7 +671,10 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 	ASN1_OBJECT		*obj;
 	EVP_PKEY		*pkey;
 	struct parse		 p;
-	int			 nid;
+	int			 nid, ip, as, sia, cp, crldp, aia, aki, ski,
+				 eku, bc, ku;
+
+	nid = ip = as = sia = cp = crldp = aia = aki = ski = eku = bc = ku = 0;
 
 	/* just fail for empty buffers, the warning was printed elsewhere */
 	if (der == NULL)
@@ -735,38 +737,58 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		obj = X509_EXTENSION_get_object(ext);
 		assert(obj != NULL);
 
-		switch (OBJ_obj2nid(obj)) {
+		switch ((nid = OBJ_obj2nid(obj))) {
 		case NID_sbgp_ipAddrBlock:
+			if (ip++ >= 1)
+				goto dup;
 			if (!sbgp_ipaddrblk(&p, ext))
 				goto out;
 			break;
 		case NID_sbgp_autonomousSysNum:
+			if (as++ > 0)
+				goto dup;
 			if (!sbgp_assysnum(&p, ext))
 				goto out;
 			break;
 		case NID_sinfo_access:
-			sia_present = 1;
+			if (sia++ > 0)
+				goto dup;
 			if (!sbgp_sia(&p, ext))
 				goto out;
 			break;
 		case NID_certificate_policies:
+			if (cp++ > 0)
+				goto dup;
 			if (!certificate_policies(&p, ext))
 				goto out;
 			break;
 		case NID_crl_distribution_points:
-			/* ignored here, handled later */
+			if (crldp++ > 0)
+				goto dup;
 			break;
 		case NID_info_access:
+			if (aia++ > 0)
+				goto dup;
 			break;
 		case NID_authority_key_identifier:
+			if (aki++ > 0)
+				goto dup;
 			break;
 		case NID_subject_key_identifier:
+			if (ski++ > 0)
+				goto dup;
 			break;
 		case NID_ext_key_usage:
+			if (eku++ > 0)
+				goto dup;
 			break;
 		case NID_basic_constraints:
+			if (bc++ > 0)
+				goto dup;
 			break;
 		case NID_key_usage:
+			if (ku++ > 0)
+				goto dup;
 			break;
 		default:
 			/* unexpected extensions warrant investigation */
@@ -845,7 +867,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 				goto out;
 			}
 		}
-		if (sia_present) {
+		if (sia) {
 			warnx("%s: unexpected SIA extension in BGPsec cert",
 			    p.fn);
 			goto out;
@@ -864,7 +886,10 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 	p.res->x509 = x;
 	return p.res;
 
-out:
+ dup:
+	warnx("%s: RFC 5280 section 4.2: duplicate %s extension", fn,
+	    OBJ_nid2sn(nid));
+ out:
 	cert_free(p.res);
 	X509_free(x);
 	return NULL;
-- 
2.20.1