Merge branch 'tmp-downgrade-sentinel' into 'master'

handshake: defer setting downgrade sentinel until version is selected

Closes #689

See merge request gnutls/gnutls!918

7 files changed, 91 insertions, 6 deletions

diff --git a/NEWS b/NEWS
index b171ef71e8..83d9b321a7 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,11 @@ See the end for copying conditions.
    a certificate. We were already enforcing the signature algorithm, but there
    was a bug in parameter checking code.
 
+** libgnutls: no longer send downgrade sentinel in TLS 1.3.
+   Previously the sentinel value was embedded to early in version
+   negotiation and was sent even on TLS 1.3. It is now sent only when
+   TLS 1.2 or earlier is negotiated (#689).
+
 ** API and ABI modifications:
 No changes since last version.
 
diff --git a/lib/ext/supported_versions.c b/lib/ext/supported_versions.c
index b7fe31f75b..b016c61c3c 100644
--- a/lib/ext/supported_versions.c
+++ b/lib/ext/supported_versions.c
@@ -63,7 +63,10 @@ supported_versions_recv_params(gnutls_session_t session,
 	int ret;
 
 	if (session->security_parameters.entity == GNUTLS_SERVER) {
+		const version_entry_st *old_vers;
+
 		vers = _gnutls_version_max(session);
+		old_vers = get_version(session);
 
 		/* do not parse this extension when we haven't TLS1.3
 		 * enabled. That is because we cannot handle earlier protocol
@@ -97,6 +100,18 @@ supported_versions_recv_params(gnutls_session_t session,
 
 				_gnutls_handshake_log("EXT[%p]: Negotiated version: %d.%d\n",
 						      session, (int)major, (int)minor);
+
+				vers = get_version(session);
+				if (old_vers != vers) {
+					/* regenerate the random value to set
+					 * downgrade sentinel if necessary
+					 */
+					ret = _gnutls_gen_server_random(session,
+									vers->id);
+					if (ret < 0)
+						return gnutls_assert_val(ret);
+				}
+
 				return 0;
 			}
 		}
diff --git a/tests/suite/tls-fuzzer/gnutls-nocert-ssl3.json b/tests/suite/tls-fuzzer/gnutls-nocert-ssl3.json
index 9bf3fa20f1..a297392255 100644
--- a/tests/suite/tls-fuzzer/gnutls-nocert-ssl3.json
+++ b/tests/suite/tls-fuzzer/gnutls-nocert-ssl3.json
@@ -18,7 +18,7 @@
          },
          {"name" : "test-export-ciphers-rejected.py",
           "comment" : "we negotiate AES even in SSL3.0",
-          "arguments" : ["--ssl3", "-p", "@PORT@"] },
+          "arguments" : ["-p", "@PORT@"] },
          {"name" : "test-client-compatibility.py",
           "arguments" : ["-p", "@PORT@", "18: IE 6 on XP",
                          "52: YandexBot 3.0 on unknown",
diff --git a/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json b/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json
index c764130306..47fcf878a4 100644
--- a/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json
+++ b/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json
@@ -33,7 +33,12 @@
                          "-e", "drop extension in TLS 1.3 session resumption",
                          "-e", "modified extension in 2nd CH in HRR handshake",
                          "-e", "renegotiation with changed limit",
-                         "-e", "renegotiation with dropped extension"] },
+                         "-e", "renegotiation with dropped extension",
+                         "-e", "added extension in 2nd CH in HRR handshake",
+                         "-e", "check server sent size in TLS 1.0 with max_fragment_length",
+                         "-e", "check server sent size in TLS 1.1 with max_fragment_length",
+                         "-e", "check server sent size in TLS 1.2 with max_fragment_length",
+                         "-e", "removed extension in 2nd CH in HRR handshake"] },
          {"name" : "test-record-size-limit.py",
           "arguments" : ["-p", "@PORT@", "--reply-AD-size", "672",
                          "--minimal-size", "512",
@@ -108,7 +113,11 @@
 	 {"name" : "test-tls13-version-negotiation.py",
 	  "arguments": ["-p", "@PORT@"]},
 	 {"name" : "test-tls13-zero-length-data.py",
-	  "arguments": ["-p", "@PORT@"]}
+	  "arguments": ["-p", "@PORT@"]},
+	 {"name" : "test-downgrade-protection.py",
+	  "comment" : "1/n-1 splitting in TLS 1.0 is not supported",
+	  "arguments": ["-p", "@PORT@", "--server-max-protocol", "TLSv1.3",
+			"-e", "TLS 1.3 downgrade check for Protocol (3, 1)"]}
      ]
     }
 ]
diff --git a/tests/suite/tls-fuzzer/gnutls-nocert.json b/tests/suite/tls-fuzzer/gnutls-nocert.json
index fe7a6fff17..e25b6b3613 100644
--- a/tests/suite/tls-fuzzer/gnutls-nocert.json
+++ b/tests/suite/tls-fuzzer/gnutls-nocert.json
@@ -248,7 +248,11 @@
                          "-e", "too large record payload in TLS 1.3",
                          "-e", "change size in TLS 1.3 session resumption",
                          "-e", "drop extension in TLS 1.3 session resumption",
-                         "-e", "modified extension in 2nd CH in HRR handshake"] },
+                         "-e", "modified extension in 2nd CH in HRR handshake",
+                         "-e", "added extension in 2nd CH in HRR handshake",
+                         "-e", "check server sent size in TLS 1.0 with max_fragment_length",
+                         "-e", "check server sent size in TLS 1.3 with max_fragment_length",
+                         "-e", "removed extension in 2nd CH in HRR handshake"] },
          {"name" : "test-record-size-limit.py",
           "comment" : "The reply includes PRF algorithm and affects the AD size",
           "arguments" : ["-p", "@PORT@", "--reply-AD-size", "827",
diff --git a/tests/suite/tls-fuzzer/tlsfuzzer b/tests/suite/tls-fuzzer/tlsfuzzer
-Subproject a520d50cf84aba0126d1e09b12fd0038af0944b
+Subproject 13479e5a44bc10e3577fc28b921c5b999a363ce
diff --git a/tests/tls13/rnd-check-rollback-val.c b/tests/tls13/rnd-check-rollback-val.c
index f573596c5e..6b7adafcb5 100644
--- a/tests/tls13/rnd-check-rollback-val.c
+++ b/tests/tls13/rnd-check-rollback-val.c
@@ -89,6 +89,8 @@ static void client(int fd)
 	gnutls_certificate_credentials_t x509_cred;
 	gnutls_session_t session;
 	gnutls_datum_t srandom;
+	unsigned try = 0;
+	gnutls_datum_t session_data = { NULL, 0 };
 
 	global_init();
 
@@ -102,6 +104,7 @@ static void client(int fd)
 					    &cli_ca3_key,
 					    GNUTLS_X509_FMT_PEM);
 
+ retry:
 	/* Initialize TLS session
 	 */
 	gnutls_init(&session, GNUTLS_CLIENT);
@@ -112,6 +115,9 @@ static void client(int fd)
 	if (ret < 0)
 		fail("cannot set TLS priorities\n");
 
+	if (try > 0)
+		gnutls_session_set_data(session, session_data.data, session_data.size);
+
 	/* put the anonymous credentials to the current session
 	 */
 	gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, x509_cred);
@@ -129,6 +135,9 @@ static void client(int fd)
 		fail("error in handshake: %s\n", gnutls_strerror(ret));
 	}
 
+	if (try > 0)
+		assert(gnutls_session_is_resumed(session));
+
 	gnutls_session_get_random(session, NULL, &srandom);
 
 	if (srandom.size != 32)
@@ -147,10 +156,28 @@ static void client(int fd)
 		fail("unexpected random data for %s\n", name);
 	}
 
-	close(fd);
+	do {
+		ret = gnutls_record_send(session, "\x00", 1);
+	} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
+
+	if (try == 0) {
+		ret = gnutls_session_get_data2(session, &session_data);
+		if (ret < 0)
+			fail("couldn't retrieve session data: %s\n",
+			     gnutls_strerror(ret));
+	}
 
 	gnutls_deinit(session);
 
+	if (try == 0) {
+		try++;
+		goto retry;
+	}
+
+	close(fd);
+
+	gnutls_free(session_data.data);
+
 	gnutls_certificate_free_credentials(x509_cred);
 
 	gnutls_global_deinit();
@@ -162,6 +189,9 @@ static void server(int fd)
 	int ret;
 	gnutls_session_t session;
 	gnutls_certificate_credentials_t x509_cred;
+	gnutls_datum_t skey;
+	unsigned try = 0;
+	unsigned char buf[16];
 
 	/* this must be called once in the program
 	 */
@@ -177,6 +207,9 @@ static void server(int fd)
 					    &server_key,
 					    GNUTLS_X509_FMT_PEM);
 
+	assert(gnutls_session_ticket_key_generate(&skey) >= 0);
+
+ retry:
 	gnutls_init(&session, GNUTLS_SERVER);
 
 	gnutls_handshake_set_timeout(session, 20 * 1000);
@@ -185,6 +218,8 @@ static void server(int fd)
 
 	gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, x509_cred);
 
+	assert(gnutls_session_ticket_enable_server(session, &skey) >= 0);
+
 	gnutls_transport_set_int(session, fd);
 
 	do {
@@ -197,9 +232,26 @@ static void server(int fd)
 	if (ret < 0)
 		fail("error in handshake: %s\n", gnutls_strerror(ret));
 
-	close(fd);
+	if (try > 0)
+		assert(gnutls_session_is_resumed(session));
+
+	do {
+		ret = gnutls_record_recv(session, buf, sizeof(buf));
+	} while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
+
+	if (ret < 0)
+		fail("server: recv did not succeed as expected: %s\n", gnutls_strerror(ret));
+
 	gnutls_deinit(session);
 
+	if (try == 0) {
+		try++;
+		goto retry;
+	}
+
+	close(fd);
+
+	gnutls_free(skey.data);
 	gnutls_certificate_free_credentials(x509_cred);
 
 	gnutls_global_deinit();