Keep track of ChannelIDService in CookieStore

net/url_request/url_request_http_job.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/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 64 matching lines @@
     CID_EPHEMERAL_COOKIE_PERSISTENT = 1,
     CID_PERSISTENT_COOKIE_EPHEMERAL = 2,
     // Value 3 was removed (CID_PERSISTENT_COOKIE_PERSISTENT)
     NO_COOKIE_STORE = 4,
     NO_CHANNEL_ID_STORE = 5,
     KNOWN_MISMATCH = 6,
     EPHEMERAL_MATCH = 7,
     EPHEMERAL_MISMATCH = 8,
     PERSISTENT_MATCH = 9,
     PERSISTENT_MISMATCH = 10,
+    EPHEMERAL_UNKNOWN = 11,
+    PERSISTENT_UNKNOWN = 12,
     EPHEMERALITY_MAX
   } ephemerality;
   const net::HttpNetworkSession::Params* params =
       context->GetNetworkSessionParams();
   net::CookieStore* cookie_store = context->cookie_store();
   if (params == nullptr || params->channel_id_service == nullptr) {
     ephemerality = NO_CHANNEL_ID_STORE;
   } else if (cookie_store == nullptr) {
     ephemerality = NO_COOKIE_STORE;
   } else if (params->channel_id_service->GetChannelIDStore()->IsEphemeral()) {
     if (cookie_store->IsEphemeral()) {
-      if (context->channel_id_service() &&
-          params->channel_id_service->GetUniqueID() ==
-              context->channel_id_service()->GetUniqueID()) {
+      if (cookie_store->GetChannelIDServiceID() == -1) {
+        ephemerality = EPHEMERAL_UNKNOWN;
+      } else if (context->channel_id_service() &&
+                 cookie_store->GetChannelIDServiceID() ==
+                     context->channel_id_service()->GetUniqueID()) {

[mmenke, 2016/03/18 21:50:45]

While I'm not familiar with just how channel ID is hooked up, this whole thing
seems very prone to bugs.  Safebrowsing, for instance, does wire up its own
ChannelIDService and CookieStore.  But since it shares an HttpNetworkSession
with the main profile, it's still presumably borked, and this code wouldn't
catch it, since the URLRequestContext's CookieStore and ChannelIDService match,
it's just the HttpNetworkSession's ChannelIDService that's wrong.

[nharper, 2016/03/18 21:59:03]

Safe browsing wires up its own CookieStore, but doesn't touch the
ChannelIDService (Ignoring my other CL). However, you correctly point out that I
meant to replace the context->channel_id_service()->GetUniqueID() line, not the
params->channel_id_service->GetUniqueID().

I.e. I should be checking that the id in the cookie store matches the one in the
HttpNetworkSession::Params's channel_id_service, since I want to be comparing
against the ChannelIDService actually in use.

         ephemerality = EPHEMERAL_MATCH;
       } else {
         ephemerality = EPHEMERAL_MISMATCH;
       }
     } else if (context->has_known_mismatched_cookie_store()) {
       ephemerality = KNOWN_MISMATCH;
     } else {
       ephemerality = CID_EPHEMERAL_COOKIE_PERSISTENT;
     }
   } else if (cookie_store->IsEphemeral()) {
     ephemerality = CID_PERSISTENT_COOKIE_EPHEMERAL;
+  } else if (cookie_store->GetChannelIDServiceID() == -1) {
+    ephemerality = PERSISTENT_UNKNOWN;
   } else if (context->channel_id_service() &&
-             params->channel_id_service->GetUniqueID() ==
+             cookie_store->GetChannelIDServiceID() ==
                  context->channel_id_service()->GetUniqueID()) {
     ephemerality = PERSISTENT_MATCH;
   } else {
     ephemerality = PERSISTENT_MISMATCH;
   }
   UMA_HISTOGRAM_ENUMERATION("Net.TokenBinding.StoreEphemerality", ephemerality,
                             EPHEMERALITY_MAX);
 }
 
 }  // namespace
@@ skipping 1479 matching lines @@
   return override_response_headers_.get() ?
              override_response_headers_.get() :
              transaction_->GetResponseInfo()->headers.get();
 }
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 }  // namespace net