Merge branch 'wip/dueno/tls13-compat' into 'master'

priority: add option to disable TLS 1.3 middlebox compatibility mode

Closes #1208

See merge request gnutls/gnutls!1410

10 files changed, 179 insertions, 11 deletions

diff --git a/.gitignore b/.gitignore
index ee1d177288..4bf0da343c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -843,6 +843,7 @@ tests/tls12-rollback-detection
 tests/tls12-server-kx-neg
 tests/tls13-cert-key-exchange
 tests/tls13-cipher-neg
+tests/tls13-compat-mode
 tests/tls13-early-data
 tests/tls13-early-data-neg
 tests/tls13-early-start
diff --git a/NEWS b/NEWS
index ba6827358d..b3590a316a 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,11 @@ Copyright (C) 2000-2016 Free Software Foundation, Inc.
 Copyright (C) 2013-2019 Nikos Mavrogiannopoulos
 See the end for copying conditions.
 
+* Version 3.7.2 (unreleased)
+
+** libgnutls: The priority string option %DISABLE_TLS13_COMPAT_MODE was added
+   to disable TLS 1.3 middlebox compatibility mode
+
 * Version 3.7.1 (released 2021-03-10)
 
 ** libgnutls: Fixed potential use-after-free in sending "key_share"
diff --git a/doc/cha-gtls-app.texi b/doc/cha-gtls-app.texi
index 36ba55e3ab..2399bf82eb 100644
--- a/doc/cha-gtls-app.texi
+++ b/doc/cha-gtls-app.texi
@@ -1610,6 +1610,10 @@ client hello.  Note that this should be set only by applications that
 try to reconnect with a downgraded protocol version. See RFC7507 for
 details.
 
+@item %DISABLE_TLS13_COMPAT_MODE @tab
+will disable TLS 1.3 middlebox compatibility mode (RFC8446, Appendix
+D.4) for non-compliant middleboxes.
+
 @item %VERIFY_ALLOW_BROKEN @tab
 will allow signatures with known to be broken algorithms (such as MD5 or
 SHA1) in certificate chains.
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index 2611b5af54..62a061e67a 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -950,6 +950,7 @@ struct gnutls_priority_st {
 	bool have_psk;
 	bool force_etm;
 	unsigned int additional_verify_flags;
+	bool tls13_compat_mode;
 
 	/* TLS_FALLBACK_SCSV */
 	bool fallback;
diff --git a/lib/handshake-tls13.c b/lib/handshake-tls13.c
index 7dd42becf1..9687707a32 100644
--- a/lib/handshake-tls13.c
+++ b/lib/handshake-tls13.c
@@ -84,11 +84,13 @@ int _gnutls13_handshake_client(gnutls_session_t session)
 	case STATE99:
 	case STATE100:
 #ifdef TLS13_APPENDIX_D4
-		/* We send it before keys are generated. That works because CCS
-		 * is always being cached and queued and not being sent directly */
-		ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE100));
-		STATE = STATE100;
-		IMED_RET("send change cipher spec", ret, 0);
+		if (session->internals.priorities->tls13_compat_mode) {
+			/* We send it before keys are generated. That works because CCS
+			 * is always being cached and queued and not being sent directly */
+			ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE100));
+			STATE = STATE100;
+			IMED_RET("send change cipher spec", ret, 0);
+		}
 #endif
 		FALLTHROUGH;
 	case STATE101:
@@ -385,9 +387,11 @@ int _gnutls13_handshake_server(gnutls_session_t session)
 		FALLTHROUGH;
 	case STATE92:
 #ifdef TLS13_APPENDIX_D4
-		ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE92));
-		STATE = STATE92;
-		IMED_RET("send change cipher spec", ret, 0);
+		if (session->internals.priorities->tls13_compat_mode) {
+			ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE92));
+			STATE = STATE92;
+			IMED_RET("send change cipher spec", ret, 0);
+		}
 #endif
 		FALLTHROUGH;
 	case STATE93:
@@ -416,7 +420,8 @@ int _gnutls13_handshake_server(gnutls_session_t session)
 #ifdef TLS13_APPENDIX_D4
 		/* don't send CCS twice: when HRR has already been
 		 * sent, CCS should have followed it (see above) */
-		if (!(session->internals.hsk_flags & HSK_HRR_SENT)) {
+		if (session->internals.priorities->tls13_compat_mode &&
+		    !(session->internals.hsk_flags & HSK_HRR_SENT)) {
 			ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE100));
 			STATE = STATE100;
 			IMED_RET("send change cipher spec", ret, 0);
diff --git a/lib/handshake.c b/lib/handshake.c
index 6c894eb68a..33bc7f7be6 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -2213,7 +2213,9 @@ static int send_client_hello(gnutls_session_t session, int again)
 		}
 
 #ifdef TLS13_APPENDIX_D4
-		if (max_ver->tls13_sem && !resuming) {
+		if (max_ver->tls13_sem &&
+		    session->internals.priorities->tls13_compat_mode &&
+		    !resuming) {
 			/* Under TLS1.3 we generate a random session ID to make
 			 * the TLS1.3 session look like a resumed TLS1.2 session */
 			ret = _gnutls_generate_session_id(session->security_parameters.
diff --git a/lib/priority.c b/lib/priority.c
index 7686c7530a..8cd8a1b260 100644
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -989,6 +989,10 @@ static void enable_server_precedence(gnutls_priority_t c)
 {
 	c->server_precedence = 1;
 }
+static void disable_tls13_compat_mode(gnutls_priority_t c)
+{
+	c->tls13_compat_mode = false;
+}
 static void dummy_func(gnutls_priority_t c)
 {
 }
@@ -2005,6 +2009,11 @@ gnutls_priority_init(gnutls_priority_t * priority_cache,
 	 * when we make it the default.
 	 */
 	(*priority_cache)->sr = SR_PARTIAL;
+	/* For now TLS 1.3 middlebox compatibility mode is enabled by default.
+	 * This will eventually be disabled by default and moved to the %COMPAT
+	 * setting.
+	 */
+	(*priority_cache)->tls13_compat_mode = true;
 	(*priority_cache)->min_record_version = 1;
 	gnutls_atomic_init(&(*priority_cache)->usage_cnt);
 
diff --git a/lib/priority_options.gperf b/lib/priority_options.gperf
index a81001dda1..6fb9ae603c 100644
--- a/lib/priority_options.gperf
+++ b/lib/priority_options.gperf
@@ -39,3 +39,4 @@ PROFILE_SUITEB192, enable_profile_suiteb192
 NEW_PADDING, dummy_func
 DEBUG_ALLOW_KEY_USAGE_VIOLATIONS, enable_server_key_usage_violations
 ALLOW_SMALL_RECORDS, enable_allow_small_records
+DISABLE_TLS13_COMPAT_MODE, disable_tls13_compat_mode
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c6d994f877..ed32dadcc5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -223,7 +223,7 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei
 	 sign-verify-newapi sign-verify-deterministic iov aead-cipher-vec \
 	 tls13-without-timeout-func buffer status-request-revoked \
 	 set_x509_ocsp_multi_cli kdf-api keylog-func handshake-write \
-	 x509cert-dntypes id-on-xmppAddr
+	 x509cert-dntypes id-on-xmppAddr tls13-compat-mode
 
 if HAVE_SECCOMP_TESTS
 ctests += dtls-with-seccomp tls-with-seccomp dtls-client-with-seccomp tls-client-with-seccomp
diff --git a/tests/tls13-compat-mode.c b/tests/tls13-compat-mode.c
new file mode 100644
index 0000000000..e8f99802df
--- /dev/null
+++ b/tests/tls13-compat-mode.c
@@ -0,0 +1,140 @@
+/*
+ * Copyright (C) 2021 Free Software Foundation, Inc.
+ *
+ * Author: 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 General Public License
+ * along with GnuTLS.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <gnutls/gnutls.h>
+#include <gnutls/x509.h>
+#include <assert.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <string.h>
+#include "cert-common.h"
+#include "eagain-common.h"
+#include "utils.h"
+
+/* This tests TLS 1.3 middlebox compatibility mode. */
+
+#define COMPAT_PRIO "NORMAL:-VERS-ALL:+VERS-TLS1.3"
+#define NO_COMPAT_PRIO COMPAT_PRIO ":%DISABLE_TLS13_COMPAT_MODE"
+
+#define HANDSHAKE_SESSION_ID_POS 34
+
+struct data {
+	bool compat;
+};
+
+static int
+handshake_callback(gnutls_session_t session, unsigned int htype,
+		   unsigned post, unsigned int incoming,
+		   const gnutls_datum_t *msg)
+{
+	unsigned pos;
+	struct data *data;
+	uint8_t s;
+
+	assert(htype == GNUTLS_HANDSHAKE_CLIENT_HELLO);
+	assert(msg->size >= HANDSHAKE_SESSION_ID_POS);
+
+	data = gnutls_session_get_ptr(session);
+
+	pos = HANDSHAKE_SESSION_ID_POS;
+	if (pos + 1 > msg->size)
+		fail("error\n");
+	s = msg->data[pos];
+
+	if (data->compat && s == 0) {
+		fail("empty session ID while compat mode is enabled\n");
+	} else if (!data->compat && s > 0) {
+		fail("non-empty session ID while compat mode is disabled\n");
+	}
+
+	return 0;
+}
+
+static void
+test(const char *name, bool client_compat, bool server_compat)
+{
+	/* Server stuff. */
+	gnutls_certificate_credentials_t serverx509cred;
+	gnutls_session_t server;
+	int sret = GNUTLS_E_AGAIN;
+	/* Client stuff. */
+	gnutls_certificate_credentials_t clientx509cred;
+	gnutls_session_t client;
+	struct data data;
+	int cret = GNUTLS_E_AGAIN;
+
+	success("%s\n", name);
+
+	/* Init server */
+	assert(gnutls_certificate_allocate_credentials(&serverx509cred) >= 0);
+	gnutls_certificate_set_x509_key_mem(serverx509cred,
+					    &server_cert, &server_key,
+					    GNUTLS_X509_FMT_PEM);
+
+	assert(gnutls_init(&server, GNUTLS_SERVER) >= 0);
+	assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, serverx509cred) >= 0);
+	assert(gnutls_priority_set_direct(server,
+					  server_compat ?
+					  COMPAT_PRIO : NO_COMPAT_PRIO,
+					  NULL) >= 0);
+	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_certificate_allocate_credentials(&clientx509cred) >= 0);
+	assert(gnutls_init(&client, GNUTLS_CLIENT) >= 0);
+	assert(gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE,
+				      clientx509cred) >= 0);
+
+	assert(gnutls_priority_set_direct(client,
+					  client_compat ?
+					  COMPAT_PRIO : NO_COMPAT_PRIO,
+					  NULL) >= 0);
+	gnutls_transport_set_push_function(client, client_push);
+	gnutls_transport_set_pull_function(client, client_pull);
+	gnutls_transport_set_ptr(client, client);
+	data.compat = client_compat;
+	gnutls_session_set_ptr(client, &data);
+	gnutls_handshake_set_hook_function(client, GNUTLS_HANDSHAKE_CLIENT_HELLO,
+					   GNUTLS_HOOK_POST,
+					   handshake_callback);
+
+	HANDSHAKE(client, server);
+
+	gnutls_deinit(client);
+	gnutls_deinit(server);
+
+	gnutls_certificate_free_credentials(serverx509cred);
+	gnutls_certificate_free_credentials(clientx509cred);
+}
+
+void doit(void)
+{
+	test("client compat, server compat", true, true);
+	test("client compat, server non-compat", true, false);
+	test("client non-compat, server compat", false, true);
+	test("client non-compat, server non-compat", false, false);
+}