From: Theo Buehler Subject: rpki-client: start reworking CMS signed object parsers To: tech@openbsd.org Date: Tue, 16 Jun 2026 18:01:51 +0200 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. 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 15 Jun 2026 17:36:53 -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,33 @@ 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, time_t signtime) +{ + struct aspa *aspa = obj; + + aspa->signtime = signtime; + 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. @@ -176,22 +204,12 @@ aspa_parse(struct cert **out_cert, const if ((aspa = calloc(1, sizeof(*aspa))) == NULL) err(1, NULL); - aspa->signtime = signtime; - if (cert->num_ips > 0) { - warnx("%s: superfluous IP Resources extension present", fn); + if (!aspa_cert_info(fn, aspa, cert)) goto out; - } - - if (x509_any_inherits(cert->x509)) { - warnx("%s: inherit elements not allowed in EE cert", fn); - 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, signtime); *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 15 Jun 2026 17:36:53 -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,57 @@ 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, time_t signtime) +{ + struct mft *mft = obj; + + mft->signtime = signtime; + + 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 +439,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); @@ -399,43 +450,16 @@ mft_parse(struct cert **out_cert, const if ((mft = calloc(1, sizeof(*mft))) == NULL) err(1, NULL); - 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); + if (!mft_parse_econtent(fn, mft, cms, cmsz)) goto out; - } - if ((mft->crl = strdup(crlfile)) == NULL) - err(1, NULL); - - if (mft_parse_econtent(fn, mft, cms, cmsz) == 0) + if (!mft_validate(fn, mft, cert, signtime)) goto out; - if (mft->signtime > mft->nextupdate) { - warnx("%s: dating issue: CMS signing-time after MFT nextUpdate", - fn); - goto out; - } + /* XXX - find a nice spot to do this. */ + mft->mftsize = len; *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 15 Jun 2026 17:36:53 -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,38 @@ 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, time_t signtime) +{ + struct roa *roa = obj; + + roa->signtime = signtime; + 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. @@ -228,31 +261,12 @@ roa_parse(struct cert **out_cert, const if ((roa = calloc(1, sizeof(struct roa))) == NULL) 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, signtime); *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 15:32:37 -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,28 @@ 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, time_t signtime) +{ + struct rsc *rsc = obj; + + rsc->signtime = signtime; + 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. @@ -363,17 +386,12 @@ rsc_parse(struct cert **out_cert, const if ((rsc = calloc(1, sizeof(struct rsc))) == NULL) 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, signtime); *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 15 Jun 2026 17:36:53 -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,38 @@ 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, time_t signtime) +{ + struct spl *spl = obj; + + spl->signtime = signtime; + 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. @@ -233,31 +266,12 @@ spl_parse(struct cert **out_cert, const if ((spl = calloc(1, sizeof(*spl))) == NULL) 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); - goto out; - } - if (cert->num_ases == 0) { - warnx("%s: AS Resources extension missing", fn); + if (!spl_cert_info(fn, spl, cert)) 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, signtime); *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 15 Jun 2026 17:36:53 -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,31 @@ 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, time_t signtime) +{ + struct tak *tak = obj; + + tak->signtime = signtime; + 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. @@ -207,20 +233,13 @@ tak_parse(struct cert **out_cert, const if ((tak = calloc(1, sizeof(struct tak))) == NULL) 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, signtime)) goto out; - } *out_cert = cert; cert = NULL;