From: Tobias Heider <tobias.heider@stusta.de>
Subject: Re: iked: don't pretend to support multiple subjectAltName
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Mon, 17 Jun 2024 18:21:02 +0200

On Mon, Jun 17, 2024 at 10:57:26AM +0200, Theo Buehler wrote:
> Per RFC 5280, section 4.2: "A certificate MUST NOT include more
> than one instance of a particular extension". X509_verify_cert()
> will fail on such a cert anyway. The below switches to using the
> X509_get_ext_d2i() API correctly. Not sure the added log_debug()
> calls are of much value; they won't do harm.

Looks correct.

I wonder if we should make the new log message more verbose since
it would help people figure out why their certs don't work.
Probably not an issue though if this failed in libressl anyway.

ok tobhe@

> 
> Index: ca.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/ca.c,v
> diff -u -p -r1.101 ca.c
> --- ca.c	13 Feb 2024 12:25:11 -0000	1.101
> +++ ca.c	25 Apr 2024 12:47:46 -0000
> @@ -1985,13 +1985,13 @@ ca_x509_subjectaltname_do(X509 *cert, in
>  	GENERAL_NAME *entry;
>  	ASN1_STRING *cstr;
>  	char idstr[IKED_ID_SIZE];
> -	int idx, ret, i, type, len;
> +	int crit, ret, i, type, len;
>  	const uint8_t *data;
>  
>  	ret = -1;
> -	idx = -1;
> -	while ((stack = X509_get_ext_d2i(cert, NID_subject_alt_name,
> -	    NULL, &idx)) != NULL) {
> +	crit = -1;
> +	if ((stack = X509_get_ext_d2i(cert, NID_subject_alt_name,
> +	    &crit, NULL)) != NULL) {
>  		for (i = 0; i < sk_GENERAL_NAME_num(stack); i++) {
>  			entry = sk_GENERAL_NAME_value(stack, i);
>  			switch (entry->type) {
> @@ -2071,12 +2071,13 @@ ca_x509_subjectaltname_do(X509 *cert, in
>  			}
>  		}
>  		sk_GENERAL_NAME_pop_free(stack, GENERAL_NAME_free);
> -		if (ret != -1)
> -			break;
> -	}
> -	if (idx == -1)
> +	} else if (crit == -2)
> +		log_debug("%s: multiple subjectAltName extensions", __func__);
> +	else if (crit == -1)
>  		log_debug("%s: did not find subjectAltName in certificate",
>  		    __func__);
> +	else
> +		log_debug("%s: failed to decode subjectAltName", __func__);
>  	return ret;
>  }
>  
>