Move channel id tests up from OpenSSL and update channelid version.

net/socket/ssl_client_socket_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/ssl_client_socket.h"
 
 #include "base/callback_helpers.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "net/base/address_list.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_log_unittest.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/mock_cert_verifier.h"
 #include "net/cert/test_root_certs.h"
 #include "net/dns/host_resolver.h"
 #include "net/http/transport_security_state.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/socket_test_util.h"
 #include "net/socket/tcp_client_socket.h"
+#include "net/ssl/default_server_bound_cert_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_config_service.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 //-----------------------------------------------------------------------------
 
 namespace net {
@@ skipping 515 matching lines @@
     }
     SetResult(result);
   }
 
   StreamSocket* socket_;
   CompletionCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DeleteSocketCallback);
 };
 
+// A ServerBoundCertStore that always returns an error when asked for a
+// certificate.
+class FailingServerBoundCertStore : public ServerBoundCertStore {
+  virtual int GetServerBoundCert(const std::string& server_identifier,
+                                 base::Time* expiration_time,
+                                 std::string* private_key_result,
+                                 std::string* cert_result,
+                                 const GetCertCallback& callback) OVERRIDE {
+    return ERR_UNEXPECTED;
+  }
+  virtual void SetServerBoundCert(const std::string& server_identifier,
+                                  base::Time creation_time,
+                                  base::Time expiration_time,
+                                  const std::string& private_key,
+                                  const std::string& cert) OVERRIDE {}
+  virtual void DeleteServerBoundCert(const std::string& server_identifier,
+                                     const base::Closure& completion_callback)
+      OVERRIDE {}
+  virtual void DeleteAllCreatedBetween(base::Time delete_begin,
+                                       base::Time delete_end,
+                                       const base::Closure& completion_callback)
+      OVERRIDE {}
+  virtual void DeleteAll(const base::Closure& completion_callback) OVERRIDE {}
+  virtual void GetAllServerBoundCerts(const GetCertListCallback& callback)
+      OVERRIDE {}
+  virtual int GetCertCount() OVERRIDE { return 0; }
+  virtual void SetForceKeepSessionState() OVERRIDE {}
+};
+
 class SSLClientSocketTest : public PlatformTest {
  public:
   SSLClientSocketTest()
       : socket_factory_(ClientSocketFactory::GetDefaultFactory()),
         cert_verifier_(new MockCertVerifier),
         transport_security_state_(new TransportSecurityState) {
     cert_verifier_->set_default_result(OK);
     context_.cert_verifier = cert_verifier_.get();
     context_.transport_security_state = transport_security_state_.get();
   }
@@ skipping 136 matching lines @@
       EXPECT_LT(0, rv);
     } else {
       // False Start is not enabled, so the handshake will not complete because
       // the server second leg is blocked.
       base::RunLoop().RunUntilIdle();
       EXPECT_FALSE(callback.have_result());
     }
   }
 };
 
+class SSLClientSocketChannelIDTest : public SSLClientSocketTest {

[wtc, 2014/05/06 17:56:24]

Nit: I think it is a good idea to copy the ConnectToTestServer and
CreateAndConnectSSLClientSocket methods to this class. Right now the code of
these two methods is inlined in the two unit tests.

[haavardm, 2014/05/07 13:53:43]

I did that to follow the code pattern of the rest of this file. In that case it
would make more sense to add it to SSLClientSocketTest. 

+ protected:
+  void EnabledChannelID() {

[Ryan Sleevi, 2014/05/06 18:45:26]

these should be "EnableChannelID()" / "EnableFailingChannelID" (note the
present, active voice rather than past-tense)

[haavardm, 2014/05/07 13:53:43]

Done.

+    cert_service_.reset(
+        new ServerBoundCertService(new DefaultServerBoundCertStore(NULL),
+                                   base::MessageLoopProxy::current()));
+    context_.server_bound_cert_service = cert_service_.get();
+  }
+
+  void EnabledFailingChannelID() {
+    cert_service_.reset(new ServerBoundCertService(
+        new FailingServerBoundCertStore(), base::MessageLoopProxy::current()));
+    context_.server_bound_cert_service = cert_service_.get();
+  }
+
+  scoped_ptr<ServerBoundCertService> cert_service_;

[wtc, 2014/05/06 17:56:24]

The cert_service_ member should be private.

+};
+
 //-----------------------------------------------------------------------------
 
 // LogContainsSSLConnectEndEvent returns true if the given index in the given
 // log is an SSL connect end event. The NSS sockets will cork in an attempt to
 // merge the first application data record with the Finished message when false
 // starting. However, in order to avoid the server timing out the handshake,
 // they'll give up waiting for application data and send the Finished after a
 // timeout. This means that an SSL connect end event may appear as a socket
 // write.
 static bool LogContainsSSLConnectEndEvent(
@@ skipping 1630 matching lines @@
 TEST_F(SSLClientSocketFalseStartTest, NoForwardSecrecy) {
   SpawnedTestServer::SSLOptions server_options;
   server_options.key_exchanges =
       SpawnedTestServer::SSLOptions::KEY_EXCHANGE_RSA;
   server_options.enable_npn = true;
   SSLConfig client_config;
   client_config.next_protos.push_back("http/1.1");
   TestFalseStart(server_options, client_config, false);
 }
 
+// Connect to a server using channel id. It should allow the connection.
+TEST_F(SSLClientSocketChannelIDTest, SendChannelID) {
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
+                                SpawnedTestServer::kLocalhost,
+                                base::FilePath());
+
+  SpawnedTestServer::SSLOptions ssl_options;

[wtc, 2014/05/06 17:56:24]

Remove the ssl_options variable in both unit tests. It is not used.
Alternatively, pass ssl_options as the second argument to the test_server
constructor.

[haavardm, 2014/05/07 13:53:43]

Done.

+  ASSERT_TRUE(test_server.Start());
+
+  AddressList addr;
+  ASSERT_TRUE(test_server.GetAddressList(&addr));
+
+  TestCompletionCallback callback;
+  scoped_ptr<StreamSocket> transport(
+      new TCPClientSocket(addr, NULL, NetLog::Source()));

[wtc, 2014/05/06 17:56:24]

Just wanted to confirm that you intended to pass NULL as the second argument to
the TCPClientSocket constructor. I saw some tests that declare a local variable
"CapturingNetLog log;" and pass &log. I guess these two tests don't need that.

[haavardm, 2014/05/07 13:53:43]

Yes, that was as intended. Some tests only pass if certain events are present in
the log. That is not needed for these tests.

+  int rv = transport->Connect(callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  EnabledChannelID();
+  SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.channel_id_enabled = true;
+
+  scoped_ptr<SSLClientSocket> sock(CreateSSLClientSocket(
+      transport.Pass(), test_server.host_port_pair(), kDefaultSSLConfig));

[wtc, 2014/05/06 17:56:24]

1. BUG: the variable ssl_config is set but not used. ssl_config instead of
kDefaultSSLConfig should be passed here.

Did the test pass in spite of this bug?

2. The FailingChannelID test has the same bug.

[haavardm, 2014/05/07 13:53:43]

Hm, the evils of copy and paste.. Yes the tests passed. The simple reason is
that channel_id_enabled is on by default.

I will set it explicitly anyway, in case the default value changes.

Done.

+
+  rv = sock->Connect(callback.callback());

[wtc, 2014/05/06 17:56:24]

Just wanted to point out that you omitted them check

    if (sock_->IsConnected()) {
      LOG(ERROR) << "SSL Socket prematurely connected";
      return false;
    }

in the original code. I think this is OK.

+  if (rv == ERR_IO_PENDING)
+    rv = callback.WaitForResult();
+   EXPECT_EQ(OK, rv);

[wtc, 2014/05/06 17:56:24]

Nit: fix indentation.

[haavardm, 2014/05/07 13:53:43]

Done.

+  EXPECT_TRUE(sock->IsConnected());
+  EXPECT_TRUE(sock->WasChannelIDSent());
+
+  sock->Disconnect();
+  EXPECT_FALSE(sock->IsConnected());
+}
+
+// Connect to a server using channel id but without sending a key. It should
+// fail.
+TEST_F(SSLClientSocketChannelIDTest, FailingChannelID) {
+  SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
+                                SpawnedTestServer::kLocalhost,
+                                base::FilePath());
+
+  SpawnedTestServer::SSLOptions ssl_options;
+  ASSERT_TRUE(test_server.Start());
+
+  AddressList addr;
+  ASSERT_TRUE(test_server.GetAddressList(&addr));
+
+  TestCompletionCallback callback;
+  scoped_ptr<StreamSocket> transport(
+      new TCPClientSocket(addr, NULL, NetLog::Source()));
+  int rv = transport->Connect(callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = callback.WaitForResult();
+  EXPECT_EQ(OK, rv);
+
+  EnabledFailingChannelID();
+  SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.channel_id_enabled = true;
+
+  scoped_ptr<SSLClientSocket> sock(CreateSSLClientSocket(
+      transport.Pass(), test_server.host_port_pair(), kDefaultSSLConfig));
+
+  rv = sock->Connect(callback.callback());
+  if (rv == ERR_IO_PENDING)
+    rv = callback.WaitForResult();
+
+  EXPECT_EQ(ERR_UNEXPECTED, rv);
+  EXPECT_FALSE(sock->IsConnected());
+}
+
 }  // namespace net