handshake: do not negotiate TLS1.3 using the legacy version fieldtmp-update-version-numbers

Previously we could end-up with a TLS1.3 connection if the TLS1.3
ID was seen on the wire. We now explicitly fallback to TLS1.2
when we see a protocol with TLS1.3 semantics in an SSL2.0 or
in the legacy version of the client hello.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

4 files changed, 25 insertions, 8 deletions

diff --git a/lib/ext/supported_versions.c b/lib/ext/supported_versions.c
index 3a11b39bd4..b7fe31f75b 100644
--- a/lib/ext/supported_versions.c
+++ b/lib/ext/supported_versions.c
@@ -133,7 +133,7 @@ supported_versions_recv_params(gnutls_session_t session,
 		if (!vers->tls13_sem)
 			return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_PARAMETER);
 
-		ret = _gnutls_negotiate_version(session, major, minor);
+		ret = _gnutls_negotiate_version(session, major, minor, 1);
 		if (ret < 0) {
 			gnutls_assert();
 			return ret;
diff --git a/lib/handshake.c b/lib/handshake.c
index ccf8299165..08481cca07 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -431,7 +431,7 @@ _gnutls_finished(gnutls_session_t session, int type, void *ret,
  */
 int
 _gnutls_negotiate_version(gnutls_session_t session,
-			  uint8_t major, uint8_t minor)
+			  uint8_t major, uint8_t minor, unsigned allow_tls13)
 {
 	const version_entry_st *vers;
 	const version_entry_st *aversion = nversion_to_entry(major, minor);
@@ -471,6 +471,12 @@ _gnutls_negotiate_version(gnutls_session_t session,
 	} else {
 		session->security_parameters.pversion = aversion;
 
+		/* we do not allow TLS1.3 negotiation using this mechanism */
+		if (aversion->tls13_sem && !allow_tls13) {
+			vers = _gnutls_legacy_version_max(session);
+			session->security_parameters.pversion = vers;
+		}
+
 		return 0;
 	}
 }
@@ -505,7 +511,7 @@ _gnutls_user_hello_func(gnutls_session_t session,
 			 * earlier, as TLS1.3 uses a different set of ciphersuites, and
 			 * thus we cannot fallback.
 			 */
-			ret = _gnutls_negotiate_version(session, major, minor);
+			ret = _gnutls_negotiate_version(session, major, minor, 0);
 			if (ret < 0) {
 				gnutls_assert();
 				return ret;
@@ -565,11 +571,15 @@ read_client_hello(gnutls_session_t session, uint8_t * data,
 
 	set_adv_version(session, major, minor);
 
-	ret = _gnutls_negotiate_version(session, major, minor);
+	ret = _gnutls_negotiate_version(session, major, minor, 0);
 	if (ret < 0)
 		return gnutls_assert_val(ret);
 
-	neg_version = get_num_version(session);
+	vers = get_version(session);
+	if (vers == NULL)
+		return gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
+
+	neg_version = vers->id;
 
 	pos += 2;
 
diff --git a/lib/handshake.h b/lib/handshake.h
index 0d617213c5..184a349a91 100644
--- a/lib/handshake.h
+++ b/lib/handshake.h
@@ -94,7 +94,8 @@ int _gnutls_server_select_suite(gnutls_session_t session, uint8_t * data,
 				unsigned int datalen, unsigned int scsv_only);
 
 int _gnutls_negotiate_version(gnutls_session_t session,
-			      uint8_t major, uint8_t minor);
+			      uint8_t major, uint8_t minor,
+			      unsigned allow_tls13);
 int _gnutls_user_hello_func(gnutls_session_t session,
 			    uint8_t major, uint8_t minor);
 
diff --git a/lib/sslv2_compat.c b/lib/sslv2_compat.c
index de762a5674..9d8fd91bf4 100644
--- a/lib/sslv2_compat.c
+++ b/lib/sslv2_compat.c
@@ -96,6 +96,7 @@ _gnutls_read_client_hello_v2(gnutls_session_t session, uint8_t * data,
 	uint8_t rnd[GNUTLS_RANDOM_SIZE], major, minor;
 	int len = datalen;
 	int neg_version;
+	const version_entry_st *vers;
 	uint16_t challenge;
 	uint8_t session_id[GNUTLS_MAX_SESSION_ID_SIZE];
 
@@ -109,12 +110,17 @@ _gnutls_read_client_hello_v2(gnutls_session_t session, uint8_t * data,
 	minor = data[pos + 1];
 	set_adv_version(session, major, minor);
 
-	ret = _gnutls_negotiate_version(session, major, minor);
+	ret = _gnutls_negotiate_version(session, major, minor, 0);
 	if (ret < 0) {
 		gnutls_assert();
 		return ret;
 	}
-	neg_version = ret;
+
+	vers = get_version(session);
+	if (vers == NULL)
+		return gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
+
+	neg_version = vers->id;
 
 	pos += 2;