Ignore client auth request in net::BidirectionalStream.

This CL makes net::BidirectionalStream ignore client auth request, and retry
with no client cert or key.

Bug: 823811
Change-Id: I968e6af703060ec0f50c3f0d0242395d8a56c567
Reviewed-on: https://chromium-review.googlesource.com/973184
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544739}

net/http/bidirectional_stream.cc

@@ -104,10 +104,6 @@ BidirectionalStream::BidirectionalStream(
                    base::Unretained(&request_info_->extra_headers)));
   }
 
-  SSLConfig server_ssl_config;
-  session->ssl_config_service()->GetSSLConfig(&server_ssl_config);
-  session->GetAlpnProtos(&server_ssl_config.alpn_protos);
-
   if (!request_info_->url.SchemeIs(url::kHttpsScheme)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
@@ -116,22 +112,11 @@ BidirectionalStream::BidirectionalStream(
     return;
   }
 
-  HttpRequestInfo http_request_info;
-  http_request_info.url = request_info_->url;
-  http_request_info.method = request_info_->method;
-  http_request_info.extra_headers = request_info_->extra_headers;
-  http_request_info.socket_tag = request_info_->socket_tag;
-  stream_request_ =
-      session->http_stream_factory()->RequestBidirectionalStreamImpl(
-          http_request_info, request_info_->priority, server_ssl_config,
-          server_ssl_config, this,
-          /* enable_ip_based_pooling = */ true,
-          /* enable_alternative_services = */ true, net_log_);
-  // Check that this call cannot fail to set a non-NULL |stream_request_|.
-  DCHECK(stream_request_);
-  // Check that HttpStreamFactory does not invoke OnBidirectionalStreamImplReady
-  // synchronously.
-  DCHECK(!stream_impl_);
+  SSLConfig ssl_config;
+  session->ssl_config_service()->GetSSLConfig(&ssl_config);
+  session->GetAlpnProtos(&ssl_config.alpn_protos);
+
+  StartRequest(ssl_config);
 }
 
 BidirectionalStream::~BidirectionalStream() {
@@ -220,6 +205,26 @@ void BidirectionalStream::PopulateNetErrorDetails(NetErrorDetails* details) {
     stream_impl_->PopulateNetErrorDetails(details);
 }
 
+void BidirectionalStream::StartRequest(const SSLConfig& ssl_config) {
+  DCHECK(!stream_request_);
+  HttpRequestInfo http_request_info;
+  http_request_info.url = request_info_->url;
+  http_request_info.method = request_info_->method;
+  http_request_info.extra_headers = request_info_->extra_headers;
+  http_request_info.socket_tag = request_info_->socket_tag;
+  stream_request_ =
+      session_->http_stream_factory()->RequestBidirectionalStreamImpl(
+          http_request_info, request_info_->priority, ssl_config, ssl_config,
+          this,
+          /* enable_ip_based_pooling = */ true,
+          /* enable_alternative_services = */ true, net_log_);
+  // Check that this call does not fail.
+  DCHECK(stream_request_);
+  // Check that HttpStreamFactory does not invoke OnBidirectionalStreamImplReady
+  // synchronously.
+  DCHECK(!stream_impl_);
+}
+
 void BidirectionalStream::OnStreamReady(bool request_headers_sent) {
   request_headers_sent_ = request_headers_sent;
   if (net_log_.IsCapturing()) {
@@ -402,7 +407,16 @@ void BidirectionalStream::OnNeedsClientAuth(const SSLConfig& used_ssl_config,
                                             SSLCertRequestInfo* cert_info) {
   DCHECK(stream_request_);
 
-  NotifyFailed(ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
+  // BidirectionalStream doesn't support client auth. It ignores client auth
+  // requests with null client cert and key.
+  SSLConfig ssl_config = used_ssl_config;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = nullptr;
+  ssl_config.client_private_key = nullptr;
+  session_->ssl_client_auth_cache()->Add(cert_info->host_and_port, nullptr,
+                                         nullptr);
+  stream_request_ = nullptr;
+  StartRequest(ssl_config);
 }
 
 void BidirectionalStream::OnHttpsProxyTunnelResponse(

net/http/bidirectional_stream.h

@@ -176,6 +176,7 @@ class NET_EXPORT BidirectionalStream : public BidirectionalStreamImpl::Delegate,
   void PopulateNetErrorDetails(NetErrorDetails* details);
 
  private:
+  void StartRequest(const SSLConfig& ssl_config);
   // BidirectionalStreamImpl::Delegate implementation:
   void OnStreamReady(bool request_headers_sent) override;
   void OnHeadersReceived(const SpdyHeaderBlock& response_headers) override;

net/http/bidirectional_stream_unittest.cc

@@ -32,6 +32,7 @@
 #include "net/socket/socket_test_util.h"
 #include "net/spdy/chromium/spdy_session.h"
 #include "net/spdy/chromium/spdy_test_util_common.h"
+#include "net/ssl/ssl_cert_request_info.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/gtest_util.h"
 #include "net/test/test_data_directory.h"
@@ -579,6 +580,70 @@ TEST_F(BidirectionalStreamTest,
   base::RunLoop().RunUntilIdle();
 }
 
+TEST_F(BidirectionalStreamTest, ClientAuthRequestIgnored) {
+  scoped_refptr<SSLCertRequestInfo> cert_request(new SSLCertRequestInfo());
+  cert_request->host_and_port = host_port_pair_;
+
+  // First attempt receives client auth request.
+  SSLSocketDataProvider ssl_data1(ASYNC, ERR_SSL_CLIENT_AUTH_CERT_NEEDED);
+  ssl_data1.next_proto = kProtoHTTP2;
+  ssl_data1.cert_request_info = cert_request.get();
+
+  session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl_data1);
+  StaticSocketDataProvider socket_data1(nullptr, 0, nullptr, 0);
+  session_deps_.socket_factory->AddSocketDataProvider(&socket_data1);
+
+  // Second attempt succeeds.
+  SpdySerializedFrame req(spdy_util_.ConstructSpdyGet(kDefaultUrl, 1, LOWEST));
+  MockWrite writes[] = {
+      CreateMockWrite(req, 0),
+  };
+  SpdySerializedFrame resp(spdy_util_.ConstructSpdyGetReply(nullptr, 0, 1));
+  SpdySerializedFrame body_frame(spdy_util_.ConstructSpdyDataFrame(1, true));
+  MockRead reads[] = {
+      CreateMockRead(resp, 1), CreateMockRead(body_frame, 2),
+      MockRead(SYNCHRONOUS, net::OK, 3),
+  };
+
+  SSLSocketDataProvider ssl_data2(ASYNC, OK);
+  ssl_data2.next_proto = kProtoHTTP2;
+  session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl_data2);
+  SequencedSocketData socket_data2(reads, arraysize(reads), writes,
+                                   arraysize(writes));
+  session_deps_.socket_factory->AddSocketDataProvider(&socket_data2);
+
+  http_session_ = SpdySessionDependencies::SpdyCreateSession(&session_deps_);
+  SpdySessionKey key(host_port_pair_, ProxyServer::Direct(),
+                     PRIVACY_MODE_DISABLED, SocketTag());
+  std::unique_ptr<BidirectionalStreamRequestInfo> request_info(
+      new BidirectionalStreamRequestInfo);
+  request_info->method = "GET";
+  request_info->url = default_url_;
+  request_info->end_stream_on_headers = true;
+  request_info->priority = LOWEST;
+
+  scoped_refptr<IOBuffer> read_buffer(new IOBuffer(kReadBufferSize));
+  std::unique_ptr<TestDelegateBase> delegate(
+      new TestDelegateBase(read_buffer.get(), kReadBufferSize));
+
+  delegate->SetRunUntilCompletion(true);
+  delegate->Start(std::move(request_info), http_session_.get());
+
+  // Ensure the certificate was added to the client auth cache.
+  scoped_refptr<X509Certificate> client_cert;
+  scoped_refptr<SSLPrivateKey> client_private_key;
+  ASSERT_TRUE(http_session_->ssl_client_auth_cache()->Lookup(
+      host_port_pair_, &client_cert, &client_private_key));
+  ASSERT_FALSE(client_cert);
+  ASSERT_FALSE(client_private_key);
+
+  const SpdyHeaderBlock& response_headers = delegate->response_headers();
+  EXPECT_EQ("200", response_headers.find(":status")->second);
+  EXPECT_EQ(1, delegate->on_data_read_count());
+  EXPECT_EQ(0, delegate->on_data_sent_count());
+  EXPECT_EQ(kProtoHTTP2, delegate->GetProtocol());
+}
+
 // Simulates user calling ReadData after END_STREAM has been received in
 // BidirectionalStreamSpdyImpl.
 TEST_F(BidirectionalStreamTest, TestReadDataAfterClose) {