Skip to content

Commit

Permalink
[M115] ExtensionUnpublishedAvailability policy should ignore malware …
Browse files Browse the repository at this point in the history
…extensions

Extensions taken down for malware are already de-activated/unloaded in
Chrome via an existing blocklisting mechanism. A user is not allowed
to re-enable these extensions. So the policy does not add any value here
by trying to disable these extensions. Moreover, making the policy
disable malware extensions has a side-effect of showing the policy
disable string in the extension's Detail view (in chrome://extensions) even when the policy is disabled. See bug description for more details.

(cherry picked from commit 94423b3)

Bug: 1449026
Change-Id: I26866d04fe9534da4b2a97e67b3b0b4424a70a33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4567823
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Anunoy Ghosh <anunoy@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1149789}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4568995
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#113}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Anunoy Ghosh authored and Chromium LUCI CQ committed May 29, 2023
1 parent a072ee0 commit c016d8a
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 11 deletions.
16 changes: 12 additions & 4 deletions chrome/browser/extensions/extension_management.cc
Expand Up @@ -336,13 +336,21 @@ bool ExtensionManagement::IsAllowedByUnpublishedAvailabilityPolicy(
if (!cws_info_service_) {
cws_info_service_ = CWSInfoService::Get(profile_);
}
// Return the current live-in-CWS status of the extension in CWS if available,
// otherwise assume it's currently published and return true.
// Return the current published status of the extension in CWS if available.
// Otherwise assume the extension is currently published and return true.
// Ignore extensions taken down for malware as they are blocklisted and
// unloaded independently of policy.
// Current publish status may not available if the policy setting just changed
// to |kDisableUnpublished|. The actual publish status will be retrieved
// by CWSInfoService separately and will trigger this same policy check.
auto live_status = cws_info_service_->IsLiveInCWS(*extension);
return live_status.value_or(true);
absl::optional<CWSInfoServiceInterface::CWSInfo> cws_info =
cws_info_service_->GetCWSInfo(*extension);
if (cws_info.has_value() && cws_info->is_present &&
cws_info->violation_type !=
CWSInfoServiceInterface::CWSViolationType::kMalware) {
return cws_info->is_live;
}
return true;
}

APIPermissionSet ExtensionManagement::GetBlockedAPIPermissions(
Expand Down
40 changes: 33 additions & 7 deletions chrome/browser/extensions/extension_management_unittest.cc
Expand Up @@ -1275,7 +1275,7 @@ TEST_F(ExtensionManagementServiceTest,
// checks.
testing::NiceMock<MockCWSInfoService> mock_cws_info_service;
SetCWSInfoService(&mock_cws_info_service);
EXPECT_CALL(mock_cws_info_service, IsLiveInCWS).Times(0);
EXPECT_CALL(mock_cws_info_service, GetCWSInfo).Times(0);
// Verify that the extensions are allowed regardless of policy setting.
SetPref(true, pref_names::kExtensionUnpublishedAvailability,
base::Value(static_cast<int>(
Expand Down Expand Up @@ -1312,7 +1312,7 @@ TEST_F(ExtensionManagementServiceTest,
// CWS publish state should not be queried when this extension is checked.
testing::NiceMock<MockCWSInfoService> mock_cws_info_service;
SetCWSInfoService(&mock_cws_info_service);
EXPECT_CALL(mock_cws_info_service, IsLiveInCWS).Times(0);
EXPECT_CALL(mock_cws_info_service, GetCWSInfo).Times(0);
// Verify that the extension is allowed.
EXPECT_TRUE(extension_management_->IsAllowedByUnpublishedAvailabilityPolicy(
normal_extension.get()));
Expand All @@ -1334,14 +1334,40 @@ TEST_F(ExtensionManagementServiceTest,
// Create a test extension.
scoped_refptr<const Extension> normal_extension =
CreateNormalExtension(kTargetExtension);
// Create mock CWSInfoService to verify when IsLiveInCWS check is called.
// Create mock CWSInfoService to verify GetCWSInfo is called.
testing::NiceMock<MockCWSInfoService> mock_cws_info_service;
SetCWSInfoService(&mock_cws_info_service);
EXPECT_CALL(mock_cws_info_service, IsLiveInCWS)
.WillOnce(testing::Return(absl::optional<bool>(true)))
.WillOnce(testing::Return(absl::optional<bool>(false)))
.WillOnce(testing::Return(absl::optional<bool>()));
// Set up responses to GetCWSInfo calls.
CWSInfoServiceInterface::CWSInfo cws_info_live = {
/*is_present=*/true,
/*is_live=*/true,
/*last_update_time=*/base::Time::Now(),
CWSInfoServiceInterface::CWSViolationType::kNone,
/*unpublished_long_ago=*/false,
/*no_privacy_practice=*/false};
CWSInfoServiceInterface::CWSInfo cws_info_not_live = {
/*is_present=*/true,
/*is_live=*/false,
/*last_update_time=*/base::Time::Now(),
CWSInfoServiceInterface::CWSViolationType::kNone,
/*unpublished_long_ago=*/false,
/*no_privacy_practice=*/false};
CWSInfoServiceInterface::CWSInfo cws_info_malware = {
/*is_present=*/true,
/*is_live=*/false,
/*last_update_time=*/base::Time::Now(),
CWSInfoServiceInterface::CWSViolationType::kMalware,
/*unpublished_long_ago=*/false,
/*no_privacy_practice=*/false};
EXPECT_CALL(mock_cws_info_service, GetCWSInfo)
.WillOnce(testing::Return(cws_info_live))
.WillOnce(testing::Return(cws_info_malware))
.WillOnce(testing::Return(cws_info_not_live))
.WillOnce(testing::Return(absl::nullopt));
// Verify that the extension is allowed when it is live in CWS.
EXPECT_TRUE(extension_management_->IsAllowedByUnpublishedAvailabilityPolicy(
normal_extension.get()));
// Verify that the extension is ignored, i.e. allowed, when it is malware.
EXPECT_TRUE(extension_management_->IsAllowedByUnpublishedAvailabilityPolicy(
normal_extension.get()));
// Verify that the extension is disallowed when it is not live in CWS.
Expand Down

0 comments on commit c016d8a

Please sign in to comment.