Download raw body.
rpki-client: start reworking CMS signed object parsers
On Tue, Jun 16, 2026 at 09:24:50PM +0200, Theo Buehler wrote:
> On Tue, Jun 16, 2026 at 06:33:17PM +0000, Job Snijders wrote:
> > On Tue, Jun 16, 2026 at 06:01:51PM +0200, Theo Buehler wrote:
> > > The diff below starts unifying various CMS signed object parsers in
> > > preparation for a lot more refactoring of this long-accumulated
> > > copy-paste mess.
> > >
> > > The signed object is passed as a void * object to various handlers, so
> > > these handlers all have the same signature.
> > >
> > > In this step, *_parse() is essentially split into allocation plus:
> > >
> > > 1. *_cert_info(), which checks some basic things on the EE cert (usually
> > > inheritance, presence or absence of RFC 3779 extensions). For MFTs
> > > it also extracts some info and hangs that off mft.
> > >
> > > 2. the already existing *_parse_econtent()
> > >
> > > 3. *_validate() that does some validation steps, sets ->signtime and
> > > ->valid. In most signed object handlers the validate step can't
> > > currently fail. This is one of the many warts we've accumulated
> > > and is marked with an /* XXX */.
> > >
> > > This is all straightforward and should not change anything.
> > >
> > > The reason I put cert_info after parse_econtent is that the latter
> > > usually has some helpers and I found it easier to reason about this if
> > > the future struct members are somewhat close to each other.
>
> job didn't like passing in the signtime to the validate functions
> because that's only really needed for manifests and a bit of a weird
> thing to do. So avoid this for now and deal with this in a later step.
>
> So this diff leaves the ->signtime and ->mftsize parts alone and drops
> the signtime argument from the validate functions. It is otherwise
> identical to the previous one.
Looks good to me. OK claudio@
> Index: aspa.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
> diff -u -p -r1.42 aspa.c
> --- aspa.c 8 Feb 2026 12:35:07 -0000 1.42
> +++ aspa.c 16 Jun 2026 19:15:05 -0000
> @@ -117,9 +117,10 @@ aspa_parse_providers(const char *fn, str
> * Returns zero on failure, non-zero on success.
> */
> static int
> -aspa_parse_econtent(const char *fn, struct aspa *aspa, const unsigned char *d,
> +aspa_parse_econtent(const char *fn, void *obj, const unsigned char *d,
> size_t dsz)
> {
> + struct aspa *aspa = obj;
> const unsigned char *oder;
> ASProviderAttestation *aspa_asn1;
> int rc = 0;
> @@ -152,6 +153,32 @@ aspa_parse_econtent(const char *fn, stru
> return rc;
> }
>
> +static int
> +aspa_cert_info(const char *fn, void *obj, const struct cert *cert)
> +{
> + if (cert->num_ips > 0) {
> + warnx("%s: superfluous IP Resources extension present", fn);
> + return 0;
> + }
> +
> + if (x509_any_inherits(cert->x509)) {
> + warnx("%s: inherit elements not allowed in EE cert", fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int
> +aspa_validate(const char *fn, void *obj, struct cert *cert)
> +{
> + struct aspa *aspa = obj;
> +
> + aspa->valid = valid_aspa(fn, cert, aspa);
> +
> + return 1; /* XXX */
> +}
> +
> /*
> * Parse a full ASPA file.
> * Returns the payload or NULL if the file was malformed.
> @@ -178,20 +205,11 @@ aspa_parse(struct cert **out_cert, const
> err(1, NULL);
> aspa->signtime = signtime;
>
> - if (cert->num_ips > 0) {
> - warnx("%s: superfluous IP Resources extension present", fn);
> - goto out;
> - }
> -
> - if (x509_any_inherits(cert->x509)) {
> - warnx("%s: inherit elements not allowed in EE cert", fn);
> + if (!aspa_cert_info(fn, aspa, cert))
> goto out;
> - }
> -
> if (!aspa_parse_econtent(fn, aspa, cms, cmsz))
> goto out;
> -
> - aspa->valid = valid_aspa(fn, cert, aspa);
> + (void)aspa_validate(fn, aspa, cert);
>
> *out_cert = cert;
> cert = NULL;
> Index: mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> diff -u -p -r1.138 mft.c
> --- mft.c 16 May 2026 07:27:03 -0000 1.138
> +++ mft.c 16 Jun 2026 19:15:05 -0000
> @@ -290,9 +290,10 @@ mft_has_unique_names_and_hashes(const ch
> * Returns 0 on failure and 1 on success.
> */
> static int
> -mft_parse_econtent(const char *fn, struct mft *mft, const unsigned char *d,
> +mft_parse_econtent(const char *fn, void *obj, const unsigned char *d,
> size_t dsz)
> {
> + struct mft *mft = obj;
> const unsigned char *oder;
> Manifest *mft_asn1;
> FileAndHash *fh;
> @@ -374,6 +375,55 @@ mft_parse_econtent(const char *fn, struc
> return rc;
> }
>
> +static int
> +mft_cert_info(const char *fn, void *obj, const struct cert *cert)
> +{
> + struct mft *mft = obj;
> + char *crlfile;
> +
> + if (!x509_inherits(cert->x509)) {
> + warnx("%s: RFC 3779 extensions not set to inherit", fn);
> + return 0;
> + }
> +
> + if ((mft->aki = strdup(cert->aki)) == NULL)
> + err(1, NULL);
> + if ((mft->sia = strdup(cert->signedobj)) == NULL)
> + err(1, NULL);
> +
> + crlfile = strrchr(cert->crl, '/');
> + if (crlfile == NULL) {
> + warnx("%s: RFC 6487 section 4.8.6: "
> + "invalid CRL distribution point", fn);
> + return 0;
> + }
> + crlfile++;
> + if (!valid_mft_filename(crlfile, strlen(crlfile)) ||
> + rtype_from_file_extension(crlfile) != RTYPE_CRL) {
> + warnx("%s: RFC 6487 section 4.8.6: CRL: "
> + "bad CRL distribution point extension", fn);
> + return 0;
> + }
> + if ((mft->crl = strdup(crlfile)) == NULL)
> + err(1, NULL);
> +
> + return 1;
> +}
> +
> +static int
> +mft_validate(const char *fn, void *obj, struct cert *cert)
> +{
> + struct mft *mft = obj;
> +
> + if (mft->signtime > mft->nextupdate) {
> + warnx("%s: dating issue: CMS signing-time after MFT nextUpdate",
> + fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> /*
> * Parse the objects that have been published in the manifest.
> * Return mft if it conforms to RFC 9286, otherwise NULL.
> @@ -387,7 +437,6 @@ mft_parse(struct cert **out_cert, const
> int rc = 0;
> size_t cmsz;
> unsigned char *cms;
> - char *crlfile;
> time_t signtime = 0;
>
> assert(*out_cert == NULL);
> @@ -402,40 +451,12 @@ mft_parse(struct cert **out_cert, const
> mft->signtime = signtime;
> mft->mftsize = len;
>
> - if ((mft->aki = strdup(cert->aki)) == NULL)
> - err(1, NULL);
> - if ((mft->sia = strdup(cert->signedobj)) == NULL)
> - err(1, NULL);
> -
> - if (!x509_inherits(cert->x509)) {
> - warnx("%s: RFC 3779 extension not set to inherit", fn);
> - goto out;
> - }
> -
> - crlfile = strrchr(cert->crl, '/');
> - if (crlfile == NULL) {
> - warnx("%s: RFC 6487 section 4.8.6: "
> - "invalid CRL distribution point", fn);
> + if (!mft_cert_info(fn, mft, cert))
> goto out;
> - }
> - crlfile++;
> - if (!valid_mft_filename(crlfile, strlen(crlfile)) ||
> - rtype_from_file_extension(crlfile) != RTYPE_CRL) {
> - warnx("%s: RFC 6487 section 4.8.6: CRL: "
> - "bad CRL distribution point extension", fn);
> - goto out;
> - }
> - if ((mft->crl = strdup(crlfile)) == NULL)
> - err(1, NULL);
> -
> - if (mft_parse_econtent(fn, mft, cms, cmsz) == 0)
> + if (!mft_parse_econtent(fn, mft, cms, cmsz))
> goto out;
> -
> - if (mft->signtime > mft->nextupdate) {
> - warnx("%s: dating issue: CMS signing-time after MFT nextUpdate",
> - fn);
> + if (!mft_validate(fn, mft, cert))
> goto out;
> - }
>
> *out_cert = cert;
> cert = NULL;
> Index: roa.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
> diff -u -p -r1.87 roa.c
> --- roa.c 9 Sep 2025 08:23:24 -0000 1.87
> +++ roa.c 16 Jun 2026 19:15:05 -0000
> @@ -64,9 +64,10 @@ ASN1_SEQUENCE(ROAIPAddress) = {
> * Returns zero on failure, non-zero on success.
> */
> static int
> -roa_parse_econtent(const char *fn, struct roa *roa, const unsigned char *d,
> +roa_parse_econtent(const char *fn, void *obj, const unsigned char *d,
> size_t dsz)
> {
> + struct roa *roa = obj;
> const unsigned char *oder;
> RouteOriginAttestation *roa_asn1;
> const ROAIPAddressFamily *addrfam;
> @@ -204,6 +205,37 @@ roa_parse_econtent(const char *fn, struc
> return rc;
> }
>
> +static int
> +roa_cert_info(const char *fn, void *obj, const struct cert *cert)
> +{
> + if (x509_any_inherits(cert->x509)) {
> + warnx("%s: inherit elements not allowed in EE cert", fn);
> + return 0;
> + }
> +
> + if (cert->num_ases > 0) {
> + warnx("%s: superfluous AS Resources extension present", fn);
> + return 0;
> + }
> +
> + if (cert->num_ips == 0) {
> + warnx("%s: no IP address present", fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int
> +roa_validate(const char *fn, void *obj, struct cert *cert)
> +{
> + struct roa *roa = obj;
> +
> + roa->valid = valid_roa(fn, cert, roa);
> +
> + return 1; /* XXX */
> +}
> +
> /*
> * Parse a full RFC 9582 file.
> * Returns the ROA or NULL if the document was malformed.
> @@ -230,29 +262,11 @@ roa_parse(struct cert **out_cert, const
> err(1, NULL);
> roa->signtime = signtime;
>
> - if (!roa_parse_econtent(fn, roa, cms, cmsz))
> - goto out;
> -
> - if (x509_any_inherits(cert->x509)) {
> - warnx("%s: inherit elements not allowed in EE cert", fn);
> - goto out;
> - }
> -
> - if (cert->num_ases > 0) {
> - warnx("%s: superfluous AS Resources extension present", fn);
> + if (!roa_cert_info(fn, roa, cert))
> goto out;
> - }
> -
> - if (cert->num_ips == 0) {
> - warnx("%s: no IP address present", fn);
> + if (!roa_parse_econtent(fn, roa, cms, cmsz))
> goto out;
> - }
> -
> - /*
> - * If the ROA isn't valid, we accept it anyway and depend upon
> - * the code around roa_read() to check the "valid" field itself.
> - */
> - roa->valid = valid_roa(fn, cert, roa);
> + (void)roa_validate(fn, roa, cert);
>
> *out_cert = cert;
> cert = NULL;
> Index: rsc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
> diff -u -p -r1.44 rsc.c
> --- rsc.c 16 May 2026 07:27:03 -0000 1.44
> +++ rsc.c 16 Jun 2026 19:15:05 -0000
> @@ -288,9 +288,10 @@ rsc_parse_checklist(const char *fn, stru
> * Returns zero on failure, non-zero on success.
> */
> static int
> -rsc_parse_econtent(const char *fn, struct rsc *rsc, const unsigned char *d,
> +rsc_parse_econtent(const char *fn, void *obj, const unsigned char *d,
> size_t dsz)
> {
> + struct rsc *rsc = obj;
> const unsigned char *oder;
> RpkiSignedChecklist *rsc_asn1;
> ResourceBlock *resources;
> @@ -339,6 +340,27 @@ rsc_parse_econtent(const char *fn, struc
> return rc;
> }
>
> +static int
> +rsc_cert_info(const char *fn, void *obj, const struct cert *cert)
> +{
> + if (x509_any_inherits(cert->x509)) {
> + warnx("%s: inherit elements not allowed in EE cert", fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int
> +rsc_validate(const char *fn, void *obj, struct cert *cert)
> +{
> + struct rsc *rsc = obj;
> +
> + rsc->valid = valid_rsc(fn, cert, rsc);
> +
> + return 1; /* XXX */
> +}
> +
> /*
> * Parse a full RFC 9323 file.
> * Returns the RSC or NULL if the object was malformed.
> @@ -365,15 +387,11 @@ rsc_parse(struct cert **out_cert, const
> err(1, NULL);
> rsc->signtime = signtime;
>
> - if (x509_any_inherits(cert->x509)) {
> - warnx("%s: inherit elements not allowed in EE cert", fn);
> + if (!rsc_cert_info(fn, rsc, cert))
> goto out;
> - }
> -
> if (!rsc_parse_econtent(fn, rsc, cms, cmsz))
> goto out;
> -
> - rsc->valid = valid_rsc(fn, cert, rsc);
> + (void)rsc_validate(fn, rsc, cert);
>
> *out_cert = cert;
> cert = NULL;
> Index: spl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/spl.c,v
> diff -u -p -r1.16 spl.c
> --- spl.c 24 Aug 2025 12:34:39 -0000 1.16
> +++ spl.c 16 Jun 2026 19:15:05 -0000
> @@ -98,9 +98,10 @@ prefix_cmp(enum afi afi, const struct ip
> * Returns zero on failure, non-zero on success.
> */
> static int
> -spl_parse_econtent(const char *fn, struct spl *spl, const unsigned char *d,
> +spl_parse_econtent(const char *fn, void *obj, const unsigned char *d,
> size_t dsz)
> {
> + struct spl *spl = obj;
> const unsigned char *oder;
> SignedPrefixList *spl_asn1;
> const AddressFamilyPrefixes *afp;
> @@ -209,6 +210,37 @@ spl_parse_econtent(const char *fn, struc
> return rc;
> }
>
> +static int
> +spl_cert_info(const char *fn, void *obj, const struct cert *cert)
> +{
> + if (x509_any_inherits(cert->x509)) {
> + warnx("%s: inherit elements not allowed in EE cert", fn);
> + return 0;
> + }
> +
> + if (cert->num_ases == 0) {
> + warnx("%s: AS Resources extension missing", fn);
> + return 0;
> + }
> +
> + if (cert->num_ips > 0) {
> + warnx("%s: superfluous IP Resources extension present", fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int
> +spl_validate(const char *fn, void *obj, struct cert *cert)
> +{
> + struct spl *spl = obj;
> +
> + spl->valid = valid_spl(fn, cert, spl);
> +
> + return 1; /* XXX */
> +}
> +
> /*
> * Parse a full Signed Prefix List file.
> * Returns the SPL, or NULL if the object was malformed.
> @@ -235,29 +267,11 @@ spl_parse(struct cert **out_cert, const
> err(1, NULL);
> spl->signtime = signtime;
>
> - if (!spl_parse_econtent(fn, spl, cms, cmsz))
> - goto out;
> -
> - if (x509_any_inherits(cert->x509)) {
> - warnx("%s: inherit elements not allowed in EE cert", fn);
> + if (!spl_cert_info(fn, spl, cert))
> goto out;
> - }
> -
> - if (cert->num_ases == 0) {
> - warnx("%s: AS Resources extension missing", fn);
> - goto out;
> - }
> -
> - if (cert->num_ips > 0) {
> - warnx("%s: superfluous IP Resources extension present", fn);
> + if (!spl_parse_econtent(fn, spl, cms, cmsz))
> goto out;
> - }
> -
> - /*
> - * If the SPL isn't valid, we accept it anyway and depend upon
> - * the code around spl_read() to check the "valid" field itself.
> - */
> - spl->valid = valid_spl(fn, cert, spl);
> + (void)spl_validate(fn, spl, cert);
>
> *out_cert = cert;
> cert = NULL;
> Index: tak.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/tak.c,v
> diff -u -p -r1.29 tak.c
> --- tak.c 2 Dec 2025 12:47:48 -0000 1.29
> +++ tak.c 16 Jun 2026 19:15:05 -0000
> @@ -140,9 +140,10 @@ parse_takey(const char *fn, const TAKey
> * Returns zero on failure, non-zero on success.
> */
> static int
> -tak_parse_econtent(const char *fn, struct tak *tak, const unsigned char *d,
> +tak_parse_econtent(const char *fn, void *obj, const unsigned char *d,
> size_t dsz)
> {
> + struct tak *tak = obj;
> const unsigned char *oder;
> TAK *tak_asn1;
> int rc = 0;
> @@ -183,6 +184,30 @@ tak_parse_econtent(const char *fn, struc
> return rc;
> }
>
> +static int
> +tak_cert_info(const char *fn, void *obj, const struct cert *cert)
> +{
> + if (!x509_inherits(cert->x509)) {
> + warnx("%s: RFC 3779 extension not set to inherit", fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int
> +tak_validate(const char *fn, void *obj, struct cert *cert)
> +{
> + struct tak *tak = obj;
> +
> + if (strcmp(cert->aki, tak->current->ski) != 0) {
> + warnx("%s: current TAKey's SKI does not match EE AKI", fn);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> /*
> * Parse a full RFC 9691 Trust Anchor Key file.
> * Returns the TAK or NULL if the object was malformed.
> @@ -209,18 +234,12 @@ tak_parse(struct cert **out_cert, const
> err(1, NULL);
> tak->signtime = signtime;
>
> - if (!x509_inherits(cert->x509)) {
> - warnx("%s: RFC 3779 extension not set to inherit", fn);
> + if (!tak_cert_info(fn, tak, cert))
> goto out;
> - }
> -
> if (!tak_parse_econtent(fn, tak, cms, cmsz))
> goto out;
> -
> - if (strcmp(cert->aki, tak->current->ski) != 0) {
> - warnx("%s: current TAKey's SKI does not match EE AKI", fn);
> + if (!tak_validate(fn, tak, cert))
> goto out;
> - }
>
> *out_cert = cert;
> cert = NULL;
>
--
:wq Claudio
rpki-client: start reworking CMS signed object parsers