From: Claudio Jeker Subject: Re: rpki-client: const shuffling for openssl 4 To: Theo Buehler Cc: tech@openbsd.org Date: Thu, 2 Apr 2026 17:12:08 +0200 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