Introducing the OffDomainInclusionDetector to analyze resource requests for suspicious activity.

chrome/browser/safe_browsing/safe_browsing_service.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 "chrome/browser/safe_browsing/safe_browsing_service.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 12 matching lines @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/prefs/tracked/tracked_preference_validation_delegate.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
 #include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer.h"
 #include "chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.h"
+#include "chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector.h"
 #include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/ping_manager.h"
 #include "chrome/browser/safe_browsing/protocol_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_database.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
@@ skipping 190 matching lines @@
         url_request_context_getter_.get()));
   }
   download_service_.reset(new safe_browsing::DownloadProtectionService(
       this, url_request_context_getter_.get()));
 #endif
 
   if (IsIncidentReportingServiceEnabled()) {
     incident_service_.reset(new safe_browsing::IncidentReportingService(
         this, url_request_context_getter_));
   }
+
+  off_domain_inclusion_detector_.reset(
+      new safe_browsing::OffDomainInclusionDetector);
 #endif
 
   // Track the safe browsing preference of existing profiles.
   // The SafeBrowsingService will be started if any existing profile has the
   // preference enabled. It will also listen for updates to the preferences.
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   if (profile_manager) {
     std::vector<Profile*> profiles = profile_manager->GetLoadedProfiles();
     for (size_t i = 0; i < profiles.size(); ++i) {
       if (profiles[i]->IsOffTheRecord())
@@ skipping 21 matching lines @@
 
   // Remove Profile creation/destruction observers.
   prefs_registrar_.RemoveAll();
 
   Stop(true);
   // The IO thread is going away, so make sure the ClientSideDetectionService
   // dtor executes now since it may call the dtor of URLFetcher which relies
   // on it.
   csd_service_.reset();
   incident_service_.reset();
   download_service_.reset();

[grt (UTC plus 2), 2014/11/19 18:29:00]

?
  off_domain_inclusion_detector_.reset();

[gab, 2014/11/19 21:29:09]

I had considered this but initially decided against it as I don't understand why
it couldn't just wait for the scoped_ptr to go out of scope when the class is
actually deleted?

The comment for csd_service_ makes sense (although it's sad that such
dependencies have to be carefully hardcoded), but nothing justifies the explicit
reset for incident_service_ and download_service_ AFAICT? And even less for the
off_domain_inclusion_detector_.

As-is, currently, nothing prevents the off_domain_inclusion_detector_ from
receiving requests while the SafeBrowsingService is shutting down (though I'm
not even convinced that would happen).
I was later thinking of hooking the incident_reporting_service_ as a weak
pointer anyways so that it wouldn't depend on that service's lifetime.

Anyways, I guess I'll add the reset here for now since it doesn't really hurt
(and may be required for some obscure reason) but I'd like to understand why
these are required.

 
   url_request_context_getter_ = NULL;
   BrowserThread::PostNonNestableTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&SafeBrowsingService::DestroyURLRequestContextOnIOThread,
                  this));
 }
 
 // Binhash verification is only enabled for UMA users for now.
 bool SafeBrowsingService::DownloadBinHashNeeded() const {
@@ skipping 53 matching lines @@
 }
 
 void SafeBrowsingService::AddDownloadManager(
     content::DownloadManager* download_manager) {
 #if defined(FULL_SAFE_BROWSING)
   if (incident_service_)
     incident_service_->AddDownloadManager(download_manager);
 #endif
 }
 
+void SafeBrowsingService::OnResourceRequest(const net::URLRequest* request) {
+#if defined(FULL_SAFE_BROWSING)
+  if (off_domain_inclusion_detector_)
+    off_domain_inclusion_detector_->OnResourceRequest(request);
+#endif
+}
+
 SafeBrowsingUIManager* SafeBrowsingService::CreateUIManager() {
   return new SafeBrowsingUIManager(this);
 }
 
 SafeBrowsingDatabaseManager* SafeBrowsingService::CreateDatabaseManager() {
 #if defined(FULL_SAFE_BROWSING)
   return new SafeBrowsingDatabaseManager(this);
 #else
   return NULL;
 #endif
@@ skipping 201 matching lines @@
   else
     Stop(false);
 
 #if defined(FULL_SAFE_BROWSING)
   if (csd_service_)
     csd_service_->SetEnabledAndRefreshState(enable);
   if (download_service_)
     download_service_->SetEnabled(enable);
 #endif
 }