ext/supported_versions: reorder client precedence if necessarytmp-supported-versions

If the client advertises TLS < 1.2 before TLS 1.3 and the server is
configured with TLS 1.3 enabled, the server should select TLS 1.3;
otherwise the client will disconnect when seeing downgrade sentinel.

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

2 files changed, 41 insertions, 21 deletions

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();