On Fri, Jan 26, 2024 at 02:11:52PM -0800, Tim wrote:
> I'm trying to troubleshoot an issue where Chrome/Chromium browsers
> randomly fail to correctly use SSL against my web server.
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
