From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: rpki-client: better extension order in cert_parse_pre
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Thu, 19 Jun 2025 10:50:12 +0200

On Thu, Jun 19, 2025 at 08:34:10AM +0200, Theo Buehler wrote:
> On Thu, Jun 19, 2025 at 07:52:33AM +0200, Claudio Jeker wrote:
> > On Thu, Jun 19, 2025 at 07:27:35AM +0200, Theo Buehler wrote:
> > > On Thu, Jun 19, 2025 at 07:14:55AM +0200, Claudio Jeker wrote:
> > > > On Thu, Jun 19, 2025 at 12:55:54AM +0200, Theo Buehler wrote:
> > > > > The random order in which cert_parse_pre's switch handles extensions
> > > > > has confused me too many times. I'd like this to match the order in
> > > > > RFC 6487, section 4.8. This isn't perfect either - ski comes before aki
> > > > > and aia comes before sia. Still, it's better.
> > > > > 
> > > > > To make this easy to review, I'll do it in a couple of steps. Here's
> > > > > the first one, covering sections 4.8.1-4.8.7.
> > > > > 
> > > > >        4.8.1.  Basic Constraints  . . . . . . . . . . . . . . . . . .  8
> > > > >        4.8.2.  Subject Key Identifier . . . . . . . . . . . . . . . .  9
> > > > >        4.8.3.  Authority Key Identifier . . . . . . . . . . . . . . .  9
> > > > >        4.8.4.  Key Usage  . . . . . . . . . . . . . . . . . . . . . .  9
> > > > >        4.8.5.  Extended Key Usage . . . . . . . . . . . . . . . . . .  9
> > > > >        4.8.6.  CRL Distribution Points  . . . . . . . . . . . . . . . 10
> > > > >        4.8.7.  Authority Information Access . . . . . . . . . . . . . 10
> > > > 
> > > > Go for it. OK claudio@
> > > 
> > > And here's the second step, moving the two sbgp extensions to their
> > > proper spot:
> > > 
> > >        4.8.7.  Authority Information Access . . . . . . . . . . . . . 10
> > >        4.8.8.  Subject Information Access . . . . . . . . . . . . . . 11
> > >        4.8.9.  Certificate Policies . . . . . . . . . . . . . . . . . 12
> > >        4.8.10. IP Resources . . . . . . . . . . . . . . . . . . . . . 12
> > >        4.8.11. AS Resources . . . . . . . . . . . . . . . . . . . . . 12
> > 
> > Sure. Do you want to add a comment how this is sorted?
> 
> Can do. I have added this:
> 
> +		/* The switch is ordered following RFC 6487, section 4.8. */
>  		switch (nid = OBJ_obj2nid(obj)) {
>  		case NID_basic_constraints:
>  			if (bc++ > 0)
> 
> I am working on refactoring, merging and streamlining the logic of
> cert_parse_ee_cert() (which is way too lax) and cert_parse_pre() (which
> is about as strict as it can be but messy since it grew organically).
> 
> Last baby step for this: make counters match.
> 
> We could drop the initialization for the nid to appease some tooling
> but I'm sure it will upset some other tools, so I left it.
> 
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.161 cert.c
> --- cert.c	19 Jun 2025 06:20:23 -0000	1.161
> +++ cert.c	19 Jun 2025 06:22:46 -0000
> @@ -815,10 +815,10 @@ cert_parse_pre(const char *fn, const uns
>  	const ASN1_BIT_STRING	*issuer_uid = NULL, *subject_uid = NULL;
>  	ASN1_OBJECT		*obj;
>  	EVP_PKEY		*pkey;
> -	int			 nid, ip, as, sia, cp, crldp, aia, aki, ski,
> -				 eku, bc, ku;
> +	int			 nid, bc, ski, aki, ku, eku, crldp, aia, sia,
> +				 cp, ip, as;
>  
> -	nid = ip = as = sia = cp = crldp = aia = aki = ski = eku = bc = ku = 0;
> +	nid = bc = ski = aki = ku = eku = crldp = aia = sia = cp = ip = as = 0;
>  
>  	/* just fail for empty buffers, the warning was printed elsewhere */
>  	if (der == NULL)
> 

OK claudio@

-- 
:wq Claudio