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 07:14:55 +0200

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@
 
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.159 cert.c
> --- cert.c	4 Jun 2025 09:18:28 -0000	1.159
> +++ cert.c	18 Jun 2025 22:49:14 -0000
> @@ -881,6 +881,34 @@ cert_parse_pre(const char *fn, const uns
>  		assert(obj != NULL);
>  
>  		switch (nid = OBJ_obj2nid(obj)) {
> +		case NID_basic_constraints:
> +			if (bc++ > 0)
> +				goto dup;
> +			break;
> +		case NID_subject_key_identifier:
> +			if (ski++ > 0)
> +				goto dup;
> +			break;
> +		case NID_authority_key_identifier:
> +			if (aki++ > 0)
> +				goto dup;
> +			break;
> +		case NID_key_usage:
> +			if (ku++ > 0)
> +				goto dup;
> +			break;
> +		case NID_ext_key_usage:
> +			if (eku++ > 0)
> +				goto dup;
> +			break;
> +		case NID_crl_distribution_points:
> +			if (crldp++ > 0)
> +				goto dup;
> +			break;
> +		case NID_info_access:
> +			if (aia++ > 0)
> +				goto dup;
> +			break;
>  		case NID_sbgp_ipAddrBlock:
>  			if (ip++ > 0)
>  				goto dup;
> @@ -908,34 +936,6 @@ cert_parse_pre(const char *fn, const uns
>  				goto dup;
>  			if (!certificate_policies(fn, cert, ext))
>  				goto out;
> -			break;
> -		case NID_crl_distribution_points:
> -			if (crldp++ > 0)
> -				goto dup;
> -			break;
> -		case NID_info_access:
> -			if (aia++ > 0)
> -				goto dup;
> -			break;
> -		case NID_authority_key_identifier:
> -			if (aki++ > 0)
> -				goto dup;
> -			break;
> -		case NID_subject_key_identifier:
> -			if (ski++ > 0)
> -				goto dup;
> -			break;
> -		case NID_ext_key_usage:
> -			if (eku++ > 0)
> -				goto dup;
> -			break;
> -		case NID_basic_constraints:
> -			if (bc++ > 0)
> -				goto dup;
> -			break;
> -		case NID_key_usage:
> -			if (ku++ > 0)
> -				goto dup;
>  			break;
>  		default:
>  			/* unexpected extensions warrant investigation */
> 

-- 
:wq Claudio