Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: some refactoring around cert_parse()
To:
tech@openbsd.org
Date:
Mon, 26 Jan 2026 07:18:14 +0100

Download raw body.

Thread
These are a few simple refactoring steps.

cert_parse() is a bit too general: it accomodates TAs, CAs, and BGPsec
router certs because of the needs of filemode. Normal mode knows what
kind of cert it wants to parse and should not have to concern itself
with checking the purpose.

So split a cert_deserialize_and_parse() out of cert_parse() only to be
used from cert.c. Add cert_parse_ca_or_brk() which checks the purpose,
so proc_parser_cert() doesn't need to. Similarly, cert_parse_ta()
deserializes and validates a TA and use that instead of cert_pares()
followed by ta_parse().

A minor thing is to split ta_validate() out of ta_parse(). That should
go away at some point since cert_check_validity_period() does not check
against the current time. This feels wrong and was done to make filemode
more useful if I remember correctly. That's for a later step.

Once these steps are in, there's some cleanup to do, such as renaming
cert_parse() to cert_parse_filemode() and ta_parse() to ta_validate().
That's for a later diff.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.214 cert.c
--- cert.c	24 Jan 2026 08:14:08 -0000	1.214
+++ cert.c	26 Jan 2026 06:13:07 -0000
@@ -1882,21 +1882,17 @@ cert_parse_ee_cert(const char *fn, int t
 }
 
 /*
- * Parse and partially validate an RPKI X509 certificate (either a trust
- * anchor or a certificate) as defined in RFC 6487.
- * Returns the parse results or NULL on failure.
+ * This is a generic parser for resource certificates and can only do as much
+ * validation as can be extracted from the bare DER. Callers should at least
+ * check the cert->purpose and consider any further validation.
  */
-struct cert *
-cert_parse(const char *fn, const unsigned char *der, size_t len)
+static struct cert *
+cert_deserialize_and_parse(const char *fn, const unsigned char *der, size_t len)
 {
 	struct cert		*cert = NULL;
 	const unsigned char	*oder;
 	X509			*x = NULL;
 
-	/* just fail for empty buffers, the warning was printed elsewhere */
-	if (der == NULL)
-		return NULL;
-
 	oder = der;
 	if ((x = d2i_X509(NULL, &der, len)) == NULL) {
 		warnx("%s: d2i_X509", fn);
@@ -1914,17 +1910,72 @@ cert_parse(const char *fn, const unsigne
 	if ((cert = cert_parse_internal(fn, x)) == NULL)
 		goto out;
 
+	X509_free(x);
+	return cert;
+
+ out:
+	cert_free(cert);
+	X509_free(x);
+	return NULL;
+}
+
+/*
+ * Parse a certificate file from its DER. Intended for .cer in a Manifest
+ * fileList, so must be a CA cert or a BGPsec router cert.
+ * Returns cert on success or NULL on failure.
+ */
+struct cert *
+cert_parse_ca_or_brk(const char *fn, const unsigned char *der, size_t len)
+{
+	struct cert *cert = NULL;
+
+	/* Handle possible parse_load_file() failure which already warned. */
+	if (der == NULL)
+		return NULL;
+
+	if ((cert = cert_deserialize_and_parse(fn, der, len)) == NULL)
+		goto out;
+
+	if (cert->purpose != CERT_PURPOSE_CA &&
+	    cert->purpose != CERT_PURPOSE_BGPSEC_ROUTER) {
+		warnx("%s: want CA or BGPsec Router cert, got %s",
+		    fn, purpose2str(cert->purpose));
+		goto out;
+	}
+
+	return cert;
+
+ out:
+	cert_free(cert);
+	return NULL;
+}
+
+/*
+ * Parse and partially validate an RPKI X509 certificate (either a trust
+ * anchor or a certificate) as defined in RFC 6487.
+ * Returns the parse results or NULL on failure.
+ */
+struct cert *
+cert_parse(const char *fn, const unsigned char *der, size_t len)
+{
+	struct cert		*cert = NULL;
+
+	/* just fail for empty buffers, the warning was printed elsewhere */
+	if (der == NULL)
+		return NULL;
+
+	if ((cert = cert_deserialize_and_parse(fn, der, len)) == NULL)
+		goto out;
+
 	if (cert->purpose == CERT_PURPOSE_EE) {
 		warnx("%s: unexpected EE cert", fn);
 		goto out;
 	}
 
-	X509_free(x);
 	return cert;
 
  out:
 	cert_free(cert);
-	X509_free(x);
 	return NULL;
 }
 
@@ -1971,12 +2022,27 @@ ta_check_pubkey(const char *fn, struct c
 	return rv;
 }
 
+static int
+ta_check_validity(const char *fn, struct cert *cert)
+{
+	time_t		 now = get_current_time();
+
+	if (cert->notbefore > now) {
+		warnx("%s: certificate not yet valid", fn);
+		return 0;
+	}
+	if (cert->notafter < now) {
+		warnx("%s: certificate has expired", fn);
+		return 0;
+	}
+
+	return 1;
+}
+
 struct cert *
 ta_parse(const char *fn, struct cert *p, const unsigned char *spki,
     size_t spkisz)
 {
-	time_t		 now = get_current_time();
-
 	if (p == NULL)
 		return NULL;
 
@@ -1988,21 +2054,35 @@ ta_parse(const char *fn, struct cert *p,
 
 	if (!ta_check_pubkey(fn, p, spki, spkisz))
 		goto out;
-
-	if (p->notbefore > now) {
-		warnx("%s: certificate not yet valid", fn);
+	if (!ta_check_validity(fn, p))
 		goto out;
-	}
-	if (p->notafter < now) {
-		warnx("%s: certificate has expired", fn);
-		goto out;
-	}
 
 	return p;
 
  out:
 	cert_free(p);
 	return NULL;
+}
+
+/*
+ * Parse a TA from its DER and validate it against SPKI from the TAL and
+ * current time.
+ * Returns a validated TA cert on success or NULL on failure.
+ */
+struct cert	*
+cert_parse_ta(const char *fn, const unsigned char *der, size_t len,
+    const unsigned char *spki, size_t spkisz)
+{
+	struct cert *cert = NULL;
+
+	/* Handle possible load_file() failure. Will warn later. */
+	if (der == NULL)
+		return NULL;
+
+	if ((cert = cert_deserialize_and_parse(fn, der, len)) == NULL)
+		return NULL;
+
+	return ta_parse(fn, cert, spki, spkisz);
 }
 
 /*
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.273 extern.h
--- extern.h	24 Jan 2026 08:11:26 -0000	1.273
+++ extern.h	26 Jan 2026 06:13:07 -0000
@@ -714,7 +714,11 @@ struct tal	*tal_read(struct ibuf *);
 void		 cert_buffer(struct ibuf *, const struct cert *);
 void		 cert_free(struct cert *);
 void		 auth_tree_free(struct auth_tree *);
+struct cert	*cert_parse_ca_or_brk(const char *, const unsigned char *,
+		    size_t);
 struct cert	*cert_parse_ee_cert(const char *, int, X509 *);
+struct cert	*cert_parse_ta(const char *, const unsigned char *, size_t,
+		    const unsigned char *, size_t);
 struct cert	*cert_parse(const char *, const unsigned char *, size_t);
 struct cert	*ta_parse(const char *, struct cert *, const unsigned char *,
 		    size_t);
Index: filemode.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
diff -u -p -r1.78 filemode.c
--- filemode.c	24 Jan 2026 08:13:10 -0000	1.78
+++ filemode.c	26 Jan 2026 06:13:07 -0000
@@ -261,8 +261,7 @@ parse_load_ta(struct tal *tal)
 	}
 
 	/* Extract certificate data. */
-	cert = cert_parse(file, f, flen);
-	cert = ta_parse(file, cert, tal->spki, tal->spkisz);
+	cert = cert_parse_ta(file, f, flen, tal->spki, tal->spkisz);
 	if (cert == NULL)
 		goto out;
 
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.175 parser.c
--- parser.c	20 Jan 2026 16:49:03 -0000	1.175
+++ parser.c	26 Jan 2026 06:13:07 -0000
@@ -585,17 +585,10 @@ proc_parser_cert(char *file, const unsig
 
 	/* Extract certificate data. */
 
-	cert = cert_parse(file, der, len);
+	cert = cert_parse_ca_or_brk(file, der, len);
 	if (cert == NULL)
 		goto out;
 
-	if (cert->purpose != CERT_PURPOSE_CA &&
-	    cert->purpose != CERT_PURPOSE_BGPSEC_ROUTER) {
-		warnx("%s: %s not allowed in a manifest", file,
-		    purpose2str(cert->purpose));
-		goto out;
-	}
-
 	a = find_issuer(file, entp->certid, cert->aki, entp->mftaki);
 	if (a == NULL)
 		goto out;
@@ -700,17 +693,15 @@ proc_parser_root_cert(struct entity *ent
 
 	file2 = parse_filepath(entp->repoid, entp->path, entp->file, DIR_VALID);
 	der = load_file(file2, &der_len);
-	cert2 = cert_parse(file2, der, der_len);
+	cert2 = cert_parse_ta(file2, der, der_len, spki, spkisz);
 	free(der);
-	cert2 = ta_parse(file2, cert2, spki, spkisz);
 
 	if (!noop) {
 		file1 = parse_filepath(entp->repoid, entp->path, entp->file,
 		    DIR_TEMP);
 		der = load_file(file1, &der_len);
-		cert1 = cert_parse(file1, der, der_len);
+		cert1 = cert_parse_ta(file1, der, der_len, spki, spkisz);
 		free(der);
-		cert1 = ta_parse(file1, cert1, spki, spkisz);
 	}
 
 	if ((cmp = proc_parser_ta_cmp(cert1, cert2)) > 0) {