rpki-client: reject certs with duplicate extension OIDs

Theo Buehler <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
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);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.