Add a ModuleLoadAnalyzer which checks modules against a whitelist

chrome/browser/safe_browsing/incident_reporting/module_load_analyzer_win.cc

+// Copyright 2016 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/incident_reporting/module_load_analyzer.h"
+
+#include "base/file_version_info.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/install_verification/win/module_verification_common.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
+#include "chrome/browser/safe_browsing/incident_reporting/suspicious_module_incident.h"
+#include "chrome/browser/safe_browsing/path_sanitizer.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/common/safe_browsing/binary_feature_extractor.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+
+#if defined(SAFE_BROWSING_DB_LOCAL)
+#include "chrome/browser/safe_browsing/local_database_manager.h"
+#elif defined(SAFE_BROWSING_DB_REMOTE)
+#include "chrome/browser/safe_browsing/remote_database_manager.h"
+#endif
+
+namespace safe_browsing {
+
+// Enables analysis and reporting of suspicious modules loaded in the process.
+const base::Feature kIncidentReportingModuleLoadAnalysis{
+    "IncidentReportingModuleLoadAnalysis", base::FEATURE_DISABLED_BY_DEFAULT};
+
+void RegisterModuleLoadAnalysis(
+    const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager) {
+  DCHECK(database_manager);
+  if (base::FeatureList::IsEnabled(kIncidentReportingModuleLoadAnalysis)) {

[grt (UTC plus 2), 2016/02/11 15:55:00]

i think the impl for this will be the same for all platforms, so move this to
the non-_win.cc file with its contents in an #if defined(OS_WIN) block a la
RegisterBinaryIntegrityAnalysis

[proberge, 2016/02/11 20:25:21]

Done.

+    scoped_refptr<SafeBrowsingService> safe_browsing_service(
+        g_browser_process->safe_browsing_service());
+
+    if (safe_browsing_service) {
+      safe_browsing_service
+          ->RegisterExtendedReportingOnlyDelayedAnalysisCallback(
+              base::Bind(&VerifyModuleLoadState, database_manager));
+    }
+  }
+}
+
+void GetLoadedSuspiciousModulesOnIOThread(

[grt (UTC plus 2), 2016/02/11 15:55:00]

rename to something like CheckModuleWhitelistOnIOThread?

[grt (UTC plus 2), 2016/02/11 15:55:00]

move into unnamed namespace on line 31

[proberge, 2016/02/11 20:25:21]

Done.

[proberge, 2016/02/11 20:25:22]

Done.

+    const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
+    scoped_ptr<IncidentReceiver> incident_receiver,
+    scoped_ptr<std::set<ModuleInfo>> module_info_set) {

[grt (UTC plus 2), 2016/02/11 15:55:00]

#include <set>

[proberge, 2016/02/11 20:25:21]

Done.

+  SCOPED_UMA_HISTOGRAM_TIMER("SBIRS.SuspiciousModuleDetectionTime");
+  scoped_ptr<std::set<base::string16>> suspicious_names(

[grt (UTC plus 2), 2016/02/11 15:55:00]

can you make this a set of FilePaths since that's what is used in
ReportIncidentsForSuspiciousModules?

[proberge, 2016/02/11 20:25:21]

Done.

+      new std::set<base::string16>);
+
+  std::set<ModuleInfo>::const_iterator module_iter(module_info_set->begin());

[grt (UTC plus 2), 2016/02/11 15:55:00]

  for (const ModuleInfo& module_info : *module_info_set) {

[proberge, 2016/02/11 20:25:22]

Done.

+  for (; module_iter != module_info_set->end(); ++module_iter) {
+    base::string16 module_file_name(base::ToLowerASCII(

[grt (UTC plus 2), 2016/02/11 15:55:00]

there's no guarantee that filenames are ascii. consider base::i18n::FoldCase

[proberge, 2016/02/11 20:25:21]

Done.

+        base::FilePath(module_iter->name).BaseName().value()));
+
+    // If not whitelisted.
+    if (!database_manager->MatchModuleWhitelistString(
+            base::UTF16ToUTF8(module_file_name)))
+      suspicious_names->insert(module_iter->name);
+  }
+
+  UMA_HISTOGRAM_COUNTS("SBIRS.SuspiciousModuleReportCount",
+                       suspicious_names->size());
+
+  if (!suspicious_names->empty()) {
+    content::BrowserThread::GetBlockingPool()
+        ->PostWorkerTaskWithShutdownBehavior(
+            FROM_HERE, base::Bind(&ReportIncidentsForSuspiciousModules,
+                                  base::Passed(std::move(suspicious_names)),

[grt (UTC plus 2), 2016/02/11 15:55:00]

#include <utility>

[proberge, 2016/02/11 20:25:21]

Done.

+                                  base::Passed(std::move(incident_receiver))),
+            base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+  }
+}
+
+void ReportIncidentsForSuspiciousModules(

[grt (UTC plus 2), 2016/02/11 15:55:00]

move into unnamed namespace

[proberge, 2016/02/11 20:25:21]

Done.

+    scoped_ptr<std::set<base::string16>> module_names,
+    scoped_ptr<IncidentReceiver> incident_receiver) {
+  PathSanitizer path_sanitizer;
+  SCOPED_UMA_HISTOGRAM_TIMER("SBIRS.SuspiciousModuleReportingTime");
+
+  for (const auto& module_name : *module_names) {
+    // TODO(proberge): Skip over modules that have already been reported.
+
+    scoped_ptr<ClientIncidentReport_IncidentData_SuspiciousModuleIncident>
+        suspicious_module(
+            new ClientIncidentReport_IncidentData_SuspiciousModuleIncident());
+
+    const base::FilePath module_path(module_name);
+
+    // Sanitized path.
+    base::FilePath sanitized_path(module_path);
+    path_sanitizer.StripHomeDirectory(&sanitized_path);
+    suspicious_module->set_path(base::WideToUTF8(sanitized_path.value()));
+
+    // Digest.
+    scoped_refptr<BinaryFeatureExtractor> binary_feature_extractor(

[grt (UTC plus 2), 2016/02/11 15:55:00]

can you create one instance of this outside of the loop and reuse it for each
module?

[proberge, 2016/02/11 20:25:22]

Done.

+        new BinaryFeatureExtractor());
+
+    binary_feature_extractor->ExtractDigest(
+        module_path, suspicious_module->mutable_digest());
+
+    // Version.
+    scoped_ptr<FileVersionInfo> version_info(
+        FileVersionInfo::CreateFileVersionInfo(module_path));
+    if (version_info) {
+      std::wstring file_version = version_info->file_version();

[grt (UTC plus 2), 2016/02/11 15:55:00]

base::string16

[proberge, 2016/02/11 20:25:21]

Done.

+      if (!file_version.empty())
+        suspicious_module->set_version(base::WideToUTF8(file_version));
+    }
+
+    // Signature.
+    binary_feature_extractor->CheckSignature(
+        module_path, suspicious_module->mutable_signature());
+
+    // Image headers.
+    if (!binary_feature_extractor->ExtractImageFeatures(
+            module_path, BinaryFeatureExtractor::kDefaultOptions,
+            suspicious_module->mutable_image_headers(),
+            nullptr /* signed_data */)) {
+      suspicious_module->clear_image_headers();
+    }
+
+    // Send the report.

[grt (UTC plus 2), 2016/02/11 15:55:00]

nit: this sends the incident to the reporting service. whether or not a report
is assembled and sent is up to the service itself.

[proberge, 2016/02/11 20:25:22]

Done.

+    incident_receiver->AddIncidentForProcess(make_scoped_ptr(

[grt (UTC plus 2), 2016/02/11 15:55:00]

while the construction of extended_reporting_only_delayed_analysis_callbacks_
ensures that this analyzer doesn't run until sometime after a profile
participating in extended safe browsing is added to the profile manager, it
doesn't guarantee that the incident will only be reported if such a profile is
still loaded at the time of report generation.
ProcessIncidentsIfCollectionComplete() will ordinarily associate the incident
with a profile participating in extended safe browsing if there is one. do you
want to ensure that these incidents are not reported if the ESB profile was
unloaded? in this case, consider doing something like extending the Incident
class so that instances can be queried for their required level of consent so
that the various spots where profile association is done can drop the incident
if needed.

[proberge, 2016/02/11 20:25:21]

Done.

+        new SuspiciousModuleIncident(std::move(suspicious_module))));
+  }
+}
+
+void VerifyModuleLoadState(
+    const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
+    scoped_ptr<IncidentReceiver> incident_receiver) {
+  scoped_ptr<std::set<ModuleInfo>> module_info_set(new std::set<ModuleInfo>);
+  if (!GetLoadedModules(module_info_set.get()))
+    return;
+
+  // PostTaskAndReply doesn't work here because we're in a sequenced blocking
+  // thread pool.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&GetLoadedSuspiciousModulesOnIOThread, database_manager,
+                 base::Passed(std::move(incident_receiver)),
+                 base::Passed(std::move(module_info_set))));
+}
+
+}  // namespace safe_browsing