Commit 5a31215e authored by davidben's avatar davidben Committed by Commit bot
Browse files

Delete TLS version fallback code in net/http.

This removes the logic in HttpNetworkTransaction to drive the TLS version
fallback. It's gone now.

Note this does come with a behavior change. TLS 1.2 intolerant servers will now
fail with their true error code rather than
ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION. That error code was intended for
diagnositics. With the fallback having been gone for several releases (by the
time this hits stable), we won't need it anymore.

Of course, this will all come back for TLS 1.3, but let's implement it in
SSLConnectJob next time. Avoid a bit of munging across layers.

BUG=621780
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2099673003
Cr-Commit-Position: refs/heads/master@{#402312}
parent 3e84f9c8
......@@ -71,20 +71,6 @@ namespace net {
namespace {
std::unique_ptr<base::Value> NetLogSSLVersionFallbackCallback(
const GURL* url,
int net_error,
uint16_t version_before,
uint16_t version_after,
NetLogCaptureMode /* capture_mode */) {
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
dict->SetString("host_and_port", GetHostAndPort(*url));
dict->SetInteger("net_error", net_error);
dict->SetInteger("version_before", version_before);
dict->SetInteger("version_after", version_after);
return std::move(dict);
}
std::unique_ptr<base::Value> NetLogSSLCipherFallbackCallback(
const GURL* url,
int net_error,
......@@ -108,7 +94,6 @@ HttpNetworkTransaction::HttpNetworkTransaction(RequestPriority priority,
request_(NULL),
priority_(priority),
headers_valid_(false),
fallback_error_code_(ERR_SSL_INAPPROPRIATE_FALLBACK),
request_headers_(),
read_buf_len_(0),
total_received_bytes_(0),
......@@ -1439,56 +1424,6 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
return OK;
}
// TODO(davidben): Remove this code once the dedicated error code is no
// longer needed and the flags to re-enable the fallback expire.
bool should_fallback = false;
uint16_t version_max = server_ssl_config_.version_max;
switch (error) {
// This could be a TLS-intolerant server or a server that chose a
// cipher suite defined only for higher protocol versions (such as
// an TLS 1.1 server that chose a TLS-1.2-only cipher suite). Fall
// back to the next lower version and retry.
case ERR_CONNECTION_CLOSED:
case ERR_SSL_PROTOCOL_ERROR:
case ERR_SSL_VERSION_OR_CIPHER_MISMATCH:
// Some servers trigger the TLS 1.1 fallback with ERR_CONNECTION_RESET
// (https://crbug.com/433406).
case ERR_CONNECTION_RESET:
// This was added for the TLS 1.0 fallback (https://crbug.com/260358) which
// has since been removed, but other servers may be relying on it for the
// TLS 1.1 fallback. It will be removed with the remainder of the fallback.
case ERR_SSL_BAD_RECORD_MAC_ALERT:
// Fallback down to a TLS 1.1 ClientHello. By default, this is rejected
// but surfaces ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION to help diagnose
// server bugs.
if (version_max >= SSL_PROTOCOL_VERSION_TLS1_2 &&
version_max > server_ssl_config_.version_min) {
version_max--;
should_fallback = true;
}
break;
case ERR_SSL_INAPPROPRIATE_FALLBACK:
// The server told us that we should not have fallen back. A buggy server
// could trigger ERR_SSL_INAPPROPRIATE_FALLBACK with the initial
// connection. |fallback_error_code_| is initialised to
// ERR_SSL_INAPPROPRIATE_FALLBACK to catch this case.
error = fallback_error_code_;
break;
}
if (should_fallback) {
net_log_.AddEvent(
NetLog::TYPE_SSL_VERSION_FALLBACK,
base::Bind(&NetLogSSLVersionFallbackCallback, &request_->url, error,
server_ssl_config_.version_max, version_max));
fallback_error_code_ = error;
server_ssl_config_.version_max = version_max;
server_ssl_config_.version_fallback = true;
ResetConnectionAndRequestForResend();
error = OK;
}
return error;
}
......
......@@ -330,12 +330,6 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction
SSLConfig server_ssl_config_;
SSLConfig proxy_ssl_config_;
// fallback_error_code contains the error code that caused the last TLS
// fallback. If the fallback connection results in
// ERR_SSL_INAPPROPRIATE_FALLBACK (i.e. the server indicated that the
// fallback should not have been needed) then we use this value to return the
// original error that triggered the fallback.
int fallback_error_code_;
// Keys to use for signing message in Token Binding header.
std::unique_ptr<crypto::ECPrivateKey> provided_token_binding_key_;
......
......@@ -122,47 +122,6 @@ class HttpNetworkTransactionSSLTest : public testing::Test {
std::vector<std::unique_ptr<HttpRequestInfo>> request_info_vector_;
};
// Tests that HttpNetworkTransaction attempts to fallback from
// TLS 1.2 to TLS 1.1.
TEST_F(HttpNetworkTransactionSSLTest, SSLFallback) {
ssl_config_service_ = new TLS12SSLConfigService;
session_params_.ssl_config_service = ssl_config_service_.get();
// |ssl_data1| is for the first handshake (TLS 1.2), which will fail
// for protocol reasons (e.g., simulating a version rollback attack).
SSLSocketDataProvider ssl_data1(ASYNC, ERR_SSL_PROTOCOL_ERROR);
mock_socket_factory_.AddSSLSocketDataProvider(&ssl_data1);
StaticSocketDataProvider data1(NULL, 0, NULL, 0);
mock_socket_factory_.AddSocketDataProvider(&data1);
// |ssl_data2| contains the handshake result for a TLS 1.1
// handshake which will be attempted after the TLS 1.2
// handshake fails.
SSLSocketDataProvider ssl_data2(ASYNC, ERR_SSL_PROTOCOL_ERROR);
mock_socket_factory_.AddSSLSocketDataProvider(&ssl_data2);
StaticSocketDataProvider data2(NULL, 0, NULL, 0);
mock_socket_factory_.AddSocketDataProvider(&data2);
HttpNetworkSession session(session_params_);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, &session);
TestCompletionCallback callback;
// This will consume |ssl_data1| and |ssl_data2|.
int rv =
callback.GetResult(trans.Start(GetRequestInfo("https://www.paypal.com/"),
callback.callback(), BoundNetLog()));
EXPECT_EQ(ERR_SSL_PROTOCOL_ERROR, rv);
SocketDataProviderArray<SocketDataProvider>& mock_data =
mock_socket_factory_.mock_data();
// Confirms that |ssl_data1| and |ssl_data2| are consumed.
EXPECT_EQ(2u, mock_data.next_index());
SSLConfig& ssl_config = GetServerSSLConfig(&trans);
// |version_max| falls back to TLS 1.1.
EXPECT_EQ(SSL_PROTOCOL_VERSION_TLS1_1, ssl_config.version_max);
EXPECT_TRUE(ssl_config.version_fallback);
}
#if !defined(OS_IOS)
TEST_F(HttpNetworkTransactionSSLTest, TokenBinding) {
ssl_config_service_ = new TokenBindingSSLConfigService;
......
......@@ -541,6 +541,9 @@ EVENT_TYPE(SSL_WRITE_ERROR)
// "version_before": <SSL version before the fallback>,
// "version_after": <SSL version after the fallback>,
// }
//
// TODO(davidben): Remove this event and the corresponding log_view_painter.js
// logic in M56.
EVENT_TYPE(SSL_VERSION_FALLBACK)
// An SSL connection needs to be retried with more cipher suites because the
......
......@@ -116,32 +116,7 @@ int InitSocketPoolHelper(ClientSocketPoolManager::SocketGroupType group_type,
connection_group = "ftp/" + connection_group;
}
if (using_ssl) {
// All connections in a group should use the same SSLConfig settings.
// Encode version_max in the connection group's name, unless it's the
// default version_max. (We want the common case to use the shortest
// encoding). A version_max of TLS 1.1 is encoded as "ssl(max:3.2)/"
// rather than "tlsv1.1/" because the actual protocol version, which
// is selected by the server, may not be TLS 1.1. Do not encode
// version_min in the connection group's name because version_min
// should be the same for all connections, whereas version_max may
// change for version fallbacks.
std::string prefix = "ssl/";
if (ssl_config_for_origin.version_max != kDefaultSSLVersionMax) {
switch (ssl_config_for_origin.version_max) {
case SSL_PROTOCOL_VERSION_TLS1_2:
prefix = "ssl(max:3.3)/";
break;
case SSL_PROTOCOL_VERSION_TLS1_1:
prefix = "ssl(max:3.2)/";
break;
case SSL_PROTOCOL_VERSION_TLS1:
prefix = "ssl(max:3.1)/";
break;
default:
CHECK(false);
break;
}
}
// Place sockets with and without deprecated ciphers into separate
// connection groups.
if (ssl_config_for_origin.deprecated_cipher_suites_enabled)
......
......@@ -3323,15 +3323,10 @@ class TestSSLConfigService : public SSLConfigService {
rev_checking_required_local_anchors_(
rev_checking_required_local_anchors),
token_binding_enabled_(token_binding_enabled),
min_version_(kDefaultSSLVersionMin),
fallback_min_version_(kDefaultSSLVersionFallbackMin) {}
min_version_(kDefaultSSLVersionMin) {}
void set_min_version(uint16_t version) { min_version_ = version; }
void set_fallback_min_version(uint16_t version) {
fallback_min_version_ = version;
}
// SSLConfigService:
void GetSSLConfig(SSLConfig* config) override {
*config = SSLConfig();
......@@ -3339,9 +3334,6 @@ class TestSSLConfigService : public SSLConfigService {
config->verify_ev_cert = ev_enabled_;
config->rev_checking_required_local_anchors =
rev_checking_required_local_anchors_;
if (fallback_min_version_) {
config->version_fallback_min = fallback_min_version_;
}
if (min_version_) {
config->version_min = min_version_;
}
......@@ -3359,7 +3351,6 @@ class TestSSLConfigService : public SSLConfigService {
const bool rev_checking_required_local_anchors_;
const bool token_binding_enabled_;
uint16_t min_version_;
uint16_t fallback_min_version_;
};
// TODO(svaldez): Update tests to use EmbeddedTestServer.
......@@ -8903,22 +8894,6 @@ TEST_F(HTTPSRequestTest, SSLSessionCacheShardTest) {
}
}
class FallbackTestURLRequestContext : public TestURLRequestContext {
public:
explicit FallbackTestURLRequestContext(bool delay_initialization)
: TestURLRequestContext(delay_initialization) {}
void set_fallback_min_version(uint16_t version) {
TestSSLConfigService* ssl_config_service = new TestSSLConfigService(
true /* check for EV */, false /* online revocation checking */,
false /* require rev. checking for local
anchors */,
false /* token binding enabled */);
ssl_config_service->set_fallback_min_version(version);
set_ssl_config_service(ssl_config_service);
}
};
class HTTPSFallbackTest : public testing::Test {
public:
HTTPSFallbackTest() : context_(true) {}
......@@ -8943,10 +8918,6 @@ class HTTPSFallbackTest : public testing::Test {
base::RunLoop().Run();
}
void set_fallback_min_version(uint16_t version) {
context_.set_fallback_min_version(version);
}
void ExpectConnection(int version) {
EXPECT_EQ(1, delegate_.response_started_count());
EXPECT_NE(0, delegate_.bytes_received());
......@@ -8965,7 +8936,7 @@ class HTTPSFallbackTest : public testing::Test {
private:
TestDelegate delegate_;
FallbackTestURLRequestContext context_;
TestURLRequestContext context_;
std::unique_ptr<URLRequest> request_;
};
......@@ -8980,7 +8951,7 @@ TEST_F(HTTPSFallbackTest, TLSv1NoFallback) {
ExpectFailure(ERR_SSL_VERSION_OR_CIPHER_MISMATCH);
}
// Tests the TLS 1.1 fallback doesn't happen but 1.2-intolerance is detected.
// Tests the TLS 1.1 fallback doesn't happen.
TEST_F(HTTPSFallbackTest, TLSv1_1NoFallback) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
......@@ -8988,167 +8959,9 @@ TEST_F(HTTPSFallbackTest, TLSv1_1NoFallback) {
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_2;
ASSERT_NO_FATAL_FAILURE(DoFallbackTest(ssl_options));
ExpectFailure(ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION);
}
// Tests the TLS 1.1 fallback when explicitly enabled.
TEST_F(HTTPSFallbackTest, TLSv1_1Fallback) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
ssl_options.tls_intolerant =
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_2;
set_fallback_min_version(SSL_PROTOCOL_VERSION_TLS1_1);
ASSERT_NO_FATAL_FAILURE(DoFallbackTest(ssl_options));
ExpectConnection(SSL_CONNECTION_VERSION_TLS1_1);
}
// Tests that the TLS 1.1 fallback, if enabled, triggers on closed connections.
TEST_F(HTTPSFallbackTest, TLSv1_1FallbackClosed) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
ssl_options.tls_intolerant =
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_2;
ssl_options.tls_intolerance_type =
SpawnedTestServer::SSLOptions::TLS_INTOLERANCE_CLOSE;
set_fallback_min_version(SSL_PROTOCOL_VERSION_TLS1_1);
ASSERT_NO_FATAL_FAILURE(DoFallbackTest(ssl_options));
ExpectConnection(SSL_CONNECTION_VERSION_TLS1_1);
}
// This test is disabled on Android because the remote test server doesn't cause
// a TCP reset.
#if !defined(OS_ANDROID)
// Tests fallback to TLS 1.1, if enabled, on connection reset.
TEST_F(HTTPSFallbackTest, TLSv1_1FallbackReset) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
ssl_options.tls_intolerant =
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_2;
ssl_options.tls_intolerance_type =
SpawnedTestServer::SSLOptions::TLS_INTOLERANCE_RESET;
set_fallback_min_version(SSL_PROTOCOL_VERSION_TLS1_1);
ASSERT_NO_FATAL_FAILURE(DoFallbackTest(ssl_options));
ExpectConnection(SSL_CONNECTION_VERSION_TLS1_1);
}
#endif // !OS_ANDROID
// Tests that we don't fallback, even if enabled, on handshake failure with
// servers that implement TLS_FALLBACK_SCSV. Also ensure that the original error
// code is reported.
TEST_F(HTTPSFallbackTest, FallbackSCSV) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
// Configure HTTPS server to be intolerant of TLS >= 1.1 in order to trigger
// a version fallback.
ssl_options.tls_intolerant =
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_1;
// Have the server process TLS_FALLBACK_SCSV so that version fallback
// connections are rejected.
ssl_options.fallback_scsv_enabled = true;
set_fallback_min_version(SSL_PROTOCOL_VERSION_TLS1_1);
ASSERT_NO_FATAL_FAILURE(DoFallbackTest(ssl_options));
// ERR_SSL_VERSION_OR_CIPHER_MISMATCH is how the server simulates version
// intolerance. If the fallback SCSV is processed when the original error
// that caused the fallback should be returned, which should be
// ERR_SSL_VERSION_OR_CIPHER_MISMATCH.
ExpectFailure(ERR_SSL_VERSION_OR_CIPHER_MISMATCH);
}
// Tests that we don't fallback, even if enabled, on connection closed with
// servers that implement TLS_FALLBACK_SCSV. Also ensure that the original error
// code is reported.
TEST_F(HTTPSFallbackTest, FallbackSCSVClosed) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
// Configure HTTPS server to be intolerant of TLS >= 1.1 in order to trigger
// a version fallback.
ssl_options.tls_intolerant =
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_1;
ssl_options.tls_intolerance_type =
SpawnedTestServer::SSLOptions::TLS_INTOLERANCE_CLOSE;
// Have the server process TLS_FALLBACK_SCSV so that version fallback
// connections are rejected.
ssl_options.fallback_scsv_enabled = true;
set_fallback_min_version(SSL_PROTOCOL_VERSION_TLS1_1);
ASSERT_NO_FATAL_FAILURE(DoFallbackTest(ssl_options));
// The original error should be replayed on rejected fallback.
ExpectFailure(ERR_CONNECTION_CLOSED);
}
// Test that fallback probe connections don't cause sessions to be cached.
TEST_F(HTTPSRequestTest, FallbackProbeNoCache) {
SpawnedTestServer::SSLOptions ssl_options(
SpawnedTestServer::SSLOptions::CERT_OK);
ssl_options.tls_intolerant =
SpawnedTestServer::SSLOptions::TLS_INTOLERANT_TLS1_2;
ssl_options.tls_intolerance_type =
SpawnedTestServer::SSLOptions::TLS_INTOLERANCE_CLOSE;
ssl_options.record_resume = true;
SpawnedTestServer test_server(
SpawnedTestServer::TYPE_HTTPS,
ssl_options,
base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
ASSERT_TRUE(test_server.Start());
SSLClientSocket::ClearSessionCache();
// Make a connection that does a probe fallback to TLSv1.1 but fails because
// fallback is disabled. We don't wish a session for this connection to be
// inserted locally.
{
TestDelegate delegate;
FallbackTestURLRequestContext context(true);
context.Init();
std::unique_ptr<URLRequest> request(context.CreateRequest(
test_server.GetURL("/"), DEFAULT_PRIORITY, &delegate));
request->Start();
base::RunLoop().Run();
EXPECT_EQ(1, delegate.response_started_count());
EXPECT_FALSE(request->status().is_success());
EXPECT_EQ(URLRequestStatus::FAILED, request->status().status());
EXPECT_EQ(ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION,
request->status().error());
}
// Now allow TLSv1.1 fallback connections and request the session cache log.
{
TestDelegate delegate;
FallbackTestURLRequestContext context(true);
context.set_fallback_min_version(SSL_PROTOCOL_VERSION_TLS1_1);
context.Init();
std::unique_ptr<URLRequest> request(context.CreateRequest(
test_server.GetURL("ssl-session-cache"), DEFAULT_PRIORITY, &delegate));
request->Start();
base::RunLoop().Run();
EXPECT_EQ(1, delegate.response_started_count());
EXPECT_NE(0, delegate.bytes_received());
EXPECT_EQ(
SSL_CONNECTION_VERSION_TLS1_1,
SSLConnectionStatusToVersion(request->ssl_info().connection_status));
EXPECT_TRUE(request->ssl_info().connection_status &
SSL_CONNECTION_VERSION_FALLBACK);
std::vector<std::string> lines;
// If no sessions were cached then the server should have seen two sessions
// inserted with no lookups.
AssertTwoDistinctSessionsInserted(delegate.data_received());
}
}
class HTTPSSessionTest : public testing::Test {
public:
HTTPSSessionTest() : default_context_(true) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment