net: don't remember TLS intolerant servers.

net: don't remember TLS intolerant servers.

I've seen a couple of reports recently where is was clear that an SNI-only
hostname had ended up in the list of TLS-intolerant servers. The result is that
the user sees the non-SNI certificate for that IP address, which doesn't match
the requested hostname. The only way to clear this is to restart Chrome.

This change partly reverts r45088 so that we will no longer remember
TLS-intolerant servers. This means that we'll perform SSLv3 fallback for every
connection, if needed. That's unfortunate for truly TLS-intolerant servers, but
it also means that we'll get back to TLS much faster in the event of a
transient network error trigger fallback.

BUG=none
TEST=net_unittests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134129

[wtc, 2012-04-26 00:49:14]

Patch set 1 LGTM.

https://chromiumcodereview.appspot.com/10218007/diff/1/net/http/http_stream_f...
File net/http/http_stream_factory_impl_job.cc (left):

https://chromiumcodereview.appspot.com/10218007/diff/1/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:1056: << origin_server.ToString();

Move this warning message to net/http/http_network_transaction.cc,
unless you think the warning message is not useful.

https://chromiumcodereview.appspot.com/10218007/diff/1/net/test/base_test_ser...
File net/test/base_test_server.h (right):

https://chromiumcodereview.appspot.com/10218007/diff/1/net/test/base_test_ser...
net/test/base_test_server.h:131: // test SSLv3 fallback.

COMMENT BUG: I was confused by this comment because I thought
by "abrutly closing a TLS connection" you meant closing the
underlying TCP connection, which is one behavior of TLS-intolerant
servers.

How about "the server will abort a TLS handshake with a handshake_failure alert"
or just "the server will abort any TLS handshakes"?

Please make the same change to the comments in the Python files.

https://chromiumcodereview.appspot.com/10218007/diff/1/net/url_request/url_re...
File net/url_request/url_request_unittest.cc (right):

https://chromiumcodereview.appspot.com/10218007/diff/1/net/url_request/url_re...
net/url_request/url_request_unittest.cc:1748: EXPECT_NE(0, d.bytes_received());

Is it easy to check that SSLv3 was used?

I guess a nonzero d.response_started_count() implies that
SSLv3 fallback must have succeeded.

[agl, 2012-04-26 17:03:29]

https://chromiumcodereview.appspot.com/10218007/diff/1/net/http/http_stream_f...
File net/http/http_stream_factory_impl_job.cc (left):

https://chromiumcodereview.appspot.com/10218007/diff/1/net/http/http_stream_f...
net/http/http_stream_factory_impl_job.cc:1056: << origin_server.ToString();
On 2012/04/26 00:49:14, wtc wrote:
> 
> Move this warning message to net/http/http_network_transaction.cc,
> unless you think the warning message is not useful.

Done.

https://chromiumcodereview.appspot.com/10218007/diff/1/net/test/base_test_ser...
File net/test/base_test_server.h (right):

https://chromiumcodereview.appspot.com/10218007/diff/1/net/test/base_test_ser...
net/test/base_test_server.h:131: // test SSLv3 fallback.
On 2012/04/26 00:49:14, wtc wrote:
> 
> COMMENT BUG: I was confused by this comment because I thought
> by "abrutly closing a TLS connection" you meant closing the
> underlying TCP connection, which is one behavior of TLS-intolerant
> servers.
> 
> How about "the server will abort a TLS handshake with a handshake_failure
alert"
> or just "the server will abort any TLS handshakes"?
> 
> Please make the same change to the comments in the Python files.

Done.

https://chromiumcodereview.appspot.com/10218007/diff/1/net/url_request/url_re...
File net/url_request/url_request_unittest.cc (right):

https://chromiumcodereview.appspot.com/10218007/diff/1/net/url_request/url_re...
net/url_request/url_request_unittest.cc:1748: EXPECT_NE(0, d.bytes_received());
On 2012/04/26 00:49:14, wtc wrote:
> Is it easy to check that SSLv3 was used?

I remembered that we have a connection_status flag for this already
(SSL_CONNECTION_SSL3_FALLBACK) so I've added a test for that.

[commit-bot: I haz the power, 2012-04-26 17:03:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/agl@chromium.org/10218007/6001

[commit-bot: I haz the power, 2012-04-26 18:45:35]

Change committed as 134129