On Tue, 30 Jan 2024 13:30:32 +0100
Theo Buehler <tb@theobuehler.org> wrote:

> This version of a diff from jsing for libssl (it applies with slight
> offsets to 7.4-stable) should fix this issue.
> 
> Could you please try this with an unpached apache-httpd?
> 
> Index: s3_lib.c
> ===================================================================
> RCS file: /cvs/src/lib/libssl/s3_lib.c,v
> diff -u -p -r1.248 s3_lib.c
> --- s3_lib.c	29 Nov 2023 13:39:34 -0000	1.248
> +++ s3_lib.c	30 Jan 2024 11:34:10 -0000
> @@ -1594,6 +1594,7 @@ ssl3_free(SSL *s)
>  	tls1_transcript_hash_free(s);
>  
>  	free(s->s3->alpn_selected);
> +	free(s->s3->alpn_wire_data);
>  
>  	freezero(s->s3->peer_quic_transport_params,
>  	    s->s3->peer_quic_transport_params_len);
> @@ -1659,6 +1660,9 @@ ssl3_clear(SSL *s)
>  	free(s->s3->alpn_selected);
>  	s->s3->alpn_selected = NULL;
>  	s->s3->alpn_selected_len = 0;
> +	free(s->s3->alpn_wire_data);
> +	s->s3->alpn_wire_data = NULL;
> +	s->s3->alpn_wire_data_len = 0;
>  
>  	freezero(s->s3->peer_quic_transport_params,
>  	    s->s3->peer_quic_transport_params_len);
> Index: ssl_local.h
> ===================================================================
> RCS file: /cvs/src/lib/libssl/ssl_local.h,v
> diff -u -p -r1.12 ssl_local.h
> --- ssl_local.h	29 Dec 2023 12:24:33 -0000	1.12
> +++ ssl_local.h	30 Jan 2024 11:34:10 -0000
> @@ -1209,6 +1209,8 @@ typedef struct ssl3_state_st {
>  	 */
>  	uint8_t *alpn_selected;
>  	size_t alpn_selected_len;
> +	uint8_t *alpn_wire_data;
> +	size_t alpn_wire_data_len;
>  
>  	/* Contains the QUIC transport params received from our peer. */
>  	uint8_t *peer_quic_transport_params;
> Index: ssl_tlsext.c
> ===================================================================
> RCS file: /cvs/src/lib/libssl/ssl_tlsext.c,v
> diff -u -p -r1.137 ssl_tlsext.c
> --- ssl_tlsext.c	28 Apr 2023 18:14:59 -0000	1.137
> +++ ssl_tlsext.c	30 Jan 2024 11:34:10 -0000
> @@ -86,33 +86,48 @@ tlsext_alpn_check_format(CBS *cbs)
>  }
>  
>  static int
> -tlsext_alpn_server_parse(SSL *s, uint16_t msg_types, CBS *cbs, int *alert)
> +tlsext_alpn_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
>  {
> -	CBS alpn, selected_cbs;
> -	const unsigned char *selected;
> -	unsigned char selected_len;
> -	int r;
> +	CBS alpn;
>  
>  	if (!CBS_get_u16_length_prefixed(cbs, &alpn))
>  		return 0;
> -
>  	if (!tlsext_alpn_check_format(&alpn))
>  		return 0;
> +	if (!CBS_stow(&alpn, &s->s3->alpn_wire_data, &s->s3->alpn_wire_data_len))
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int
> +tlsext_alpn_server_process(SSL *s, uint16_t msg_type, int *alert)
> +{
> +	const unsigned char *selected;
> +	unsigned char selected_len;
> +	CBS alpn, selected_cbs;
> +	int cb_ret;
>  
>  	if (s->ctx->alpn_select_cb == NULL)
>  		return 1;
>  
> +	if (s->s3->alpn_wire_data == NULL) {
> +		*alert = SSL_AD_INTERNAL_ERROR;
> +		return 0;
> +	}
> +	CBS_init(&alpn, s->s3->alpn_wire_data, s->s3->alpn_wire_data_len);
> +
>  	/*
>  	 * XXX - A few things should be considered here:
>  	 * 1. Ensure that the same protocol is selected on session resumption.
>  	 * 2. Should the callback be called even if no ALPN extension was sent?
>  	 * 3. TLSv1.2 and earlier: ensure that SNI has already been processed.
>  	 */
> -	r = s->ctx->alpn_select_cb(s, &selected, &selected_len,
> +	cb_ret = s->ctx->alpn_select_cb(s, &selected, &selected_len,
>  	    CBS_data(&alpn), CBS_len(&alpn),
>  	    s->ctx->alpn_select_cb_arg);
>  
> -	if (r == SSL_TLSEXT_ERR_OK) {
> +	if (cb_ret == SSL_TLSEXT_ERR_OK) {
>  		CBS_init(&selected_cbs, selected, selected_len);
>  
>  		if (!CBS_stow(&selected_cbs, &s->s3->alpn_selected,
> @@ -125,7 +140,7 @@ tlsext_alpn_server_parse(SSL *s, uint16_
>  	}
>  
>  	/* On SSL_TLSEXT_ERR_NOACK behave as if no callback was present. */
> -	if (r == SSL_TLSEXT_ERR_NOACK)
> +	if (cb_ret == SSL_TLSEXT_ERR_NOACK)
>  		return 1;
>  
>  	*alert = SSL_AD_NO_APPLICATION_PROTOCOL;
> @@ -1972,6 +1987,7 @@ struct tls_extension_funcs {
>  	int (*needs)(SSL *s, uint16_t msg_type);
>  	int (*build)(SSL *s, uint16_t msg_type, CBB *cbb);
>  	int (*parse)(SSL *s, uint16_t msg_type, CBS *cbs, int *alert);
> +	int (*process)(SSL *s, uint16_t msg_type, int *alert);
>  };
>  
>  struct tls_extension {
> @@ -2123,6 +2139,7 @@ static const struct tls_extension tls_ex
>  			.needs = tlsext_alpn_server_needs,
>  			.build = tlsext_alpn_server_build,
>  			.parse = tlsext_alpn_server_parse,
> +			.process = tlsext_alpn_server_process,
>  		},
>  	},
>  	{
> @@ -2391,6 +2408,14 @@ tlsext_clienthello_hash_extension(SSL *s
>  	return 1;
>  }
>  
> +static void
> +tlsext_cleanup(SSL *s)
> +{
> +	free(s->s3->alpn_wire_data);
> +	s->s3->alpn_wire_data = NULL;
> +	s->s3->alpn_wire_data_len = 0;
> +}
> +
>  static int
>  tlsext_parse(SSL *s, int is_server, uint16_t msg_type, CBS *cbs, int *alert)
>  {
> @@ -2466,6 +2491,36 @@ tlsext_parse(SSL *s, int is_server, uint
>  	return 0;
>  }
>  
> +static int
> +tlsext_process(SSL *s, int is_server, uint16_t msg_type, int *alert)
> +{
> +	const struct tls_extension_funcs *ext;
> +	const struct tls_extension *tlsext;
> +	int alert_desc;
> +	size_t idx;
> +
> +	alert_desc = SSL_AD_DECODE_ERROR;
> +
> +	/* Run processing for present TLS extensions, in a defined order. */
> +	for (idx = 0; idx < N_TLS_EXTENSIONS; idx++) {
> +		tlsext = &tls_extensions[idx];
> +		if ((s->s3->hs.extensions_seen & (1 << idx)) == 0)
> +			continue;
> +		ext = tlsext_funcs(tlsext, is_server);
> +		if (ext->process == NULL)
> +			continue;
> +		if (!ext->process(s, msg_type, &alert_desc))
> +			goto err;
> +	}
> +
> +	return 1;
> +
> + err:
> +	*alert = alert_desc;
> +
> +	return 0;
> +}
> +
>  static void
>  tlsext_server_reset_state(SSL *s)
>  {
> @@ -2486,11 +2541,23 @@ tlsext_server_build(SSL *s, uint16_t msg
>  int
>  tlsext_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
>  {
> +	int ret = 0;
> +
>  	/* XXX - this should be done by the caller... */
>  	if (msg_type == SSL_TLSEXT_MSG_CH)
>  		tlsext_server_reset_state(s);
>  
> -	return tlsext_parse(s, 1, msg_type, cbs, alert);
> +	if (!tlsext_parse(s, 1, msg_type, cbs, alert))
> +		goto err;
> +	if (!tlsext_process(s, 1, msg_type, alert))
> +		goto err;
> +
> +	ret = 1;
> +
> + err:
> +	tlsext_cleanup(s);
> +
> +	return ret;
>  }
>  
>  static void

	After applying this patch, I was not able to replicate the
error on Chrome after a few minutes of testing on a couple different
clients.

	I will leave this version running and report any issues.

--TimH