Download raw body.
rpki-client: some refactoring around cert_parse()
On Mon, Jan 26, 2026 at 07:18:14AM +0100, Theo Buehler wrote:
> 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.
OK claudio@
I'm a bit on the fence with all those specialised functions but I know you
have more to follow.
> 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) {
>
--
:wq Claudio
rpki-client: some refactoring around cert_parse()