Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: split extension parsing out of cert_parse_pre()
To:
tech@openbsd.org
Date:
Mon, 30 Jun 2025 13:34:33 +0200

Download raw body.

Thread
Entirely straightforward refactor: extract a helper from cert_parse_pre()
so it can be shared with cert_parse_ee_cert(). I haven't hooked this
into cert_parse_ee_cert() yet because that makes the diff messy.
It will be the next step.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.174 cert.c
--- cert.c	30 Jun 2025 11:15:47 -0000	1.174
+++ cert.c	30 Jun 2025 11:21:09 -0000
@@ -1087,6 +1087,130 @@ cert_check_purpose(const char *fn, X509 
 }
 
 /*
+ * Parse extensions in a resource certificate following RFC 6487, section 4.8.
+ *
+ * Check issuance, basic constraints, and key usage bits are consistent and
+ * determine what kind of cert we were passed: annoyingly, callers can't
+ * distinguish BGPsec router certs (a rare kind of EE cert) from CA certs,
+ * as they both are .cer files in a Manifest.
+ *
+ * Then walk the list of extensions, handle each one and ensure there are no
+ * duplicates. Store the relevant information in cert.
+ */
+
+static int
+cert_parse_extensions(const char *fn, struct cert *cert, X509 *x)
+{
+	X509_EXTENSION	*ext;
+	ASN1_OBJECT	*obj;
+	int		 extsz, i, nid;
+	int		 bc, ski, aki, ku, eku, crldp, aia, sia, cp, ip, as;
+
+	nid = bc = ski = aki = ku = eku = crldp = aia = sia = cp = ip = as = 0;
+
+	/*
+	 * Check issuance, basic constraints and (extended) key usage bits are
+	 * appropriate for a resource cert. Covers RFC 6487 4.8.1, 4.8.4, 4.8.5.
+	 */
+	if ((cert->purpose = cert_check_purpose(fn, x)) == CERT_PURPOSE_INVALID)
+		goto out;
+
+	/* Look for X509v3 extensions. */
+	if ((extsz = X509_get_ext_count(x)) <= 0) {
+		warnx("%s: certificate without X.509v3 extensions", fn);
+		goto out;
+	}
+
+	for (i = 0; i < extsz; i++) {
+		ext = X509_get_ext(x, i);
+		assert(ext != NULL);
+		obj = X509_EXTENSION_get_object(ext);
+		assert(obj != NULL);
+
+		/* The switch is ordered following RFC 6487, section 4.8. */
+		switch (nid = OBJ_obj2nid(obj)) {
+		case NID_basic_constraints:
+			if (bc++ > 0)
+				goto dup;
+			break;
+		case NID_subject_key_identifier:
+			if (ski++ > 0)
+				goto dup;
+			break;
+		case NID_authority_key_identifier:
+			if (aki++ > 0)
+				goto dup;
+			break;
+		case NID_key_usage:
+			if (ku++ > 0)
+				goto dup;
+			break;
+		case NID_ext_key_usage:
+			if (eku++ > 0)
+				goto dup;
+			break;
+		case NID_crl_distribution_points:
+			if (crldp++ > 0)
+				goto dup;
+			break;
+		case NID_info_access:
+			if (aia++ > 0)
+				goto dup;
+			if (!cert_aia(fn, cert, ext))
+				goto out;
+			break;
+		case NID_sinfo_access:
+			if (sia++ > 0)
+				goto dup;
+			if (!cert_sia(fn, cert, ext))
+				goto out;
+			break;
+		case NID_certificate_policies:
+			if (cp++ > 0)
+				goto dup;
+			if (!certificate_policies(fn, cert, ext))
+				goto out;
+			break;
+		case NID_sbgp_ipAddrBlock:
+			if (ip++ > 0)
+				goto dup;
+			if (!sbgp_ipaddrblk(fn, cert, ext))
+				goto out;
+			break;
+		case NID_sbgp_autonomousSysNum:
+			if (as++ > 0)
+				goto dup;
+			if (!sbgp_assysnum(fn, cert, ext))
+				goto out;
+			break;
+		default:
+			/* unexpected extensions warrant investigation */
+			{
+				char objn[64];
+
+				OBJ_obj2txt(objn, sizeof(objn), obj, 0);
+				if (X509_EXTENSION_get_critical(ext)) {
+					warnx("%s: unknown critical extension "
+					    "%s (NID %d)", fn, objn, nid);
+					goto out;
+				}
+				warnx("%s: ignoring %s (NID %d)",
+				    fn, objn, nid);
+			}
+			break;
+		}
+	}
+
+	return 1;
+
+ dup:
+	warnx("%s: RFC 5280 section 4.2: duplicate extension: %s", fn,
+	    nid2str(nid));
+ out:
+	return 0;
+}
+
+/*
  * Lightweight version of cert_parse_pre() for EE certs.
  * Parses the two RFC 3779 extensions, and performs some sanity checks.
  * Returns cert on success and NULL on failure.
@@ -1170,16 +1294,10 @@ cert_parse_pre(const char *fn, const uns
 	struct cert		*cert;
 	const unsigned char	*oder;
 	size_t			 j;
-	int			 i, extsz;
 	X509			*x = NULL;
-	X509_EXTENSION		*ext = NULL;
 	const ASN1_BIT_STRING	*issuer_uid = NULL, *subject_uid = NULL;
-	ASN1_OBJECT		*obj;
 	EVP_PKEY		*pkey;
-	int			 nid, bc, ski, aki, ku, eku, crldp, aia, sia,
-				 cp, ip, as;
-
-	nid = bc = ski = aki = ku = eku = crldp = aia = sia = cp = ip = as = 0;
+	int			 nid;
 
 	/* just fail for empty buffers, the warning was printed elsewhere */
 	if (der == NULL)
@@ -1233,98 +1351,8 @@ cert_parse_pre(const char *fn, const uns
 	if (!cert_check_subject_and_issuer(fn, x))
 		goto out;
 
-	/*
-	 * Check issuance, basic constraints and (extended) key usage bits are
-	 * appropriate for a resource cert. Covers RFC 6487 4.8.1, 4.8.4, 4.8.5.
-	 */
-	if ((cert->purpose = cert_check_purpose(fn, x)) == CERT_PURPOSE_INVALID)
-		goto out;
-
-	/* Look for X509v3 extensions. */
-	if ((extsz = X509_get_ext_count(x)) <= 0) {
-		warnx("%s: certificate without X.509v3 extensions", fn);
+	if (!cert_parse_extensions(fn, cert, x))
 		goto out;
-	}
-
-	for (i = 0; i < extsz; i++) {
-		ext = X509_get_ext(x, i);
-		assert(ext != NULL);
-		obj = X509_EXTENSION_get_object(ext);
-		assert(obj != NULL);
-
-		/* The switch is ordered following RFC 6487, section 4.8. */
-		switch (nid = OBJ_obj2nid(obj)) {
-		case NID_basic_constraints:
-			if (bc++ > 0)
-				goto dup;
-			break;
-		case NID_subject_key_identifier:
-			if (ski++ > 0)
-				goto dup;
-			break;
-		case NID_authority_key_identifier:
-			if (aki++ > 0)
-				goto dup;
-			break;
-		case NID_key_usage:
-			if (ku++ > 0)
-				goto dup;
-			break;
-		case NID_ext_key_usage:
-			if (eku++ > 0)
-				goto dup;
-			break;
-		case NID_crl_distribution_points:
-			if (crldp++ > 0)
-				goto dup;
-			break;
-		case NID_info_access:
-			if (aia++ > 0)
-				goto dup;
-			if (!cert_aia(fn, cert, ext))
-				goto out;
-			break;
-		case NID_sinfo_access:
-			if (sia++ > 0)
-				goto dup;
-			if (!cert_sia(fn, cert, ext))
-				goto out;
-			break;
-		case NID_certificate_policies:
-			if (cp++ > 0)
-				goto dup;
-			if (!certificate_policies(fn, cert, ext))
-				goto out;
-			break;
-		case NID_sbgp_ipAddrBlock:
-			if (ip++ > 0)
-				goto dup;
-			if (!sbgp_ipaddrblk(fn, cert, ext))
-				goto out;
-			break;
-		case NID_sbgp_autonomousSysNum:
-			if (as++ > 0)
-				goto dup;
-			if (!sbgp_assysnum(fn, cert, ext))
-				goto out;
-			break;
-		default:
-			/* unexpected extensions warrant investigation */
-			{
-				char objn[64];
-
-				OBJ_obj2txt(objn, sizeof(objn), obj, 0);
-				if (X509_EXTENSION_get_critical(ext)) {
-					warnx("%s: unknown critical extension "
-					    "%s (NID %d)", fn, objn, nid);
-					goto out;
-				}
-				warnx("%s: ignoring %s (NID %d)",
-				    fn, objn, nid);
-			}
-			break;
-		}
-	}
 
 	if (!x509_get_aki(x, fn, &cert->aki))
 		goto out;
@@ -1392,9 +1420,6 @@ cert_parse_pre(const char *fn, const uns
 	cert->x509 = x;
 	return cert;
 
- dup:
-	warnx("%s: RFC 5280 section 4.2: duplicate extension: %s", fn,
-	    nid2str(nid));
  out:
 	cert_free(cert);
 	X509_free(x);