Remove SSL_set_fallback_version.

Ding-dong the fallback's dead.
https://mailarchive.ietf.org/arch/msg/tls/xfCh7D7hISFs5x-eA0xHwksoLrc

Also we'll need to tweak the versioning code slightly to implement
supported_versions and it's nice to have this out of the way.

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

8 files changed, 14 insertions, 124 deletions

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index a8d5abd9..256e3a03 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -695,9 +695,7 @@ OPENSSL_EXPORT uint32_t SSL_get_options(const SSL *ssl);
  * version; see RFC 7507 for details.
  *
  * DO NOT ENABLE THIS if your application attempts a normal handshake. Only use
- * this in explicit fallback retries, following the guidance in RFC 7507.
- *
- * This flag is deprecated. Use |SSL_set_fallback_version| instead. */
+ * this in explicit fallback retries, following the guidance in RFC 7507. */
 #define SSL_MODE_SEND_FALLBACK_SCSV 0x00000400L
 
 /* SSL_CTX_set_mode enables all modes set in |mode| (which should be one or more
@@ -3093,22 +3091,6 @@ OPENSSL_EXPORT const SSL_CIPHER *SSL_get_pending_cipher(const SSL *ssl);
 OPENSSL_EXPORT void SSL_CTX_set_retain_only_sha256_of_client_certs(SSL_CTX *ctx,
                                                                    int enable);
 
-/* SSL_set_fallback_version, on a client, sets the effective maximum protocol
- * version. This may be used when implementing a version fallback to work around
- * buggy servers.
- *
- * For purposes of the TLS protocol itself, including assembling the ClientHello
- * and which ServerHello versions are accepted, this value is used as the
- * maximum version. However, if this value differs from the real maximum
- * version, as set by |SSL_set_max_version|, TLS_FALLBACK_SCSV (see RFC 7507)
- * will be sent. Further, the TLS 1.3 anti-downgrade logic will be conditioned
- * on the true maximum version.
- *
- * For instance, a fallback from a TLS 1.3 ClientHello to a TLS 1.2 ClientHello
- * should set this value to |TLS1_2_VERSION| and call |SSL_set_max_version| with
- * |TLS1_3_VERSION|. */
-OPENSSL_EXPORT void SSL_set_fallback_version(SSL *ssl, uint16_t version);
-
 
 /* Deprecated functions. */
 
@@ -4017,11 +3999,6 @@ struct ssl_st {
    * is normalized in DTLS. */
   uint16_t min_version;
 
-  /* fallback_version is the effective maximum acceptable protocol version for
-   * use with a version fallback, or zero if unset. Note this version is
-   * normalized in DTLS. */
-  uint16_t fallback_version;
-
   uint16_t max_send_fragment;
 
   /* There are 2 BIO's even though they are normally both the same. This is so
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 238906bf..4e4cf5c6 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -581,8 +581,7 @@ end:
 
 static int ssl_write_client_cipher_list(SSL *ssl, CBB *out,
                                         uint16_t min_version,
-                                        uint16_t max_version,
-                                        uint16_t real_max_version) {
+                                        uint16_t max_version) {
   /* Prepare disabled cipher masks. */
   ssl_set_client_disabled(ssl);
 
@@ -636,8 +635,7 @@ static int ssl_write_client_cipher_list(SSL *ssl, CBB *out,
     }
   }
 
-  if ((ssl->mode & SSL_MODE_SEND_FALLBACK_SCSV) ||
-      real_max_version > max_version) {
+  if (ssl->mode & SSL_MODE_SEND_FALLBACK_SCSV) {
     if (!CBB_add_u16(&child, SSL3_CK_FALLBACK_SCSV & 0xffff)) {
       return 0;
     }
@@ -647,9 +645,8 @@ static int ssl_write_client_cipher_list(SSL *ssl, CBB *out,
 }
 
 int ssl_add_client_hello_body(SSL *ssl, CBB *body) {
-  uint16_t min_version, max_version, real_max_version;
-  if (!ssl_get_full_version_range(ssl, &min_version, &max_version,
-                                  &real_max_version)) {
+  uint16_t min_version, max_version;
+  if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
     return 0;
   }
 
@@ -676,8 +673,7 @@ int ssl_add_client_hello_body(SSL *ssl, CBB *body) {
 
   size_t header_len =
       SSL_is_dtls(ssl) ? DTLS1_HM_HEADER_LENGTH : SSL3_HM_HEADER_LENGTH;
-  if (!ssl_write_client_cipher_list(ssl, body, min_version, max_version,
-                                    real_max_version) ||
+  if (!ssl_write_client_cipher_list(ssl, body, min_version, max_version) ||
       !CBB_add_u8(body, 1 /* one compression method */) ||
       !CBB_add_u8(body, 0 /* null compression */) ||
       !ssl_add_clienthello_tlsext(ssl, body, header_len + CBB_len(body))) {
@@ -837,9 +833,8 @@ static int ssl3_get_server_hello(SSL *ssl) {
 
   server_version = ssl->method->version_from_wire(server_wire_version);
 
-  uint16_t min_version, max_version, real_max_version;
-  if (!ssl_get_full_version_range(ssl, &min_version, &max_version,
-                                  &real_max_version) ||
+  uint16_t min_version, max_version;
+  if (!ssl_get_version_range(ssl, &min_version, &max_version) ||
       server_version < min_version || server_version > max_version) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
     al = SSL_AD_PROTOCOL_VERSION;
diff --git a/ssl/internal.h b/ssl/internal.h
index c090094a..0bc0b8ed 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1505,21 +1505,10 @@ int ssl3_can_false_start(const SSL *ssl);
  * |version|. */
 const SSL3_ENC_METHOD *ssl3_get_enc_method(uint16_t version);
 
-/* ssl_get_full_version_range sets |*out_min_version|, |*out_fallback_version|,
- * and |*out_max_version| to the minimum, fallback, and maximum enabled protocol
- * versions, respectively. The fallback version is the effective maximium
- * version used throughout the stack and the maximum version is the true maximum
- * for downgrade purposes. */
-int ssl_get_full_version_range(const SSL *ssl, uint16_t *out_min_version,
-                               uint16_t *out_fallback_version,
-                               uint16_t *out_max_version);
-
-/* ssl_get_version_range sets |*out_min_version| and
- * |*out_effective_max_version| to the minimum and maximum enabled protocol
- * versions, respectively. Note that, if there is a fallback version set, it
- * returns it as the maximum version. */
+/* ssl_get_version_range sets |*out_min_version| and |*out_max_version| to the
+ * minimum and maximum enabled protocol versions, respectively. */
 int ssl_get_version_range(const SSL *ssl, uint16_t *out_min_version,
-                          uint16_t *out_effective_max_version);
+                          uint16_t *out_max_version);
 
 /* ssl3_protocol_version returns |ssl|'s protocol version. It is an error to
  * call this function before the version is determined. */
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 1e1f7527..aa039992 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -965,10 +965,6 @@ void SSL_set_max_version(SSL *ssl, uint16_t version) {
   ssl->max_version = ssl->method->version_from_wire(version);
 }
 
-void SSL_set_fallback_version(SSL *ssl, uint16_t version) {
-  ssl->fallback_version = ssl->method->version_from_wire(version);
-}
-
 uint32_t SSL_CTX_set_options(SSL_CTX *ctx, uint32_t options) {
   ctx->options |= options;
   return ctx->options;
@@ -2682,9 +2678,8 @@ const struct {
 
 static const size_t kVersionsLen = OPENSSL_ARRAY_SIZE(kVersions);
 
-int ssl_get_full_version_range(const SSL *ssl, uint16_t *out_min_version,
-                               uint16_t *out_fallback_version,
-                               uint16_t *out_max_version) {
+int ssl_get_version_range(const SSL *ssl, uint16_t *out_min_version,
+                          uint16_t *out_max_version) {
   /* For historical reasons, |SSL_OP_NO_DTLSv1| aliases |SSL_OP_NO_TLSv1|, but
    * DTLS 1.0 should be mapped to TLS 1.1. */
   uint32_t options = ssl->options;
@@ -2743,32 +2738,16 @@ int ssl_get_full_version_range(const SSL *ssl, uint16_t *out_min_version,
     }
   }
 
-  uint16_t fallback_version = max_version;
-  if (ssl->fallback_version != 0 && ssl->fallback_version < fallback_version) {
-    fallback_version = ssl->fallback_version;
-  }
-
-  if (!any_enabled || fallback_version < min_version) {
+  if (!any_enabled) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION);
     return 0;
   }
 
   *out_min_version = min_version;
-  *out_fallback_version = fallback_version;
   *out_max_version = max_version;
   return 1;
 }
 
-int ssl_get_version_range(const SSL *ssl, uint16_t *out_min_version,
-                          uint16_t *out_effective_max_version) {
-  /* This function returns the effective maximum version and not the fallback
-   * version. */
-  uint16_t real_max_version_unused;
-  return ssl_get_full_version_range(ssl, out_min_version,
-                                    out_effective_max_version,
-                                    &real_max_version_unused);
-}
-
 uint16_t ssl3_protocol_version(const SSL *ssl) {
   assert(ssl->s3->have_version);
   return ssl->method->version_from_wire(ssl->version);
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 1fe8ce0c..bef323ee 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1370,9 +1370,6 @@ static bool DoExchange(bssl::UniquePtr<SSL_SESSION> *out_session,
   if (config->max_version != 0) {
     SSL_set_max_version(ssl.get(), (uint16_t)config->max_version);
   }
-  if (config->fallback_version != 0) {
-    SSL_set_fallback_version(ssl.get(), (uint16_t)config->fallback_version);
-  }
   if (config->mtu != 0) {
     SSL_set_options(ssl.get(), SSL_OP_NO_QUERY_MTU);
     SSL_set_mtu(ssl.get(), config->mtu);
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 854bcea1..9ae72ef5 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4235,51 +4235,6 @@ func addVersionNegotiationTests() {
 		// TODO(davidben): This test should fail once TLS 1.3 is final
 		// and the fallback signal restored.
 	})
-
-	// Test that FALLBACK_SCSV is sent and that the downgrade signal works
-	// behave correctly when both real maximum and fallback versions are
-	// set.
-	testCases = append(testCases, testCase{
-		name: "Downgrade-TLS12-Client-Fallback",
-		config: Config{
-			Bugs: ProtocolBugs{
-				FailIfNotFallbackSCSV: true,
-			},
-		},
-		flags: []string{
-			"-max-version", strconv.Itoa(VersionTLS13),
-			"-fallback-version", strconv.Itoa(VersionTLS12),
-		},
-		// TODO(davidben): This test should fail once TLS 1.3 is final
-		// and the fallback signal restored.
-	})
-	testCases = append(testCases, testCase{
-		name: "Downgrade-TLS12-Client-FallbackEqualsMax",
-		flags: []string{
-			"-max-version", strconv.Itoa(VersionTLS12),
-			"-fallback-version", strconv.Itoa(VersionTLS12),
-		},
-	})
-
-	// On TLS 1.2 fallback, 1.3 ServerHellos are forbidden. (We would rather
-	// just have such connections fail than risk getting confused because we
-	// didn't sent the 1.3 ClientHello.)
-	testCases = append(testCases, testCase{
-		name: "Downgrade-TLS12-Fallback-CheckVersion",
-		config: Config{
-			Bugs: ProtocolBugs{
-				NegotiateVersion:      VersionTLS13,
-				FailIfNotFallbackSCSV: true,
-			},
-		},
-		flags: []string{
-			"-max-version", strconv.Itoa(VersionTLS13),
-			"-fallback-version", strconv.Itoa(VersionTLS12),
-		},
-		shouldFail:    true,
-		expectedError: ":UNSUPPORTED_PROTOCOL:",
-	})
-
 }
 
 func addMinimumVersionTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 677aa545..1f01166e 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -145,7 +145,6 @@ const Flag<int> kIntFlags[] = {
   { "-resume-count", &TestConfig::resume_count },
   { "-min-version", &TestConfig::min_version },
   { "-max-version", &TestConfig::max_version },
-  { "-fallback-version", &TestConfig::fallback_version },
   { "-mtu", &TestConfig::mtu },
   { "-export-keying-material", &TestConfig::export_keying_material },
   { "-expect-total-renegotiations", &TestConfig::expect_total_renegotiations },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 8ed74ac0..93c22ce4 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -66,7 +66,6 @@ struct TestConfig {
   std::string expected_signed_cert_timestamps;
   int min_version = 0;
   int max_version = 0;
-  int fallback_version = 0;
   int mtu = 0;
   bool implicit_handshake = false;
   bool use_early_callback = false;