Merge branch 'tmp-supported-versions' into 'master'

ext/supported_versions: reorder client precedence if necessary

Closes #837

See merge request gnutls/gnutls!1074

3 files changed, 44 insertions, 21 deletions

diff --git a/NEWS b/NEWS
index 538256a0b7..4165de6e7a 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,9 @@ See the end for copying conditions.
 ** libgnutls: added interoperability tests with gnutls 2.12.x; addressed
    issue with large record handling due to random padding (#811).
 
+** libgnutls: the server now selects the highest TLS protocol version,
+   if TLS 1.3 is enabled and the client advertises an older protocol version first (#837).
+
 ** API and ABI modifications:
 gnutls_aead_cipher_encryptv2: Added
 gnutls_aead_cipher_decryptv2: Added
diff --git a/lib/ext/supported_versions.c b/lib/ext/supported_versions.c
index 8d52fad5c0..1b9c295795 100644
--- a/lib/ext/supported_versions.c
+++ b/lib/ext/supported_versions.c
@@ -63,6 +63,7 @@ supported_versions_recv_params(gnutls_session_t session,
 
 	if (session->security_parameters.entity == GNUTLS_SERVER) {
 		const version_entry_st *old_vers;
+		const version_entry_st *cli_vers = NULL;
 
 		vers = _gnutls_version_max(session);
 		old_vers = get_version(session);
@@ -94,29 +95,41 @@ supported_versions_recv_params(gnutls_session_t session,
 			_gnutls_handshake_log("EXT[%p]: Found version: %d.%d\n",
 					      session, (int)major, (int)minor);
 
-			if (_gnutls_nversion_is_supported(session, major, minor)) {
-				session->security_parameters.pversion = nversion_to_entry(major, minor);
-
-				_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;
-			}
+			if (!_gnutls_nversion_is_supported(session, major, minor))
+				continue;
+
+			/* Prefer the latest possible version
+			 * regardless of the client's precedence.  See
+			 * https://gitlab.com/gnutls/gnutls/issues/837
+			 * for the rationale.
+			 */
+			if (!cli_vers ||
+			    major > cli_vers->major ||
+			    (major == cli_vers->major &&
+			     minor > cli_vers->minor))
+				cli_vers = nversion_to_entry(major, minor);
+		}
+
+		if (!cli_vers)
+			return gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
+
+		session->security_parameters.pversion = cli_vers;
+
+		_gnutls_handshake_log("EXT[%p]: Negotiated version: %d.%d\n",
+				      session,
+				      (int)cli_vers->major,
+				      (int)cli_vers->minor);
+
+		if (old_vers != cli_vers) {
+			/* regenerate the random value to set
+			 * downgrade sentinel if necessary
+			 */
+			ret = _gnutls_gen_server_random(session, cli_vers->id);
+			if (ret < 0)
+				return gnutls_assert_val(ret);
 		}
 
-		/* if we are here, none of the versions were acceptable */
-		return gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
+		return 0;
 	} else { /* client */
 
 		if (!have_creds_for_tls13(session)) {
diff --git a/tests/version-checks.c b/tests/version-checks.c
index 4f9bac2261..2b6badff17 100644
--- a/tests/version-checks.c
+++ b/tests/version-checks.c
@@ -171,6 +171,13 @@ void doit(void)
 	reset_buffers();
 	try("NORMAL:-VERS-TLS-ALL:+VERS-TLS1.3:+VERS-TLS1.2", GNUTLS_TLS1_3);
 	reset_buffers();
+	/* If TLS 1.3 is enabled in the server, prefer the latest
+	 * possible version regardless of the client's precedence.
+	 * See https://gitlab.com/gnutls/gnutls/issues/837 for the
+	 * rationale.
+	 */
+	try("NORMAL:-VERS-TLS-ALL:+VERS-TLS1.2:+VERS-TLS1.3", GNUTLS_TLS1_3);
+	reset_buffers();
 #ifdef ENABLE_SSL3
 	try("NORMAL:-VERS-TLS-ALL:+VERS-SSL3.0", -1);
 	reset_buffers();