On Thu, Apr 02, 2026 at 05:39:57PM +0200, Theo Buehler wrote:
> On Thu, Apr 02, 2026 at 05:12:08PM +0200, Claudio Jeker wrote:
> > 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.
> 
> Indeed.
> 
> > 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.
> 
> True. We have regress that ensures that openssl 3 doesn't break and I
> will make sure to add the openssl 4 version once released so we notice
> if we break things.
> 
> Should I annotate the casts with
> 
> 	/* Needed for OpenSSL 3 and LibreSSL */
> 	if ((os = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) {
> 
> I know I will forget at some point....

Yes, put a reminder in for future us.
 
> > 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.
> 
> Thanks. Makes sense. I was thinking the maintenance burden is smaller
> with fewer hacks, but maybe I tried too hard to minimize those...
> 
> Then I suggest I rename certificate_policies() into cert_policies()
> and land the diff.
 
Sounds good.

-- 
:wq Claudio