Revert "Add Render Process Sandbox Policy feature tests"
This reverts commit cbbfffe534a0017c63f27229096289cb220937c6.
Reason for revert: suspect this CL has caused
RendererSandboxSettings/RendererFeatureSandboxWinTest.RendererGeneratedPolicyTest
to be flaky on multiple bots.
Bug: 1209051
Original change's description:
> Add Render Process Sandbox Policy feature tests
>
> This is primarily a Windows change, but some refactoring was necessary
> in order to make RendererSandboxedProcessLauncherDelegate usable in test
> code and this class exists on multiple flavors.
>
> Bug: 1266582
> Change-Id: Id9aefb3e3eed2e3e375c8f14e0e90127cb86fc3e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3260947
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Jonathan Ross <jonross@chromium.org>
> Commit-Queue: Emily Andrews <emiled@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#940462}
TBR=jonross@chromium.org,wfh@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,emiled@microsoft.com
Change-Id: I1efbf754758da3eca26b888e3b473b09356a0e3e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1266582
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3273699
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Olivia Yingst <huiyingst@google.com>
Reviewed-by: Yuki Yamada <yukiy@chromium.org>
Reviewed-by: Hui Yingst <nigi@chromium.org>
Owners-Override: Yuki Yamada <yukiy@chromium.org>
Commit-Queue: Olivia Yingst <huiyingst@google.com>
Cr-Commit-Position: refs/heads/main@{#940558}
NOKEYCHECK=True
GitOrigin-RevId: bcd1a8f73f3858472384a24c00b74c2922b0b9f4
diff --git a/policy/BUILD.gn b/policy/BUILD.gn
index 6b23a2a..9a21da3 100644
--- a/policy/BUILD.gn
+++ b/policy/BUILD.gn
@@ -190,10 +190,7 @@
"win/mf_cdm_sandbox_type_unittest.cc",
"win/sandbox_win_unittest.cc",
]
- deps += [
- ":sandbox_test_utils",
- "//sandbox/win:sandbox",
- ]
+ deps += [ "//sandbox/win:sandbox" ]
data = [
"//base/test/data/pe_image/pe_image_test_32.dll",
"//base/test/data/pe_image/pe_image_test_64.dll",
@@ -201,19 +198,3 @@
]
}
}
-
-source_set("sandbox_test_utils") {
- testonly = true
-
- if (is_win) {
- sources = [
- "win/sandbox_test_utils.cc",
- "win/sandbox_test_utils.h",
- ]
-
- deps = [
- "//sandbox/win:sandbox",
- "//testing/gtest",
- ]
- }
-}
diff --git a/policy/features.cc b/policy/features.cc
index 1a19cdd..5a53ee2 100644
--- a/policy/features.cc
+++ b/policy/features.cc
@@ -37,10 +37,6 @@
// Enables GPU Low Privilege AppContainer when combined with kGpuAppContainer.
const base::Feature kGpuLPAC{"GpuLPAC", base::FEATURE_ENABLED_BY_DEFAULT};
-// Enables Renderer AppContainer
-const base::Feature kRendererAppContainer{"RendererAppContainer",
- base::FEATURE_DISABLED_BY_DEFAULT};
-
#endif // defined(OS_WIN)
#if !defined(OS_ANDROID)
diff --git a/policy/features.h b/policy/features.h
index 34f4bb5..c39549d 100644
--- a/policy/features.h
+++ b/policy/features.h
@@ -26,7 +26,6 @@
SANDBOX_POLICY_EXPORT extern const base::Feature kWinSboxDisableExtensionPoints;
SANDBOX_POLICY_EXPORT extern const base::Feature kGpuAppContainer;
SANDBOX_POLICY_EXPORT extern const base::Feature kGpuLPAC;
-SANDBOX_POLICY_EXPORT extern const base::Feature kRendererAppContainer;
#endif // defined(OS_WIN)
#if !defined(OS_ANDROID)
diff --git a/policy/win/sandbox_test_utils.cc b/policy/win/sandbox_test_utils.cc
deleted file mode 100644
index 7cb2701..0000000
--- a/policy/win/sandbox_test_utils.cc
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright 2021 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 "sandbox/policy/win/sandbox_test_utils.h"
-
-#include "base/strings/strcat.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace sandbox {
-namespace policy {
-
-constexpr wchar_t kBaseSecurityDescriptor[] = L"D:(A;;GA;;;WD)";
-constexpr wchar_t kRegistryRead[] = L"registryRead";
-constexpr wchar_t klpacPnpNotifications[] = L"lpacPnpNotifications";
-
-std::vector<Sid> GetCapabilitySids(
- const std::initializer_list<std::wstring>& capabilities) {
- std::vector<Sid> sids;
- for (const auto& capability : capabilities) {
- sids.emplace_back(Sid::FromNamedCapability(capability.c_str()));
- }
- return sids;
-}
-
-std::wstring GetAccessAllowedForCapabilities(
- const std::initializer_list<std::wstring>& capabilities) {
- std::wstring sddl = kBaseSecurityDescriptor;
- for (const auto& capability : GetCapabilitySids(capabilities)) {
- std::wstring sid_string;
- CHECK(capability.ToSddlString(&sid_string));
- base::StrAppend(&sddl, {L"(A;;GRGX;;;", sid_string, L")"});
- }
- return sddl;
-}
-
-void EqualSidList(const std::vector<Sid>& left, const std::vector<Sid>& right) {
- EXPECT_EQ(left.size(), right.size());
- auto result = std::mismatch(left.cbegin(), left.cend(), right.cbegin(),
- [](const auto& left_sid, const auto& right_sid) {
- return !!::EqualSid(left_sid.GetPSID(),
- right_sid.GetPSID());
- });
- EXPECT_EQ(result.first, left.cend());
-}
-
-void CheckCapabilities(
- AppContainerBase* profile,
- const std::initializer_list<std::wstring>& additional_capabilities) {
- auto additional_caps = GetCapabilitySids(additional_capabilities);
- auto impersonation_caps =
- GetCapabilitySids({kChromeInstallFiles, klpacPnpNotifications,
- kLpacChromeInstallFiles, kRegistryRead});
- auto base_caps = GetCapabilitySids(
- {klpacPnpNotifications, kLpacChromeInstallFiles, kRegistryRead});
-
- impersonation_caps.insert(impersonation_caps.end(), additional_caps.begin(),
- additional_caps.end());
- base_caps.insert(base_caps.end(), additional_caps.begin(),
- additional_caps.end());
-
- EqualSidList(impersonation_caps, profile->GetImpersonationCapabilities());
- EqualSidList(base_caps, profile->GetCapabilities());
-}
-} // namespace policy
-} // namespace sandbox
\ No newline at end of file
diff --git a/policy/win/sandbox_test_utils.h b/policy/win/sandbox_test_utils.h
deleted file mode 100644
index dc869c9..0000000
--- a/policy/win/sandbox_test_utils.h
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2021 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.
-
-#ifndef SANDBOX_POLICY_WIN_SANDBOX_TEST_UTILS_H_
-#define SANDBOX_POLICY_WIN_SANDBOX_TEST_UTILS_H_
-
-#include <string>
-#include <vector>
-
-#include "sandbox/win/src/app_container_base.h"
-#include "sandbox/win/src/sid.h"
-
-namespace sandbox {
-namespace policy {
-
-constexpr wchar_t kChromeInstallFiles[] = L"chromeInstallFiles";
-constexpr wchar_t kLpacChromeInstallFiles[] = L"lpacChromeInstallFiles";
-
-std::vector<Sid> GetCapabilitySids(
- const std::initializer_list<std::wstring>& capabilities);
-
-std::wstring GetAccessAllowedForCapabilities(
- const std::initializer_list<std::wstring>& capabilities);
-
-void EqualSidList(const std::vector<Sid>& left, const std::vector<Sid>& right);
-
-void CheckCapabilities(
- AppContainerBase* profile,
- const std::initializer_list<std::wstring>& additional_capabilities);
-} // namespace policy
-} // namespace sandbox
-
-#endif // SANDBOX_POLICY_WIN_SANDBOX_TEST_UTILS_H_
\ No newline at end of file
diff --git a/policy/win/sandbox_win.cc b/policy/win/sandbox_win.cc
index 30a2498..a58d8ae 100644
--- a/policy/win/sandbox_win.cc
+++ b/policy/win/sandbox_win.cc
@@ -579,7 +579,10 @@
if (base::win::GetVersion() < base::win::Version::WIN8)
return false;
- return base::FeatureList::IsEnabled(features::kRendererAppContainer);
+ static const base::Feature kRendererAppContainer{
+ "RendererAppContainer", base::FEATURE_DISABLED_BY_DEFAULT};
+
+ return base::FeatureList::IsEnabled(kRendererAppContainer);
}
ResultCode SetJobMemoryLimit(const base::CommandLine& cmd_line,
diff --git a/policy/win/sandbox_win_unittest.cc b/policy/win/sandbox_win_unittest.cc
index f45d320..2499414 100644
--- a/policy/win/sandbox_win_unittest.cc
+++ b/policy/win/sandbox_win_unittest.cc
@@ -20,6 +20,7 @@
#include "base/notreached.h"
#include "base/path_service.h"
#include "base/scoped_native_library.h"
+#include "base/strings/strcat.h"
#include "base/test/scoped_feature_list.h"
#include "base/win/windows_version.h"
#include "build/build_config.h"
@@ -27,10 +28,11 @@
#include "sandbox/policy/mojom/sandbox.mojom.h"
#include "sandbox/policy/sandbox_type.h"
#include "sandbox/policy/switches.h"
-#include "sandbox/policy/win/sandbox_test_utils.h"
+#include "sandbox/win/src/app_container_base.h"
#include "sandbox/win/src/sandbox_factory.h"
#include "sandbox/win/src/sandbox_policy.h"
#include "sandbox/win/src/sandbox_policy_diagnostic.h"
+#include "sandbox/win/src/sid.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -41,10 +43,16 @@
namespace policy {
namespace {
+
+constexpr wchar_t kBaseSecurityDescriptor[] = L"D:(A;;GA;;;WD)";
constexpr char kAppContainerId[] = "SandboxWinTest";
constexpr wchar_t kPackageSid[] =
L"S-1-15-2-1505217662-1870513255-555216753-1864132992-3842232122-"
L"1807018979-869957911";
+constexpr wchar_t kChromeInstallFiles[] = L"chromeInstallFiles";
+constexpr wchar_t kLpacChromeInstallFiles[] = L"lpacChromeInstallFiles";
+constexpr wchar_t kRegistryRead[] = L"registryRead";
+constexpr wchar_t klpacPnpNotifications[] = L"lpacPnpNotifications";
class TestTargetPolicy : public TargetPolicy {
public:
@@ -147,6 +155,26 @@
scoped_refptr<AppContainerBase> app_container_;
};
+std::vector<Sid> GetCapabilitySids(
+ const std::initializer_list<std::wstring>& capabilities) {
+ std::vector<Sid> sids;
+ for (const auto& capability : capabilities) {
+ sids.emplace_back(Sid::FromNamedCapability(capability.c_str()));
+ }
+ return sids;
+}
+
+std::wstring GetAccessAllowedForCapabilities(
+ const std::initializer_list<std::wstring>& capabilities) {
+ std::wstring sddl = kBaseSecurityDescriptor;
+ for (const auto& capability : GetCapabilitySids(capabilities)) {
+ std::wstring sid_string;
+ CHECK(capability.ToSddlString(&sid_string));
+ base::StrAppend(&sddl, {L"(A;;GRGX;;;", sid_string, L")"});
+ }
+ return sddl;
+}
+
// Drops a temporary file granting RX access to a list of capabilities.
bool DropTempFileWithSecurity(
const base::ScopedTempDir& temp_dir,
@@ -166,6 +194,35 @@
return !!result;
}
+void EqualSidList(const std::vector<Sid>& left, const std::vector<Sid>& right) {
+ EXPECT_EQ(left.size(), right.size());
+ auto result = std::mismatch(left.cbegin(), left.cend(), right.cbegin(),
+ [](const auto& left_sid, const auto& right_sid) {
+ return !!::EqualSid(left_sid.GetPSID(),
+ right_sid.GetPSID());
+ });
+ EXPECT_EQ(result.first, left.cend());
+}
+
+void CheckCapabilities(
+ AppContainerBase* profile,
+ const std::initializer_list<std::wstring>& additional_capabilities) {
+ auto additional_caps = GetCapabilitySids(additional_capabilities);
+ auto impersonation_caps =
+ GetCapabilitySids({kChromeInstallFiles, klpacPnpNotifications,
+ kLpacChromeInstallFiles, kRegistryRead});
+ auto base_caps = GetCapabilitySids(
+ {klpacPnpNotifications, kLpacChromeInstallFiles, kRegistryRead});
+
+ impersonation_caps.insert(impersonation_caps.end(), additional_caps.begin(),
+ additional_caps.end());
+ base_caps.insert(base_caps.end(), additional_caps.begin(),
+ additional_caps.end());
+
+ EqualSidList(impersonation_caps, profile->GetImpersonationCapabilities());
+ EqualSidList(base_caps, profile->GetCapabilities());
+}
+
class SandboxWinTest : public ::testing::Test {
public:
SandboxWinTest() {}
