commit f98b582ded9840cd3e3b6365d8d484b1da78c7f8
author Steven Valdez <svaldez@google.com> Tue Dec 12 11:29:53 2017 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Dec 12 17:20:07 2017 +0000
tree b6c8d7286c7b621bf3cc6e331e9309b2585c8d44
parent 6fe960d174f43738875933be9eeb48f4dd0c0521

Fix tls13_variant check to check max_version.

Change-Id: I946e8903d745b0bf0c48701d16aa866464233ab7
Reviewed-on: https://boringssl-review.googlesource.com/24044
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>

ssl/handshake_client.cc

diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index b801e82..cdda459 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -464,7 +464,7 @@
     hs->session_id_len = ssl->session->session_id_length;
     OPENSSL_memcpy(hs->session_id, ssl->session->session_id,
                    hs->session_id_len);
-  } else if (ssl_is_resumption_variant(ssl->tls13_variant)) {
+  } else if (ssl_is_resumption_variant(hs->max_version, ssl->tls13_variant)) {
     hs->session_id_len = sizeof(hs->session_id);
     if (!RAND_bytes(hs->session_id, hs->session_id_len)) {
       return ssl_hs_error;

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 5d043a8..4151d2b 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -404,9 +404,10 @@
 // TLS 1.3 resumption experiment.
 bool ssl_is_resumption_experiment(uint16_t version);
 
-// ssl_is_resumption_variant returns whether the version corresponds to a
+// ssl_is_resumption_variant returns whether the variant corresponds to a
 // TLS 1.3 resumption experiment.
-bool ssl_is_resumption_variant(enum tls13_variant_t variant);
+bool ssl_is_resumption_variant(uint16_t max_version,
+                               enum tls13_variant_t variant);
 
 // ssl_is_resumption_client_ccs_experiment returns whether the version
 // corresponds to a TLS 1.3 resumption experiment that sends a client CCS.

ssl/ssl_versions.cc

diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc
index 15b0294..2406bd8 100644
--- a/ssl/ssl_versions.cc
+++ b/ssl/ssl_versions.cc
@@ -412,7 +412,11 @@
          version == TLS1_3_DRAFT22_VERSION;
 }
 
-bool ssl_is_resumption_variant(enum tls13_variant_t variant) {
+bool ssl_is_resumption_variant(uint16_t max_version,
+                               enum tls13_variant_t variant) {
+  if (max_version < TLS1_3_VERSION) {
+    return false;
+  }
   return variant == tls13_experiment || variant == tls13_experiment2 ||
          variant == tls13_experiment3 || variant == tls13_draft22;
 }

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 0e2d011..fbd016a 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -11353,6 +11353,20 @@
 			tls13Variant: variant,
 		})
 
+		// Test that the client omits the fake session ID when the max version is TLS 1.2 and below.
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			name:     "TLS12NoSessionID-" + name,
+			config: Config{
+				MaxVersion: VersionTLS13,
+				Bugs: ProtocolBugs{
+					ExpectNoTLS12Session: true,
+				},
+			},
+			tls13Variant: variant,
+			flags:        []string{"-max-version", strconv.Itoa(VersionTLS12)},
+		})
+
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			name:     "EarlyData-Client-" + name,