TLS: Choose the max version supported by the client, not first.

This change is based on interpreting TLS 1.3 draft 18.

Change-Id: I727961aff2f7318bcbbc8bf6d62b7d6ad3e62da9
Reviewed-on: https://boringssl-review.googlesource.com/11921
Reviewed-by: David Benjamin <davidben@google.com>

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 8db9bee..9200b85 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -578,6 +578,9 @@
       return 0;
     }
 
+    /* Choose the newest commonly-supported version advertised by the client.
+     * The client orders the versions according to its preferences, but we're
+     * not required to honor the client's preferences. */
     int found_version = 0;
     while (CBS_len(&versions) != 0) {
       uint16_t ext_version;
@@ -590,10 +593,10 @@
         continue;
       }
       if (min_version <= ext_version &&
-          ext_version <= max_version) {
+          ext_version <= max_version &&
+          (!found_version || version < ext_version)) {
         version = ext_version;
         found_version = 1;
-        break;
       }
     }
 

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 03a0ef9..a5c075c 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4357,6 +4357,19 @@
 		expectedVersion: VersionTLS12,
 	})
 
+	// Test that the maximum version is selected regardless of the
+	// client-sent order.
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "IgnoreClientVersionOrder",
+		config: Config{
+			Bugs: ProtocolBugs{
+				SendSupportedVersions: []uint16{VersionTLS12, tls13DraftVersion},
+			},
+		},
+		expectedVersion: VersionTLS13,
+	})
+
 	// Test for version tolerance.
 	testCases = append(testCases, testCase{
 		testType: serverTest,