From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: rpki-client: simplify cert/crl signature type checking
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Tue, 11 Jun 2024 09:00:48 +0200

On Mon, Jun 10, 2024 at 11:36:03PM +0200, Theo Buehler wrote:
> X509_CRL_get_signature_nid(crl) does OBJ_obj2nid(crl->sig_alg->algorithm)
> and similarly for X509_get_signature_nid(). That's a bit simpler than
> what we currently have. The proper error check for OBJ_obj2nid() is to
> see if nid is NID_undef, so do that, although it wouldn't really be
> necessary here. I can leave that out to preserve behavior if that's
> preferred.
> 
> PS: The sig_alg members of X509_CRL_seq_tt and X509_seq_tt aren't optional
> (there's no ASN1_TFLG_OPTIONAL) and neither is the algorithm in
> X509_ALGOR_seq_tt, so none of these is NULL after a successful call to
> d2i_X509_CRL and d2i_X509 and the get_signature_nid API is safe to call.

Lovley, this is much nicer. Ok claudio@
 
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.145 cert.c
> --- cert.c	10 Jun 2024 10:50:13 -0000	1.145
> +++ cert.c	10 Jun 2024 21:07:22 -0000
> @@ -797,9 +797,7 @@ cert_parse_pre(const char *fn, const uns
>  	int			 i, extsz;
>  	X509			*x = NULL;
>  	X509_EXTENSION		*ext = NULL;
> -	const X509_ALGOR	*palg;
>  	const ASN1_BIT_STRING	*piuid = NULL, *psuid = NULL;
> -	const ASN1_OBJECT	*cobj;
>  	ASN1_OBJECT		*obj;
>  	EVP_PKEY		*pkey;
>  	int			 nid, ip, as, sia, cp, crldp, aia, aki, ski,
> @@ -832,13 +830,10 @@ cert_parse_pre(const char *fn, const uns
>  		goto out;
>  	}
>  
> -	X509_get0_signature(NULL, &palg, x);
> -	if (palg == NULL) {
> -		warnx("%s: X509_get0_signature", fn);
> +	if ((nid = X509_get_signature_nid(x)) == NID_undef) {
> +		warnx("%s: unknown signature type", fn);
>  		goto out;
>  	}
> -	X509_ALGOR_get0(&cobj, NULL, NULL, palg);
> -	nid = OBJ_obj2nid(cobj);
>  	if (experimental && nid == NID_ecdsa_with_SHA256) {
>  		if (verbose)
>  			warnx("%s: P-256 support is experimental", fn);
> Index: crl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
> diff -u -p -r1.37 crl.c
> --- crl.c	5 Jun 2024 13:36:28 -0000	1.37
> +++ crl.c	10 Jun 2024 21:09:55 -0000
> @@ -165,9 +165,7 @@ crl_parse(const char *fn, const unsigned
>  {
>  	const unsigned char	*oder;
>  	struct crl		*crl;
> -	const X509_ALGOR	*palg;
>  	const X509_NAME		*name;
> -	const ASN1_OBJECT	*cobj;
>  	const ASN1_TIME		*at;
>  	int			 count, nid, rc = 0;
>  
> @@ -200,13 +198,10 @@ crl_parse(const char *fn, const unsigned
>  	if (!x509_valid_name(fn, "issuer", name))
>  		goto out;
>  
> -	X509_CRL_get0_signature(crl->x509_crl, NULL, &palg);
> -	if (palg == NULL) {
> -		warnx("%s: X509_CRL_get0_signature", fn);
> +	if ((nid = X509_CRL_get_signature_nid(crl->x509_crl)) == NID_undef) {
> +		warnx("%s: unknown signature type", fn);
>  		goto out;
>  	}
> -	X509_ALGOR_get0(&cobj, NULL, NULL, palg);
> -	nid = OBJ_obj2nid(cobj);
>  	if (experimental && nid == NID_ecdsa_with_SHA256) {
>  		if (verbose)
>  			warnx("%s: P-256 support is experimental", fn);
> 

-- 
:wq Claudio