On Tue, Jan 20, 2026 at 05:57:04PM +0100, Theo Buehler wrote:
> On Tue, Jan 20, 2026 at 12:50:05PM +0000, Job Snijders wrote:
> > On Tue, Jan 20, 2026 at 09:02:12AM +0100, Theo Buehler wrote:
> > > This is an almost entirely mechanical diff. The pkey hanging off
> > > struct tal always confuses me since pkey always makes me think of
> > > EVP_PKEY. The combo with pk and opk in a couple of functions makes
> > > this worse.
> > > 
> > > So: rename tal->pkey{,sz} to tal->spki{,sz} and pk/opk to pkey/opkey
> > > and adjust a couple of nearby comments. Update from RFC 7730 to RFC
> > > 8630 while there.
> > 
> > To me it is not immediately clear from the new (or old) variable names
> > 'pkey' and 'opkey' what those variables might contain, perhaps the names
> > 'tal_pkey' and 'cert_pkey' would've been more descriptive? Anyhow...
> 
> So here's the rename you requested. I will do a %s/badcert/out followup
> in cert.c to get that out of the way.
 
On comment:

> -	if (EVP_PKEY_cmp(pk, opk) != 1) {
> +	if (EVP_PKEY_cmp(cert_pkey, tal_pkey) != 1) {

This revereses the order of arguments (which does not matter) but I just
wanted to point that out.

OK claudio@

> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.212 cert.c
> --- cert.c	20 Jan 2026 16:49:03 -0000	1.212
> +++ cert.c	20 Jan 2026 16:52:10 -0000
> @@ -1937,20 +1937,20 @@ static int
>  ta_check_pubkey(const char *fn, struct cert *cert, const unsigned char *spki,
>      size_t spkisz)
>  {
> -	EVP_PKEY	*pk, *opk;
> +	EVP_PKEY	*cert_pkey, *tal_pkey;
>  	int		 rv = 0;
>  
>  	/* first check pubkey against the one from the TAL */
> -	pk = d2i_PUBKEY(NULL, &spki, spkisz);
> -	if (pk == NULL) {
> +	tal_pkey = d2i_PUBKEY(NULL, &spki, spkisz);
> +	if (tal_pkey == NULL) {
>  		warnx("%s: RFC 6487 (trust anchor): bad TAL pubkey", fn);
>  		goto badcert;
>  	}
> -	if ((opk = X509_get0_pubkey(cert->x509)) == NULL) {
> +	if ((cert_pkey = X509_get0_pubkey(cert->x509)) == NULL) {
>  		warnx("%s: RFC 6487 (trust anchor): missing pubkey", fn);
>  		goto badcert;
>  	}
> -	if (EVP_PKEY_cmp(pk, opk) != 1) {
> +	if (EVP_PKEY_cmp(cert_pkey, tal_pkey) != 1) {
>  		warnx("%s: RFC 6487 (trust anchor): "
>  		    "pubkey does not match TAL pubkey", fn);
>  		goto badcert;
> @@ -1960,14 +1960,14 @@ ta_check_pubkey(const char *fn, struct c
>  	 * Do not replace with a <= 0 check since OpenSSL 3 broke that:
>  	 * https://github.com/openssl/openssl/issues/24575
>  	 */
> -	if (X509_verify(cert->x509, pk) != 1) {
> +	if (X509_verify(cert->x509, tal_pkey) != 1) {
>  		warnx("%s: failed to verify signature", fn);
>  		goto badcert;
>  	}
>  
>  	rv = 1;
>   badcert:
> -	EVP_PKEY_free(pk);
> +	EVP_PKEY_free(tal_pkey);
>  	return rv;
>  }
>  
> Index: filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> diff -u -p -r1.77 filemode.c
> --- filemode.c	20 Jan 2026 16:49:03 -0000	1.77
> +++ filemode.c	20 Jan 2026 16:52:10 -0000
> @@ -283,11 +283,11 @@ out:
>  static struct tal *
>  find_tal(struct cert *cert)
>  {
> -	EVP_PKEY	*pk, *opk;
> +	EVP_PKEY	*cert_pkey, *tal_pkey;
>  	struct tal	*tal;
>  	int		 i;
>  
> -	if ((opk = X509_get0_pubkey(cert->x509)) == NULL)
> +	if ((cert_pkey = X509_get0_pubkey(cert->x509)) == NULL)
>  		return NULL;
>  
>  	for (i = 0; i < TALSZ_MAX; i++) {
> @@ -297,14 +297,14 @@ find_tal(struct cert *cert)
>  			break;
>  		tal = talobj[i];
>  		spki = tal->spki;
> -		pk = d2i_PUBKEY(NULL, &spki, tal->spkisz);
> -		if (pk == NULL)
> +		tal_pkey = d2i_PUBKEY(NULL, &spki, tal->spkisz);
> +		if (tal_pkey == NULL)
>  			continue;
> -		if (EVP_PKEY_cmp(pk, opk) == 1) {
> -			EVP_PKEY_free(pk);
> +		if (EVP_PKEY_cmp(cert_pkey, tal_pkey) == 1) {
> +			EVP_PKEY_free(tal_pkey);
>  			return tal;
>  		}
> -		EVP_PKEY_free(pk);
> +		EVP_PKEY_free(tal_pkey);
>  	}
>  	return NULL;
>  }
> 

-- 
:wq Claudio