On Tue, 30 Jan 2024 13:30:32 +0100
Theo Buehler <[email protected]> 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
