Remove Channel ID code from SSLClientSocketImpl

Bug: 875053
Change-Id: I3d798801537d6ede56d2f7acd8c26982e7291313
Reviewed-on: https://chromium-review.googlesource.com/c/1325568
Commit-Queue: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606568}

net/log/net_log_event_type_list.h

@@ -487,22 +487,6 @@ EVENT_TYPE(SSL_CLIENT_CERT_REQUESTED)
 //   }
 EVENT_TYPE(SSL_PRIVATE_KEY_OP)
 
-// The start/end of getting a Channel ID key.
-//
-// The START event contains these parameters:
-//   {
-//     "ephemeral": <Whether or not the Channel ID store is ephemeral>,
-//     "service": <Unique identifier for the ChannelIDService used>,
-//     "store": <Unique identifier for the ChannelIDStore used>,
-//   }
-//
-// The END event may contain these parameters:
-//   {
-//     "net_error": <Net error code>,
-//     "key": <Hex-encoded EC point of public key (uncompressed point format)>,
-//   }
-EVENT_TYPE(SSL_GET_CHANNEL_ID)
-
 // A client certificate (or none) was provided to the SSL library to be sent
 // to the SSL server.
 // The following parameters are attached to the event:

net/socket/ssl_client_socket_impl.cc

@@ -86,32 +86,6 @@ std::unique_ptr<base::Value> NetLogPrivateKeyOperationCallback(
   return std::move(value);
 }
 
-std::unique_ptr<base::Value> NetLogChannelIDLookupCallback(
-    ChannelIDService* channel_id_service,
-    NetLogCaptureMode capture_mode) {
-  ChannelIDStore* store = channel_id_service->GetChannelIDStore();
-  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
-  dict->SetBoolean("ephemeral", store->IsEphemeral());
-  dict->SetString("service", base::HexEncode(&channel_id_service,
-                                             sizeof(channel_id_service)));
-  dict->SetString("store", base::HexEncode(&store, sizeof(store)));
-  return std::move(dict);
-}
-
-std::unique_ptr<base::Value> NetLogChannelIDLookupCompleteCallback(
-    crypto::ECPrivateKey* key,
-    int result,
-    NetLogCaptureMode capture_mode) {
-  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
-  dict->SetInteger("net_error", result);
-  std::string raw_key;
-  if (result == OK && key && key->ExportRawPublicKey(&raw_key)) {
-    std::string key_to_log = base::HexEncode(raw_key.data(), raw_key.length());
-    dict->SetString("key", key_to_log);
-  }
-  return std::move(dict);
-}
-
 std::unique_ptr<base::Value> NetLogSSLInfoCallback(
     SSLClientSocketImpl* socket,
     NetLogCaptureMode capture_mode) {
@@ -450,7 +424,6 @@ SSLClientSocketImpl::SSLClientSocketImpl(
       was_ever_used_(false),
       cert_verifier_(context.cert_verifier),
       cert_transparency_verifier_(context.cert_transparency_verifier),
-      channel_id_service_(context.channel_id_service),
       transport_(std::move(transport_socket)),
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
@@ -459,7 +432,6 @@ SSLClientSocketImpl::SSLClientSocketImpl(
       in_confirm_handshake_(false),
       disconnected_(false),
       negotiated_protocol_(kProtoUnknown),
-      channel_id_sent_(false),
       certificate_verified_(false),
       certificate_requested_(false),
       signature_result_(kSSLClientSocketNoPendingResult),
@@ -542,7 +514,6 @@ void SSLClientSocketImpl::Disconnect() {
 
   // Shut down anything that may call us back.
   cert_verifier_request_.reset();
-  channel_id_request_.Cancel();
   weak_factory_.InvalidateWeakPtrs();
   transport_adapter_.reset();
 
@@ -654,7 +625,6 @@ bool SSLClientSocketImpl::GetSSLInfo(SSLInfo* ssl_info) {
   ssl_info->public_key_hashes = server_cert_verify_result_.public_key_hashes;
   ssl_info->client_cert_sent =
       ssl_config_.send_client_cert && ssl_config_.client_cert.get();
-  ssl_info->channel_id_sent = channel_id_sent_;
   ssl_info->pinning_failure_log = pinning_failure_log_;
   ssl_info->ocsp_result = server_cert_verify_result_.ocsp_result;
   ssl_info->is_fatal_cert_error = is_fatal_cert_error_;
@@ -731,14 +701,6 @@ void SSLClientSocketImpl::GetSSLCertRequestInfo(
   }
 }
 
-ChannelIDService* SSLClientSocketImpl::GetChannelIDService() const {
-  return channel_id_service_;
-}
-
-crypto::ECPrivateKey* SSLClientSocketImpl::GetChannelIDKey() const {
-  return channel_id_key_.get();
-}
-
 void SSLClientSocketImpl::ApplySocketTag(const SocketTag& tag) {
   return transport_->socket()->ApplySocketTag(tag);
 }
@@ -929,11 +891,6 @@ int SSLClientSocketImpl::Init() {
     return ERR_UNEXPECTED;
   }
 
-  // TLS channel ids.
-  if (IsChannelIDEnabled()) {
-    SSL_enable_tls_channel_id(ssl_.get());
-  }
-
   if (!ssl_config_.alpn_protos.empty()) {
     std::vector<uint8_t> wire_protos =
         SerializeNextProtos(ssl_config_.alpn_protos);
@@ -982,12 +939,6 @@ int SSLClientSocketImpl::DoHandshake() {
   int net_error = OK;
   if (rv <= 0) {
     int ssl_error = SSL_get_error(ssl_.get(), rv);
-    if (ssl_error == SSL_ERROR_WANT_CHANNEL_ID_LOOKUP) {
-      // The server supports channel ID. Stop to look one up before returning to
-      // the handshake.
-      next_handshake_state_ = STATE_CHANNEL_ID_LOOKUP;
-      return OK;
-    }
     if (ssl_error == SSL_ERROR_WANT_X509_LOOKUP &&
         !ssl_config_.send_client_cert) {
       return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
@@ -1072,40 +1023,6 @@ int SSLClientSocketImpl::DoHandshakeComplete(int result) {
   return OK;
 }
 
-int SSLClientSocketImpl::DoChannelIDLookup() {
-  NetLogParametersCallback callback = base::Bind(
-      &NetLogChannelIDLookupCallback, base::Unretained(channel_id_service_));
-  net_log_.BeginEvent(NetLogEventType::SSL_GET_CHANNEL_ID, callback);
-  next_handshake_state_ = STATE_CHANNEL_ID_LOOKUP_COMPLETE;
-  return channel_id_service_->GetOrCreateChannelID(
-      host_and_port_.host(), &channel_id_key_,
-      base::Bind(&SSLClientSocketImpl::OnHandshakeIOComplete,
-                 base::Unretained(this)),
-      &channel_id_request_);
-}
-
-int SSLClientSocketImpl::DoChannelIDLookupComplete(int result) {
-  net_log_.EndEvent(NetLogEventType::SSL_GET_CHANNEL_ID,
-                    base::Bind(&NetLogChannelIDLookupCompleteCallback,
-                               channel_id_key_.get(), result));
-  if (result < 0)
-    return result;
-
-  // Hand the key to OpenSSL. Check for error in case OpenSSL rejects the key
-  // type.
-  DCHECK(channel_id_key_);
-  crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
-  if (!SSL_set1_tls_channel_id(ssl_.get(), channel_id_key_->key())) {
-    LOG(ERROR) << "Failed to set Channel ID.";
-    return ERR_FAILED;
-  }
-
-  // Return to the handshake.
-  channel_id_sent_ = true;
-  next_handshake_state_ = STATE_HANDSHAKE;
-  return OK;
-}
-
 int SSLClientSocketImpl::DoVerifyCert(int result) {
   DCHECK(start_cert_verification_time_.is_null());
 
@@ -1317,13 +1234,6 @@ int SSLClientSocketImpl::DoHandshakeLoop(int last_io_result) {
       case STATE_HANDSHAKE_COMPLETE:
         rv = DoHandshakeComplete(rv);
         break;
-      case STATE_CHANNEL_ID_LOOKUP:
-        DCHECK_EQ(OK, rv);
-        rv = DoChannelIDLookup();
-        break;
-      case STATE_CHANNEL_ID_LOOKUP_COMPLETE:
-        rv = DoChannelIDLookupComplete(rv);
-        break;
       case STATE_VERIFY_CERT:
         DCHECK_EQ(OK, rv);
         rv = DoVerifyCert(rv);
@@ -1687,7 +1597,6 @@ std::string SSLClientSocketImpl::GetSessionCacheKey() const {
   result.append(ssl_session_cache_shard_);
 
   result.push_back('/');
-  result.push_back(ssl_config_.channel_id_enabled ? '1' : '0');
   result.push_back(ssl_config_.version_interference_probe ? '1' : '0');
   return result;
 }
@@ -1810,10 +1719,6 @@ void SSLClientSocketImpl::RecordNegotiatedProtocol() const {
                             negotiated_protocol_, kProtoLast + 1);
 }
 
-bool SSLClientSocketImpl::IsChannelIDEnabled() const {
-  return ssl_config_.channel_id_enabled && channel_id_service_;
-}
-
 int SSLClientSocketImpl::MapLastOpenSSLError(
     int ssl_error,
     const crypto::OpenSSLErrStackTracer& tracer,

net/socket/ssl_client_socket_impl.h

@@ -27,7 +27,6 @@
 #include "net/socket/next_proto.h"
 #include "net/socket/socket_bio_adapter.h"
 #include "net/socket/ssl_client_socket.h"
-#include "net/ssl/channel_id_service.h"
 #include "net/ssl/openssl_ssl_util.h"
 #include "net/ssl/ssl_client_cert_type.h"
 #include "net/ssl/ssl_config.h"
@@ -102,8 +101,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
   void DumpMemoryStats(SocketMemoryStats* stats) const override;
   void GetSSLCertRequestInfo(
       SSLCertRequestInfo* cert_request_info) const override;
-  ChannelIDService* GetChannelIDService() const override;
-  crypto::ECPrivateKey* GetChannelIDKey() const override;
 
   void ApplySocketTag(const SocketTag& tag) override;
 
@@ -142,8 +139,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
 
   int DoHandshake();
   int DoHandshakeComplete(int result);
-  int DoChannelIDLookup();
-  int DoChannelIDLookupComplete(int result);
   int DoVerifyCert(int result);
   int DoVerifyCertComplete(int result);
   void DoConnectCallback(int result);
@@ -218,9 +213,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
   // in a UMA histogram.
   void RecordNegotiatedProtocol() const;
 
-  // Returns whether TLS channel ID is enabled.
-  bool IsChannelIDEnabled() const;
-
   // Returns the net error corresponding to the most recent OpenSSL
   // error. ssl_error is the output of SSL_get_error.
   int MapLastOpenSSLError(int ssl_error,
@@ -271,9 +263,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
   ct::CTVerifyResult ct_verify_result_;
   CTVerifier* cert_transparency_verifier_;
 
-  // The service for retrieving Channel ID keys.  May be NULL.
-  ChannelIDService* channel_id_service_;
-
   // OpenSSL stuff
   bssl::UniquePtr<SSL> ssl_;
 
@@ -290,8 +279,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
     STATE_NONE,
     STATE_HANDSHAKE,
     STATE_HANDSHAKE_COMPLETE,
-    STATE_CHANNEL_ID_LOOKUP,
-    STATE_CHANNEL_ID_LOOKUP_COMPLETE,
     STATE_VERIFY_CERT,
     STATE_VERIFY_CERT_COMPLETE,
   };
@@ -304,10 +291,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
   bool disconnected_;
 
   NextProto negotiated_protocol_;
-  // Written by the |channel_id_service_|.
-  std::unique_ptr<crypto::ECPrivateKey> channel_id_key_;
-  // True if a channel ID was sent.
-  bool channel_id_sent_;
   // If non-null, the newly-established to be inserted into the session cache
   // once certificate verification is done.
   bssl::UniquePtr<SSL_SESSION> pending_session_;
@@ -315,8 +298,6 @@ class SSLClientSocketImpl : public SSLClientSocket,
   bool certificate_verified_;
   // Set to true if a CertificateRequest was received.
   bool certificate_requested_;
-  // The request handle for |channel_id_service_|.
-  ChannelIDService::Request channel_id_request_;
 
   int signature_result_;
   std::vector<uint8_t> signature_;

net/socket/ssl_client_socket_unittest.cc

@@ -59,8 +59,6 @@
 #include "net/socket/stream_socket.h"
 #include "net/socket/tcp_client_socket.h"
 #include "net/socket/tcp_server_socket.h"
-#include "net/ssl/channel_id_service.h"
-#include "net/ssl/default_channel_id_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_config_service.h"
 #include "net/ssl/ssl_connection_status_flags.h"
@@ -707,57 +705,6 @@ class DeleteSocketCallback : public TestCompletionCallbackBase {
   DISALLOW_COPY_AND_ASSIGN(DeleteSocketCallback);
 };
 
-// A ChannelIDStore that always returns an error when asked for a
-// channel id.
-class FailingChannelIDStore : public ChannelIDStore {
-  int GetChannelID(const std::string& server_identifier,
-                   std::unique_ptr<crypto::ECPrivateKey>* key_result,
-                   GetChannelIDCallback callback) override {
-    return ERR_UNEXPECTED;
-  }
-  void SetChannelID(std::unique_ptr<ChannelID> channel_id) override {}
-  void DeleteChannelID(const std::string& server_identifier,
-                       base::OnceClosure completion_callback) override {}
-  void DeleteForDomainsCreatedBetween(
-      const base::Callback<bool(const std::string&)>& domain_predicate,
-      base::Time delete_begin,
-      base::Time delete_end,
-      base::OnceClosure completion_callback) override {}
-  void DeleteAll(base::OnceClosure completion_callback) override {}
-  void GetAllChannelIDs(GetChannelIDListCallback callback) override {}
-  int GetChannelIDCount() override { return 0; }
-  void SetForceKeepSessionState() override {}
-  void Flush() override {}
-  bool IsEphemeral() override { return true; }
-};
-
-// A ChannelIDStore that asynchronously returns an error when asked for a
-// channel id.
-class AsyncFailingChannelIDStore : public ChannelIDStore {
-  int GetChannelID(const std::string& server_identifier,
-                   std::unique_ptr<crypto::ECPrivateKey>* key_result,
-                   GetChannelIDCallback callback) override {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(std::move(callback), ERR_UNEXPECTED,
-                                  server_identifier, nullptr));
-    return ERR_IO_PENDING;
-  }
-  void SetChannelID(std::unique_ptr<ChannelID> channel_id) override {}
-  void DeleteChannelID(const std::string& server_identifier,
-                       base::OnceClosure completion_callback) override {}
-  void DeleteForDomainsCreatedBetween(
-      const base::Callback<bool(const std::string&)>& domain_predicate,
-      base::Time delete_begin,
-      base::Time delete_end,
-      base::OnceClosure completion_callback) override {}
-  void DeleteAll(base::OnceClosure completion_callback) override {}
-  void GetAllChannelIDs(GetChannelIDListCallback callback) override {}
-  int GetChannelIDCount() override { return 0; }
-  void SetForceKeepSessionState() override {}
-  void Flush() override {}
-  bool IsEphemeral() override { return true; }
-};
-
 // A mock ExpectCTReporter that remembers the latest violation that was
 // reported and the number of violations reported.
 class MockExpectCTReporter : public TransportSecurityState::ExpectCTReporter {
@@ -1184,32 +1131,6 @@ class SSLClientSocketFalseStartTest : public SSLClientSocketTest {
   }
 };
 
-class SSLClientSocketChannelIDTest : public SSLClientSocketTest {
- protected:
-  SSLClientSocketChannelIDTest() = default;
-
-  void EnableChannelID() {
-    channel_id_service_.reset(
-        new ChannelIDService(new DefaultChannelIDStore(NULL)));
-    context_.channel_id_service = channel_id_service_.get();
-  }
-
-  void EnableFailingChannelID() {
-    channel_id_service_.reset(
-        new ChannelIDService(new FailingChannelIDStore()));
-    context_.channel_id_service = channel_id_service_.get();
-  }
-
-  void EnableAsyncFailingChannelID() {
-    channel_id_service_.reset(
-        new ChannelIDService(new AsyncFailingChannelIDStore()));
-    context_.channel_id_service = channel_id_service_.get();
-  }
-
- private:
-  std::unique_ptr<ChannelIDService> channel_id_service_;
-};
-
 // Provides a response to the 0RTT request indicating whether it was received
 // as early data.
 class ZeroRTTResponse : public test_server::HttpResponse {
@@ -3287,95 +3208,6 @@ TEST_F(SSLClientSocketFalseStartTest, NoSessionResumptionBadFinished) {
   EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
 }
 
-// Connect to a server using channel id. It should allow the connection.
-TEST_F(SSLClientSocketChannelIDTest, SendChannelID) {
-  SpawnedTestServer::SSLOptions ssl_options;
-
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  EnableChannelID();
-  SSLConfig ssl_config;
-  ssl_config.channel_id_enabled = true;
-
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
-
-  EXPECT_THAT(rv, IsOk());
-  EXPECT_TRUE(sock_->IsConnected());
-  SSLInfo ssl_info;
-  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
-  EXPECT_TRUE(ssl_info.channel_id_sent);
-
-  sock_->Disconnect();
-  EXPECT_FALSE(sock_->IsConnected());
-}
-
-// Connect to a server using Channel ID but failing to look up the Channel
-// ID. It should fail.
-TEST_F(SSLClientSocketChannelIDTest, FailingChannelID) {
-  SpawnedTestServer::SSLOptions ssl_options;
-
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  EnableFailingChannelID();
-  SSLConfig ssl_config;
-  ssl_config.channel_id_enabled = true;
-
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
-
-  // TODO(haavardm@opera.com): Due to differences in threading, Linux returns
-  // ERR_UNEXPECTED while Mac and Windows return ERR_PROTOCOL_ERROR. Accept all
-  // error codes for now.
-  // http://crbug.com/373670
-  EXPECT_NE(OK, rv);
-  EXPECT_FALSE(sock_->IsConnected());
-}
-
-// Connect to a server using Channel ID but asynchronously failing to look up
-// the Channel ID. It should fail.
-TEST_F(SSLClientSocketChannelIDTest, FailingChannelIDAsync) {
-  SpawnedTestServer::SSLOptions ssl_options;
-
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  EnableAsyncFailingChannelID();
-  SSLConfig ssl_config;
-  ssl_config.channel_id_enabled = true;
-
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
-
-  EXPECT_THAT(rv, IsError(ERR_UNEXPECTED));
-  EXPECT_FALSE(sock_->IsConnected());
-}
-
-// Tests that session caches are sharded by whether Channel ID is enabled.
-TEST_F(SSLClientSocketChannelIDTest, ChannelIDShardSessionCache) {
-  SpawnedTestServer::SSLOptions ssl_options;
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  EnableChannelID();
-
-  // Connect without Channel ID.
-  SSLConfig ssl_config;
-  ssl_config.channel_id_enabled = false;
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
-  SSLInfo ssl_info;
-  EXPECT_TRUE(sock_->GetSSLInfo(&ssl_info));
-  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
-  EXPECT_FALSE(ssl_info.channel_id_sent);
-
-  // Enable Channel ID and connect again. This needs a full handshake to assert
-  // Channel ID.
-  ssl_config.channel_id_enabled = true;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
-  EXPECT_TRUE(sock_->GetSSLInfo(&ssl_info));
-  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
-  EXPECT_TRUE(ssl_info.channel_id_sent);
-}
-
 // Server preference should win in ALPN.
 TEST_F(SSLClientSocketTest, Alpn) {
   SpawnedTestServer::SSLOptions server_options;