rpms/gnutls
5
0
Fork 0
Code Issues Pull Requests Releases Activity

handshake: only shuffle extensions in the first Client Hello

Browse Source 
Related: RHEL-58246
Signed-off-by: Daiki Ueno <dueno@redhat.com>
This commit is contained in:
Daiki Ueno 2025-02-12 17:31:50 +09:00
parent 050ed4a962
commit dd046ab8d8
2 changed files with 215 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

213
gnutls-3.8.9-limit-shuffle-extensions.patch Normal file
View File

@ -0,0 +1,213 @@
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

2
gnutls.spec
View File

@ -30,6 +30,8 @@ Patch: gnutls-3.7.6-fips-sha1-sigver.patch
Patch: gnutls-3.8.8-tests-ktls-skip-tls12-chachapoly.patch
# not upstreamed: https://gitlab.com/gnutls/gnutls/-/merge_requests/1932
Patch: gnutls-3.8.9-allow-rsa-pkcs1-encrypt.patch
# upstreamed: https://gitlab.com/gnutls/gnutls/-/merge_requests/1930
Patch: gnutls-3.8.9-limit-shuffle-extensions.patch
%bcond_without bootstrap
%bcond_without dane