From: Theo Buehler <tb@theobuehler.org>
Subject: Re: rpki-client: refactor manifest thisUpdate / nextUpdate parsing
To: Job Snijders <job@openbsd.org>
Cc: tech@openbsd.org
Date: Sun, 4 Feb 2024 01:47:35 +0100

> > @@ -301,8 +247,15 @@ mft_parse_econtent(const unsigned char *
> >  	if (p->res->seqnum == NULL)
> >  		goto out;
> >  
> > -	if (!mft_parse_time(mft->thisUpdate, mft->nextUpdate, p))
> > +	if (!x509_get_time(mft->thisUpdate, &p->res->thisupdate)) {
> > +		warn("%s: parsing manifest thisUpdate failed", p->fn);
> >  		goto out;
> > +	}
> > +
> > +	if (!x509_get_time(mft->nextUpdate, &p->res->nextupdate)) {
> > +		warn("%s: parsing manifest nextUpdate failed", p->fn);
> > +		goto out;
> > +	}
> 
> I think you should keep the nextupdate >= thisupdate check here for
> filemode. I think it is unclear if "MUST be later" means > or >=, so I
> would keep the check as it is.

For posterity: the approach is fine, but it needs some additional checks
because OpenSSL doesn't require RFC 5280 conformance for GeneralizedTime
DER encoding. (This is legitimate for a general purpose ASN.1 parser,
but a bit dubious for an ASN.1 parser that primarily exists to deal with
certs and related structures.)