Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: SPKI in TALs
To:
tech@openbsd.org
Date:
Tue, 20 Jan 2026 09:02:12 +0100

Download raw body.

Thread
This is an almost entirely mechanical diff. The pkey hanging off struct tal
always confuses me since pkey always makes me think of EVP_PKEY. The combo
with pk and opk in a couple of functions makes this worse.

So: rename tal->pkey{,sz} to tal->spki{,sz} and pk/opk to pkey/opkey and
adjust a couple of nearby comments. Update from RFC 7730 to RFC 8630 while
there.

There's one additional change: in tal_parse_buffer() we currently accept
trailing garbage in the TAL (for example, you can just append AAAA to the
Base64 encoded SPKI in any *.tal right now and we won't notice). Check
that we consumed the full thing as we usually do. I'll land this separately.

We could add such checks to all the d2i_PUBKEY calls, but I'm not sure
it's worth it.

PS: ta_check_pubkey() has this annoying thing that we can't really get
our hands on the raw DER of the cert's SPKI to compare it with the TAL.
It's always going to be a version re-encoded by libcrypto. We could of
course memmem(), but even if that returns non-NULL, we can't be sure we
actually found the cert's SPKI.

Index: usr.sbin/rpki-client/cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.211 cert.c
--- usr.sbin/rpki-client/cert.c	19 Jan 2026 10:12:20 -0000	1.211
+++ usr.sbin/rpki-client/cert.c	20 Jan 2026 07:35:42 -0000
@@ -1934,23 +1934,23 @@ cert_parse(const char *fn, const unsigne
  * 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)
+ta_check_pubkey(const char *fn, struct cert *cert, const unsigned char *spki,
+    size_t spkisz)
 {
-	EVP_PKEY	*pk, *opk;
+	EVP_PKEY	*pkey, *opkey;
 	int		 rv = 0;
 
 	/* first check pubkey against the one from the TAL */
-	pk = d2i_PUBKEY(NULL, &pkey, pkeysz);
-	if (pk == NULL) {
+	pkey = d2i_PUBKEY(NULL, &spki, spkisz);
+	if (pkey == NULL) {
 		warnx("%s: RFC 6487 (trust anchor): bad TAL pubkey", fn);
 		goto badcert;
 	}
-	if ((opk = X509_get0_pubkey(cert->x509)) == NULL) {
+	if ((opkey = X509_get0_pubkey(cert->x509)) == NULL) {
 		warnx("%s: RFC 6487 (trust anchor): missing pubkey", fn);
 		goto badcert;
 	}
-	if (EVP_PKEY_cmp(pk, opk) != 1) {
+	if (EVP_PKEY_cmp(pkey, opkey) != 1) {
 		warnx("%s: RFC 6487 (trust anchor): "
 		    "pubkey does not match TAL pubkey", fn);
 		goto badcert;
@@ -1960,14 +1960,14 @@ ta_check_pubkey(const char *fn, struct c
 	 * 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) {
+	if (X509_verify(cert->x509, pkey) != 1) {
 		warnx("%s: failed to verify signature", fn);
 		goto badcert;
 	}
 
 	rv = 1;
  badcert:
-	EVP_PKEY_free(pk);
+	EVP_PKEY_free(pkey);
 	return rv;
 }
 
Index: usr.sbin/rpki-client/extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.270 extern.h
--- usr.sbin/rpki-client/extern.h	16 Jan 2026 11:25:27 -0000	1.270
+++ usr.sbin/rpki-client/extern.h	20 Jan 2026 07:35:42 -0000
@@ -169,7 +169,7 @@ RB_HEAD(nca_tree, nonfunc_ca);
 RB_PROTOTYPE(nca_tree, nonfunc_ca, entry, ncacmp);
 
 /*
- * The TAL file conforms to RFC 7730.
+ * The TAL file conforms to RFC 8630.
  * It is the top-level structure of RPKI and defines where we can find
  * certificates for TAs (trust anchors).
  * It also includes the public key for verifying those trust anchor
@@ -178,8 +178,8 @@ RB_PROTOTYPE(nca_tree, nonfunc_ca, entry
 struct tal {
 	char		**uri; /* well-formed rsync URIs */
 	size_t		 num_uris;
-	unsigned char	*pkey; /* DER-encoded public key */
-	size_t		 pkeysz; /* length of pkey */
+	unsigned char	*spki; /* DER-encoded subjectPublicKeyInfo */
+	size_t		 spkisz; /* length of SPKI */
 	char		*descr; /* basename of tal file */
 	int		 id; /* ID of this TAL */
 };
Index: usr.sbin/rpki-client/filemode.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
diff -u -p -r1.76 filemode.c
--- usr.sbin/rpki-client/filemode.c	16 Jan 2026 11:25:27 -0000	1.76
+++ usr.sbin/rpki-client/filemode.c	20 Jan 2026 07:35:42 -0000
@@ -262,7 +262,7 @@ parse_load_ta(struct tal *tal)
 
 	/* Extract certificate data. */
 	cert = cert_parse(file, f, flen);
-	cert = ta_parse(file, cert, tal->pkey, tal->pkeysz);
+	cert = ta_parse(file, cert, tal->spki, tal->spkisz);
 	if (cert == NULL)
 		goto out;
 
@@ -283,28 +283,28 @@ out:
 static struct tal *
 find_tal(struct cert *cert)
 {
-	EVP_PKEY	*pk, *opk;
+	EVP_PKEY	*pkey, *opkey;
 	struct tal	*tal;
 	int		 i;
 
-	if ((opk = X509_get0_pubkey(cert->x509)) == NULL)
+	if ((opkey = X509_get0_pubkey(cert->x509)) == NULL)
 		return NULL;
 
 	for (i = 0; i < TALSZ_MAX; i++) {
-		const unsigned char *pkey;
+		const unsigned char *spki;
 
 		if (talobj[i] == NULL)
 			break;
 		tal = talobj[i];
-		pkey = tal->pkey;
-		pk = d2i_PUBKEY(NULL, &pkey, tal->pkeysz);
-		if (pk == NULL)
+		spki = tal->spki;
+		pkey = d2i_PUBKEY(NULL, &spki, tal->spkisz);
+		if (pkey == NULL)
 			continue;
-		if (EVP_PKEY_cmp(pk, opk) == 1) {
-			EVP_PKEY_free(pk);
+		if (EVP_PKEY_cmp(pkey, opkey) == 1) {
+			EVP_PKEY_free(pkey);
 			return tal;
 		}
-		EVP_PKEY_free(pk);
+		EVP_PKEY_free(pkey);
 	}
 	return NULL;
 }
@@ -613,7 +613,7 @@ proc_parser_file(char *file, unsigned ch
 		expires = NULL;
 		notafter = NULL;
 		if ((tal = find_tal(cert)) != NULL) {
-			cert = ta_parse(file, cert, tal->pkey, tal->pkeysz);
+			cert = ta_parse(file, cert, tal->spki, tal->spkisz);
 			status = (cert != NULL);
 			if (status) {
 				expires = &cert->expires;
Index: usr.sbin/rpki-client/main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
diff -u -p -r1.303 main.c
--- usr.sbin/rpki-client/main.c	31 Dec 2025 12:51:52 -0000	1.303
+++ usr.sbin/rpki-client/main.c	20 Jan 2026 07:35:42 -0000
@@ -500,11 +500,11 @@ queue_add_from_tal(struct tal *tal)
 		return;
 	}
 
-	/* steal the pkey from the tal structure */
-	data = tal->pkey;
-	tal->pkey = NULL;
+	/* steal the spki from the tal structure */
+	data = tal->spki;
+	tal->spki = NULL;
 	entityq_add(NULL, nfile, RTYPE_CER, DIR_UNKNOWN, repo, data,
-	    tal->pkeysz, tal->id, tal->id, NULL);
+	    tal->spkisz, tal->id, tal->id, NULL);
 }
 
 /*
Index: usr.sbin/rpki-client/print.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
diff -u -p -r1.73 print.c
--- usr.sbin/rpki-client/print.c	13 Jan 2026 21:36:17 -0000	1.73
+++ usr.sbin/rpki-client/print.c	20 Jan 2026 07:35:42 -0000
@@ -106,8 +106,8 @@ tal_print(const struct tal *p)
 	X509_PUBKEY		*pubkey;
 	size_t			 i;
 
-	der = p->pkey;
-	if ((pubkey = d2i_X509_PUBKEY(NULL, &der, p->pkeysz)) == NULL)
+	der = p->spki;
+	if ((pubkey = d2i_X509_PUBKEY(NULL, &der, p->spkisz)) == NULL)
 		errx(1, "d2i_X509_PUBKEY failed");
 
 	if ((ski = x509_pubkey_get_ski(pubkey, p->descr)) == NULL)
Index: usr.sbin/rpki-client/tal.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/tal.c,v
diff -u -p -r1.42 tal.c
--- usr.sbin/rpki-client/tal.c	1 Aug 2025 13:46:06 -0000	1.42
+++ usr.sbin/rpki-client/tal.c	20 Jan 2026 07:35:42 -0000
@@ -126,21 +126,26 @@ tal_parse_buffer(const char *fn, char *b
 
 	/* Now the Base64-encoded public key. */
 	if ((base64_decode(buf, len, &der, &dersz)) == -1) {
-		warnx("%s: RFC 7730 section 2.1: subjectPublicKeyInfo: "
+		warnx("%s: RFC 8630 section 2.1: subjectPublicKeyInfo: "
 		    "bad public key", fn);
 		goto out;
 	}
 
-	tal->pkey = der;
-	tal->pkeysz = dersz;
+	tal->spki = der;
+	tal->spkisz = dersz;
 
 	/* Make sure it's a valid public key. */
 	pkey = d2i_PUBKEY(NULL, (const unsigned char **)&der, dersz);
 	if (pkey == NULL) {
-		warnx("%s: RFC 7730 section 2.1: subjectPublicKeyInfo: "
+		warnx("%s: RFC 8630 section 2.1: subjectPublicKeyInfo: "
 		    "failed public key parse", fn);
 		goto out;
 	}
+	if (der != tal->spki + tal->spkisz) {
+		warnx("%s: RFC 8630 section 2.1: subjectPublicKeyInfo: "
+		    "%td bytes of trailing garbage", fn,
+		    tal->spki + tal->spkisz - der);
+	}
 	rc = 1;
 out:
 	if (rc == 0) {
@@ -152,7 +157,7 @@ out:
 }
 
 /*
- * Parse a TAL from "buf" conformant to RFC 7730 originally from a file
+ * Parse a TAL from "buf" conformant to RFC 8630 originally from a file
  * named "fn".
  * Returns the encoded data or NULL on syntax failure.
  */
@@ -198,7 +203,7 @@ tal_free(struct tal *p)
 		for (i = 0; i < p->num_uris; i++)
 			free(p->uri[i]);
 
-	free(p->pkey);
+	free(p->spki);
 	free(p->uri);
 	free(p->descr);
 	free(p);
@@ -214,7 +219,7 @@ tal_buffer(struct ibuf *b, const struct 
 	size_t	 i;
 
 	io_simple_buffer(b, &p->id, sizeof(p->id));
-	io_buf_buffer(b, p->pkey, p->pkeysz);
+	io_buf_buffer(b, p->spki, p->spkisz);
 	io_str_buffer(b, p->descr);
 	io_simple_buffer(b, &p->num_uris, sizeof(p->num_uris));
 
@@ -237,10 +242,10 @@ tal_read(struct ibuf *b)
 		err(1, NULL);
 
 	io_read_buf(b, &p->id, sizeof(p->id));
-	io_read_buf_alloc(b, (void **)&p->pkey, &p->pkeysz);
+	io_read_buf_alloc(b, (void **)&p->spki, &p->spkisz);
 	io_read_str(b, &p->descr);
 	io_read_buf(b, &p->num_uris, sizeof(p->num_uris));
-	if (p->pkeysz <= 0 || p->num_uris <= 0)
+	if (p->spkisz <= 0 || p->num_uris <= 0)
 		errx(1, "tal_read: bad message");
 
 	if ((p->uri = calloc(p->num_uris, sizeof(char *))) == NULL)
Index: regress/usr.sbin/rpki-client/test-cert.c
===================================================================
RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-cert.c,v
diff -u -p -r1.27 test-cert.c
--- regress/usr.sbin/rpki-client/test-cert.c	23 Oct 2025 05:26:25 -0000	1.27
+++ regress/usr.sbin/rpki-client/test-cert.c	20 Jan 2026 07:35:42 -0000
@@ -85,7 +85,7 @@ main(int argc, char *argv[])
 			free(buf);
 			if (p == NULL)
 				break;
-			p = ta_parse(cert_path, p, tal->pkey, tal->pkeysz);
+			p = ta_parse(cert_path, p, tal->spki, tal->spkisz);
 			tal_free(tal);
 			if (p == NULL)
 				break;