From: Theo Buehler <tb@theobuehler.org>
Subject: rpki-client: reject certs with duplicate extension OIDs
To: tech@openbsd.org
Date: Fri, 12 Jun 2026 17:40:27 +0200

We check this for extensions we know about in cert_parse_extensions().
I think we should keep doing it there since we need to keep track of the
extensions we encountered anyway.

While cert_parse_extensions() rejects certs with critical extensions we
don't know about, we allow duplicates non-critical ones mainly because
that's annoying to keep track of. LibreSSL's libcrypto checks for this
and rejects the cert when caching the extensions, OpenSSL 4 adds an
EXFLAG_DUPLICATE flag and accepts the cert, OpenSSL 3 ignores
duplicates. In short: we get to do it ourselves.

This check is basically lifted from libcrypto's x509_purp.c with a few
extra contortions due to const sprinkling and making things opaque. The
warning is the same as the one already present in cert_parse_extensions().

I do not NULL check X509_EXTENSION_get_object() because the extension
parsed, so an OID is present.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.237 cert.c
--- cert.c	16 May 2026 07:27:03 -0000	1.237
+++ cert.c	12 Jun 2026 15:36:24 -0000
@@ -34,6 +34,68 @@ int certid = TALSZ_MAX;
 
 static pthread_rwlock_t	cert_lk = PTHREAD_RWLOCK_INITIALIZER;
 
+/* Helper to sort a STACK_OF(X509_EXTENSION) by OID. */
+static int
+cert_extension_oid_cmp(const X509_EXTENSION *const *a,
+    const X509_EXTENSION *const *b)
+{
+	/* XXX - cast away const for OpenSSL 3 and LibreSSL */
+	const ASN1_OBJECT *ao = X509_EXTENSION_get_object((X509_EXTENSION *)*a);
+	const ASN1_OBJECT *bo = X509_EXTENSION_get_object((X509_EXTENSION *)*b);
+
+	return OBJ_cmp(ao, bo);
+}
+
+/*
+ * Per RFC 5280, Section 4.2, a certificate MUST NOT include more than
+ * one instance of a particular extension.
+ * Returns 1 if certificate conforms to this and 0 otherwise.
+ */
+static int
+cert_extension_oids_are_unique(const char *fn, const struct cert *cert)
+{
+	const X509 *x509 = cert->x509;
+	const STACK_OF(X509_EXTENSION) *cexts = NULL;
+	STACK_OF(X509_EXTENSION) *exts = NULL;
+	const X509_EXTENSION *prev, *curr;
+	const ASN1_OBJECT *obj;
+	int i, nid, rc = 0;
+
+	if (X509_get_ext_count(x509) <= 1)
+		goto done;
+
+	if ((cexts = X509_get0_extensions(x509)) == NULL)
+		goto out;
+
+	if ((exts = sk_X509_EXTENSION_dup(cexts)) == NULL)
+		goto out;
+
+	(void)sk_X509_EXTENSION_set_cmp_func(exts, cert_extension_oid_cmp);
+	sk_X509_EXTENSION_sort(exts);
+
+	prev = sk_X509_EXTENSION_value(exts, 0);
+	for (i = 1; i < sk_X509_EXTENSION_num(exts); i++) {
+		curr = sk_X509_EXTENSION_value(exts, i);
+		if (cert_extension_oid_cmp(&prev, &curr) == 0) {
+			/* XXX - cast away const for OpenSSL 3 and LibreSSL */
+			obj = X509_EXTENSION_get_object((X509_EXTENSION *)curr);
+			nid = OBJ_obj2nid(obj);
+			warnx("%s: RFC 5280 section 4.2: duplicate extension: "
+			   "%s", fn, nid2str(nid));
+			goto out;
+		}
+		prev = curr;
+	}
+
+ done:
+	rc = 1;
+
+ out:
+	sk_X509_EXTENSION_free(exts);
+
+	return rc;
+}
+
 /*
  * Check the cert's purpose: the cA bit in basic constraints distinguishes
  * between TA/CA and EE/BGPsec router and the key usage bits must match.
@@ -60,6 +122,9 @@ cert_check_purpose(const char *fn, struc
 		warnx("%s: could not cache X509v3 extensions", fn);
 		goto out;
 	}
+
+	if (!cert_extension_oids_are_unique(fn, cert))
+		goto out;
 
 	ext_flags = X509_get_extension_flags(x);