ext/pre_shared_key: fix binder calculation when HRR is sent

In that case, ClientHello1 and HelloRetryRequest are included in the
PSK binder computation, not only the truncated ClientHello2.

Signed-off-by: Daiki Ueno <dueno@redhat.com>

3 files changed, 48 insertions, 14 deletions

diff --git a/lib/ext/pre_shared_key.c b/lib/ext/pre_shared_key.c
index 0d3468c40d..0fa7df2d27 100644
--- a/lib/ext/pre_shared_key.c
+++ b/lib/ext/pre_shared_key.c
@@ -58,20 +58,31 @@ compute_binder_key(const mac_entry_st *prf,
 }
 
 static int
-compute_psk_binder(unsigned entity,
+compute_psk_binder(gnutls_session_t session,
 		const mac_entry_st *prf, unsigned binders_length, unsigned hash_size,
 		int exts_length, int ext_offset,
 		const gnutls_datum_t *psk, const gnutls_datum_t *client_hello,
 		void *out)
 {
 	int ret;
-	unsigned extensions_len_pos;
+	unsigned client_hello_pos, extensions_len_pos;
 	gnutls_buffer_st handshake_buf;
 	uint8_t binder_key[MAX_HASH_SIZE];
 
 	_gnutls_buffer_init(&handshake_buf);
 
-	if (entity == GNUTLS_CLIENT) {
+	if (session->security_parameters.entity == GNUTLS_CLIENT) {
+		if (session->internals.hsk_flags & HSK_HRR_RECEIVED) {
+			ret = gnutls_buffer_append_data(&handshake_buf,
+							(const void *) session->internals.handshake_hash_buffer.data,
+							session->internals.handshake_hash_buffer.length);
+			if (ret < 0) {
+				gnutls_assert();
+				goto error;
+			}
+		}
+
+		client_hello_pos = handshake_buf.length;
 		ret = gnutls_buffer_append_data(&handshake_buf,
 				(const void *)  client_hello->data,
 				client_hello->size);
@@ -81,23 +92,40 @@ compute_psk_binder(unsigned entity,
 		}
 
 		/* This is a ClientHello message */
-		handshake_buf.data[0] = GNUTLS_HANDSHAKE_CLIENT_HELLO;
+		handshake_buf.data[client_hello_pos] = GNUTLS_HANDSHAKE_CLIENT_HELLO;
 
 		/*
 		 * At this point we have not yet added the binders to the ClientHello,
 		 * but we have to overwrite the size field, pretending as if binders
 		 * of the correct length were present.
 		 */
-		_gnutls_write_uint24(handshake_buf.length + binders_length - 2, &handshake_buf.data[1]);
-		_gnutls_write_uint16(handshake_buf.length + binders_length - ext_offset,
-				&handshake_buf.data[ext_offset]);
+		_gnutls_write_uint24(handshake_buf.length - client_hello_pos + binders_length - 2, &handshake_buf.data[client_hello_pos + 1]);
+		_gnutls_write_uint16(handshake_buf.length - client_hello_pos + binders_length - ext_offset,
+				&handshake_buf.data[client_hello_pos + ext_offset]);
 
-		extensions_len_pos = handshake_buf.length - exts_length - 2;
+		extensions_len_pos = handshake_buf.length - client_hello_pos - exts_length - 2;
 		_gnutls_write_uint16(exts_length + binders_length + 2,
-				&handshake_buf.data[extensions_len_pos]);
+				&handshake_buf.data[client_hello_pos + extensions_len_pos]);
 	} else {
-		if (unlikely(client_hello->size <= binders_length))
-			return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+		if (session->internals.hsk_flags & HSK_HRR_SENT) {
+			if (unlikely(session->internals.handshake_hash_buffer.length <= client_hello->size)) {
+				ret = gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+				goto error;
+			}
+
+			ret = gnutls_buffer_append_data(&handshake_buf,
+							(const void *) session->internals.handshake_hash_buffer.data,
+							session->internals.handshake_hash_buffer.length - client_hello->size);
+			if (ret < 0) {
+				gnutls_assert();
+				goto error;
+			}
+		}
+
+		if (unlikely(client_hello->size <= binders_length)) {
+			ret = gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+			goto error;
+		}
 
 		ret = gnutls_buffer_append_data(&handshake_buf,
 						(const void *) client_hello->data,
@@ -194,7 +222,7 @@ client_send_params(gnutls_session_t session,
 	client_hello.data = extdata->data+sizeof(mbuffer_st);
 	client_hello.size = extdata->length-sizeof(mbuffer_st);
 
-	ret = compute_psk_binder(GNUTLS_CLIENT, prf,
+	ret = compute_psk_binder(session, prf,
 				 hash_size+1, hash_size, extdata->length-pos,
 				 ext_offset, &key, &client_hello,
 				 binder_value);
@@ -314,7 +342,7 @@ static int server_recv_params(gnutls_session_t session,
 	/* Compute the binder value for this PSK */
 	prf = pskcred->binder_algo;
 	hash_size = prf->output_size;
-	ret = compute_psk_binder(GNUTLS_SERVER, prf, psk_parser.binder_len+2, hash_size, 0, 0,
+	ret = compute_psk_binder(session, prf, psk_parser.binder_len+2, hash_size, 0, 0,
 				 &key, &full_client_hello,
 				 binder_value);
 	if (ret < 0) {
diff --git a/lib/tls13/hello_retry.c b/lib/tls13/hello_retry.c
index 51f545ec00..5676c52780 100644
--- a/lib/tls13/hello_retry.c
+++ b/lib/tls13/hello_retry.c
@@ -89,6 +89,9 @@ int _gnutls13_send_hello_retry_request(gnutls_session_t session, unsigned again)
 		/* reset extensions sent by this session to allow re-sending them */
 		session->internals.used_exts = 0;
 
+		if (session->key.psk_needs_free)
+			_gnutls_free_temp_key_datum(&session->key.psk);
+
 		bufel = _gnutls_buffer_to_mbuffer(&buf);
 	}
 
diff --git a/tests/psk-file.c b/tests/psk-file.c
index a73031193f..e1e058ffe9 100644
--- a/tests/psk-file.c
+++ b/tests/psk-file.c
@@ -87,7 +87,7 @@ static void client(int sd, const char *prio, const char *user, const gnutls_datu
 
 	/* Initialize TLS session
 	 */
-	gnutls_init(&session, GNUTLS_CLIENT);
+	gnutls_init(&session, GNUTLS_CLIENT|GNUTLS_KEY_SHARE_TOP);
 
 	/* Use default priorities */
 	assert(gnutls_priority_set_direct(session, prio, NULL)>=0);
@@ -392,6 +392,9 @@ void doit(void)
 	run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+DHE-PSK", NULL, "non-hex", &key, 0, 0, GNUTLS_E_FATAL_ALERT_RECEIVED, GNUTLS_E_KEYFILE_ERROR);
 	run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+DHE-PSK", NULL, "unknown", &key, 0, 0, GNUTLS_E_FATAL_ALERT_RECEIVED, GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 	run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:+DHE-PSK", NULL, "jas", &wrong_key, 0, 0, GNUTLS_E_FATAL_ALERT_RECEIVED, GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
+
+	/* try with HelloRetryRequest and PSK */
+	run_test2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+DHE-PSK:-GROUP-ALL:+GROUP-FFDHE2048:+GROUP-FFDHE4096", "NORMAL:-VERS-ALL:+VERS-TLS1.3:+DHE-PSK:-GROUP-ALL:+GROUP-FFDHE4096", "jas", &key, 0, GNUTLS_KX_DHE_PSK, 0, 0);
 }
 
 #endif				/* _WIN32 */