ext/supported_versions: reorder client precedence if necessary

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>

diff --git a/NEWS b/NEWS
index e0320042c3769c33cde1a00b47ca92314aa7fef8..b8278e11d90e91eda5196ff2515320e3316f5290 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,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 8d52fad5c0305e9b9d6fcf2e992f47fbec791d32..1b9c295795a09dec3327a194a4fd368b7bbbf644 100644 (file)
--- 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 4f9bac22615b47f8c73f3eeb4d5ebeb266579b7b..2b6badff176c5967b887c85b541eb02dd096908a 100644 (file)
--- 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();