Check Channel ID key on each request and log to UMA whether they match

BUG=548423

Review URL: https://codereview.chromium.org/1679413002

Cr-Commit-Position: refs/heads/master@{#376845}

10 files changed, 109 insertions, 0 deletions

diff --git a/chrome/browser/extensions/api/socket/tls_socket_unittest.cc b/chrome/browser/extensions/api/socket/tls_socket_unittest.cc
index 15f9218..4c04b35 100644
--- a/chrome/browser/extensions/api/socket/tls_socket_unittest.cc
+++ b/chrome/browser/extensions/api/socket/tls_socket_unittest.cc
@@ -76,6 +76,7 @@ class MockSSLClientSocket : public net::SSLClientSocket {
   MOCK_CONST_METHOD0(GetChannelIDService, net::ChannelIDService*());
   MOCK_METHOD2(GetSignedEKMForTokenBinding,
                net::Error(crypto::ECPrivateKey*, std::vector<uint8_t>*));
+  MOCK_CONST_METHOD0(GetChannelIDKey, crypto::ECPrivateKey*());
   MOCK_CONST_METHOD0(GetSSLFailureState, net::SSLFailureState());
   bool IsConnected() const override { return true; }
 
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index d118ac4..43222a4 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -47,6 +47,7 @@
 #include "net/spdy/spdy_protocol.h"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
+#include "net/ssl/channel_id_service.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_failure_state.h"
@@ -58,6 +59,63 @@
 
 namespace net {
 
+namespace {
+
+void DoNothingAsyncCallback(int result){};
+void RecordChannelIDKeyMatch(SSLClientSocket* ssl_socket,
+                             ChannelIDService* channel_id_service,
+                             std::string host) {
+  SSLInfo ssl_info;
+  ssl_socket->GetSSLInfo(&ssl_info);
+  if (!ssl_info.channel_id_sent)
+    return;
+  scoped_ptr<crypto::ECPrivateKey> request_key;
+  ChannelIDService::Request request;
+  int result = channel_id_service->GetOrCreateChannelID(
+      host, &request_key, base::Bind(&DoNothingAsyncCallback), &request);
+  // GetOrCreateChannelID only returns ERR_IO_PENDING before its first call
+  // (over the lifetime of the ChannelIDService) has completed or if it is
+  // creating a new key. The key that is being looked up here should already
+  // have been looked up before the channel ID was sent on the ssl socket, so
+  // the expectation is that this call will return synchronously. If this does
+  // return ERR_IO_PENDING, treat that as any other lookup failure and cancel
+  // the async request.
+  if (result == ERR_IO_PENDING)
+    request.Cancel();
+  crypto::ECPrivateKey* socket_key = ssl_socket->GetChannelIDKey();
+
+  // This enum is used for an UMA histogram - do not change or re-use values.
+  enum {
+    NO_KEYS = 0,
+    MATCH = 1,
+    SOCKET_KEY_MISSING = 2,
+    REQUEST_KEY_MISSING = 3,
+    KEYS_DIFFER = 4,
+    KEY_LOOKUP_ERROR = 5,
+    KEY_MATCH_MAX
+  } match;
+  if (result != OK) {
+    match = KEY_LOOKUP_ERROR;
+  } else if (!socket_key && !request_key) {
+    match = NO_KEYS;
+  } else if (!socket_key) {
+    match = SOCKET_KEY_MISSING;
+  } else if (!request_key) {
+    match = REQUEST_KEY_MISSING;
+  } else {
+    match = KEYS_DIFFER;
+    std::string raw_socket_key, raw_request_key;
+    if (socket_key->ExportRawPublicKey(&raw_socket_key) &&
+        request_key->ExportRawPublicKey(&raw_request_key) &&
+        raw_socket_key == raw_request_key) {
+      match = MATCH;
+    }
+  }
+  UMA_HISTOGRAM_ENUMERATION("Net.TokenBinding.KeyMatch", match, KEY_MATCH_MAX);
+}
+
+}  // namespace
+
 // Returns parameters associated with the start of a HTTP stream job.
 scoped_ptr<base::Value> NetLogHttpStreamJobCallback(
     const NetLog::Source& source,
@@ -1278,6 +1336,13 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
 
   next_state_ = STATE_CREATE_STREAM_COMPLETE;
 
+  if (using_ssl_ && connection_->socket()) {
+    SSLClientSocket* ssl_socket =
+        static_cast<SSLClientSocket*>(connection_->socket());
+    RecordChannelIDKeyMatch(ssl_socket, session_->params().channel_id_service,
+                            server_.HostForURL());
+  }
+
   // We only set the socket motivation if we're the first to use
   // this socket.  Is there a race for two SPDY requests?  We really
   // need to plumb this through to the connect level.
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 072cbe0..f10965c 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -833,6 +833,11 @@ Error MockClientSocket::GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
   return ERR_NOT_IMPLEMENTED;
 }
 
+crypto::ECPrivateKey* MockClientSocket::GetChannelIDKey() const {
+  NOTREACHED();
+  return NULL;
+}
+
 SSLFailureState MockClientSocket::GetSSLFailureState() const {
   return IsConnected() ? SSL_FAILURE_NONE : SSL_FAILURE_UNKNOWN;
 }
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 3470b5d..7f200549 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -581,6 +581,7 @@ class MockClientSocket : public SSLClientSocket {
   ChannelIDService* GetChannelIDService() const override;
   Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                     std::vector<uint8_t>* out) override;
+  crypto::ECPrivateKey* GetChannelIDKey() const override;
   SSLFailureState GetSSLFailureState() const override;
 
  protected:
diff --git a/net/socket/ssl_client_socket.h b/net/socket/ssl_client_socket.h
index 9f6551a..10affda 100644
--- a/net/socket/ssl_client_socket.h
+++ b/net/socket/ssl_client_socket.h
@@ -153,6 +153,11 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
   virtual Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                             std::vector<uint8_t>* out) = 0;
 
+  // This method is only for debugging crbug.com/548423 and will be removed when
+  // that bug is closed. This returns the channel ID key that was used when
+  // establishing the connection (or NULL if no channel ID was used).
+  virtual crypto::ECPrivateKey* GetChannelIDKey() const = 0;
+
   // Returns the state of the handshake when it failed, or |SSL_FAILURE_NONE| if
   // the handshake succeeded. This is used to classify causes of the TLS version
   // fallback.
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 9526c1c..38f2a78 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -521,6 +521,10 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> {
   // verified, and may not be called within an NSS callback.
   void CacheSessionIfNecessary();
 
+  crypto::ECPrivateKey* GetChannelIDKey() const {
+    return channel_id_key_.get();
+  }
+
  private:
   friend class base::RefCountedThreadSafe<Core>;
   ~Core();
@@ -3198,6 +3202,10 @@ Error SSLClientSocketNSS::GetSignedEKMForTokenBinding(
   return ERR_NOT_IMPLEMENTED;
 }
 
+crypto::ECPrivateKey* SSLClientSocketNSS::GetChannelIDKey() const {
+  return core_->GetChannelIDKey();
+}
+
 SSLFailureState SSLClientSocketNSS::GetSSLFailureState() const {
   if (completed_handshake_)
     return SSL_FAILURE_NONE;
diff --git a/net/socket/ssl_client_socket_nss.h b/net/socket/ssl_client_socket_nss.h
index 0df9d4f..d4d1740 100644
--- a/net/socket/ssl_client_socket_nss.h
+++ b/net/socket/ssl_client_socket_nss.h
@@ -101,6 +101,7 @@ class SSLClientSocketNSS : public SSLClientSocket {
   ChannelIDService* GetChannelIDService() const override;
   Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                     std::vector<uint8_t>* out) override;
+  crypto::ECPrivateKey* GetChannelIDKey() const override;
   SSLFailureState GetSSLFailureState() const override;
 
  private:
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index e36346f..9e23a4d 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -588,6 +588,10 @@ Error SSLClientSocketOpenSSL::GetSignedEKMForTokenBinding(
   return OK;
 }
 
+crypto::ECPrivateKey* SSLClientSocketOpenSSL::GetChannelIDKey() const {
+  return channel_id_key_.get();
+}
+
 SSLFailureState SSLClientSocketOpenSSL::GetSSLFailureState() const {
   return ssl_failure_state_;
 }
diff --git a/net/socket/ssl_client_socket_openssl.h b/net/socket/ssl_client_socket_openssl.h
index 70d195a..d1078a9 100644
--- a/net/socket/ssl_client_socket_openssl.h
+++ b/net/socket/ssl_client_socket_openssl.h
@@ -78,6 +78,7 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   ChannelIDService* GetChannelIDService() const override;
   Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
                                     std::vector<uint8_t>* out) override;
+  crypto::ECPrivateKey* GetChannelIDKey() const override;
   SSLFailureState GetSSLFailureState() const override;
 
   // SSLSocket implementation.
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index a171164..53d202e 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -28256,6 +28256,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Net.TokenBinding.KeyMatch" enum="TokenBinding.KeyMatch">
+  <owner>nharper@chromium.org</owner>
+  <summary>
+    Logs on each request that is sent on a connection where Channel ID was sent
+    whether the key that would be used for Token Binding matches the key used
+    for Channel ID.
+  </summary>
+</histogram>
+
 <histogram name="Net.TokenBinding.Support" enum="TokenBinding.Support">
   <owner>nharper@chromium.org</owner>
   <summary>
@@ -81350,6 +81359,15 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="1" label="Renegotiation patched"/>
 </enum>
 
+<enum name="TokenBinding.KeyMatch" type="int">
+  <int value="0" label="No keys"/>
+  <int value="1" label="Keys match"/>
+  <int value="2" label="Socket (Channel ID) key missing"/>
+  <int value="3" label="Request (Token Binding) key missing"/>
+  <int value="4" label="Keys present but don't match"/>
+  <int value="5" label="Error looking up request key"/>
+</enum>
+
 <enum name="TokenBinding.Support" type="int">
   <int value="0" label="DISABLED"/>
   <int value="1" label="CLIENT_ONLY"/>