Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: rpki-client: const shuffling for openssl 4
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Thu, 2 Apr 2026 17:12:08 +0200

Download raw body.

Thread
On Thu, Apr 02, 2026 at 04:40:43PM +0200, Theo Buehler wrote:
> On Sat, Mar 28, 2026 at 10:21:15AM +0100, Theo Buehler wrote:
> > OpenSSL 4 went on a const sprinkling spree which requires us to think
> > about how we can compile cleanly with various versions of the libs so
> > we can catch more serious warnings.
> > 
> > We can adapt the xissuer in print.c to be const. It hasn't been const
> > because X509_get_issuer_name() isn't const correct.
> > 
> > cert.c is a bit tricky even if it only parses things and hence doesn't
> > modify libcrypto objects it doesn't own.
> > 
> > There are two pieces to the puzzle. 
> > 
> > In cert_check_spki() the pubkey is a libcrypto-internal pointer hanging
> > off cert->x509, which is then passed to the very const-incorrect getter
> > X509_PUBKEY_get0_param(): that's a piece of art which hands back pointers
> > to things deeper down in the x509 - some of them const, some non-const.
> > OpenSSL 3 made its X509_PUBKEY argument const, but their X509_ALGOR **
> > still isn't. I don't believe they thought about it in #11894 as they had
> > a _cmp() vs _eq() bikeshed to sort out.
> > 
> > Our choices are to cast the pubkey coming from X509_get_X509_PUBKEY()
> > or the one passed to X509_PUBKEY_get0_param(). Since the latter call is
> > tricky I chose the former.
> > 
> > Of course I saved the best for last:
> > 
> > The individual extension handlers don't take a const X509_EXTENSION *
> > mainly because each of them calls X509V3_EXT_d2i() which should have
> > been const but isn't. We could cast away const in all nine of its
> > callers, which might be the least evil approach if we can live with a 
> > it of churn. That works out nicely except that we also need to cast the
> > ext passed to X509_EXTENSION_get_object(). Happy to go this way if you
> > prefer that.
> 
> That diff would be the below.
> 
> > As an aside, a single wrapper function that allows casting away const
> > only once seems a bad idea:
> > 
> > /*
> >  * Decode extension data from DER. It's the caller's responsibility to
> >  * assign the return value to the correct pointer type and to free it.
> >  */
> > static void *
> > cert_yolo_decode_extension_data(const X509_EXTENSION *ext)
> 
> Maybe we could get away with this by calling it X509V3_EXT_d2i_const()
> or something. Still not a fan.
> 
> > {
> > 	/* XXX - const correct only in OpenSSL 4. */
> > 	return X509V3_EXT_d2i((X509_EXTENSION *)ext);
> > }
> > 
> > I don't see how to add belts and suspenders to make this interface less
> > terrible and trappy.
> > 
> > Yet another approach is the below: cast away const from X509_get_ext().
> > "grep -w ext cert.c" will show you that ext is only passed to 
> > X509V3_EXT_d2i() and X509_EXTENSION_get_object() already mentioned above
> > and to X509_EXTENSION_get_critical() which is already const correct.
> 
> It would be nice to get a version of this diff in so the next rpki-client
> release can support OpenSSL 4 ootb (needs some portable massaging on top,
> but that isn't hard). It works well in my testing.
> 
> I know the new certificate_policies() line below is too long. We could
> wrap it or rename the function to cert_policies(). And then I need to
> figure out what to do about the sbgp_* functions...

As usual there is not enough lipstick to make this pig pretty.

Even if libressl fixes the API to follow openssl 4 we still need to
support openssl 3 for a while so the cast will remain for a while.

That said I do prefer the diff below a tad bit better.
My reasoning being that this makes the rpki-client code more const
correct and pushes the cast to the openssl calls. While your first diff
removes the const early on but this leaves the rpki-client code incorrect.
 
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.224 cert.c
> --- cert.c	3 Feb 2026 16:21:37 -0000	1.224
> +++ cert.c	28 Mar 2026 12:40:39 -0000
> @@ -354,8 +354,12 @@ cert_check_spki(const char *fn, struct c
>  	const void		*pval = NULL;
>  	int			 rc = 0;
>  
> -	/* Should be called _get0_. It returns a pointer owned by cert->x509. */
> -	if ((pubkey = X509_get_X509_PUBKEY(cert->x509)) == NULL) {
> +	/*
> +	 * Should be called _get0_. It returns a pointer owned by cert->x509.
> +	 * XXX - cast away const for OpenSSL 4.
> +	 */
> +	pubkey = (X509_PUBKEY *)X509_get_X509_PUBKEY(cert->x509);
> +	if (pubkey == NULL) {
>  		warnx("%s: RFC 6487, 4.7: certificate without SPKI", fn);
>  		goto out;
>  	}
> @@ -418,7 +422,7 @@ cert_check_spki(const char *fn, struct c
>  }
>  
>  static int
> -cert_ski(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_ski(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	ASN1_OCTET_STRING	*os = NULL;
>  	unsigned char		 md[EVP_MAX_MD_SIZE];
> @@ -433,7 +437,7 @@ cert_ski(const char *fn, struct cert *ce
>  		goto out;
>  	}
>  
> -	if ((os = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((os = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.2: error parsing SKI", fn);
>  		goto out;
>  	}
> @@ -465,7 +469,7 @@ cert_ski(const char *fn, struct cert *ce
>  }
>  
>  static int
> -cert_aki(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_aki(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	AUTHORITY_KEYID	*akid = NULL;
>  	int		 length, rc = 0;
> @@ -478,7 +482,7 @@ cert_aki(const char *fn, struct cert *ce
>  		goto out;
>  	}
>  
> -	if ((akid = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((akid = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.3: error parsing AKI", fn);
>  		goto out;
>  	}
> @@ -513,7 +517,7 @@ cert_aki(const char *fn, struct cert *ce
>   * Parse CRL distribution point per RFC 6487, section 4.8.6.
>   */
>  static int
> -cert_crldp(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_crldp(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	CRL_DIST_POINTS	*crldp = NULL;
>  	DIST_POINT	*dp;
> @@ -535,7 +539,7 @@ cert_crldp(const char *fn, struct cert *
>  		goto out;
>  	}
>  
> -	if ((crldp = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((crldp = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.6: CRL distribution point: "
>  		    "failed extension parse", fn);
>  		goto out;
> @@ -614,7 +618,7 @@ cert_crldp(const char *fn, struct cert *
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -cert_aia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_aia(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	AUTHORITY_INFO_ACCESS	*aia = NULL;
>  	ACCESS_DESCRIPTION	*ad;
> @@ -636,7 +640,7 @@ cert_aia(const char *fn, struct cert *ce
>  		goto out;
>  	}
>  
> -	if ((aia = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((aia = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.7: AIA: failed extension parse",
>  		    fn);
>  		goto out;
> @@ -694,7 +698,7 @@ cert_aia(const char *fn, struct cert *ce
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -cert_ca_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_ca_sia(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	AUTHORITY_INFO_ACCESS	*sia = NULL;
>  	ACCESS_DESCRIPTION	*ad;
> @@ -711,7 +715,7 @@ cert_ca_sia(const char *fn, struct cert 
>  		goto out;
>  	}
>  
> -	if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((sia = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.8: SIA: failed extension parse",
>  		    fn);
>  		goto out;
> @@ -833,7 +837,7 @@ cert_ca_sia(const char *fn, struct cert 
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -cert_ee_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_ee_sia(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	AUTHORITY_INFO_ACCESS	*sia = NULL;
>  	ACCESS_DESCRIPTION	*ad;
> @@ -849,7 +853,7 @@ cert_ee_sia(const char *fn, struct cert 
>  		goto out;
>  	}
>  
> -	if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((sia = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.8: SIA: failed extension parse",
>  		    fn);
>  		goto out;
> @@ -922,7 +926,7 @@ cert_ee_sia(const char *fn, struct cert 
>  }
>  
>  static int
> -cert_sia(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +cert_sia(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	switch (cert->purpose) {
>  	case CERT_PURPOSE_TA:
> @@ -944,7 +948,7 @@ cert_sia(const char *fn, struct cert *ce
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -certificate_policies(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +certificate_policies(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	STACK_OF(POLICYINFO)		*policies = NULL;
>  	POLICYINFO			*policy;
> @@ -959,7 +963,7 @@ certificate_policies(const char *fn, str
>  		goto out;
>  	}
>  
> -	if ((policies = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((policies = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.9: certificatePolicies: "
>  		    "failed extension parse", fn);
>  		goto out;
> @@ -1224,7 +1228,7 @@ sbgp_parse_ipaddrblk(const char *fn, con
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -sbgp_ipaddrblk(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +sbgp_ipaddrblk(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	IPAddrBlocks	*addrblk = NULL;
>  	int		 rc = 0;
> @@ -1235,7 +1239,7 @@ sbgp_ipaddrblk(const char *fn, struct ce
>  		goto out;
>  	}
>  
> -	if ((addrblk = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((addrblk = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.10: sbgp-ipAddrBlock: "
>  		    "failed extension parse", fn);
>  		goto out;
> @@ -1455,7 +1459,7 @@ sbgp_parse_assysnum(const char *fn, cons
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -sbgp_assysnum(const char *fn, struct cert *cert, X509_EXTENSION *ext)
> +sbgp_assysnum(const char *fn, struct cert *cert, const X509_EXTENSION *ext)
>  {
>  	ASIdentifiers		*asidentifiers = NULL;
>  	int			 rc = 0;
> @@ -1466,7 +1470,7 @@ sbgp_assysnum(const char *fn, struct cer
>  		goto out;
>  	}
>  
> -	if ((asidentifiers = X509V3_EXT_d2i(ext)) == NULL) {
> +	if ((asidentifiers = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
>  		warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
>  		    "failed extension parse", fn);
>  		goto out;
> @@ -1498,8 +1502,8 @@ static int
>  cert_parse_extensions(const char *fn, struct cert *cert)
>  {
>  	X509		*x = cert->x509;
> -	X509_EXTENSION	*ext;
> -	ASN1_OBJECT	*obj;
> +	const X509_EXTENSION *ext;
> +	const ASN1_OBJECT *obj;
>  	int		 extsz, i, nid;
>  	int		 bc, ski, aki, ku, eku, crldp, aia, sia, cp, ip, as;
>  
> @@ -1517,7 +1521,7 @@ cert_parse_extensions(const char *fn, st
>  	for (i = 0; i < extsz; i++) {
>  		ext = X509_get_ext(x, i);
>  		assert(ext != NULL);
> -		obj = X509_EXTENSION_get_object(ext);
> +		obj = X509_EXTENSION_get_object((X509_EXTENSION *)ext);
>  		assert(obj != NULL);
>  
>  		/* The switch is ordered following RFC 6487, section 4.8. */
> Index: print.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
> diff -u -p -r1.74 print.c
> --- print.c	20 Jan 2026 16:49:03 -0000	1.74
> +++ print.c	27 Mar 2026 19:36:55 -0000
> @@ -378,7 +378,7 @@ crl_print(const struct crl *p)
>  {
>  	STACK_OF(X509_REVOKED)	*revlist;
>  	X509_REVOKED *rev;
> -	X509_NAME *xissuer;
> +	const X509_NAME *xissuer;
>  	int i;
>  	char *issuer, *serial;
>  	time_t t;
> 

-- 
:wq Claudio