Skip to content

Commit

Permalink
dlp: Return all restrictions on IsFilesTransferRestricted.
Browse files Browse the repository at this point in the history 
In order to be able to cache IsFilesTransferRestricted results
in the DLP daemon, Chrome should return not only the restricted
files, but the evaluated restriction level for the files.

Pending dlp_service.proto update.

Bug: b/269742560
Test: Tests are updated.
Change-Id: I1b04c6d1d3017be70aa39a938af3d3a153654612
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4272387
Reviewed-by: Aida Zolic <aidazolic@chromium.org>
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Aya Elsayed <ayaelattar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115637}
  • Loading branch information
Sergey Poromov authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent 14e7488 commit 6225f6a
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 61 deletions.
35 changes: 27 additions & 8 deletions chrome/browser/ash/dbus/dlp_files_policy_service_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,15 @@ void DlpFilesPolicyServiceProvider::IsFilesTransferRestricted(
policy::DlpFilesController* files_controller =
rules_manager->GetDlpFilesController();
if (!files_controller) {
std::vector<std::pair<policy::DlpFilesController::FileDaemonInfo,
dlp::RestrictionLevel>>
response_files;
for (const auto& file : files_info) {
response_files.emplace_back(file,
::dlp::RestrictionLevel::LEVEL_UNSPECIFIED);
}
RespondWithRestrictedFilesTransfer(method_call, std::move(response_sender),
std::move(files_info));
std::move(response_files));
return;
}

Expand All @@ -185,15 +192,27 @@ void DlpFilesPolicyServiceProvider::IsFilesTransferRestricted(
void DlpFilesPolicyServiceProvider::RespondWithRestrictedFilesTransfer(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender,
const std::vector<policy::DlpFilesController::FileDaemonInfo>&
restricted_files) {
const std::vector<std::pair<policy::DlpFilesController::FileDaemonInfo,
dlp::RestrictionLevel>>& requested_files) {
dlp::IsFilesTransferRestrictedResponse response_proto;

for (const auto& file : restricted_files) {
dlp::FileMetadata* file_metadata = response_proto.add_restricted_files();
file_metadata->set_inode(file.inode);
file_metadata->set_path(file.path.value());
file_metadata->set_source_url(file.source_url.spec());
for (const auto& [file, level] : requested_files) {
// Daemon still uses the old logic to rely on these fields.
// TODO(b/259182892): Remove when it's not used.
if (level == ::dlp::RestrictionLevel::LEVEL_BLOCK ||
level == ::dlp::RestrictionLevel::LEVEL_WARN_CANCEL) {
dlp::FileMetadata* file_metadata = response_proto.add_restricted_files();
file_metadata->set_inode(file.inode);
file_metadata->set_path(file.path.value());
file_metadata->set_source_url(file.source_url.spec());
}
dlp::FileRestriction* files_restriction =
response_proto.add_files_restrictions();
files_restriction->mutable_file_metadata()->set_inode(file.inode);
files_restriction->mutable_file_metadata()->set_path(file.path.value());
files_restriction->mutable_file_metadata()->set_source_url(
file.source_url.spec());
files_restriction->set_restriction_level(level);
}
std::unique_ptr<dbus::Response> response =
dbus::Response::FromMethodCall(method_call);
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ash/dbus/dlp_files_policy_service_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ class DlpFilesPolicyServiceProvider
void RespondWithRestrictedFilesTransfer(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender,
const std::vector<policy::DlpFilesController::FileDaemonInfo>&
restricted_files);
const std::vector<std::pair<policy::DlpFilesController::FileDaemonInfo,
dlp::RestrictionLevel>>& requested_files);

base::WeakPtrFactory<DlpFilesPolicyServiceProvider> weak_ptr_factory_{this};
};
Expand Down
83 changes: 53 additions & 30 deletions chrome/browser/ash/policy/dlp/dlp_files_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,8 +859,16 @@ void DlpFilesController::CheckIfDownloadAllowed(
FileAction::kDownload,
base::BindOnce( // TODO(b/270015718): Unify to ReturnIfActionAllowed.
[](CheckIfDlpAllowedCallback result_callback,
const std::vector<FileDaemonInfo>& restricted_files) {
bool is_allowed = restricted_files.empty();
const std::vector<std::pair<
FileDaemonInfo, ::dlp::RestrictionLevel>>& files_levels) {
bool is_allowed = true;
for (const auto& [file, level] : files_levels) {
if (level == ::dlp::RestrictionLevel::LEVEL_BLOCK ||
level == ::dlp::RestrictionLevel::LEVEL_WARN_CANCEL) {
is_allowed = false;
break;
}
}
if (!is_allowed) {
ShowNotification(
kDownloadBlockedNotificationId,
Expand Down Expand Up @@ -977,7 +985,7 @@ void DlpFilesController::IsFilesTransferRestricted(

DlpFileDestination deduplication_dst;

std::vector<FileDaemonInfo> restricted_files;
std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>> files_levels;
std::vector<FileDaemonInfo> warned_files;
std::vector<DlpConfidentialFile> dialog_files;
absl::optional<std::string> destination_pattern;
Expand Down Expand Up @@ -1009,22 +1017,36 @@ void DlpFilesController::IsFilesTransferRestricted(
destination_pattern, rule_metadata, level);
}

if (level == DlpRulesManager::Level::kBlock) {
restricted_files.push_back(file);
DlpHistogramEnumeration(dlp::kFileActionBlockedUMA, files_action);
} else if (level == DlpRulesManager::Level::kWarn) {
warned_files.push_back(file);
warned_source_patterns.emplace_back(source_pattern);
warned_rules_metadata.emplace_back(rule_metadata);
if (files_action != FileAction::kDownload) {
dialog_files.emplace_back(file.path);
switch (level) {
case DlpRulesManager::Level::kBlock: {
files_levels.push_back({file, ::dlp::RestrictionLevel::LEVEL_BLOCK});
DlpHistogramEnumeration(dlp::kFileActionBlockedUMA, files_action);
break;
}
case DlpRulesManager::Level::kNotSet:
case DlpRulesManager::Level::kAllow: {
files_levels.push_back({file, ::dlp::RestrictionLevel::LEVEL_ALLOW});
break;
}
case DlpRulesManager::Level::kReport: {
files_levels.push_back({file, ::dlp::RestrictionLevel::LEVEL_REPORT});
break;
}
case DlpRulesManager::Level::kWarn: {
warned_files.push_back(file);
warned_source_patterns.emplace_back(source_pattern);
warned_rules_metadata.emplace_back(rule_metadata);
if (files_action != FileAction::kDownload) {
dialog_files.emplace_back(file.path);
}
DlpHistogramEnumeration(dlp::kFileActionWarnedUMA, files_action);
break;
}
DlpHistogramEnumeration(dlp::kFileActionWarnedUMA, files_action);
}
}

if (warned_files.empty()) {
std::move(result_callback).Run(std::move(restricted_files));
std::move(result_callback).Run(std::move(files_levels));
return;
}

Expand All @@ -1034,12 +1056,12 @@ void DlpFilesController::IsFilesTransferRestricted(
}

warn_dialog_widget_ = warn_notifier_->ShowDlpFilesWarningDialog(
base::BindOnce(
&DlpFilesController::OnDlpWarnDialogReply,
weak_ptr_factory_.GetWeakPtr(), std::move(restricted_files),
std::move(warned_files), std::move(warned_source_patterns),
std::move(warned_rules_metadata), std::move(deduplication_dst),
destination_pattern, files_action, std::move(result_callback)),
base::BindOnce(&DlpFilesController::OnDlpWarnDialogReply,
weak_ptr_factory_.GetWeakPtr(), std::move(files_levels),
std::move(warned_files), std::move(warned_source_patterns),
std::move(warned_rules_metadata),
std::move(deduplication_dst), destination_pattern,
files_action, std::move(result_callback)),
std::move(dialog_files), dst_component, destination_pattern,
files_action);
}
Expand Down Expand Up @@ -1190,7 +1212,8 @@ void DlpFilesController::SetFileSystemContextForTesting(
}

void DlpFilesController::OnDlpWarnDialogReply(
std::vector<FileDaemonInfo> restricted_files,
std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>>
files_levels,
std::vector<FileDaemonInfo> warned_files,
std::vector<std::string> warned_src_patterns,
std::vector<DlpRulesManager::RuleMetadata> warned_rules_metadata,
Expand All @@ -1199,21 +1222,21 @@ void DlpFilesController::OnDlpWarnDialogReply(
FileAction files_action,
IsFilesTransferRestrictedCallback callback,
bool should_proceed) {
if (!should_proceed) {
restricted_files.insert(restricted_files.end(),
std::make_move_iterator(warned_files.begin()),
std::make_move_iterator(warned_files.end()));
} else {
DCHECK(warned_files.size() == warned_src_patterns.size());
DCHECK(warned_files.size() == warned_rules_metadata.size());
for (size_t i = 0; i < warned_files.size(); ++i) {
DCHECK(warned_files.size() == warned_src_patterns.size());
DCHECK(warned_files.size() == warned_rules_metadata.size());
for (size_t i = 0; i < warned_files.size(); ++i) {
if (should_proceed) {
DlpHistogramEnumeration(dlp::kFileActionWarnProceededUMA, files_action);
MaybeReportEvent(warned_files[i].inode, warned_files[i].path,
warned_src_patterns[i], dst, dst_pattern,
warned_rules_metadata[i], absl::nullopt);
}
files_levels.emplace_back(warned_files[i],
should_proceed
? ::dlp::RestrictionLevel::LEVEL_WARN_PROCEED
: ::dlp::RestrictionLevel::LEVEL_WARN_CANCEL);
}
std::move(callback).Run(std::move(restricted_files));
std::move(callback).Run(std::move(files_levels));
}

void DlpFilesController::ReturnDisallowedTransfers(
Expand Down
9 changes: 5 additions & 4 deletions chrome/browser/ash/policy/dlp/dlp_files_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ class DlpFilesController {
using CheckIfDlpAllowedCallback = base::OnceCallback<void(bool is_allowed)>;
using GetDlpMetadataCallback =
base::OnceCallback<void(std::vector<DlpFileMetadata>)>;
using IsFilesTransferRestrictedCallback =
base::OnceCallback<void(const std::vector<FileDaemonInfo>&)>;
using IsFilesTransferRestrictedCallback = base::OnceCallback<void(
const std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>>&)>;

explicit DlpFilesController(const DlpRulesManager& rules_manager);
DlpFilesController(const DlpFilesController& other) = delete;
Expand Down Expand Up @@ -262,9 +262,10 @@ class DlpFilesController {
private:
// Called back from warning dialog. Passes blocked files sources along
// to |callback|. In case |should_proceed| is true, passes only
// |restricted_files_sources|, otherwise passes also |warned_files_sources|.
// |files_levels|, otherwise passes also |warned_files_sources|.
void OnDlpWarnDialogReply(
std::vector<FileDaemonInfo> restricted_files_sources,
std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>>
files_levels,
std::vector<FileDaemonInfo> warned_files_sources,
std::vector<std::string> warned_src_patterns,
std::vector<DlpRulesManager::RuleMetadata> warned_rules_metadata,
Expand Down
61 changes: 44 additions & 17 deletions chrome/browser/ash/policy/dlp/dlp_files_controller_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1310,10 +1310,12 @@ TEST_F(DlpFilesControllerTest, CheckReportingOnIsFilesTransferRestricted) {

std::vector<DlpFilesController::FileDaemonInfo> transferred_files = {file1,
file2};
std::vector<DlpFilesController::FileDaemonInfo> disallowed_files = {file1};
std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>> files_levels =
{{file1, ::dlp::RestrictionLevel::LEVEL_BLOCK},
{file2, ::dlp::RestrictionLevel::LEVEL_ALLOW}};

MockIsFilesTransferRestrictedCallback cb;
EXPECT_CALL(cb, Run(disallowed_files)).Times(::testing::AnyNumber());
EXPECT_CALL(cb, Run(files_levels)).Times(::testing::AnyNumber());

auto event_builder = DlpPolicyEventBuilder::Event(
kExampleSourcePattern1, kRuleName1, kRuleId1,
Expand Down Expand Up @@ -1387,10 +1389,12 @@ TEST_F(DlpFilesControllerTest, CheckReportingOnMixedCalls) {

std::vector<DlpFilesController::FileDaemonInfo> transferred_files = {file1,
file2};
std::vector<DlpFilesController::FileDaemonInfo> disallowed_files = {file1};
std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>> files_levels =
{{file1, ::dlp::RestrictionLevel::LEVEL_BLOCK},
{file2, ::dlp::RestrictionLevel::LEVEL_ALLOW}};

MockIsFilesTransferRestrictedCallback cb;
EXPECT_CALL(cb, Run(disallowed_files)).Times(1);
EXPECT_CALL(cb, Run(files_levels)).Times(1);

auto event_builder = DlpPolicyEventBuilder::Event(
kExampleSourcePattern1, kRuleName1, kRuleId1,
Expand Down Expand Up @@ -1848,8 +1852,20 @@ TEST_P(DlpFilesExternalDestinationTest, IsFilesTransferRestricted_Component) {
DlpFilesController::FileDaemonInfo(kInode3, base::FilePath(),
kExampleUrl3)});

std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>> files_levels =
{{DlpFilesController::FileDaemonInfo(kInode1, base::FilePath(),
kExampleUrl1),
::dlp::RestrictionLevel::LEVEL_BLOCK},
{DlpFilesController::FileDaemonInfo(kInode2, base::FilePath(),
kExampleUrl2),
::dlp::RestrictionLevel::LEVEL_ALLOW},
{DlpFilesController::FileDaemonInfo(kInode3, base::FilePath(),
kExampleUrl3),
::dlp::RestrictionLevel::LEVEL_BLOCK}};

MockIsFilesTransferRestrictedCallback cb;
EXPECT_CALL(cb, Run(disallowed_files)).Times(1);
EXPECT_CALL(cb, Run(files_levels)).Times(1);

EXPECT_CALL(*rules_manager_,
IsRestrictedComponent(_, expected_component, _, _, _))
.WillOnce(
Expand Down Expand Up @@ -1996,17 +2012,20 @@ TEST_P(DlpFilesUrlDestinationTest, IsFilesTransferRestricted_Url) {

const auto histogram_tester = base::HistogramTester();

std::vector<DlpFilesController::FileDaemonInfo> disallowed_files;
std::vector<std::pair<FileDaemonInfo, ::dlp::RestrictionLevel>> files_levels;
std::vector<std::string> disallowed_source_patterns;
std::vector<std::string> triggered_rule_names;
std::vector<std::string> triggered_rule_ids;

for (size_t i = 0; i < transferred_files.size(); ++i) {
if (levels[i] == DlpRulesManager::Level::kBlock) {
disallowed_files.emplace_back(transferred_files[i]);
files_levels.emplace_back(transferred_files[i],
::dlp::RestrictionLevel::LEVEL_BLOCK);
disallowed_source_patterns.emplace_back(source_patterns[i]);
triggered_rule_names.emplace_back(rules_names[i]);
triggered_rule_ids.emplace_back(rules_ids[i]);
} else {
files_levels.emplace_back(transferred_files[i],
::dlp::RestrictionLevel::LEVEL_ALLOW);
}
}
EXPECT_CALL(*rules_manager_, IsRestrictedDestination(_, _, _, _, _, _))
Expand All @@ -2027,15 +2046,15 @@ TEST_P(DlpFilesUrlDestinationTest, IsFilesTransferRestricted_Url) {
.Times(::testing::AnyNumber());

MockIsFilesTransferRestrictedCallback cb;
EXPECT_CALL(cb, Run(disallowed_files)).Times(1);
EXPECT_CALL(cb, Run(files_levels)).Times(1);

files_controller_->IsFilesTransferRestricted(
transferred_files,
DlpFilesController::DlpFileDestination(destination_url),
DlpFilesController::FileAction::kDownload, cb.Get());

ASSERT_EQ(events.size(), disallowed_files.size());
for (size_t i = 0u; i < disallowed_files.size(); ++i) {
ASSERT_EQ(events.size(), disallowed_source_patterns.size());
for (size_t i = 0u; i < disallowed_source_patterns.size(); ++i) {
EXPECT_THAT(
events[i],
IsDlpPolicyEvent(CreateDlpPolicyEvent(
Expand All @@ -2044,8 +2063,9 @@ TEST_P(DlpFilesUrlDestinationTest, IsFilesTransferRestricted_Url) {
triggered_rule_ids[i], DlpRulesManager::Level::kBlock)));
}

int blocked_downloads =
disallowed_files.empty() ? 0 : disallowed_files.size();
int blocked_downloads = disallowed_source_patterns.empty()
? 0
: disallowed_source_patterns.size();

EXPECT_THAT(
histogram_tester.GetAllSamples(GetDlpHistogramPrefix() +
Expand Down Expand Up @@ -2296,10 +2316,17 @@ TEST_P(DlpFilesWarningDialogContentTest,
IsFilesTransferRestricted_WarningDialogContent) {
auto transfer_info = GetParam();
std::vector<DlpFilesController::FileDaemonInfo> warned_files;
std::vector<
std::pair<DlpFilesController::FileDaemonInfo, ::dlp::RestrictionLevel>>
files_levels;
for (size_t i = 0; i < transfer_info.file_sources.size(); ++i) {
warned_files.emplace_back(transfer_info.file_inodes[i],
base::FilePath(transfer_info.file_paths[i]),
transfer_info.file_sources[i]);
DlpFilesController::FileDaemonInfo file_info(
transfer_info.file_inodes[i],
base::FilePath(transfer_info.file_paths[i]),
transfer_info.file_sources[i]);
warned_files.emplace_back(file_info);
files_levels.emplace_back(file_info,
::dlp::RestrictionLevel::LEVEL_WARN_CANCEL);
}
storage::ExternalMountPoints* mount_points =
storage::ExternalMountPoints::GetSystemInstance();
Expand Down Expand Up @@ -2347,7 +2374,7 @@ TEST_P(DlpFilesWarningDialogContentTest,
.Times(1);

MockIsFilesTransferRestrictedCallback cb;
EXPECT_CALL(cb, Run(warned_files)).Times(1);
EXPECT_CALL(cb, Run(files_levels)).Times(1);

auto dst_url = mount_points->CreateExternalFileSystemURL(
blink::StorageKey(), "removable",
Expand Down

0 comments on commit 6225f6a

Please sign in to comment.