Ensure that when using False Start + client auth, bad client certificates are not cached

net/http/http_network_transaction_unittest.cc

Index: net/http/http_network_transaction_unittest.cc
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 260de13f9a3cbe2fdbe1ade93d82454d1b82077a..cb193ed7b7dc7a36de1c4bd23763da8d4bf3be89 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -20,6 +20,7 @@
 #include "net/base/net_log.h"
 #include "net/base/net_log_unittest.h"
 #include "net/base/request_priority.h"
+#include "net/base/ssl_cert_request_info.h"
 #include "net/base/ssl_config_service_defaults.h"
 #include "net/base/ssl_info.h"
 #include "net/base/test_completion_callback.h"
@@ -8207,4 +8208,148 @@ TEST_F(HttpNetworkTransactionTest, NPNMispredict) {
   EXPECT_EQ("hello world", contents);
 }
 
+// Test that a "bad" client certificate is not cached when TLS False
+// Start is not being used.
+TEST_F(HttpNetworkTransactionTest, ClientAuthCertCache_NoFalseStart) {
+  SessionDependencies session_deps;
+
+  // When False Start is disabled, the result of Connect() is the actual
+  // result of the SSL handshake.
+  // [ssl_]data1 = Aborted handshake for client auth, TLSv1

[Ryan Hamilton, 2011/01/05 17:35:02]

Would it make sense to elaborate on the comment here?  If I'm reading this code
right, the test does the following:
1) Attempts to establish an SSL connection without providing a SSL client
certificate.  This causes a failure with the error code
ERR_SSL_CLIENT_AUTH_CERT_NEEDED.
2) Reconnect, and decline to provide a certificate (which should have similar
behavior to providing a client certificate that the server rejects) by setting a
NULL entry in the SSL cache.   This should result in ERR_SSL_PROTOCOL_ERROR and
the entry from the cache should be removed.

Is that right?

I assume, also, that the second test is basically doing the same thing, except
that the ssl errors are coming not from the connect, but rather from the reads?

+  // [ssl_]data2 = Failed handshake due to client auth, TLSv1
+  // [ssl_]data3 = Failed handshake, SSLv3 fallback (http://crbug.com/66517)

[Ryan Hamilton, 2011/01/05 17:35:02]

Is ssl_data3 used?  I only see one place where ERR_SSL_PROTOCOL_ERROR is
checked, but I would have expected to see 2 places...

+
+  SSLSocketDataProvider ssl_data1(true /* async */,
+                                  net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
+  SSLSocketDataProvider ssl_data2(true /* async */,
+                                  net::ERR_SSL_PROTOCOL_ERROR);
+  SSLSocketDataProvider ssl_data3(true /* async */,
+                                  net::ERR_SSL_PROTOCOL_ERROR);
+  scoped_refptr<SSLCertRequestInfo> cert_request(new SSLCertRequestInfo());
+  cert_request->host_and_port = "www.example.com:443";
+  ssl_data1.cert_request_info = cert_request.get();
+  ssl_data2.cert_request_info = cert_request.get();
+  ssl_data3.cert_request_info = cert_request.get();
+
+  net::StaticSocketDataProvider data1(NULL, 0, NULL, 0);
+  net::StaticSocketDataProvider data2(NULL, 0, NULL, 0);
+  net::StaticSocketDataProvider data3(NULL, 0, NULL, 0);
+
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl_data1);
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl_data2);
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl_data3);
+  session_deps.socket_factory.AddSocketDataProvider(&data1);
+  session_deps.socket_factory.AddSocketDataProvider(&data2);
+  session_deps.socket_factory.AddSocketDataProvider(&data3);
+
+  scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+  scoped_ptr<HttpNetworkTransaction> trans(new HttpNetworkTransaction(session));
+
+  net::HttpRequestInfo request_info;
+  request_info.url = GURL("https://www.example.com/");
+  request_info.method = "GET";
+  request_info.load_flags = net::LOAD_NORMAL;
+
+  TestCompletionCallback callback;
+  int rv = trans->Start(&request_info, &callback, net::BoundNetLog());
+  ASSERT_EQ(net::ERR_IO_PENDING, rv);
+  rv = callback.WaitForResult();
+  ASSERT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv);
+
+  // Supply a dummy certificate. The SSLClientAuthCertCache tests ensure
+  // this test should behave the same as if this test used a real cert.
+  rv = trans->RestartWithCertificate(NULL, &callback);
+  ASSERT_EQ(net::ERR_IO_PENDING, rv);
+
+  // Ensure the certificate was added to the client auth cache before
+  // allowing the connection to continue restarting.
+  scoped_refptr<X509Certificate> client_cert;
+  ASSERT_TRUE(session->ssl_client_auth_cache()->Lookup("www.example.com:443",
+                                                       &client_cert));
+  ASSERT_EQ(NULL, client_cert.get());
+  rv = callback.WaitForResult();
+
+  // Now that the new handshake has failed, ensure that the client
+  // certificate was removed from the client auth cache.
+  ASSERT_EQ(net::ERR_SSL_PROTOCOL_ERROR, rv);
+  ASSERT_FALSE(session->ssl_client_auth_cache()->Lookup("www.example.com:443",
+                                                        &client_cert));
+}
+
+// Test that a "bad" client certificate is not cached when TLS False
+// Start is being used.
+TEST_F(HttpNetworkTransactionTest, ClientAuthCertCache_FalseStart) {
+  SessionDependencies session_deps;
+
+  // [ssl_]data1 = Aborted handshake (due to client auth), TLSv1
+  // [ssl_]data2 = Failed handshake, TLSv1
+  // [ssl_]data3 = Failed handshake, SSLv3 fallback (http://crbug.com/66517)
+  // When False Start is used, the Connect() call is an artificial success.
+  SSLSocketDataProvider ssl_data1(true /* async */, net::OK);
+  SSLSocketDataProvider ssl_data2(true /* async */, net::OK);
+  SSLSocketDataProvider ssl_data3(true /* async */, net::OK);
+
+  scoped_refptr<SSLCertRequestInfo> cert_request(new SSLCertRequestInfo());
+  cert_request->host_and_port = "www.example.com:443";
+  ssl_data1.cert_request_info = cert_request.get();
+  ssl_data2.cert_request_info = cert_request.get();
+  ssl_data3.cert_request_info = cert_request.get();
+
+  // When TLS False Start is used, the SSL handshake is completed by the
+  // first attempt to Read from the socket. Ensure that even if the error is
+  // delayed until Read, the client cert is still evicted from the cache.
+  net::MockRead data1_reads[] = {
+    net::MockRead(true /* async */, net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED),
+  };
+  net::MockRead data2_reads[] = {
+    net::MockRead(true /* async */, net::ERR_SSL_PROTOCOL_ERROR),
+  };
+  net::StaticSocketDataProvider data1(
+      data1_reads, arraysize(data1_reads), NULL, 0);
+  net::StaticSocketDataProvider data2(
+      data2_reads, arraysize(data2_reads), NULL, 0);
+  net::StaticSocketDataProvider data3(
+      data2_reads, arraysize(data2_reads), NULL, 0);
+
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl_data1);
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl_data2);
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl_data3);
+  session_deps.socket_factory.AddSocketDataProvider(&data1);
+  session_deps.socket_factory.AddSocketDataProvider(&data2);
+  session_deps.socket_factory.AddSocketDataProvider(&data3);
+
+  scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+  scoped_ptr<HttpNetworkTransaction> trans(new HttpNetworkTransaction(session));
+
+  net::HttpRequestInfo request_info;
+  request_info.url = GURL("https://www.example.com/");
+  request_info.method = "GET";
+  request_info.load_flags = net::LOAD_NORMAL;
+
+  TestCompletionCallback callback;
+  int rv = trans->Start(&request_info, &callback, net::BoundNetLog());
+  ASSERT_EQ(net::ERR_IO_PENDING, rv);
+  rv = callback.WaitForResult();
+  ASSERT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv);
+
+  // Supply a dummy certificate. The SSLClientAuthCertCache tests ensure
+  // this test should behave the same as if this test used a real cert.
+  rv = trans->RestartWithCertificate(NULL, &callback);
+  ASSERT_EQ(net::ERR_IO_PENDING, rv);
+
+  // Ensure the certificate was added to the client auth cache before
+  // allowing the connection to continue restarting.
+  scoped_refptr<X509Certificate> client_cert;
+  ASSERT_TRUE(session->ssl_client_auth_cache()->Lookup("www.example.com:443",
+                                                       &client_cert));
+  ASSERT_EQ(NULL, client_cert.get());
+  rv = callback.WaitForResult();
+
+  // Now that the new handshake has failed, ensure that the client
+  // certificate was removed from the client auth cache.
+  ASSERT_EQ(net::ERR_SSL_PROTOCOL_ERROR, rv);
+  ASSERT_FALSE(session->ssl_client_auth_cache()->Lookup("www.example.com:443",
+                                                        &client_cert));
+}
+
 }  // namespace net