From: Bob Beck <beck@obtuse.com>
Subject: Re: isakmpd: don't reach into ASN1_STRING [step 1/2]
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org, beck@openbsd.org
Date: Mon, 24 Nov 2025 11:10:48 -0700

Ok beck@

> On Nov 24, 2025, at 07:31, Theo Buehler <tb@theobuehler.org> wrote:
> 
> beck and davidben have plans to make ASN1_STRING opaque in OpenSSL 4 [1].
> This breaks somewhere in the vicinity of 200 ports, so I don't think we'll
> flip the switch anytime soon in libressl, but I want base to be ready for
> that since that makes my life easier.
> 
> This is the first of two steps to convert isakmpd. ASN1_STRING_length()
> and ASN1_STRING_get0_data() are dumb accessors. The latter returns a
> const pointer, so add a minor adjustment for that, the rest is entirely
> straightforward.
> 
> (In case you wonder: x509_cert_subjectaltname() isn't static because it
> is "used" from regress/isakmpd/x509. That thing is completely broken and
> hasn't been successfully compiled since reyk removed math_mp.h back in
> 2010, possibly much longer.)
> 
> [1]: https://github.com/openssl/openssl/issues/29117
> 
> Index: x509.c
> ===================================================================
> RCS file: /cvs/src/sbin/isakmpd/x509.c,v
> diff -u -p -r1.126 x509.c
> --- x509.c	28 Apr 2024 16:43:42 -0000	1.126
> +++ x509.c	24 Nov 2025 14:03:09 -0000
> @@ -1098,11 +1098,11 @@ x509_cert_obtain(u_int8_t *id, size_t id
> 
> /* Returns a pointer to the subjectAltName information of X509 certificate.  */
> int
> -x509_cert_subjectaltname(X509 *scert, u_int8_t **altname, u_int32_t *len)
> +x509_cert_subjectaltname(X509 *scert, const u_int8_t **altname, u_int32_t *len)
> {
> 	X509_EXTENSION		*subjectaltname;
> 	ASN1_OCTET_STRING	*sanasn1data;
> -	u_int8_t		*sandata;
> +	const u_int8_t		*sandata;
> 	int			 extpos, santype, sanlen;
> 
> 	extpos = X509_get_ext_by_NID(scert, NID_subject_alt_name, -1);
> @@ -1114,14 +1114,15 @@ x509_cert_subjectaltname(X509 *scert, u_
> 	subjectaltname = X509_get_ext(scert, extpos);
> 	sanasn1data = X509_EXTENSION_get_data(subjectaltname);
> 
> -	if (!subjectaltname || !sanasn1data || !sanasn1data->data ||
> -	    sanasn1data->length < 4) {
> +	if (!subjectaltname || !sanasn1data ||
> +	    !ASN1_STRING_get0_data(sanasn1data) ||
> +	     ASN1_STRING_length(sanasn1data) < 4) {
> 		log_print("x509_cert_subjectaltname: invalid "
> 		    "subjectaltname extension");
> 		return 0;
> 	}
> 	/* SSL does not handle unknown ASN stuff well, do it by hand.  */
> -	sandata = sanasn1data->data;
> +	sandata = ASN1_STRING_get0_data(sanasn1data);
> 	santype = sandata[2] & 0x3f;
> 	sanlen = sandata[3];
> 	sandata += 4;
> @@ -1131,7 +1132,7 @@ x509_cert_subjectaltname(X509 *scert, u_
> 	 * extra stuff in subjectAltName, so we will just take the first
> 	 * salen bytes, and not worry about what follows.
> 	 */
> -	if (sanlen + 4 > sanasn1data->length) {
> +	if (sanlen + 4 > ASN1_STRING_length(sanasn1data)) {
> 		log_print("x509_cert_subjectaltname: subjectaltname invalid "
> 		    "length");
> 		return 0;
> @@ -1148,7 +1149,7 @@ x509_cert_get_subjects(void *scert, int 
> 	X509		*cert = scert;
> 	X509_NAME	*subject;
> 	int		type;
> -	u_int8_t	*altname;
> +	const u_int8_t	*altname;
> 	u_int32_t	altlen;
> 	u_int8_t	*buf = 0;
> 	unsigned char	*ubuf;
> Index: x509.h
> ===================================================================
> RCS file: /cvs/src/sbin/isakmpd/x509.h,v
> diff -u -p -r1.22 x509.h
> --- x509.h	5 Aug 2007 09:43:09 -0000	1.22
> +++ x509.h	24 Nov 2025 14:03:09 -0000
> @@ -82,7 +82,7 @@ int		x509_ca_count(void);
> 
> char           *x509_DN_string(u_int8_t *, size_t);
> int             x509_cert_insert(int, void *);
> -int             x509_cert_subjectaltname(X509 * cert, u_char **, u_int *);
> +int             x509_cert_subjectaltname(X509 * cert, const u_char **, u_int *);
> X509           *x509_from_asn(u_char *, u_int);
> int             x509_generate_kn(int, X509 *);
> int             x509_read_from_dir(X509_STORE *, char *, int, int *);