Add support for enabling subset of security events

Bug: 1163668
Change-Id: If5d37113d20e44b719f0e0162ed079e25853b833
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612932Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Dominique Fauteux-Chapleau <domfc@chromium.org>
Commit-Queue: Marc-André Decoste <mad@chromium.org>
Auto-Submit: Marc-André Decoste <mad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843191}

chrome/browser/enterprise/connectors/common.h

@@ -28,6 +28,7 @@ constexpr char kKeyBlockPasswordProtected[] = "block_password_protected";
 constexpr char kKeyBlockLargeFiles[] = "block_large_files";
 constexpr char kKeyBlockUnsupportedFileTypes[] = "block_unsupported_file_types";
 constexpr char kKeyMinimumDataSize[] = "minimum_data_size";
+constexpr char kKeyEnabledEventNames[] = "enabled_event_names";
 
 enum class ReportingConnector {
   SECURITY_EVENT,
@@ -74,6 +75,7 @@ struct ReportingSettings {
   ~ReportingSettings();
 
   GURL reporting_url;
+  std::set<std::string> enabled_event_names;
   std::string dm_token;
 
   // Indicates if the report should be made for the profile, or the browser if

chrome/browser/enterprise/connectors/reporting_service_settings.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/enterprise/connectors/reporting_service_settings.h"
 
 #include "chrome/browser/enterprise/connectors/service_provider_config.h"
+#include "chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.h"
 #include "components/policy/core/browser/url_util.h"
 
 namespace enterprise_connectors {
@@ -23,6 +24,29 @@ ReportingServiceSettings::ReportingServiceSettings(
     service_provider_ =
         service_provider_config.GetServiceProvider(*service_provider_name);
   }
+
+  const base::Value* enabled_event_name_list_value =
+      settings_value.FindListKey(kKeyEnabledEventNames);
+  if (enabled_event_name_list_value) {
+    for (const base::Value& enabled_event_name_value :
+         enabled_event_name_list_value->GetList()) {
+      if (enabled_event_name_value.is_string())
+        enabled_event_names_.insert(enabled_event_name_value.GetString());
+      else
+        DVLOG(1) << "Enabled event name list contains a non string value!";
+    }
+  } else {
+    // When the list of enabled event names is not set, we assume all events are
+    // enabled. This is to support the feature of selecting the "All always on"
+    // option in the policy UI, which means to always enable all events, even
+    // when new events may be added in the future. And this is also to support
+    // existing customer policies that were created before we introduced the
+    // concept of enabling/disabling events.
+    for (auto* event_name :
+         extensions::SafeBrowsingPrivateEventRouter::kAllEvents) {
+      enabled_event_names_.insert(event_name);
+    }
+  }
 }
 
 base::Optional<ReportingSettings>
@@ -35,12 +59,16 @@ ReportingServiceSettings::GetReportingSettings() const {
   settings.reporting_url = GURL(service_provider_->reporting_url());
   DCHECK(settings.reporting_url.is_valid());
 
+  settings.enabled_event_names.insert(enabled_event_names_.begin(),
+                                      enabled_event_names_.end());
   return settings;
 }
 
 bool ReportingServiceSettings::IsValid() const {
-  // The settings are valid only if a provider was given.
-  return service_provider_;
+  // The settings are valid only if a provider was given, and events are
+  // enabled. The list could be empty. The absence of a list means "all events",
+  // but the presence of an empty list means "no events".
+  return service_provider_ && !enabled_event_names_.empty();
 }
 
 ReportingServiceSettings::ReportingServiceSettings(ReportingServiceSettings&&) =

chrome/browser/enterprise/connectors/reporting_service_settings.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_ENTERPRISE_CONNECTORS_REPORTING_SERVICE_SETTINGS_H_
 #define CHROME_BROWSER_ENTERPRISE_CONNECTORS_REPORTING_SERVICE_SETTINGS_H_
 
+#include <set>
 #include <string>
 
 #include "base/optional.h"
@@ -36,6 +37,9 @@ class ReportingServiceSettings {
   // implies that a corresponding service provider doesn't exist and that these
   // settings are not valid.
   const ServiceProviderConfig::ServiceProvider* service_provider_ = nullptr;
+
+  // The events that are enabled for the current service provider.
+  std::set<std::string> enabled_event_names_;
 };
 
 }  // namespace enterprise_connectors

chrome/browser/enterprise/connectors/reporting_service_settings_unittest.cc

@@ -22,17 +22,28 @@ struct TestParam {
   ReportingSettings* expected_settings;
 };
 
-constexpr char kNormalSettings[] = R"({ "service_provider": "google" })";
-
 constexpr char kNoProviderSettings[] = "{}";
 
-ReportingSettings* NormalSettings() {
+constexpr char kNormalSettingsWithoutEvents[] =
+    R"({ "service_provider": "google" })";
+
+constexpr char kNormalSettingsWithEvents[] =
+    R"({ "service_provider": "google",
+         "enabled_event_names" : ["event 1", "event 2", "event 3"]
+       })";
+
+ReportingSettings* NoSettings() {
+  return nullptr;
+}
+
+ReportingSettings* NormalSettingsWithoutEvents() {
   static base::NoDestructor<ReportingSettings> settings;
   return settings.get();
 }
 
-ReportingSettings* NoSettings() {
-  return nullptr;
+ReportingSettings* NormalSettingsWithEvents() {
+  static base::NoDestructor<ReportingSettings> settings;
+  return settings.get();
 }
 
 }  // namespace
@@ -41,11 +52,16 @@ class ReportingServiceSettingsTest : public testing::TestWithParam<TestParam> {
  public:
   const char* settings_value() const { return GetParam().settings_value; }
   ReportingSettings* expected_settings() const {
-    // Set the GURL field dynamically to avoid static initialization issues.
-    if (GetParam().expected_settings == NormalSettings() &&
+    // Set the settings fields dynamically to avoid static initialization issue.
+    if (GetParam().expected_settings == NormalSettingsWithoutEvents() &&
         !GetParam().expected_settings->reporting_url.is_valid()) {
       GetParam().expected_settings->reporting_url =
           GURL("https://chromereporting-pa.googleapis.com/v1/events");
+    } else if (GetParam().expected_settings == NormalSettingsWithEvents() &&
+               GetParam().expected_settings->enabled_event_names.empty()) {
+      GetParam().expected_settings->enabled_event_names.insert("event 1");
+      GetParam().expected_settings->enabled_event_names.insert("event 2");
+      GetParam().expected_settings->enabled_event_names.insert("event 3");
     }
     return GetParam().expected_settings;
   }
@@ -64,17 +80,25 @@ TEST_P(ReportingServiceSettingsTest, Test) {
 
   auto reporting_settings = service_settings.GetReportingSettings();
   ASSERT_EQ((expected_settings() != nullptr), reporting_settings.has_value());
-  if (reporting_settings.has_value()) {
-    ASSERT_EQ(expected_settings(), NormalSettings());
+  if (expected_settings() == NormalSettingsWithoutEvents()) {
+    ASSERT_TRUE(reporting_settings->reporting_url.is_valid());
     ASSERT_EQ(expected_settings()->reporting_url,
               reporting_settings.value().reporting_url);
+  } else if (expected_settings() == NormalSettingsWithEvents()) {
+    ASSERT_FALSE(reporting_settings->enabled_event_names.empty());
+    ASSERT_EQ(expected_settings()->enabled_event_names,
+              reporting_settings.value().enabled_event_names);
+  } else {
+    ASSERT_EQ(expected_settings(), NoSettings());
   }
 }
 
 INSTANTIATE_TEST_CASE_P(
     ,
     ReportingServiceSettingsTest,
-    testing::Values(TestParam(kNormalSettings, NormalSettings()),
-                    TestParam(kNoProviderSettings, NoSettings())));
+    testing::Values(
+        TestParam(kNoProviderSettings, NoSettings()),
+        TestParam(kNormalSettingsWithoutEvents, NormalSettingsWithoutEvents()),
+        TestParam(kNormalSettingsWithEvents, NormalSettingsWithEvents())));
 
 }  // namespace enterprise_connectors

chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.cc

@@ -147,6 +147,7 @@ const char SafeBrowsingPrivateEventRouter::kKeyMalwareCategory[] =
 const char SafeBrowsingPrivateEventRouter::kKeyEvidenceLockerFilePath[] =
     "evidenceLockerFilepath";
 
+// All new event names should be added to the kAllEvents array below!
 const char SafeBrowsingPrivateEventRouter::kKeyPasswordReuseEvent[] =
     "passwordReuseEvent";
 const char SafeBrowsingPrivateEventRouter::kKeyPasswordChangedEvent[] =
@@ -159,6 +160,16 @@ const char SafeBrowsingPrivateEventRouter::kKeySensitiveDataEvent[] =
     "sensitiveDataEvent";
 const char SafeBrowsingPrivateEventRouter::kKeyUnscannedFileEvent[] =
     "unscannedFileEvent";
+// All new event names should be added to this array!
+const char* SafeBrowsingPrivateEventRouter::kAllEvents[6] = {
+    SafeBrowsingPrivateEventRouter::kKeyPasswordReuseEvent,
+    SafeBrowsingPrivateEventRouter::kKeyPasswordChangedEvent,
+    SafeBrowsingPrivateEventRouter::kKeyDangerousDownloadEvent,
+    SafeBrowsingPrivateEventRouter::kKeyInterstitialEvent,
+    SafeBrowsingPrivateEventRouter::kKeySensitiveDataEvent,
+    SafeBrowsingPrivateEventRouter::kKeyUnscannedFileEvent,
+};
+
 const char SafeBrowsingPrivateEventRouter::kKeyUnscannedReason[] =
     "unscannedReason";
 
@@ -207,8 +218,10 @@ void SafeBrowsingPrivateEventRouter::OnPolicySpecifiedPasswordReuseDetected(
   }
 
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyPasswordReuseEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyPasswordReuseEvent, std::move(settings.value()),
@@ -243,8 +256,10 @@ void SafeBrowsingPrivateEventRouter::OnPolicySpecifiedPasswordChanged(
   }
 
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyPasswordChangedEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(kKeyPasswordChangedEvent, std::move(settings.value()),
                       base::BindOnce(
@@ -286,8 +301,10 @@ void SafeBrowsingPrivateEventRouter::OnDangerousDownloadOpened(
   }
 
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyDangerousDownloadEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyDangerousDownloadEvent, std::move(settings.value()),
@@ -345,8 +362,10 @@ void SafeBrowsingPrivateEventRouter::OnSecurityInterstitialShown(
   }
 
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyInterstitialEvent) == 0) {
     return;
+  }
 
   PrefService* prefs = Profile::FromBrowserContext(context_)->GetPrefs();
   auto event_result =
@@ -402,8 +421,10 @@ void SafeBrowsingPrivateEventRouter::OnSecurityInterstitialProceeded(
   }
 
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyInterstitialEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyInterstitialEvent, std::move(settings.value()),
@@ -461,8 +482,10 @@ void SafeBrowsingPrivateEventRouter::OnDangerousDeepScanningResult(
     const std::string& malware_category,
     const std::string& evidence_locker_filepath) {
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyDangerousDownloadEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyDangerousDownloadEvent, std::move(settings.value()),
@@ -522,8 +545,10 @@ void SafeBrowsingPrivateEventRouter::OnSensitiveDataEvent(
     const int64_t content_size,
     safe_browsing::EventResult event_result) {
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeySensitiveDataEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeySensitiveDataEvent, std::move(settings.value()),
@@ -579,8 +604,10 @@ void SafeBrowsingPrivateEventRouter::OnAnalysisConnectorWarningBypassed(
     const enterprise_connectors::ContentAnalysisResponse::Result& result,
     const int64_t content_size) {
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeySensitiveDataEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeySensitiveDataEvent, std::move(settings.value()),
@@ -636,8 +663,10 @@ void SafeBrowsingPrivateEventRouter::OnUnscannedFileEvent(
     const int64_t content_size,
     safe_browsing::EventResult event_result) {
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyUnscannedFileEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyUnscannedFileEvent, std::move(settings.value()),
@@ -686,8 +715,10 @@ void SafeBrowsingPrivateEventRouter::OnDangerousDownloadEvent(
     const int64_t content_size,
     safe_browsing::EventResult event_result) {
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyDangerousDownloadEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyDangerousDownloadEvent, std::move(settings.value()),
@@ -730,8 +761,10 @@ void SafeBrowsingPrivateEventRouter::OnDangerousDownloadWarningBypassed(
     const std::string& mime_type,
     const int64_t content_size) {
   auto settings = GetReportingSettings();
-  if (!settings.has_value())
+  if (!settings.has_value() ||
+      settings->enabled_event_names.count(kKeyDangerousDownloadEvent) == 0) {
     return;
+  }
 
   ReportRealtimeEvent(
       kKeyDangerousDownloadEvent, std::move(settings.value()),
@@ -989,6 +1022,19 @@ void SafeBrowsingPrivateEventRouter::ReportRealtimeEvent(
     const std::string& name,
     enterprise_connectors::ReportingSettings settings,
     EventBuilder event_builder) {
+#ifndef NDEBUG
+  // Make sure that the event is included in the kAllEvents array.
+  bool found = false;
+  for (auto* known_event_name :
+       extensions::SafeBrowsingPrivateEventRouter::kAllEvents) {
+    if (name == known_event_name) {
+      found = true;
+      break;
+    }
+  }
+  DCHECK(found);
+#endif
+
   // Copy the DM token since |settings| is about to move.
   std::string dm_token = settings.dm_token;
   IfAuthorized(dm_token,

chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.h

@@ -92,6 +92,8 @@ class SafeBrowsingPrivateEventRouter
   static const char kKeyInterstitialEvent[];
   static const char kKeySensitiveDataEvent[];
   static const char kKeyUnscannedFileEvent[];
+  static const char* kAllEvents[6];
+
   static const char kKeyUnscannedReason[];
 
   // String constants for the "trigger" event field.  This corresponds to

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.cc

@@ -365,9 +365,11 @@ void SetAnalysisConnector(PrefService* prefs,
       machine_scope ? policy::POLICY_SCOPE_MACHINE : policy::POLICY_SCOPE_USER);
 }
 
-void SetOnSecurityEventReporting(PrefService* prefs,
-                                 bool enabled,
-                                 bool machine_scope) {
+void SetOnSecurityEventReporting(
+    PrefService* prefs,
+    bool enabled,
+    const std::set<std::string>& enabled_event_names,
+    bool machine_scope) {
   ListPrefUpdate settings_list(prefs,
                                enterprise_connectors::kOnSecurityEventPref);
   DCHECK(settings_list.Get());
@@ -377,6 +379,14 @@ void SetOnSecurityEventReporting(PrefService* prefs,
 
       settings.SetKey(enterprise_connectors::kKeyServiceProvider,
                       base::Value("google"));
+      if (!enabled_event_names.empty()) {
+        base::Value enabled_event_name_list(base::Value::Type::LIST);
+        for (const auto& enabled_event_name : enabled_event_names) {
+          enabled_event_name_list.Append(enabled_event_name);
+        }
+        settings.SetKey(enterprise_connectors::kKeyEnabledEventNames,
+                        std::move(enabled_event_name_list));
+      }
       settings_list->Append(std::move(settings));
     }
     prefs->SetInteger(enterprise_connectors::kOnSecurityEventScopePref,

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.h

@@ -150,9 +150,11 @@ void SetAnalysisConnector(PrefService* prefs,
                           enterprise_connectors::AnalysisConnector connector,
                           const std::string& pref_value,
                           bool machine_scope = true);
-void SetOnSecurityEventReporting(PrefService* prefs,
-                                 bool enabled,
-                                 bool machine_scope = true);
+void SetOnSecurityEventReporting(
+    PrefService* prefs,
+    bool enabled,
+    const std::set<std::string>& enabled_event_names = std::set<std::string>(),
+    bool machine_scope = true);
 void ClearAnalysisConnector(PrefService* prefs,
                             enterprise_connectors::AnalysisConnector connector);
 

components/policy/resources/policy_templates.json

@@ -21804,11 +21804,16 @@
           'type': 'object',
           'properties': {
             'service_provider': { 'type': 'string' },
+            'enabled_event_names': {
+              'type': 'array',
+              'items': { 'type': 'string' },
+            },
           },
         },
       },
       'example_value': [{
         'service_provider': 'Google',
+        'enabled_event_names': ['passwordChangedEvent', 'sensitiveDataEvent'],
       }],
       'features': {
         'dynamic_refresh': True,
@@ -21822,7 +21827,7 @@
       'tags': [],
       'desc': '''List of Chrome Enterprise Connectors services settings to be applied to the <ph name="ON_SECURITY_EVENT_ENTERPRISE_CONNECTOR">OnSecurityEvent</ph> Enterprise Connector, which triggers when a security event occurs in Chrome. This includes negative verdicts from analysis Enterprise Connectors, password reuse, navigations to unsafe pages and other security sensitive user actions.
 
-      The <ph name="ENTERPRISE_CONNECTOR_SERVICE_PROVIDER_FIELD">service_provider</ph> field identifies which reporting service provider the settings correspond to.
+      The <ph name="ENTERPRISE_CONNECTOR_SERVICE_PROVIDER_FIELD">service_provider</ph> field identifies which reporting service provider the settings correspond to and the <ph name="ENTERPRISE_CONNECTOR_ENABLED_EVENT_NAMES_FIELD">enabled_event_names</ph> field identifies which events are enabled for this provider.
 
       This policy can only be set from the Google Admin console.'''
     },