214 lines
6.9 KiB
Diff
214 lines
6.9 KiB
Diff
From dc5ee80c3a28577e9de0f82fb08164e4c02b96af Mon Sep 17 00:00:00 2001
|
|
From: Daiki Ueno <ueno@gnu.org>
|
|
Date: Sun, 9 Feb 2025 10:31:20 +0900
|
|
Subject: [PATCH] handshake: only shuffle extensions in the first Client Hello
|
|
|
|
RFC 8446 section 4.1.2 states that the second Client Hello after HRR
|
|
should preserve the same content as the first Client Hello with
|
|
limited exceptions. Since GnuTLS 3.8.5, however, the library started
|
|
shuffling the order of extensions for privacy reasons and that didn't
|
|
comply with the RFC, leading to a connectivity issue against the
|
|
server configuration with a stricter check on that.
|
|
|
|
Signed-off-by: Daiki Ueno <ueno@gnu.org>
|
|
---
|
|
lib/gnutls_int.h | 4 +++
|
|
lib/hello_ext.c | 41 ++++++++++++++++---------
|
|
lib/state.c | 2 ++
|
|
tests/tls13/hello_retry_request.c | 51 ++++++++++++++++++++++++++++---
|
|
4 files changed, 79 insertions(+), 19 deletions(-)
|
|
|
|
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
|
|
index d10a028b59..572de5aba3 100644
|
|
--- a/lib/gnutls_int.h
|
|
+++ b/lib/gnutls_int.h
|
|
@@ -1666,6 +1666,10 @@ typedef struct {
|
|
/* Compression method for certificate compression */
|
|
gnutls_compression_method_t compress_certificate_method;
|
|
|
|
+ /* To shuffle extension sending order */
|
|
+ extensions_t client_hello_exts[MAX_EXT_TYPES];
|
|
+ bool client_hello_exts_set;
|
|
+
|
|
/* If you add anything here, check _gnutls_handshake_internal_state_clear().
|
|
*/
|
|
} internals_st;
|
|
diff --git a/lib/hello_ext.c b/lib/hello_ext.c
|
|
index 40af8c2b10..d677addd75 100644
|
|
--- a/lib/hello_ext.c
|
|
+++ b/lib/hello_ext.c
|
|
@@ -438,8 +438,6 @@ int _gnutls_gen_hello_extensions(gnutls_session_t session,
|
|
int pos, ret;
|
|
size_t i;
|
|
hello_ext_ctx_st ctx;
|
|
- /* To shuffle extension sending order */
|
|
- extensions_t indices[MAX_EXT_TYPES];
|
|
|
|
msg &= GNUTLS_EXT_FLAG_SET_ONLY_FLAGS_MASK;
|
|
|
|
@@ -469,26 +467,39 @@ int _gnutls_gen_hello_extensions(gnutls_session_t session,
|
|
ret - 4);
|
|
}
|
|
|
|
- /* Initializing extensions array */
|
|
- for (i = 0; i < MAX_EXT_TYPES; i++) {
|
|
- indices[i] = i;
|
|
- }
|
|
+ if (msg & GNUTLS_EXT_FLAG_CLIENT_HELLO &&
|
|
+ !session->internals.client_hello_exts_set) {
|
|
+ /* Initializing extensions array */
|
|
+ for (i = 0; i < MAX_EXT_TYPES; i++) {
|
|
+ session->internals.client_hello_exts[i] = i;
|
|
+ }
|
|
|
|
- if (!session->internals.priorities->no_shuffle_extensions) {
|
|
- /* Ordering padding and pre_shared_key as last extensions */
|
|
- swap_exts(indices, MAX_EXT_TYPES - 2, GNUTLS_EXTENSION_DUMBFW);
|
|
- swap_exts(indices, MAX_EXT_TYPES - 1,
|
|
- GNUTLS_EXTENSION_PRE_SHARED_KEY);
|
|
+ if (!session->internals.priorities->no_shuffle_extensions) {
|
|
+ /* Ordering padding and pre_shared_key as last extensions */
|
|
+ swap_exts(session->internals.client_hello_exts,
|
|
+ MAX_EXT_TYPES - 2, GNUTLS_EXTENSION_DUMBFW);
|
|
+ swap_exts(session->internals.client_hello_exts,
|
|
+ MAX_EXT_TYPES - 1,
|
|
+ GNUTLS_EXTENSION_PRE_SHARED_KEY);
|
|
|
|
- ret = shuffle_exts(indices, MAX_EXT_TYPES - 2);
|
|
- if (ret < 0)
|
|
- return gnutls_assert_val(ret);
|
|
+ ret = shuffle_exts(session->internals.client_hello_exts,
|
|
+ MAX_EXT_TYPES - 2);
|
|
+ if (ret < 0)
|
|
+ return gnutls_assert_val(ret);
|
|
+ }
|
|
+ session->internals.client_hello_exts_set = true;
|
|
}
|
|
|
|
/* hello_ext_send() ensures we don't send duplicates, in case
|
|
* of overridden extensions */
|
|
for (i = 0; i < MAX_EXT_TYPES; i++) {
|
|
- size_t ii = indices[i];
|
|
+ size_t ii;
|
|
+
|
|
+ if (msg & GNUTLS_EXT_FLAG_CLIENT_HELLO)
|
|
+ ii = session->internals.client_hello_exts[i];
|
|
+ else
|
|
+ ii = i;
|
|
+
|
|
if (!extfunc[ii])
|
|
continue;
|
|
|
|
diff --git a/lib/state.c b/lib/state.c
|
|
index 9d3ece7570..43e961cfa2 100644
|
|
--- a/lib/state.c
|
|
+++ b/lib/state.c
|
|
@@ -516,6 +516,8 @@ static void handshake_internal_state_clear1(gnutls_session_t session)
|
|
|
|
session->internals.hrr_cs[0] = CS_INVALID_MAJOR;
|
|
session->internals.hrr_cs[1] = CS_INVALID_MINOR;
|
|
+
|
|
+ session->internals.client_hello_exts_set = false;
|
|
}
|
|
|
|
/* This function will clear all the variables in internals
|
|
diff --git a/tests/tls13/hello_retry_request.c b/tests/tls13/hello_retry_request.c
|
|
index f407b64234..6c5f698f01 100644
|
|
--- a/tests/tls13/hello_retry_request.c
|
|
+++ b/tests/tls13/hello_retry_request.c
|
|
@@ -51,14 +51,37 @@ static void tls_log_func(int level, const char *str)
|
|
}
|
|
|
|
#define HANDSHAKE_SESSION_ID_POS 34
|
|
+#define MAX_EXT_TYPES 64
|
|
|
|
struct ctx_st {
|
|
unsigned hrr_seen;
|
|
unsigned hello_counter;
|
|
uint8_t session_id[32];
|
|
size_t session_id_len;
|
|
+ unsigned extensions[MAX_EXT_TYPES];
|
|
+ size_t extensions_size1;
|
|
+ size_t extensions_size2;
|
|
};
|
|
|
|
+static int ext_callback(void *_ctx, unsigned tls_id, const unsigned char *data,
|
|
+ unsigned size)
|
|
+{
|
|
+ struct ctx_st *ctx = _ctx;
|
|
+ if (ctx->hello_counter == 0) {
|
|
+ assert(ctx->extensions_size1 < MAX_EXT_TYPES);
|
|
+ ctx->extensions[ctx->extensions_size1++] = tls_id;
|
|
+ } else {
|
|
+ assert(ctx->extensions_size2 < MAX_EXT_TYPES);
|
|
+ if (tls_id != ctx->extensions[ctx->extensions_size2]) {
|
|
+ fail("extension doesn't match at position %zu, %u != %u\n",
|
|
+ ctx->extensions_size2, tls_id,
|
|
+ ctx->extensions[ctx->extensions_size2]);
|
|
+ }
|
|
+ ctx->extensions_size2++;
|
|
+ }
|
|
+ return 0;
|
|
+}
|
|
+
|
|
static int hello_callback(gnutls_session_t session, unsigned int htype,
|
|
unsigned post, unsigned int incoming,
|
|
const gnutls_datum_t *msg)
|
|
@@ -73,15 +96,25 @@ static int hello_callback(gnutls_session_t session, unsigned int htype,
|
|
post == GNUTLS_HOOK_POST) {
|
|
size_t session_id_len;
|
|
uint8_t *session_id;
|
|
+ unsigned pos = HANDSHAKE_SESSION_ID_POS;
|
|
+ gnutls_datum_t mmsg;
|
|
+ int ret;
|
|
|
|
- assert(msg->size > HANDSHAKE_SESSION_ID_POS + 1);
|
|
- session_id_len = msg->data[HANDSHAKE_SESSION_ID_POS];
|
|
- session_id = &msg->data[HANDSHAKE_SESSION_ID_POS + 1];
|
|
+ assert(msg->size > pos + 1);
|
|
+ session_id_len = msg->data[pos];
|
|
+ session_id = &msg->data[pos + 1];
|
|
+
|
|
+ SKIP8(pos, msg->size);
|
|
+ SKIP16(pos, msg->size);
|
|
+ SKIP8(pos, msg->size);
|
|
+
|
|
+ mmsg.data = &msg->data[pos];
|
|
+ mmsg.size = msg->size - pos;
|
|
|
|
if (ctx->hello_counter > 0) {
|
|
assert(msg->size > 4);
|
|
if (msg->data[0] != 0x03 || msg->data[1] != 0x03) {
|
|
- fail("version is %d.%d expected 3,3\n",
|
|
+ fail("version is %d.%d expected 3.3\n",
|
|
(int)msg->data[0], (int)msg->data[1]);
|
|
}
|
|
|
|
@@ -95,6 +128,12 @@ static int hello_callback(gnutls_session_t session, unsigned int htype,
|
|
ctx->session_id_len = session_id_len;
|
|
memcpy(ctx->session_id, session_id, session_id_len);
|
|
|
|
+ ret = gnutls_ext_raw_parse(ctx, ext_callback, &mmsg, 0);
|
|
+ if (ret < 0) {
|
|
+ fail("unable to parse extensions: %s\n",
|
|
+ gnutls_strerror(ret));
|
|
+ }
|
|
+
|
|
ctx->hello_counter++;
|
|
}
|
|
|
|
@@ -164,6 +203,10 @@ void doit(void)
|
|
myfail("group doesn't match the expected: %s\n",
|
|
gnutls_group_get_name(gnutls_group_get(server)));
|
|
|
|
+ if (ctx.extensions_size1 != ctx.extensions_size2)
|
|
+ myfail("the number of extensions don't match in second Client Hello: %zu != %zu\n",
|
|
+ ctx.extensions_size1, ctx.extensions_size2);
|
|
+
|
|
gnutls_bye(client, GNUTLS_SHUT_WR);
|
|
gnutls_bye(server, GNUTLS_SHUT_WR);
|
|
|
|
--
|
|
2.48.1
|
|
|