From 2e22b13b3949c093dd07dde98feddddc9013beb0 Mon Sep 17 00:00:00 2001 From: Wiktor Garbacz Date: Wed, 11 Sep 2019 02:39:07 -0700 Subject: [PATCH] Enable namespaces by default PiperOrigin-RevId: 268417712 Change-Id: I496d76e8a90665627b9be2bb5f9872a5df1c84e4 --- sandboxed_api/examples/stringop/lib/sandbox.h | 1 - sandboxed_api/examples/sum/lib/sandbox.h | 1 - sandboxed_api/sandbox.cc | 1 - .../custom_fork/custom_fork_sandbox.cc | 1 - .../examples/network/network_sandbox.cc | 1 - .../network_proxy/networkproxy_sandbox.cc | 1 - .../examples/static/static_sandbox.cc | 1 - .../sandbox2/examples/tool/sandbox2tool.cc | 2 -- .../sandbox2/examples/zlib/zpipe_sandbox.cc | 1 - sandboxed_api/sandbox2/namespace_test.cc | 4 ---- sandboxed_api/sandbox2/policy_test.cc | 4 ---- sandboxed_api/sandbox2/policybuilder.cc | 3 --- sandboxed_api/sandbox2/policybuilder.h | 24 +++++++++---------- sandboxed_api/sandbox2/policybuilder_test.cc | 1 - sandboxed_api/sandbox2/sandbox2_test.cc | 4 ---- sandboxed_api/sandbox2/stack_trace.cc | 2 -- sandboxed_api/sandbox2/stack_trace_test.cc | 2 -- 17 files changed, 11 insertions(+), 43 deletions(-) diff --git a/sandboxed_api/examples/stringop/lib/sandbox.h b/sandboxed_api/examples/stringop/lib/sandbox.h index ac02522..452629b 100644 --- a/sandboxed_api/examples/stringop/lib/sandbox.h +++ b/sandboxed_api/examples/stringop/lib/sandbox.h @@ -46,7 +46,6 @@ class StringopSapiSandbox : public StringopSandbox { __NR_close, }) .AddFile("/etc/localtime") - .EnableNamespaces() .BuildOrDie(); } }; diff --git a/sandboxed_api/examples/sum/lib/sandbox.h b/sandboxed_api/examples/sum/lib/sandbox.h index e6e2711..7f46439 100644 --- a/sandboxed_api/examples/sum/lib/sandbox.h +++ b/sandboxed_api/examples/sum/lib/sandbox.h @@ -48,7 +48,6 @@ class SumSapiSandbox : public SumSandbox { __NR_close, }) .AddFile("/etc/localtime") - .EnableNamespaces() .BuildOrDie(); } }; diff --git a/sandboxed_api/sandbox.cc b/sandboxed_api/sandbox.cc index 2756aac..2eaab90 100644 --- a/sandboxed_api/sandbox.cc +++ b/sandboxed_api/sandbox.cc @@ -56,7 +56,6 @@ Sandbox::~Sandbox() { // are single-threaded and require ~30 basic syscalls. void InitDefaultPolicyBuilder(sandbox2::PolicyBuilder* builder) { (*builder) - .EnableNamespaces() .AllowRead() .AllowWrite() .AllowExit() diff --git a/sandboxed_api/sandbox2/examples/custom_fork/custom_fork_sandbox.cc b/sandboxed_api/sandbox2/examples/custom_fork/custom_fork_sandbox.cc index 8476a17..6d1a7bf 100644 --- a/sandboxed_api/sandbox2/examples/custom_fork/custom_fork_sandbox.cc +++ b/sandboxed_api/sandbox2/examples/custom_fork/custom_fork_sandbox.cc @@ -56,7 +56,6 @@ std::unique_ptr GetPolicy() { defined(THREAD_SANITIZER) .AllowMmap() #endif - .EnableNamespaces() .BuildOrDie(); } diff --git a/sandboxed_api/sandbox2/examples/network/network_sandbox.cc b/sandboxed_api/sandbox2/examples/network/network_sandbox.cc index a6c4460..c31a9fe 100644 --- a/sandboxed_api/sandbox2/examples/network/network_sandbox.cc +++ b/sandboxed_api/sandbox2/examples/network/network_sandbox.cc @@ -43,7 +43,6 @@ namespace { std::unique_ptr GetPolicy(absl::string_view sandboxee_path) { return sandbox2::PolicyBuilder() - .EnableNamespaces() .AllowExit() .AllowMmap() .AllowRead() diff --git a/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_sandbox.cc b/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_sandbox.cc index 605f9b1..e7704c7 100644 --- a/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_sandbox.cc +++ b/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_sandbox.cc @@ -32,7 +32,6 @@ namespace { std::unique_ptr GetPolicy(absl::string_view sandboxee_path) { return sandbox2::PolicyBuilder() .AllowExit() - .EnableNamespaces() .AllowMmap() .AllowRead() .AllowWrite() diff --git a/sandboxed_api/sandbox2/examples/static/static_sandbox.cc b/sandboxed_api/sandbox2/examples/static/static_sandbox.cc index eebf3f9..69e242f 100644 --- a/sandboxed_api/sandbox2/examples/static/static_sandbox.cc +++ b/sandboxed_api/sandbox2/examples/static/static_sandbox.cc @@ -93,7 +93,6 @@ std::unique_ptr GetPolicy() { #else .BlockSyscallWithErrno(__NR_openat, ENOENT) #endif - .EnableNamespaces() .BuildOrDie(); } diff --git a/sandboxed_api/sandbox2/examples/tool/sandbox2tool.cc b/sandboxed_api/sandbox2/examples/tool/sandbox2tool.cc index fab55f6..4ea5e7d 100644 --- a/sandboxed_api/sandbox2/examples/tool/sandbox2tool.cc +++ b/sandboxed_api/sandbox2/examples/tool/sandbox2tool.cc @@ -144,8 +144,6 @@ int main(int argc, char** argv) { builder.AddPolicyOnSyscall(__NR_tee, {KILL}); builder.DangerDefaultAllowAll(); - builder.EnableNamespaces(); - if (absl::GetFlag(FLAGS_sandbox2tool_need_networking)) { builder.AllowUnrestrictedNetworking(); } diff --git a/sandboxed_api/sandbox2/examples/zlib/zpipe_sandbox.cc b/sandboxed_api/sandbox2/examples/zlib/zpipe_sandbox.cc index 5f5abc0..3840a49 100644 --- a/sandboxed_api/sandbox2/examples/zlib/zpipe_sandbox.cc +++ b/sandboxed_api/sandbox2/examples/zlib/zpipe_sandbox.cc @@ -57,7 +57,6 @@ std::unique_ptr GetPolicy() { .AllowStaticStartup() .AllowSystemMalloc() .AllowExit() - .EnableNamespaces() .BlockSyscallWithErrno(__NR_access, ENOENT) .BuildOrDie(); } diff --git a/sandboxed_api/sandbox2/namespace_test.cc b/sandboxed_api/sandbox2/namespace_test.cc index d4d190e..bead3a8 100644 --- a/sandboxed_api/sandbox2/namespace_test.cc +++ b/sandboxed_api/sandbox2/namespace_test.cc @@ -49,7 +49,6 @@ TEST(NamespaceTest, FileNamespaceWorks) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't restrict the syscalls at all .DangerDefaultAllowAll() - .EnableNamespaces() .AddFileAt(path, "/binary_path") .TryBuild()); @@ -69,7 +68,6 @@ TEST(NamespaceTest, UserNamespaceWorks) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't restrict the syscalls at all .DangerDefaultAllowAll() - .EnableNamespaces() .TryBuild()); Sandbox2 sandbox(std::move(executor), std::move(policy)); @@ -104,7 +102,6 @@ TEST(NamespaceTest, UserNamespaceIDMapWritten) { std::vector args = {path, "3", "1000", "1000"}; auto executor = absl::make_unique(path, args); SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() - .EnableNamespaces() // Don't restrict the syscalls at all .DangerDefaultAllowAll() .TryBuild()); @@ -165,7 +162,6 @@ TEST_F(HostnameTest, Default) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't restrict the syscalls at all .DangerDefaultAllowAll() - .EnableNamespaces() .TryBuild()); Try("sandbox2", std::move(policy)); EXPECT_EQ(code_, 0); diff --git a/sandboxed_api/sandbox2/policy_test.cc b/sandboxed_api/sandbox2/policy_test.cc index 6f6758a..0ca0939 100644 --- a/sandboxed_api/sandbox2/policy_test.cc +++ b/sandboxed_api/sandbox2/policy_test.cc @@ -162,7 +162,6 @@ std::unique_ptr MinimalTestcasePolicy() { .AllowExit() .BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_access, ENOENT) - .EnableNamespaces() .BuildOrDie(); } @@ -200,7 +199,6 @@ TEST(MinimalTest, MinimalSharedBinaryWorks) { // New glibc accesses /etc/ld.so.preload .BlockSyscallWithErrno(__NR_access, ENOENT) .BlockSyscallWithErrno(__NR_prlimit64, EPERM) - .EnableNamespaces() .AddLibrariesForBinary(path) .BuildOrDie(); @@ -223,7 +221,6 @@ TEST(MallocTest, SystemMallocWorks) { .AllowStaticStartup() .AllowSystemMalloc() .AllowExit() - .EnableNamespaces() .BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_access, ENOENT) .BuildOrDie(); @@ -259,7 +256,6 @@ TEST(MultipleSyscalls, AddPolicyOnSyscallsWorks) { .AddPolicyOnSyscalls({__NR_getresuid, __NR_getresgid}, {ERRNO(42)}) .AddPolicyOnSyscalls({__NR_read, __NR_write}, {ERRNO(43)}) .AddPolicyOnSyscall(__NR_umask, {DENY}) - .EnableNamespaces() .BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_access, ENOENT) .BuildOrDie(); diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index ac29150..83d35e5 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -665,9 +665,6 @@ std::vector PolicyBuilder::ResolveBpfFunc(BpfFunc f) { } sapi::StatusOr> PolicyBuilder::TryBuild() { - CHECK_NE(use_namespaces_, disable_namespaces_) - << "Namespaces should either be enabled (by calling EnableNamespaces(), " - "AddFile(), etc.) or disabled (by calling DisableNamespaces())"; if (!last_status_.ok()) { return last_status_; } diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index f153e7a..3fb04ae 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/sandboxed_api/sandbox2/policybuilder.h @@ -447,28 +447,26 @@ class PolicyBuilder final { // Enables the use of namespaces. // - // Namespaces are automatically enabled when using namespace helper features - // (e.g. AddFile), therefore it is only necessary to explicitly enable - // namespaces when not using any other namespace helper feature. + // Namespaces are enabled by default. + // This is a no-op. + ABSL_DEPRECATED("Namespaces are enabled by default; no need to call this") PolicyBuilder& EnableNamespaces() { - CHECK(!disable_namespaces_) - << "Namespaces cannot be both disabled and enabled"; - use_namespaces_ = true; + CHECK(use_namespaces_) << "Namespaces cannot be both disabled and enabled"; + requires_namespaces_ = true; return *this; } // Disables the use of namespaces. // - // Sandbox2 with namespaces enabled is the recommended mode and will be the - // default in future, then calling this function will be necessary in order - // to use Sandbox2 without namespaces. + // Call in order to use Sandbox2 without namespaces. + // This is not recommended. PolicyBuilder& DisableNamespaces() { - CHECK(!use_namespaces_) + CHECK(!requires_namespaces_) << "Namespaces cannot be both disabled and enabled. You're probably " "using features that implicitly enable namespaces (SetHostname, " "AddFile, AddDirectory, AddDataDependency, AddLibrariesForBinary or " "similar)"; - disable_namespaces_ = true; + use_namespaces_ = false; return *this; } @@ -525,8 +523,8 @@ class PolicyBuilder final { void StoreDescription(PolicyBuilderDescription* pb_description); Mounts mounts_; - bool use_namespaces_ = false; - bool disable_namespaces_ = false; + bool use_namespaces_ = true; + bool requires_namespaces_ = false; bool allow_unrestricted_networking_ = false; std::string hostname_ = kDefaultHostname; diff --git a/sandboxed_api/sandbox2/policybuilder_test.cc b/sandboxed_api/sandbox2/policybuilder_test.cc index 5c6da64..dd5a63e 100644 --- a/sandboxed_api/sandbox2/policybuilder_test.cc +++ b/sandboxed_api/sandbox2/policybuilder_test.cc @@ -194,7 +194,6 @@ std::string PolicyBuilderTest::Run(std::vector args, TEST_F(PolicyBuilderTest, TestCanOnlyBuildOnce) { PolicyBuilder b; - b.EnableNamespaces(); ASSERT_THAT(b.BuildOrDie(), NotNull()); ASSERT_DEATH(b.BuildOrDie(), "Can only build policy once"); } diff --git a/sandboxed_api/sandbox2/sandbox2_test.cc b/sandboxed_api/sandbox2/sandbox2_test.cc index 1a3f290..70a1a94 100644 --- a/sandboxed_api/sandbox2/sandbox2_test.cc +++ b/sandboxed_api/sandbox2/sandbox2_test.cc @@ -125,7 +125,6 @@ TEST(RunAsyncTest, SandboxeeExternalKill) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't restrict the syscalls at all. .DangerDefaultAllowAll() - .EnableNamespaces() .TryBuild()); Sandbox2 sandbox(std::move(executor), std::move(policy)); ASSERT_TRUE(sandbox.RunAsync()); @@ -148,7 +147,6 @@ TEST(RunAsyncTest, SandboxeeTimeoutWithStacktraces) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't restrict the syscalls at all. .DangerDefaultAllowAll() - .EnableNamespaces() .TryBuild()); Sandbox2 sandbox(std::move(executor), std::move(policy)); ASSERT_TRUE(sandbox.RunAsync()); @@ -169,7 +167,6 @@ TEST(RunAsyncTest, SandboxeeTimeoutDisabledStacktraces) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't restrict the syscalls at all. .DangerDefaultAllowAll() - .EnableNamespaces() .CollectStacktracesOnTimeout(false) .TryBuild()); Sandbox2 sandbox(std::move(executor), std::move(policy)); @@ -191,7 +188,6 @@ TEST(RunAsyncTest, SandboxeeViolationDisabledStacktraces) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() // Don't allow anything - Make sure that we'll crash. - .EnableNamespaces() .CollectStacktracesOnViolation(false) .TryBuild()); Sandbox2 sandbox(std::move(executor), std::move(policy)); diff --git a/sandboxed_api/sandbox2/stack_trace.cc b/sandboxed_api/sandbox2/stack_trace.cc index bfcc3e5..e892c0b 100644 --- a/sandboxed_api/sandbox2/stack_trace.cc +++ b/sandboxed_api/sandbox2/stack_trace.cc @@ -114,8 +114,6 @@ std::unique_ptr StackTracePeer::GetPolicy(pid_t target_pid, JEQ32(static_cast(1), ALLOW), }) - .EnableNamespaces() - // Add proc maps. .AddFileAt(maps_file, file::JoinPath("/proc", absl::StrCat(target_pid), "maps")) diff --git a/sandboxed_api/sandbox2/stack_trace_test.cc b/sandboxed_api/sandbox2/stack_trace_test.cc index e5921b2..82ae00d 100644 --- a/sandboxed_api/sandbox2/stack_trace_test.cc +++ b/sandboxed_api/sandbox2/stack_trace_test.cc @@ -82,7 +82,6 @@ void SymbolizationWorksCommon( policybuilder // Don't restrict the syscalls at all. .DangerDefaultAllowAll() - .EnableNamespaces() .AddFile(path) .AddLibrariesForBinary(path) .AddFileAt(temp_filename, "/proc/cpuinfo"); @@ -180,7 +179,6 @@ TEST(StackTraceTest, SymbolizationTrustedFilesOnly) { SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder{} // Don't restrict the syscalls at all. .DangerDefaultAllowAll() - .EnableNamespaces() .AddFile(path) .AddLibrariesForBinary(path) .TryBuild());