Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: split a few things out of ta_parse()
To:
tech@openbsd.org
Date:
Mon, 19 Jan 2026 08:47:54 +0100

Download raw body.

Thread
This pulls the non-inheritance check for TAs into cert_parse_extensions()
and adds a check that the INRs are a non-empty set. The latter is redundant
with existing checks for presence of at least one of ASIdentifiers and
IPAddrBlocks combined with non-inheritance but it does not hurt to be
explicit.

The second change is splitting everything to do with the SPKI into a
helper since the current logic is messy and has completely unrelated
things interleaved. In a follow-up I'll also split out the validity
check. Later on ta_parse() will be renamed into something more
appropriate.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.210 cert.c
--- cert.c	13 Jan 2026 21:36:17 -0000	1.210
+++ cert.c	14 Jan 2026 20:11:20 -0000
@@ -1738,6 +1738,19 @@ cert_parse_extensions(const char *fn, st
 		goto out;
 	}
 
+	if (cert->purpose == CERT_PURPOSE_TA) {
+		if (x509_any_inherits(cert->x509)) {
+			warnx("%s: RFC 8630, 2.3: Trust Anchor INRs "
+			    "must not inherit", fn);
+			goto out;
+		}
+		if (cert->num_ips == 0 && cert->num_ases == 0) {
+			warnx("%s: RFC 8630, 2.3: Trust Anchor INR set "
+			    "must not be empty", fn);
+			goto out;
+		}
+	}
+
 	if (cert->purpose == CERT_PURPOSE_BGPSEC_ROUTER) {
 		if (ip != 0) {
 			warnx("%s: RFC 8209, 3.1.3.4: BGPsec Router cert "
@@ -1915,15 +1928,17 @@ cert_parse(const char *fn, const unsigne
 	return NULL;
 }
 
-struct cert *
-ta_parse(const char *fn, struct cert *p, const unsigned char *pkey,
+/*
+ * Check that the subjectPublicKeyInfo from the TAL matches the one in the cert.
+ * Verify that this key signed the cert.
+ * Returns 1 on success and 0 on failure.
+ */
+static int
+ta_check_pubkey(const char *fn, struct cert *cert, const unsigned char *pkey,
     size_t pkeysz)
 {
 	EVP_PKEY	*pk, *opk;
-	time_t		 now = get_current_time();
-
-	if (p == NULL)
-		return NULL;
+	int		 rv = 0;
 
 	/* first check pubkey against the one from the TAL */
 	pk = d2i_PUBKEY(NULL, &pkey, pkeysz);
@@ -1931,7 +1946,7 @@ ta_parse(const char *fn, struct cert *p,
 		warnx("%s: RFC 6487 (trust anchor): bad TAL pubkey", fn);
 		goto badcert;
 	}
-	if ((opk = X509_get0_pubkey(p->x509)) == NULL) {
+	if ((opk = X509_get0_pubkey(cert->x509)) == NULL) {
 		warnx("%s: RFC 6487 (trust anchor): missing pubkey", fn);
 		goto badcert;
 	}
@@ -1941,37 +1956,52 @@ ta_parse(const char *fn, struct cert *p,
 		goto badcert;
 	}
 
-	if (p->notbefore > now) {
-		warnx("%s: certificate not yet valid", fn);
-		goto badcert;
-	}
-	if (p->notafter < now) {
-		warnx("%s: certificate has expired", fn);
+	/*
+	 * Do not replace with a <= 0 check since OpenSSL 3 broke that:
+	 * https://github.com/openssl/openssl/issues/24575
+	 */
+	if (X509_verify(cert->x509, pk) != 1) {
+		warnx("%s: failed to verify signature", fn);
 		goto badcert;
 	}
+
+	rv = 1;
+ badcert:
+	EVP_PKEY_free(pk);
+	return rv;
+}
+
+/* XXX - this really doesn't parse anything except the SPKI in the TAL. */
+struct cert *
+ta_parse(const char *fn, struct cert *p, const unsigned char *pkey,
+    size_t pkeysz)
+{
+	time_t		 now = get_current_time();
+
+	if (p == NULL)
+		return NULL;
+
 	if (p->purpose != CERT_PURPOSE_TA) {
 		warnx("%s: expected trust anchor purpose, got %s", fn,
 		    purpose2str(p->purpose));
 		goto badcert;
 	}
-	/*
-	 * Do not replace with a <= 0 check since OpenSSL 3 broke that:
-	 * https://github.com/openssl/openssl/issues/24575
-	 */
-	if (X509_verify(p->x509, pk) != 1) {
-		warnx("%s: failed to verify signature", fn);
+
+	if (!ta_check_pubkey(fn, p, pkey, pkeysz))
+		goto badcert;
+
+	if (p->notbefore > now) {
+		warnx("%s: certificate not yet valid", fn);
 		goto badcert;
 	}
-	if (x509_any_inherits(p->x509)) {
-		warnx("%s: Trust anchor IP/AS resources may not inherit", fn);
+	if (p->notafter < now) {
+		warnx("%s: certificate has expired", fn);
 		goto badcert;
 	}
 
-	EVP_PKEY_free(pk);
 	return p;
 
  badcert:
-	EVP_PKEY_free(pk);
 	cert_free(p);
 	return NULL;
 }