From: Claudio Jeker Subject: Re: rpki-client: further reworking of cms_parse_validate() To: Theo Buehler Cc: tech@openbsd.org Date: Thu, 10 Sep 2026 13:50:50 +0200 On Mon, Sep 07, 2026 at 03:44:47PM +0200, Theo Buehler wrote: > This diff pulls the signed object handling into cms_parse_validate(), > which simplifies the signature quite a bit. I kept signed_object_parse() > as a thin wrapper of cms_parse_validate() since I'm not yet sure about > the next steps. We may well need a signed_object_parse_filemode(), for > example. > > We need to pull up cms_object_from_rtype() a little. Since we now call > sobj->parse_econtent() while the cms object is still alive, we can get > rid of the malloc/memcpy in cms_extract_econtent(). > > The rest is straightforward. Indeed. OK claudio@ > Index: cms.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v > diff -u -p -r1.66 cms.c > --- cms.c 7 Sep 2026 12:29:27 -0000 1.66 > +++ cms.c 7 Sep 2026 13:01:54 -0000 > @@ -32,7 +32,7 @@ > extern int filemode; > > static int > -cms_extract_econtent(const char *fn, CMS_ContentInfo *cms, unsigned char **res, > +cms_extract_econtent(const char *fn, CMS_ContentInfo *cms, const uint8_t **res, > size_t *rsz) > { > ASN1_OCTET_STRING **os = NULL; > @@ -53,15 +53,7 @@ cms_extract_econtent(const char *fn, CMS > return 0; > } > > - /* > - * The eContent in os is owned by the cms object and it has to outlive > - * it for further processing by the signedObject handlers. Since there > - * is no convenient API for this purpose, duplicate it by hand. > - */ > - if ((*res = malloc(*rsz)) == NULL) > - err(1, NULL); > - memcpy(*res, ASN1_STRING_get0_data(*os), *rsz); > - > + *res = ASN1_STRING_get0_data(*os); > return 1; > } > > @@ -302,21 +294,45 @@ cms_check_SignerInfo(const char *fn, CMS > return 1; > } > > -static int > -cms_parse_validate(struct cert **out_cert, const char *fn, int talid, > - const unsigned char *der, size_t len, const ASN1_OBJECT *oid, > - unsigned char **res, size_t *rsz, time_t *signtime) > +static const struct signed_obj * > +cms_object_from_rtype(const char *fn, enum rtype rtype) > { > + switch (rtype) { > + case RTYPE_ASPA: > + return aspa_obj(); > + case RTYPE_MFT: > + return mft_obj(); > + case RTYPE_ROA: > + return roa_obj(); > + case RTYPE_RSC: > + return rsc_obj(); > + case RTYPE_SPL: > + return spl_obj(); > + case RTYPE_TAK: > + return tak_obj(); > + default: > + errx(1, "%s: unsupported signed object", fn); > + } > +} > + > +static void * > +cms_parse_validate(struct cert **out_cert, const char *fn, enum rtype rtype, > + int talid, const unsigned char *der, size_t len) > +{ > + void *obj = NULL, *ret_obj = NULL; > + const struct signed_obj *sobj; > struct cert *cert = NULL; > const unsigned char *oder; > CMS_ContentInfo *cms = NULL; > STACK_OF(X509) *certs = NULL; > STACK_OF(X509_CRL) *crls = NULL; > - int rc = 0; > + const uint8_t *econtent = NULL; > + size_t econtent_len = 0; > + time_t signtime = 0; > > assert(*out_cert == NULL); > > - *signtime = 0; > + sobj = cms_object_from_rtype(fn, rtype); > > /* just fail for empty buffers, the warning was printed elsewhere */ > if (der == NULL) > @@ -382,90 +398,42 @@ cms_parse_validate(struct cert **out_cer > goto out; > > /* RFC 6488 section 3 verify the CMS */ > - if (!cms_check_SignerInfo(fn, cms, oid, cert, signtime)) > + if (!cms_check_SignerInfo(fn, cms, sobj->oid(), cert, &signtime)) > goto out; > > - if (*signtime > cert->notafter) > + if (signtime > cert->notafter) > warnx("%s: dating issue: CMS signing-time after X.509 notAfter", > fn); > > - if (!cms_extract_econtent(fn, cms, res, rsz)) > + if (!cms_extract_econtent(fn, cms, &econtent, &econtent_len)) > + goto out; > + > + obj = sobj->new(len, signtime); > + if (!sobj->cert_info(fn, obj, cert)) > + goto out; > + if (!sobj->parse_econtent(fn, obj, econtent, econtent_len)) > + goto out; > + if (!sobj->validate(fn, obj, cert)) > goto out; > > *out_cert = cert; > cert = NULL; > > - rc = 1; > + ret_obj = obj; > + obj = NULL; > + > out: > + sobj->free(obj); > cert_free(cert); > sk_X509_CRL_pop_free(crls, X509_CRL_free); > sk_X509_free(certs); > CMS_ContentInfo_free(cms); > - return rc; > -} > - > -static const struct signed_obj * > -cms_object_from_rtype(const char *fn, enum rtype rtype) > -{ > - switch (rtype) { > - case RTYPE_ASPA: > - return aspa_obj(); > - case RTYPE_MFT: > - return mft_obj(); > - case RTYPE_ROA: > - return roa_obj(); > - case RTYPE_RSC: > - return rsc_obj(); > - case RTYPE_SPL: > - return spl_obj(); > - case RTYPE_TAK: > - return tak_obj(); > - default: > - errx(1, "%s: unsupported signed object", fn); > - } > + return ret_obj; > } > > void * > signed_object_parse(struct cert **out_cert, const char *fn, enum rtype rtype, > int talid, const unsigned char *der, size_t len) > { > - const struct signed_obj *sobj; > - const ASN1_OBJECT *oid; > - void *obj = NULL; > - struct cert *cert = NULL; > - unsigned char *cms = NULL; > - size_t cmsz = 0; > - time_t signtime = 0; > - int rc = 0; > - > - assert(*out_cert == NULL); > - > - sobj = cms_object_from_rtype(fn, rtype); > - oid = sobj->oid(); > - > - if (!cms_parse_validate(&cert, fn, talid, der, len, oid, &cms, &cmsz, > - &signtime)) > - return NULL; > - > - obj = sobj->new(len, signtime); > - if (!sobj->cert_info(fn, obj, cert)) > - goto out; > - if (!sobj->parse_econtent(fn, obj, cms, cmsz)) > - goto out; > - if (!sobj->validate(fn, obj, cert)) > - goto out; > - > - *out_cert = cert; > - cert = NULL; > - > - rc = 1; > - > - out: > - if (rc == 0) { > - sobj->free(obj); > - obj = NULL; > - } > - cert_free(cert); > - free(cms); > - return obj; > + return cms_parse_validate(out_cert, fn, rtype, talid, der, len); > } > -- :wq Claudio