Commit cb08fe42 authored by Dominique Fauteux-Chapleau's avatar Dominique Fauteux-Chapleau Committed by Commit Bot

Hook OnFileAttached to ConnectorsManager

This makes OnFileAttached an alternative to legacy policies to
configure scanning settings. This is gated by a new feature.

Bug: 1067631
Change-Id: Ib97200da493176a7a4254d2f891c1625c85d42cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162268Reviewed-by: default avatarDominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: default avatarRoger Tawa <rogerta@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762071}
parent 5e409647
......@@ -39,6 +39,7 @@ AnalysisServiceSettings::AnalysisServiceSettings(
// Add the patterns to the settings, which configures settings.matcher and
// settings.*_pattern_settings. No enable patterns implies the settings are
// invalid.
matcher_ = std::make_unique<url_matcher::URLMatcher>();
url_matcher::URLMatcherConditionSet::ID id(0);
const base::Value* enable = settings_value.FindListKey(kKeyEnable);
if (enable && enable->is_list()) {
......@@ -94,7 +95,8 @@ base::Optional<AnalysisSettings> AnalysisServiceSettings::GetAnalysisSettings(
if (!IsValid())
return base::nullopt;
auto matches = matcher_.MatchURL(url);
DCHECK(matcher_);
auto matches = matcher_->MatchURL(url);
if (matches.empty())
return base::nullopt;
......@@ -141,7 +143,7 @@ void AnalysisServiceSettings::AddUrlPatternSettings(
const base::ListValue* url_list_value = nullptr;
url_list->GetAsList(&url_list_value);
DCHECK(url_list_value);
policy::url_util::AddFilters(&matcher_, enabled, id, url_list_value);
policy::url_util::AddFilters(matcher_.get(), enabled, id, url_list_value);
} else {
return;
}
......@@ -195,6 +197,8 @@ bool AnalysisServiceSettings::IsValid() const {
return true;
}
AnalysisServiceSettings::AnalysisServiceSettings(AnalysisServiceSettings&&) =
default;
AnalysisServiceSettings::~AnalysisServiceSettings() = default;
AnalysisServiceSettings::URLPatternSettings::URLPatternSettings() = default;
......
......@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_ENTERPRISE_CONNECTORS_ANALYSIS_SERVICE_SETTINGS_H_
#define CHROME_BROWSER_ENTERPRISE_CONNECTORS_ANALYSIS_SERVICE_SETTINGS_H_
#include <memory>
#include "base/values.h"
#include "chrome/browser/enterprise/connectors/common.h"
#include "components/url_matcher/url_matcher.h"
......@@ -15,6 +17,7 @@ namespace enterprise_connectors {
class AnalysisServiceSettings {
public:
explicit AnalysisServiceSettings(const base::Value& settings_value);
AnalysisServiceSettings(AnalysisServiceSettings&&);
~AnalysisServiceSettings();
// Get the settings to apply to a specific analysis. base::nullopt implies no
......@@ -63,7 +66,7 @@ class AnalysisServiceSettings {
// condition set IDs returned after matching against a URL can be used to
// check |enabled_patterns_settings| and |disable_patterns_settings| to
// obtain URL-specific settings.
url_matcher::URLMatcher matcher_;
std::unique_ptr<url_matcher::URLMatcher> matcher_;
// These members map URL patterns to corresponding settings. If an entry in
// the "enabled" or "disabled" lists contains more than one pattern in its
......
......@@ -10,6 +10,7 @@
#include "base/memory/singleton.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/enterprise/connectors/connectors_prefs.h"
#include "components/policy/core/browser/url_util.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
......@@ -18,6 +19,9 @@
namespace enterprise_connectors {
const base::Feature kEnterpriseConnectorsEnabled{
"EnterpriseConnectorsEnabled", base::FEATURE_DISABLED_BY_DEFAULT};
namespace {
base::ListValue AllPatterns() {
......@@ -49,6 +53,18 @@ bool MatchURLAgainstPatterns(const GURL& url,
return has_scan_match;
}
const char* ConnectorToPref(AnalysisConnector connector) {
switch (connector) {
case AnalysisConnector::BULK_DATA_ENTRY:
case AnalysisConnector::FILE_DOWNLOADED:
// TODO(crbug/1067631): Return the corresponding prefs for these analysis
// connectors once they exist.
return nullptr;
case AnalysisConnector::FILE_ATTACHED:
return kOnFileAttachedPref;
}
}
} // namespace
// ConnectorsManager implementation---------------------------------------------
......@@ -61,11 +77,59 @@ ConnectorsManager* ConnectorsManager::GetInstance() {
return base::Singleton<ConnectorsManager>::get();
}
bool ConnectorsManager::IsConnectorEnabled(AnalysisConnector connector) {
if (!base::FeatureList::IsEnabled(kEnterpriseConnectorsEnabled))
return false;
if (connector_settings_.count(connector) == 1)
return true;
const char* pref = ConnectorToPref(connector);
return pref && g_browser_process->local_state()->HasPrefPath(pref);
}
void ConnectorsManager::GetAnalysisSettings(const GURL& url,
AnalysisConnector connector,
AnalysisSettingsCallback callback) {
if (IsConnectorEnabled(connector)) {
GetAnalysisSettingsFromConnectorPolicy(url, connector, std::move(callback));
} else {
std::move(callback).Run(
GetAnalysisSettingsFromLegacyPolicies(url, connector));
}
}
void ConnectorsManager::GetAnalysisSettingsFromConnectorPolicy(
const GURL& url,
AnalysisConnector connector,
AnalysisSettingsCallback callback) {
if (connector_settings_.count(connector) == 0)
CacheConnectorPolicy(connector);
// If the connector is still not in memory, it means the pref is set to an
// empty list or that it is not a list.
if (connector_settings_.count(connector) == 0) {
std::move(callback).Run(base::nullopt);
return;
}
// While multiple services can be set by the connector policies, only the
// first one is considered for now.
std::move(callback).Run(
GetAnalysisSettingsFromLegacyPolicies(url, connector));
connector_settings_[connector][0].GetAnalysisSettings(url));
}
void ConnectorsManager::CacheConnectorPolicy(AnalysisConnector connector) {
// Connectors with non-existing policies should not reach this code.
const char* pref = ConnectorToPref(connector);
DCHECK(pref);
const base::ListValue* policy_value =
g_browser_process->local_state()->GetList(pref);
if (policy_value && policy_value->is_list()) {
for (const base::Value& service_settings : policy_value->GetList())
connector_settings_[connector].emplace_back(service_settings);
}
}
bool ConnectorsManager::DelayUntilVerdict(AnalysisConnector connector) const {
......@@ -201,4 +265,13 @@ std::set<std::string> ConnectorsManager::MatchURLAgainstLegacyPolicies(
return tags;
}
void ConnectorsManager::Reset() {
connector_settings_.clear();
}
const ConnectorsManager::AnalysisConnectorsSettings&
ConnectorsManager::GetAnalysisConnectorsSettingsForTesting() const {
return connector_settings_;
}
} // namespace enterprise_connectors
......@@ -8,7 +8,9 @@
#include <set>
#include "base/callback_forward.h"
#include "base/feature_list.h"
#include "base/optional.h"
#include "chrome/browser/enterprise/connectors/analysis_service_settings.h"
#include "chrome/browser/enterprise/connectors/common.h"
#include "url/gurl.h"
......@@ -19,6 +21,11 @@ struct DefaultSingletonTraits;
namespace enterprise_connectors {
// Controls whether the Enterprise Connectors policies should be read by
// ConnectorsManager. Legacy policies will be read as a fallback if this feature
// is disabled.
extern const base::Feature kEnterpriseConnectorsEnabled;
// Manages access to Connector policies. This class is responsible for caching
// the Connector policies, validate them against approved service providers and
// provide a simple interface to them.
......@@ -29,16 +36,24 @@ class ConnectorsManager {
using AnalysisSettingsCallback =
base::OnceCallback<void(base::Optional<AnalysisSettings>)>;
// Map used to cache analysis connectors settings.
using AnalysisConnectorsSettings =
std::map<AnalysisConnector, std::vector<AnalysisServiceSettings>>;
static ConnectorsManager* GetInstance();
// Validates which settings should be applied to an analysis connector event
// against cached policies.
// against cached policies. This function will prioritize new connector
// policies over legacy ones if they are set.
void GetAnalysisSettings(const GURL& url,
AnalysisConnector connector,
AnalysisSettingsCallback callback);
bool DelayUntilVerdict(AnalysisConnector connector) const;
// Clears any cached values.
void Reset();
// Public legacy functions.
// These functions are used to interact with legacy policies and should only
// be called while the connectors equivalent isn't available. They should be
......@@ -48,6 +63,10 @@ class ConnectorsManager {
bool MatchURLAgainstLegacyDlpPolicies(const GURL& url, bool upload) const;
bool MatchURLAgainstLegacyMalwarePolicies(const GURL& url, bool upload) const;
// Public testing functions.
const AnalysisConnectorsSettings& GetAnalysisConnectorsSettingsForTesting()
const;
private:
friend struct base::DefaultSingletonTraits<ConnectorsManager>;
......@@ -56,6 +75,21 @@ class ConnectorsManager {
ConnectorsManager();
~ConnectorsManager();
// Checks if the corresponding connector is enabled and to be used with the
// given URL.
bool IsConnectorEnabled(AnalysisConnector connector);
// Validates which settings should be applied to an analysis connector event
// against connector policies. Cache the policy value the first time this is
// called for every different connector.
void GetAnalysisSettingsFromConnectorPolicy(
const GURL& url,
AnalysisConnector connector,
AnalysisSettingsCallback callback);
// Read and cache the policy corresponding to |connector|.
void CacheConnectorPolicy(AnalysisConnector connector);
// Private legacy functions.
// These functions are used to interact with legacy policies and should stay
// private. They should be removed once legacy policies are deprecated.
......@@ -72,6 +106,10 @@ class ConnectorsManager {
std::set<std::string> MatchURLAgainstLegacyPolicies(const GURL& url,
bool upload) const;
// Cached values of the connector policies. Updated when a connector is first
// used or when a policy is updated.
AnalysisConnectorsSettings connector_settings_;
};
} // namespace enterprise_connectors
......
......@@ -4,11 +4,14 @@
#include "chrome/browser/enterprise/connectors/connectors_manager.h"
#include "base/json/json_reader.h"
#include "base/optional.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/enterprise/connectors/connectors_prefs.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/testing_profile_manager.h"
......@@ -68,34 +71,42 @@ constexpr safe_browsing::DelayDeliveryUntilVerdictValues
DELAY_UPLOADS_AND_DOWNLOADS,
};
constexpr char kEmptySettingsPref[] = "[]";
constexpr char kNormalSettingsPref[] = R"([
{
"service_provider": "Google",
"enable": [
{"url_list": ["*"], "tags": ["dlp", "malware"]},
],
"disable": [
{"url_list": ["no.dlp.com", "no.dlp.or.malware.ca"], "tags": ["dlp"]},
{"url_list": ["no.malware.com", "no.dlp.or.malware.ca"],
"tags": ["malware"]},
],
"block_until_verdict": 1,
"block_password_protected": true,
"block_large_files": true,
"block_unsupported_file_types": true,
},
])";
constexpr char kDlpAndMalwareUrl[] = "https://foo.com";
constexpr char kOnlyDlpUrl[] = "https://no.malware.com";
constexpr char kOnlyMalwareUrl[] = "https://no.dlp.com";
constexpr char kNoTagsUrl[] = "https://no.dlp.or.malware.ca";
// constexpr const char* kAllConnectorsTestUrls[] = {
// kDlpAndMalwareUrl, kOnlyDlpUrl, kOnlyMalwareUrl, kNoTagsUrl
//};
} // namespace
// Tests that permutations of legacy policies produce expected settings from a
// ConnectorsManager instance. T is a type used to iterate over policies with a
// {NONE, DOWNLOADS, UPLOADS, UPLOADS_AND_DOWNLOADS} pattern without testing
// every single permutation since these settings are independent.
template <typename T>
class ConnectorsManagerLegacyPoliciesTest
: public testing::TestWithParam<std::tuple<AnalysisConnector, T>> {
class ConnectorsManagerTest : public testing::Test {
public:
ConnectorsManagerLegacyPoliciesTest<T>()
ConnectorsManagerTest()
: profile_manager_(TestingBrowserProcess::GetGlobal()) {
scoped_feature_list_.InitWithFeatures(
{safe_browsing::kContentComplianceEnabled,
safe_browsing::kMalwareScanEnabled},
{});
EXPECT_TRUE(profile_manager_.SetUp());
profile_ = profile_manager_.CreateTestingProfile("test-user");
connectors_manager_ = ConnectorsManager::GetInstance();
}
AnalysisConnector connector() const { return std::get<0>(this->GetParam()); }
T tested_policy() const { return std::get<1>(this->GetParam()); }
bool upload_scan() const {
return connector() != AnalysisConnector::FILE_DOWNLOADED;
}
void ValidateSettings(const AnalysisSettings& settings) {
......@@ -118,10 +129,56 @@ class ConnectorsManagerLegacyPoliciesTest
[&settings](base::Optional<AnalysisSettings> tmp_settings) {
settings = std::move(tmp_settings);
});
connectors_manager_->GetAnalysisSettings(url, connector, callback);
ConnectorsManager::GetInstance()->GetAnalysisSettings(url, connector,
callback);
return settings;
}
void SetConnectorPref(const char* pref, const char* pref_value) {
auto maybe_pref_value =
base::JSONReader::Read(pref_value, base::JSON_ALLOW_TRAILING_COMMAS);
ASSERT_TRUE(maybe_pref_value.has_value());
g_browser_process->local_state()->Set(pref, maybe_pref_value.value());
}
protected:
content::BrowserTaskEnvironment task_environment_;
base::test::ScopedFeatureList scoped_feature_list_;
TestingProfileManager profile_manager_;
TestingProfile* profile_;
GURL url_ = GURL("https://google.com");
// Set to the default value of their legacy policy.
std::set<std::string> expected_tags_ = {};
BlockUntilVerdict expected_block_until_verdict_ = BlockUntilVerdict::NO_BLOCK;
bool expected_block_password_protected_files_ = false;
bool expected_block_large_files_ = false;
bool expected_block_unsupported_file_types_ = false;
};
// Tests that permutations of legacy policies produce expected settings from a
// ConnectorsManager instance. T is a type used to iterate over policies with a
// {NONE, DOWNLOADS, UPLOADS, UPLOADS_AND_DOWNLOADS} pattern without testing
// every single permutation since these settings are independent.
template <typename T>
class ConnectorsManagerLegacyPoliciesTest
: public ConnectorsManagerTest,
public testing::WithParamInterface<std::tuple<AnalysisConnector, T>> {
public:
ConnectorsManagerLegacyPoliciesTest<T>() {
scoped_feature_list_.InitWithFeatures(
{safe_browsing::kContentComplianceEnabled,
safe_browsing::kMalwareScanEnabled},
{});
}
AnalysisConnector connector() const { return std::get<0>(this->GetParam()); }
T tested_policy() const { return std::get<1>(this->GetParam()); }
bool upload_scan() const {
return connector() != AnalysisConnector::FILE_DOWNLOADED;
}
void TestPolicy() {
upload_scan() ? TestPolicyOnUpload() : TestPolicyOnDownload();
}
......@@ -218,21 +275,6 @@ class ConnectorsManagerLegacyPoliciesTest
auto no_settings = GetAnalysisSettingsSync(url_, connector());
ASSERT_FALSE(no_settings.has_value());
}
protected:
content::BrowserTaskEnvironment task_environment_;
base::test::ScopedFeatureList scoped_feature_list_;
TestingProfileManager profile_manager_;
TestingProfile* profile_;
ConnectorsManager* connectors_manager_;
GURL url_ = GURL("https://google.com");
// Set to the default value of their legacy policy.
std::set<std::string> expected_tags_ = {};
BlockUntilVerdict expected_block_until_verdict_ = BlockUntilVerdict::NO_BLOCK;
bool expected_block_password_protected_files_ = false;
bool expected_block_large_files_ = false;
bool expected_block_unsupported_file_types_ = false;
};
class ConnectorsManagerBlockLargeFileTest
......@@ -354,4 +396,152 @@ INSTANTIATE_TEST_SUITE_P(
testing::Combine(testing::ValuesIn(kAllAnalysisConnectors),
testing::ValuesIn(kAllDelayDeliveryUntilVerdictValues)));
using ConnectorsManagerNoFeatureTest = ConnectorsManagerTest;
TEST_F(ConnectorsManagerNoFeatureTest, OnFileAttached) {
SetConnectorPref(kOnFileAttachedPref, kNormalSettingsPref);
// Ensure that the default legacy settings are read synchronously.
int i = 0;
expected_tags_ = {"dlp"};
for (const char* url :
{kDlpAndMalwareUrl, kOnlyDlpUrl, kOnlyMalwareUrl, kNoTagsUrl}) {
ConnectorsManager::GetInstance()->GetAnalysisSettings(
GURL(url), AnalysisConnector::FILE_ATTACHED,
base::BindLambdaForTesting(
[this, &i](base::Optional<AnalysisSettings> settings) {
ASSERT_TRUE(settings.has_value());
ValidateSettings(settings.value());
++i;
}));
}
ASSERT_EQ(i, 4);
// No cached settings imply the connector value was never read.
ASSERT_TRUE(ConnectorsManager::GetInstance()
->GetAnalysisConnectorsSettingsForTesting()
.empty());
}
class ConnectorsManagerConnectorPoliciesTest
: public ConnectorsManagerTest,
public testing::WithParamInterface<const char*> {
public:
ConnectorsManagerConnectorPoliciesTest() {
scoped_feature_list_.InitWithFeatures({kEnterpriseConnectorsEnabled}, {});
}
~ConnectorsManagerConnectorPoliciesTest() override {
ConnectorsManager::GetInstance()->Reset();
}
const char* url() const { return GetParam(); }
void SetUpExpectedSettings(const char* pref) {
auto expected_settings = ExpectedSettings(pref, url());
expect_settings_ = expected_settings.has_value();
if (expected_settings.has_value()) {
expected_tags_ = expected_settings.value().tags;
expected_block_until_verdict_ =
expected_settings.value().block_until_verdict;
expected_block_password_protected_files_ =
expected_settings.value().block_password_protected_files;
expected_block_unsupported_file_types_ =
expected_settings.value().block_unsupported_file_types;
expected_block_large_files_ = expected_settings.value().block_large_files;
}
}
protected:
base::Optional<AnalysisSettings> ExpectedSettings(const char* pref,
const char* url) {
if (pref == kEmptySettingsPref || url == kNoTagsUrl)
return base::nullopt;
AnalysisSettings settings;
settings.block_until_verdict = BlockUntilVerdict::BLOCK;
settings.block_password_protected_files = true;
settings.block_large_files = true;
settings.block_unsupported_file_types = true;
if (url == kDlpAndMalwareUrl)
settings.tags = {"dlp", "malware"};
else if (url == kOnlyDlpUrl)
settings.tags = {"dlp"};
else if (url == kOnlyMalwareUrl)
settings.tags = {"malware"};
return settings;
}
bool expect_settings_;
};
TEST_P(ConnectorsManagerConnectorPoliciesTest, OnFileAttached) {
ASSERT_TRUE(ConnectorsManager::GetInstance()
->GetAnalysisConnectorsSettingsForTesting()
.empty());
SetConnectorPref(kOnFileAttachedPref, kNormalSettingsPref);
SetUpExpectedSettings(kNormalSettingsPref);
// Verify that the expected settings are returned normally.
bool called = false;
ConnectorsManager::GetInstance()->GetAnalysisSettings(
GURL(url()), AnalysisConnector::FILE_ATTACHED,
base::BindLambdaForTesting(
[this, &called](base::Optional<AnalysisSettings> settings) {
ASSERT_EQ(expect_settings_, settings.has_value());
if (settings.has_value())
ValidateSettings(settings.value());
called = true;
}));
ASSERT_TRUE(called);
// Verify that the expected settings are also returned by the cached settings.
const auto& cached_settings = ConnectorsManager::GetInstance()
->GetAnalysisConnectorsSettingsForTesting();
ASSERT_EQ(1u, cached_settings.size());
ASSERT_EQ(1u, cached_settings.count(AnalysisConnector::FILE_ATTACHED));
ASSERT_EQ(1u, cached_settings.at(AnalysisConnector::FILE_ATTACHED).size());
auto settings = cached_settings.at(AnalysisConnector::FILE_ATTACHED)
.at(0)
.GetAnalysisSettings(GURL(url()));
ASSERT_EQ(expect_settings_, settings.has_value());
if (settings.has_value())
ValidateSettings(settings.value());
}
TEST_P(ConnectorsManagerConnectorPoliciesTest, OnFileAttached_EmptyList) {
// If the connector's settings list is empty, no analysis settings are ever
// returned.
ASSERT_TRUE(ConnectorsManager::GetInstance()
->GetAnalysisConnectorsSettingsForTesting()
.empty());
SetConnectorPref(kOnFileAttachedPref, kEmptySettingsPref);
bool called = false;
ConnectorsManager::GetInstance()->GetAnalysisSettings(
GURL(url()), AnalysisConnector::FILE_ATTACHED,
base::BindLambdaForTesting(
[&called](base::Optional<AnalysisSettings> settings) {
ASSERT_FALSE(settings.has_value());
called = true;
}));
ASSERT_TRUE(called);
ASSERT_TRUE(ConnectorsManager::GetInstance()
->GetAnalysisConnectorsSettingsForTesting()
.empty());
}
INSTANTIATE_TEST_CASE_P(ConnectorsManagerConnectorPoliciesTest,
ConnectorsManagerConnectorPoliciesTest,
testing::Values(kDlpAndMalwareUrl,
kOnlyDlpUrl,
kOnlyMalwareUrl,
kNoTagsUrl));
} // namespace enterprise_connectors
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment