From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: rpki-client: split a few things out of ta_parse()
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Mon, 19 Jan 2026 10:37:40 +0100

On Mon, Jan 19, 2026 at 08:47:54AM +0100, Theo Buehler wrote:
> This pulls the non-inheritance check for TAs into cert_parse_extensions()
> and adds a check that the INRs are a non-empty set. The latter is redundant
> with existing checks for presence of at least one of ASIdentifiers and
> IPAddrBlocks combined with non-inheritance but it does not hurt to be
> explicit.
> 
> The second change is splitting everything to do with the SPKI into a
> helper since the current logic is messy and has completely unrelated
> things interleaved. In a follow-up I'll also split out the validity
> check. Later on ta_parse() will be renamed into something more
> appropriate.

OK claudio@
I wonder if the XXX in the ta_parse() comment is really needed. I
understand that this no longer does a real parse but either we just rename
the function (ta_check, ta_validate) or keep ta_parse and just drop the
XXX. I guess you already have something in mind for this :)
 
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.210 cert.c
> --- cert.c	13 Jan 2026 21:36:17 -0000	1.210
> +++ cert.c	14 Jan 2026 20:11:20 -0000
> @@ -1738,6 +1738,19 @@ cert_parse_extensions(const char *fn, st
>  		goto out;
>  	}
>  
> +	if (cert->purpose == CERT_PURPOSE_TA) {
> +		if (x509_any_inherits(cert->x509)) {
> +			warnx("%s: RFC 8630, 2.3: Trust Anchor INRs "
> +			    "must not inherit", fn);
> +			goto out;
> +		}
> +		if (cert->num_ips == 0 && cert->num_ases == 0) {
> +			warnx("%s: RFC 8630, 2.3: Trust Anchor INR set "
> +			    "must not be empty", fn);
> +			goto out;
> +		}
> +	}
> +
>  	if (cert->purpose == CERT_PURPOSE_BGPSEC_ROUTER) {
>  		if (ip != 0) {
>  			warnx("%s: RFC 8209, 3.1.3.4: BGPsec Router cert "
> @@ -1915,15 +1928,17 @@ cert_parse(const char *fn, const unsigne
>  	return NULL;
>  }
>  
> -struct cert *
> -ta_parse(const char *fn, struct cert *p, const unsigned char *pkey,
> +/*
> + * Check that the subjectPublicKeyInfo from the TAL matches the one in the cert.
> + * Verify that this key signed the cert.
> + * Returns 1 on success and 0 on failure.
> + */
> +static int
> +ta_check_pubkey(const char *fn, struct cert *cert, const unsigned char *pkey,
>      size_t pkeysz)
>  {
>  	EVP_PKEY	*pk, *opk;
> -	time_t		 now = get_current_time();
> -
> -	if (p == NULL)
> -		return NULL;
> +	int		 rv = 0;
>  
>  	/* first check pubkey against the one from the TAL */
>  	pk = d2i_PUBKEY(NULL, &pkey, pkeysz);
> @@ -1931,7 +1946,7 @@ ta_parse(const char *fn, struct cert *p,
>  		warnx("%s: RFC 6487 (trust anchor): bad TAL pubkey", fn);
>  		goto badcert;
>  	}
> -	if ((opk = X509_get0_pubkey(p->x509)) == NULL) {
> +	if ((opk = X509_get0_pubkey(cert->x509)) == NULL) {
>  		warnx("%s: RFC 6487 (trust anchor): missing pubkey", fn);
>  		goto badcert;
>  	}
> @@ -1941,37 +1956,52 @@ ta_parse(const char *fn, struct cert *p,
>  		goto badcert;
>  	}
>  
> -	if (p->notbefore > now) {
> -		warnx("%s: certificate not yet valid", fn);
> -		goto badcert;
> -	}
> -	if (p->notafter < now) {
> -		warnx("%s: certificate has expired", fn);
> +	/*
> +	 * 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) {
> +		warnx("%s: failed to verify signature", fn);
>  		goto badcert;
>  	}
> +
> +	rv = 1;
> + badcert:
> +	EVP_PKEY_free(pk);
> +	return rv;
> +}
> +
> +/* XXX - this really doesn't parse anything except the SPKI in the TAL. */
> +struct cert *
> +ta_parse(const char *fn, struct cert *p, const unsigned char *pkey,
> +    size_t pkeysz)
> +{
> +	time_t		 now = get_current_time();
> +
> +	if (p == NULL)
> +		return NULL;
> +
>  	if (p->purpose != CERT_PURPOSE_TA) {
>  		warnx("%s: expected trust anchor purpose, got %s", fn,
>  		    purpose2str(p->purpose));
>  		goto badcert;
>  	}
> -	/*
> -	 * Do not replace with a <= 0 check since OpenSSL 3 broke that:
> -	 * https://github.com/openssl/openssl/issues/24575
> -	 */
> -	if (X509_verify(p->x509, pk) != 1) {
> -		warnx("%s: failed to verify signature", fn);
> +
> +	if (!ta_check_pubkey(fn, p, pkey, pkeysz))
> +		goto badcert;
> +
> +	if (p->notbefore > now) {
> +		warnx("%s: certificate not yet valid", fn);
>  		goto badcert;
>  	}
> -	if (x509_any_inherits(p->x509)) {
> -		warnx("%s: Trust anchor IP/AS resources may not inherit", fn);
> +	if (p->notafter < now) {
> +		warnx("%s: certificate has expired", fn);
>  		goto badcert;
>  	}
>  
> -	EVP_PKEY_free(pk);
>  	return p;
>  
>   badcert:
> -	EVP_PKEY_free(pk);
>  	cert_free(p);
>  	return NULL;
>  }
> 

-- 
:wq Claudio