From 23135619773e6ec087ff2abc65405bd4d5676bad Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Mon, 7 Jul 2025 11:15:45 +0900
Subject: [PATCH] handshake: clear HSK_PSK_SELECTED is when resetting binders
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a TLS 1.3 handshake involves HRR and resumption or PSK, and the
second Client Hello omits PSK, the server would result in a NULL
pointer dereference as the PSK binder information is cleared while the
HSK_PSK_SELECTED flag is still set. This makes sure that
HSK_PSK_SELECTED flag is always cleared when the PSK binders are
reset. This also makes it clear the HSK_PSK_SELECTED flag is valid
only during a handshake; after that, whether PSK is used can be
checked with gnutls_auth_client_get_type.

Reported by Stefan Bühler.

Signed-off-by: Daiki Ueno <ueno@gnu.org>
---
 .gitignore                            |   1 +
 NEWS                                  |   4 +
 lib/handshake.c                       |  25 +++-
 lib/state.c                           |   4 +-
 tests/Makefile.am                     |   2 +
 tests/tls13/hello_retry_request_psk.c | 173 ++++++++++++++++++++++++++
 6 files changed, 205 insertions(+), 4 deletions(-)
 create mode 100644 tests/tls13/hello_retry_request_psk.c

Index: gnutls-3.8.9/lib/handshake.c
===================================================================
--- gnutls-3.8.9.orig/lib/handshake.c
+++ gnutls-3.8.9/lib/handshake.c
@@ -589,9 +589,28 @@ static int set_auth_types(gnutls_session
 		/* Under TLS1.3 this returns a KX which matches the negotiated
 		 * groups from the key shares; if we are resuming then the KX seen
 		 * here doesn't match the original session. */
-		if (!session->internals.resumed)
-			kx = gnutls_kx_get(session);
-		else
+		if (!session->internals.resumed) {
+			const gnutls_group_entry_st *group = get_group(session);
+
+			if (session->internals.hsk_flags & HSK_PSK_SELECTED) {
+				if (group) {
+					kx = group->pk == GNUTLS_PK_DH ?
+						     GNUTLS_KX_DHE_PSK :
+						     GNUTLS_KX_ECDHE_PSK;
+				} else {
+					kx = GNUTLS_KX_PSK;
+				}
+			} else if (group) {
+				/* Not necessarily be RSA, but just to
+				 * make _gnutls_map_kx_get_cred below
+				 * work.
+				 */
+				kx = group->pk == GNUTLS_PK_DH ?
+					     GNUTLS_KX_DHE_RSA :
+					     GNUTLS_KX_ECDHE_RSA;
+			} else
+				kx = GNUTLS_KX_UNKNOWN;
+		} else
 			kx = GNUTLS_KX_UNKNOWN;
 	} else {
 		/* TLS1.2 or earlier, kx is associated with ciphersuite */
Index: gnutls-3.8.9/lib/state.c
===================================================================
--- gnutls-3.8.9.orig/lib/state.c
+++ gnutls-3.8.9/lib/state.c
@@ -202,7 +202,8 @@ gnutls_kx_algorithm_t gnutls_kx_get(gnut
 		const gnutls_group_entry_st *group = get_group(session);
 
 		if (ver->tls13_sem) {
-			if (session->internals.hsk_flags & HSK_PSK_SELECTED) {
+			if (gnutls_auth_client_get_type(session) ==
+			    GNUTLS_CRD_PSK) {
 				if (group) {
 					if (group->pk == GNUTLS_PK_DH)
 						return GNUTLS_KX_DHE_PSK;
@@ -349,6 +350,7 @@ void reset_binders(gnutls_session_t sess
 	_gnutls_free_temp_key_datum(&session->key.binders[0].psk);
 	_gnutls_free_temp_key_datum(&session->key.binders[1].psk);
 	memset(session->key.binders, 0, sizeof(session->key.binders));
+	session->internals.hsk_flags &= ~HSK_PSK_SELECTED;
 }
 
 /* Check whether certificate credentials of type @cert_type are set
Index: gnutls-3.8.9/tests/Makefile.am
===================================================================
--- gnutls-3.8.9.orig/tests/Makefile.am
+++ gnutls-3.8.9/tests/Makefile.am
@@ -131,6 +131,8 @@ ctests += tls13/hello_retry_request
 
 ctests += tls13/hello_retry_request_resume
 
+ctests += tls13/hello_retry_request_psk
+
 ctests += tls13/psk-ext
 
 ctests += tls13/key_update
Index: gnutls-3.8.9/tests/tls13/hello_retry_request_psk.c
===================================================================
--- /dev/null
+++ gnutls-3.8.9/tests/tls13/hello_retry_request_psk.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright (C) 2017-2025 Red Hat, Inc.
+ *
+ * Author: Nikos Mavrogiannopoulos, Daiki Ueno
+ *
+ * This file is part of GnuTLS.
+ *
+ * GnuTLS is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GnuTLS is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+
+#include <string.h>
+#include <gnutls/gnutls.h>
+#include <assert.h>
+
+#include "cert-common.h"
+#include "utils.h"
+#include "tls13/ext-parse.h"
+#include "eagain-common.h"
+
+/* This program exercises the case where a TLS 1.3 handshake ends up
+ * with HRR, and the first CH includes PSK while the 2nd CH omits
+ * it */
+
+const char *testname = "hello entry request";
+
+const char *side = "";
+
+#define myfail(fmt, ...) fail("%s: " fmt, testname, ##__VA_ARGS__)
+
+static void tls_log_func(int level, const char *str)
+{
+	fprintf(stderr, "%s|<%d>| %s", side, level, str);
+}
+
+struct ctx_st {
+	unsigned hrr_seen;
+	unsigned hello_counter;
+};
+
+static int pskfunc(gnutls_session_t session, const char *username,
+		   gnutls_datum_t *key)
+{
+	if (debug)
+		printf("psk: username %s\n", username);
+	key->data = gnutls_malloc(4);
+	key->data[0] = 0xDE;
+	key->data[1] = 0xAD;
+	key->data[2] = 0xBE;
+	key->data[3] = 0xEF;
+	key->size = 4;
+	return 0;
+}
+
+static int hello_callback(gnutls_session_t session, unsigned int htype,
+			  unsigned post, unsigned int incoming,
+			  const gnutls_datum_t *msg)
+{
+	struct ctx_st *ctx = gnutls_session_get_ptr(session);
+	assert(ctx != NULL);
+
+	if (htype == GNUTLS_HANDSHAKE_HELLO_RETRY_REQUEST)
+		ctx->hrr_seen = 1;
+
+	if (htype == GNUTLS_HANDSHAKE_CLIENT_HELLO) {
+		if (post == GNUTLS_HOOK_POST)
+			ctx->hello_counter++;
+		else {
+			/* Unset the PSK credential to omit the extension */
+			gnutls_credentials_set(session, GNUTLS_CRD_PSK, NULL);
+		}
+	}
+
+	return 0;
+}
+
+void doit(void)
+{
+	int sret, cret;
+	gnutls_psk_server_credentials_t scred;
+	gnutls_psk_client_credentials_t ccred;
+	gnutls_certificate_credentials_t ccred2;
+	gnutls_session_t server, client;
+	/* Need to enable anonymous KX specifically. */
+	const gnutls_datum_t key = { (void *)"DEADBEEF", 8 };
+
+	struct ctx_st ctx;
+	memset(&ctx, 0, sizeof(ctx));
+
+	global_init();
+
+	gnutls_global_set_log_function(tls_log_func);
+	if (debug)
+		gnutls_global_set_log_level(9);
+
+	/* Init server */
+	assert(gnutls_psk_allocate_server_credentials(&scred) >= 0);
+	gnutls_psk_set_server_credentials_function(scred, pskfunc);
+
+	gnutls_init(&server, GNUTLS_SERVER);
+
+	assert(gnutls_priority_set_direct(
+		       server,
+		       "NORMAL:-VERS-ALL:+VERS-TLS1.3:-GROUP-ALL:+GROUP-X25519:+DHE-PSK",
+		       NULL) >= 0);
+
+	gnutls_credentials_set(server, GNUTLS_CRD_PSK, scred);
+	gnutls_transport_set_push_function(server, server_push);
+	gnutls_transport_set_pull_function(server, server_pull);
+	gnutls_transport_set_ptr(server, server);
+
+	/* Init client */
+	assert(gnutls_psk_allocate_client_credentials(&ccred) >= 0);
+	gnutls_psk_set_client_credentials(ccred, "test", &key,
+					  GNUTLS_PSK_KEY_HEX);
+	assert(gnutls_certificate_allocate_credentials(&ccred2) >= 0);
+
+	assert(gnutls_init(&client, GNUTLS_CLIENT | GNUTLS_KEY_SHARE_TOP) >= 0);
+
+	gnutls_session_set_ptr(client, &ctx);
+
+	cret = gnutls_priority_set_direct(
+		client,
+		"NORMAL:-VERS-ALL:+VERS-TLS1.3:-GROUP-ALL:+GROUP-SECP256R1:+GROUP-X25519:+DHE-PSK",
+		NULL);
+	if (cret < 0)
+		myfail("cannot set TLS 1.3 priorities\n");
+
+	gnutls_credentials_set(client, GNUTLS_CRD_PSK, ccred);
+	gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, ccred2);
+	gnutls_transport_set_push_function(client, client_push);
+	gnutls_transport_set_pull_function(client, client_pull);
+	gnutls_transport_set_ptr(client, client);
+
+	gnutls_handshake_set_hook_function(client, GNUTLS_HANDSHAKE_ANY,
+					   GNUTLS_HOOK_BOTH, hello_callback);
+
+	HANDSHAKE_EXPECT(client, server, GNUTLS_E_AGAIN,
+			 GNUTLS_E_INSUFFICIENT_CREDENTIALS);
+
+	assert(ctx.hrr_seen != 0);
+
+	gnutls_bye(client, GNUTLS_SHUT_WR);
+	gnutls_bye(server, GNUTLS_SHUT_WR);
+
+	gnutls_deinit(client);
+	gnutls_deinit(server);
+
+	gnutls_psk_free_server_credentials(scred);
+	gnutls_psk_free_client_credentials(ccred);
+	gnutls_certificate_free_credentials(ccred2);
+
+	gnutls_global_deinit();
+	reset_buffers();
+}
