Enable namespaces by default

PiperOrigin-RevId: 268417712
Change-Id: I496d76e8a90665627b9be2bb5f9872a5df1c84e4
This commit is contained in:
Wiktor Garbacz 2019-09-11 02:39:07 -07:00 committed by Copybara-Service
parent 4034f1235e
commit 2e22b13b39
17 changed files with 11 additions and 43 deletions

View File

@ -46,7 +46,6 @@ class StringopSapiSandbox : public StringopSandbox {
__NR_close, __NR_close,
}) })
.AddFile("/etc/localtime") .AddFile("/etc/localtime")
.EnableNamespaces()
.BuildOrDie(); .BuildOrDie();
} }
}; };

View File

@ -48,7 +48,6 @@ class SumSapiSandbox : public SumSandbox {
__NR_close, __NR_close,
}) })
.AddFile("/etc/localtime") .AddFile("/etc/localtime")
.EnableNamespaces()
.BuildOrDie(); .BuildOrDie();
} }
}; };

View File

@ -56,7 +56,6 @@ Sandbox::~Sandbox() {
// are single-threaded and require ~30 basic syscalls. // are single-threaded and require ~30 basic syscalls.
void InitDefaultPolicyBuilder(sandbox2::PolicyBuilder* builder) { void InitDefaultPolicyBuilder(sandbox2::PolicyBuilder* builder) {
(*builder) (*builder)
.EnableNamespaces()
.AllowRead() .AllowRead()
.AllowWrite() .AllowWrite()
.AllowExit() .AllowExit()

View File

@ -56,7 +56,6 @@ std::unique_ptr<sandbox2::Policy> GetPolicy() {
defined(THREAD_SANITIZER) defined(THREAD_SANITIZER)
.AllowMmap() .AllowMmap()
#endif #endif
.EnableNamespaces()
.BuildOrDie(); .BuildOrDie();
} }

View File

@ -43,7 +43,6 @@ namespace {
std::unique_ptr<sandbox2::Policy> GetPolicy(absl::string_view sandboxee_path) { std::unique_ptr<sandbox2::Policy> GetPolicy(absl::string_view sandboxee_path) {
return sandbox2::PolicyBuilder() return sandbox2::PolicyBuilder()
.EnableNamespaces()
.AllowExit() .AllowExit()
.AllowMmap() .AllowMmap()
.AllowRead() .AllowRead()

View File

@ -32,7 +32,6 @@ namespace {
std::unique_ptr<sandbox2::Policy> GetPolicy(absl::string_view sandboxee_path) { std::unique_ptr<sandbox2::Policy> GetPolicy(absl::string_view sandboxee_path) {
return sandbox2::PolicyBuilder() return sandbox2::PolicyBuilder()
.AllowExit() .AllowExit()
.EnableNamespaces()
.AllowMmap() .AllowMmap()
.AllowRead() .AllowRead()
.AllowWrite() .AllowWrite()

View File

@ -93,7 +93,6 @@ std::unique_ptr<sandbox2::Policy> GetPolicy() {
#else #else
.BlockSyscallWithErrno(__NR_openat, ENOENT) .BlockSyscallWithErrno(__NR_openat, ENOENT)
#endif #endif
.EnableNamespaces()
.BuildOrDie(); .BuildOrDie();
} }

View File

@ -144,8 +144,6 @@ int main(int argc, char** argv) {
builder.AddPolicyOnSyscall(__NR_tee, {KILL}); builder.AddPolicyOnSyscall(__NR_tee, {KILL});
builder.DangerDefaultAllowAll(); builder.DangerDefaultAllowAll();
builder.EnableNamespaces();
if (absl::GetFlag(FLAGS_sandbox2tool_need_networking)) { if (absl::GetFlag(FLAGS_sandbox2tool_need_networking)) {
builder.AllowUnrestrictedNetworking(); builder.AllowUnrestrictedNetworking();
} }

View File

@ -57,7 +57,6 @@ std::unique_ptr<sandbox2::Policy> GetPolicy() {
.AllowStaticStartup() .AllowStaticStartup()
.AllowSystemMalloc() .AllowSystemMalloc()
.AllowExit() .AllowExit()
.EnableNamespaces()
.BlockSyscallWithErrno(__NR_access, ENOENT) .BlockSyscallWithErrno(__NR_access, ENOENT)
.BuildOrDie(); .BuildOrDie();
} }

View File

@ -49,7 +49,6 @@ TEST(NamespaceTest, FileNamespaceWorks) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
// Don't restrict the syscalls at all // Don't restrict the syscalls at all
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.AddFileAt(path, "/binary_path") .AddFileAt(path, "/binary_path")
.TryBuild()); .TryBuild());
@ -69,7 +68,6 @@ TEST(NamespaceTest, UserNamespaceWorks) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
// Don't restrict the syscalls at all // Don't restrict the syscalls at all
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.TryBuild()); .TryBuild());
Sandbox2 sandbox(std::move(executor), std::move(policy)); Sandbox2 sandbox(std::move(executor), std::move(policy));
@ -104,7 +102,6 @@ TEST(NamespaceTest, UserNamespaceIDMapWritten) {
std::vector<std::string> args = {path, "3", "1000", "1000"}; std::vector<std::string> args = {path, "3", "1000", "1000"};
auto executor = absl::make_unique<Executor>(path, args); auto executor = absl::make_unique<Executor>(path, args);
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
.EnableNamespaces()
// Don't restrict the syscalls at all // Don't restrict the syscalls at all
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.TryBuild()); .TryBuild());
@ -165,7 +162,6 @@ TEST_F(HostnameTest, Default) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
// Don't restrict the syscalls at all // Don't restrict the syscalls at all
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.TryBuild()); .TryBuild());
Try("sandbox2", std::move(policy)); Try("sandbox2", std::move(policy));
EXPECT_EQ(code_, 0); EXPECT_EQ(code_, 0);

View File

@ -162,7 +162,6 @@ std::unique_ptr<Policy> MinimalTestcasePolicy() {
.AllowExit() .AllowExit()
.BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_prlimit64, EPERM)
.BlockSyscallWithErrno(__NR_access, ENOENT) .BlockSyscallWithErrno(__NR_access, ENOENT)
.EnableNamespaces()
.BuildOrDie(); .BuildOrDie();
} }
@ -200,7 +199,6 @@ TEST(MinimalTest, MinimalSharedBinaryWorks) {
// New glibc accesses /etc/ld.so.preload // New glibc accesses /etc/ld.so.preload
.BlockSyscallWithErrno(__NR_access, ENOENT) .BlockSyscallWithErrno(__NR_access, ENOENT)
.BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_prlimit64, EPERM)
.EnableNamespaces()
.AddLibrariesForBinary(path) .AddLibrariesForBinary(path)
.BuildOrDie(); .BuildOrDie();
@ -223,7 +221,6 @@ TEST(MallocTest, SystemMallocWorks) {
.AllowStaticStartup() .AllowStaticStartup()
.AllowSystemMalloc() .AllowSystemMalloc()
.AllowExit() .AllowExit()
.EnableNamespaces()
.BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_prlimit64, EPERM)
.BlockSyscallWithErrno(__NR_access, ENOENT) .BlockSyscallWithErrno(__NR_access, ENOENT)
.BuildOrDie(); .BuildOrDie();
@ -259,7 +256,6 @@ TEST(MultipleSyscalls, AddPolicyOnSyscallsWorks) {
.AddPolicyOnSyscalls({__NR_getresuid, __NR_getresgid}, {ERRNO(42)}) .AddPolicyOnSyscalls({__NR_getresuid, __NR_getresgid}, {ERRNO(42)})
.AddPolicyOnSyscalls({__NR_read, __NR_write}, {ERRNO(43)}) .AddPolicyOnSyscalls({__NR_read, __NR_write}, {ERRNO(43)})
.AddPolicyOnSyscall(__NR_umask, {DENY}) .AddPolicyOnSyscall(__NR_umask, {DENY})
.EnableNamespaces()
.BlockSyscallWithErrno(__NR_prlimit64, EPERM) .BlockSyscallWithErrno(__NR_prlimit64, EPERM)
.BlockSyscallWithErrno(__NR_access, ENOENT) .BlockSyscallWithErrno(__NR_access, ENOENT)
.BuildOrDie(); .BuildOrDie();

View File

@ -665,9 +665,6 @@ std::vector<sock_filter> PolicyBuilder::ResolveBpfFunc(BpfFunc f) {
} }
sapi::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() { sapi::StatusOr<std::unique_ptr<Policy>> 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()) { if (!last_status_.ok()) {
return last_status_; return last_status_;
} }

View File

@ -447,28 +447,26 @@ class PolicyBuilder final {
// Enables the use of namespaces. // Enables the use of namespaces.
// //
// Namespaces are automatically enabled when using namespace helper features // Namespaces are enabled by default.
// (e.g. AddFile), therefore it is only necessary to explicitly enable // This is a no-op.
// namespaces when not using any other namespace helper feature. ABSL_DEPRECATED("Namespaces are enabled by default; no need to call this")
PolicyBuilder& EnableNamespaces() { PolicyBuilder& EnableNamespaces() {
CHECK(!disable_namespaces_) CHECK(use_namespaces_) << "Namespaces cannot be both disabled and enabled";
<< "Namespaces cannot be both disabled and enabled"; requires_namespaces_ = true;
use_namespaces_ = true;
return *this; return *this;
} }
// Disables the use of namespaces. // Disables the use of namespaces.
// //
// Sandbox2 with namespaces enabled is the recommended mode and will be the // Call in order to use Sandbox2 without namespaces.
// default in future, then calling this function will be necessary in order // This is not recommended.
// to use Sandbox2 without namespaces.
PolicyBuilder& DisableNamespaces() { PolicyBuilder& DisableNamespaces() {
CHECK(!use_namespaces_) CHECK(!requires_namespaces_)
<< "Namespaces cannot be both disabled and enabled. You're probably " << "Namespaces cannot be both disabled and enabled. You're probably "
"using features that implicitly enable namespaces (SetHostname, " "using features that implicitly enable namespaces (SetHostname, "
"AddFile, AddDirectory, AddDataDependency, AddLibrariesForBinary or " "AddFile, AddDirectory, AddDataDependency, AddLibrariesForBinary or "
"similar)"; "similar)";
disable_namespaces_ = true; use_namespaces_ = false;
return *this; return *this;
} }
@ -525,8 +523,8 @@ class PolicyBuilder final {
void StoreDescription(PolicyBuilderDescription* pb_description); void StoreDescription(PolicyBuilderDescription* pb_description);
Mounts mounts_; Mounts mounts_;
bool use_namespaces_ = false; bool use_namespaces_ = true;
bool disable_namespaces_ = false; bool requires_namespaces_ = false;
bool allow_unrestricted_networking_ = false; bool allow_unrestricted_networking_ = false;
std::string hostname_ = kDefaultHostname; std::string hostname_ = kDefaultHostname;

View File

@ -194,7 +194,6 @@ std::string PolicyBuilderTest::Run(std::vector<std::string> args,
TEST_F(PolicyBuilderTest, TestCanOnlyBuildOnce) { TEST_F(PolicyBuilderTest, TestCanOnlyBuildOnce) {
PolicyBuilder b; PolicyBuilder b;
b.EnableNamespaces();
ASSERT_THAT(b.BuildOrDie(), NotNull()); ASSERT_THAT(b.BuildOrDie(), NotNull());
ASSERT_DEATH(b.BuildOrDie(), "Can only build policy once"); ASSERT_DEATH(b.BuildOrDie(), "Can only build policy once");
} }

View File

@ -125,7 +125,6 @@ TEST(RunAsyncTest, SandboxeeExternalKill) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
// Don't restrict the syscalls at all. // Don't restrict the syscalls at all.
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.TryBuild()); .TryBuild());
Sandbox2 sandbox(std::move(executor), std::move(policy)); Sandbox2 sandbox(std::move(executor), std::move(policy));
ASSERT_TRUE(sandbox.RunAsync()); ASSERT_TRUE(sandbox.RunAsync());
@ -148,7 +147,6 @@ TEST(RunAsyncTest, SandboxeeTimeoutWithStacktraces) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
// Don't restrict the syscalls at all. // Don't restrict the syscalls at all.
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.TryBuild()); .TryBuild());
Sandbox2 sandbox(std::move(executor), std::move(policy)); Sandbox2 sandbox(std::move(executor), std::move(policy));
ASSERT_TRUE(sandbox.RunAsync()); ASSERT_TRUE(sandbox.RunAsync());
@ -169,7 +167,6 @@ TEST(RunAsyncTest, SandboxeeTimeoutDisabledStacktraces) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder()
// Don't restrict the syscalls at all. // Don't restrict the syscalls at all.
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.CollectStacktracesOnTimeout(false) .CollectStacktracesOnTimeout(false)
.TryBuild()); .TryBuild());
Sandbox2 sandbox(std::move(executor), std::move(policy)); Sandbox2 sandbox(std::move(executor), std::move(policy));
@ -191,7 +188,6 @@ TEST(RunAsyncTest, SandboxeeViolationDisabledStacktraces) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
PolicyBuilder() PolicyBuilder()
// Don't allow anything - Make sure that we'll crash. // Don't allow anything - Make sure that we'll crash.
.EnableNamespaces()
.CollectStacktracesOnViolation(false) .CollectStacktracesOnViolation(false)
.TryBuild()); .TryBuild());
Sandbox2 sandbox(std::move(executor), std::move(policy)); Sandbox2 sandbox(std::move(executor), std::move(policy));

View File

@ -114,8 +114,6 @@ std::unique_ptr<Policy> StackTracePeer::GetPolicy(pid_t target_pid,
JEQ32(static_cast<unsigned int>(1), ALLOW), JEQ32(static_cast<unsigned int>(1), ALLOW),
}) })
.EnableNamespaces()
// Add proc maps. // Add proc maps.
.AddFileAt(maps_file, .AddFileAt(maps_file,
file::JoinPath("/proc", absl::StrCat(target_pid), "maps")) file::JoinPath("/proc", absl::StrCat(target_pid), "maps"))

View File

@ -82,7 +82,6 @@ void SymbolizationWorksCommon(
policybuilder policybuilder
// Don't restrict the syscalls at all. // Don't restrict the syscalls at all.
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.AddFile(path) .AddFile(path)
.AddLibrariesForBinary(path) .AddLibrariesForBinary(path)
.AddFileAt(temp_filename, "/proc/cpuinfo"); .AddFileAt(temp_filename, "/proc/cpuinfo");
@ -180,7 +179,6 @@ TEST(StackTraceTest, SymbolizationTrustedFilesOnly) {
SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder{} SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder{}
// Don't restrict the syscalls at all. // Don't restrict the syscalls at all.
.DangerDefaultAllowAll() .DangerDefaultAllowAll()
.EnableNamespaces()
.AddFile(path) .AddFile(path)
.AddLibrariesForBinary(path) .AddLibrariesForBinary(path)
.TryBuild()); .TryBuild());