Use namespaced policy in most tests

Drive-by some test cleanups.

PiperOrigin-RevId: 510134967
Change-Id: I40328a644690865c5cc0a0eb265222ebf7ff83e0

sandboxed_api/sandbox2/BUILD.bazel

@@ -614,7 +614,6 @@ cc_test(
         ":mounts",
         "//sandboxed_api:testing",
         "//sandboxed_api/util:file_base",
-        "//sandboxed_api/util:file_helpers",
         "//sandboxed_api/util:status_matchers",
         "//sandboxed_api/util:temp_file",
         "@com_google_absl//absl/strings",
@@ -826,7 +825,6 @@ cc_test(
         ":sandbox2",
         "//sandboxed_api:config",
         "//sandboxed_api:testing",
-        "//sandboxed_api/sandbox2/util:bpf_helper",
         "//sandboxed_api/util:status_matchers",
         "@com_google_googletest//:gtest_main",
     ],
@@ -916,7 +914,6 @@ cc_test(
         ":sanitizer",
         ":util",
         "//sandboxed_api:testing",
-        "//sandboxed_api/sandbox2/util:bpf_helper",
         "//sandboxed_api/util:status_matchers",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/log",
@@ -932,7 +929,6 @@ cc_test(
     deps = [
         ":util",
         "//sandboxed_api:testing",
-        "//sandboxed_api/util:file_base",
         "@com_google_googletest//:gtest_main",
     ],
 )

sandboxed_api/sandbox2/CMakeLists.txt

@@ -988,7 +988,6 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
   )
   target_link_libraries(sandbox2_sandbox2_test PRIVATE
     absl::strings
-    sandbox2::bpf_helper
     sapi::config
     sandbox2::sandbox2
     sapi::testing
@@ -1036,7 +1035,6 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
     OUTPUT_NAME util_test
   )
   target_link_libraries(sandbox2_util_test PRIVATE
-    sapi::file_base
     sapi::testing
     sandbox2::util
     sapi::test_main

sandboxed_api/sandbox2/buffer_test.cc

@@ -64,7 +64,6 @@ TEST(BufferTest, TestImplementation) {
 
 std::unique_ptr<Policy> BufferTestcasePolicy() {
   auto s2p = PolicyBuilder()
-                 .DisableNamespaces()
                  .AllowDynamicStartup()
                  .AllowExit()
                  .AllowSafeFcntl()

sandboxed_api/sandbox2/ipc_test.cc

@@ -51,7 +51,6 @@ TEST_P(IPCTest, MapFDByNamePreExecve) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());
@@ -89,7 +88,6 @@ TEST_P(IPCTest, MapFDByNamePostExecve) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());
@@ -122,7 +120,6 @@ TEST(IPCTest, NoMappedFDsPreExecve) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());

sandboxed_api/sandbox2/limits_test.cc

@@ -28,7 +28,6 @@
 #include "sandboxed_api/sandbox2/policybuilder.h"
 #include "sandboxed_api/sandbox2/result.h"
 #include "sandboxed_api/sandbox2/sandbox2.h"
-#include "sandboxed_api/sandbox2/util/bpf_helper.h"
 #include "sandboxed_api/testing.h"
 #include "sandboxed_api/util/status_matchers.h"
 
@@ -45,7 +44,6 @@ TEST(LimitsTest, RLimitASMmapUnderLimit) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             sandbox2::PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());
@@ -64,7 +62,6 @@ TEST(LimitsTest, RLimitASMmapAboveLimit) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             sandbox2::PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());
@@ -83,7 +80,6 @@ TEST(LimitsTest, RLimitASAllocaSmallUnderLimit) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             sandbox2::PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());
@@ -102,7 +98,6 @@ TEST(LimitsTest, RLimitASAllocaBigUnderLimit) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             sandbox2::PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());
@@ -121,7 +116,6 @@ TEST(LimitsTest, RLimitASAllocaBigAboveLimit) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             sandbox2::PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());

sandboxed_api/sandbox2/mounts_test.cc

@@ -23,7 +23,6 @@
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/testing.h"
-#include "sandboxed_api/util/file_helpers.h"
 #include "sandboxed_api/util/path.h"
 #include "sandboxed_api/util/status_matchers.h"
 #include "sandboxed_api/util/temp_file.h"

sandboxed_api/sandbox2/notify_test.cc

@@ -44,7 +44,6 @@ using ::testing::Eq;
 // chosen because unlikely to be called by a regular program.
 std::unique_ptr<Policy> NotifyTestcasePolicy() {
   return PolicyBuilder()
-      .DisableNamespaces()
       .AllowDynamicStartup()  // For PrintPidAndComms and includes
                               // AllowStaticStartup
       .AllowExit()

sandboxed_api/sandbox2/policy.h

@@ -57,7 +57,6 @@ class Policy final {
   friend class MonitorBase;
   friend class PtraceMonitor;
   friend class PolicyBuilder;
-  friend class PolicyBuilderPeer;  // For testing
   friend class StackTracePeer;
 
   // Private constructor only called by the PolicyBuilder.

sandboxed_api/sandbox2/policy_test.cc

@@ -32,7 +32,6 @@
 #include "sandboxed_api/sandbox2/policybuilder.h"
 #include "sandboxed_api/sandbox2/result.h"
 #include "sandboxed_api/sandbox2/sandbox2.h"
-#include "sandboxed_api/sandbox2/syscall.h"
 #include "sandboxed_api/sandbox2/util/bpf_helper.h"
 #include "sandboxed_api/testing.h"
 
@@ -46,11 +45,10 @@ PolicyBuilder CreatePolicyTestPolicyBuilder() {
   sandbox2::PolicyBuilder builder;
 
   if constexpr (sapi::host_os::IsAndroid()) {
-    builder.AllowDynamicStartup();
+    builder.DisableNamespaces().AllowDynamicStartup();
   }
 
-  builder.DisableNamespaces()
+  builder.AllowStaticStartup()
-      .AllowStaticStartup()
       .AllowExit()
       .AllowRead()
       .AllowWrite()

sandboxed_api/sandbox2/sandbox2_test.cc

@@ -37,6 +37,19 @@
 namespace sandbox2 {
 namespace {
 
+PolicyBuilder CreateDefaultPolicyBuilder(absl::string_view path) {
+  PolicyBuilder builder;
+  // Don't restrict the syscalls at all.
+  builder.DangerDefaultAllowAll();
+  if constexpr (sapi::sanitizers::IsAny()) {
+    builder.AddLibrariesForBinary(path);
+  }
+  if constexpr (sapi::sanitizers::IsAny()) {
+    builder.AddDirectory("/proc");
+  }
+  return builder;
+}
+
 using ::sapi::GetTestSourcePath;
 using ::testing::Eq;
 using ::testing::HasSubstr;
@@ -54,12 +67,8 @@ TEST(SandboxCoreDumpTest, AbortWithoutCoreDumpReturnsSignaled) {
   };
   auto executor = std::make_unique<Executor>(path, args);
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPolicyBuilder(path)
-                            PolicyBuilder()
+                                             .TryBuild());
-                                .DisableNamespaces()
-                                // Don't restrict the syscalls at all.
-                                .DangerDefaultAllowAll()
-                                .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   auto result = sandbox.Run();
 
@@ -78,11 +87,7 @@ TEST(TsyncTest, TsyncNoMemoryChecks) {
   executor->set_enable_sandbox_before_exec(false);
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            PolicyBuilder()
+                            CreateDefaultPolicyBuilder(path).TryBuild());
-                                .DisableNamespaces()
-                                // Don't restrict the syscalls at all.
-                                .DangerDefaultAllowAll()
-                                .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   auto result = sandbox.Run();
 
@@ -105,42 +110,15 @@ TEST(ExecutorTest, ExecutorFdConstructor) {
   auto executor = std::make_unique<Executor>(fd, args, envs);
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            PolicyBuilder()
+                            CreateDefaultPolicyBuilder(path).TryBuild());
-                                .DisableNamespaces()
-                                // Don't restrict the syscalls at all.
-                                .DangerDefaultAllowAll()
-                                .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   auto result = sandbox.Run();
 
   ASSERT_EQ(result.final_status(), Result::OK);
 }
 
-// Tests that we return the correct state when the sandboxee was killed by an
-// external signal. Also make sure that we do not have the stack trace.
-TEST(RunAsyncTest, SandboxeeExternalKill) {
-  const std::string path = GetTestSourcePath("sandbox2/testcases/sleep");
-
-  std::vector<std::string> args = {path};
-  std::vector<std::string> envs;
-  auto executor = std::make_unique<Executor>(path, args, envs);
-
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            PolicyBuilder()
-                                // Don't restrict the syscalls at all.
-                                .DangerDefaultAllowAll()
-                                .TryBuild());
-  Sandbox2 sandbox(std::move(executor), std::move(policy));
-  ASSERT_TRUE(sandbox.RunAsync());
-  sleep(1);
-  sandbox.Kill();
-  auto result = sandbox.AwaitResult();
-  EXPECT_EQ(result.final_status(), Result::EXTERNAL_KILL);
-  EXPECT_THAT(result.GetStackTrace(), IsEmpty());
-}
-
 // Tests that we return the correct state when the sandboxee timed out.
-TEST(RunAsyncTest, SandboxeeTimeoutWithStacktraces) {
+TEST(StackTraceTest, StackTraceOnTimeoutWorks) {
   SKIP_ANDROID;
   const std::string path = GetTestSourcePath("sandbox2/testcases/sleep");
 
@@ -161,6 +139,26 @@ TEST(RunAsyncTest, SandboxeeTimeoutWithStacktraces) {
   EXPECT_THAT(result.GetStackTrace(), HasSubstr("sleep"));
 }
 
+// Tests that we return the correct state when the sandboxee was killed by an
+// external signal. Also make sure that we do not have the stack trace.
+TEST(RunAsyncTest, SandboxeeExternalKill) {
+  const std::string path = GetTestSourcePath("sandbox2/testcases/sleep");
+
+  std::vector<std::string> args = {path};
+  std::vector<std::string> envs;
+  auto executor = std::make_unique<Executor>(path, args, envs);
+
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
+                            CreateDefaultPolicyBuilder(path).TryBuild());
+  Sandbox2 sandbox(std::move(executor), std::move(policy));
+  ASSERT_TRUE(sandbox.RunAsync());
+  sleep(1);
+  sandbox.Kill();
+  auto result = sandbox.AwaitResult();
+  EXPECT_EQ(result.final_status(), Result::EXTERNAL_KILL);
+  EXPECT_THAT(result.GetStackTrace(), IsEmpty());
+}
+
 // Tests that we do not collect stack traces if it was disabled (signaled).
 TEST(RunAsyncTest, SandboxeeTimeoutDisabledStacktraces) {
   const std::string path = GetTestSourcePath("sandbox2/testcases/sleep");
@@ -169,10 +167,29 @@ TEST(RunAsyncTest, SandboxeeTimeoutDisabledStacktraces) {
   std::vector<std::string> envs;
   auto executor = std::make_unique<Executor>(path, args, envs);
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPolicyBuilder(path)
-                            PolicyBuilder()
+                                             .CollectStacktracesOnTimeout(false)
-                                // Don't restrict the syscalls at all.
+                                             .TryBuild());
-                                .DangerDefaultAllowAll()
+  Sandbox2 sandbox(std::move(executor), std::move(policy));
+  ASSERT_TRUE(sandbox.RunAsync());
+  sandbox.set_walltime_limit(absl::Seconds(1));
+  auto result = sandbox.AwaitResult();
+  EXPECT_EQ(result.final_status(), Result::TIMEOUT);
+  EXPECT_THAT(result.GetStackTrace(), IsEmpty());
+}
+
+// Tests that we do not collect stack traces if it was disabled (violation).
+TEST(RunAsyncTest, SandboxeeViolationDisabledStacktraces) {
+  const std::string path = GetTestSourcePath("sandbox2/testcases/sleep");
+
+  std::vector<std::string> args = {path};
+  std::vector<std::string> envs;
+  auto executor = std::make_unique<Executor>(path, args, envs);
+
+  SAPI_ASSERT_OK_AND_ASSIGN(
+      auto policy, PolicyBuilder()
+                       // Don't allow anything - Make sure that we'll crash.
+                       .CollectStacktracesOnViolation(false)
                        .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   ASSERT_TRUE(sandbox.RunAsync());
@@ -186,12 +203,9 @@ TEST(RunAsyncTest, SandboxeeNotKilledWhenStartingThreadFinishes) {
   std::vector<std::string> args = {path};
   auto executor = std::make_unique<Executor>(path, args);
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPolicyBuilder(path)
-                            PolicyBuilder()
+                                             .CollectStacktracesOnExit(true)
-                                // Don't restrict the syscalls at all.
+                                             .TryBuild());
-                                .DangerDefaultAllowAll()
-                                .CollectStacktracesOnExit(true)
-                                .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   std::thread sandbox_start_thread([&sandbox]() { sandbox.RunAsync(); });
   sandbox_start_thread.join();
@@ -207,9 +221,8 @@ TEST(StarvationTest, MonitorIsNotStarvedByTheSandboxee) {
   auto executor = std::make_unique<Executor>(path, args, envs);
   executor->limits()->set_walltime_limit(absl::Seconds(5));
 
-  SAPI_ASSERT_OK_AND_ASSIGN(
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-      auto policy,
+                            CreateDefaultPolicyBuilder(path).TryBuild());
-      PolicyBuilder().DisableNamespaces().DangerDefaultAllowAll().TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
 
   auto start = absl::Now();

sandboxed_api/sandbox2/sanitizer_test.cc

@@ -28,7 +28,6 @@
 #include "gtest/gtest.h"
 #include "absl/container/flat_hash_set.h"
 #include "absl/log/log.h"
-#include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/executor.h"
@@ -37,7 +36,6 @@
 #include "sandboxed_api/sandbox2/result.h"
 #include "sandboxed_api/sandbox2/sandbox2.h"
 #include "sandboxed_api/sandbox2/util.h"
-#include "sandboxed_api/sandbox2/util/bpf_helper.h"
 #include "sandboxed_api/testing.h"
 #include "sandboxed_api/util/status_matchers.h"
 
@@ -120,7 +118,6 @@ TEST(SanitizerTest, TestSandboxedBinary) {
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
                             PolicyBuilder()
-                                .DisableNamespaces()
                                 // Don't restrict the syscalls at all.
                                 .DangerDefaultAllowAll()
                                 .TryBuild());

sandboxed_api/sandbox2/stack_trace_test.cc

@@ -59,7 +59,7 @@ using ::testing::Not;
 
 // Test that symbolization of stack traces works.
 void SymbolizationWorksCommon(
-    const std::function<void(PolicyBuilder*)>& modify_policy) {
+    std::function<void(PolicyBuilder*)> modify_policy = {}) {
   const std::string path = GetTestSourcePath("sandbox2/testcases/symbolize");
   std::vector<std::string> args = {path, "1"};
 
@@ -78,7 +78,9 @@ void SymbolizationWorksCommon(
                            .AddFile(path)
                            .AddLibrariesForBinary(path)
                            .AddFileAt(temp_filename, "/proc/cpuinfo");
-  modify_policy(&policybuilder);
+  if (modify_policy) {
+    modify_policy(&policybuilder);
+  }
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy, policybuilder.TryBuild());
 
   Sandbox2 s2(std::make_unique<Executor>(path, args), std::move(policy));
@@ -95,7 +97,7 @@ TEST(StackTraceTest, SymbolizationWorksNonSandboxedLibunwind) {
   absl::FlagSaver fs;
   absl::SetFlag(&FLAGS_sandbox_libunwind_crash_handler, false);
 
-  SymbolizationWorksCommon([](PolicyBuilder*) {});
+  SymbolizationWorksCommon();
 }
 
 TEST(StackTraceTest, SymbolizationWorksSandboxedLibunwind) {
@@ -103,7 +105,7 @@ TEST(StackTraceTest, SymbolizationWorksSandboxedLibunwind) {
   absl::FlagSaver fs;
   absl::SetFlag(&FLAGS_sandbox_libunwind_crash_handler, true);
 
-  SymbolizationWorksCommon([](PolicyBuilder*) {});
+  SymbolizationWorksCommon();
 }
 
 TEST(StackTraceTest, SymbolizationWorksSandboxedLibunwindProcDirMounted) {

sandboxed_api/sandbox2/testcases/BUILD.bazel

@@ -40,6 +40,7 @@ cc_binary(
     testonly = True,
     srcs = ["abort.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
     deps = ["//sandboxed_api/util:raw_logging"],
 )
 
@@ -56,6 +57,7 @@ cc_binary(
     testonly = True,
     srcs = ["buffer.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
     deps = [
         "//sandboxed_api/sandbox2:buffer",
         "//sandboxed_api/sandbox2:comms",
@@ -68,6 +70,7 @@ cc_binary(
     testonly = True,
     srcs = ["ipc.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
     deps = [
         "//sandboxed_api/sandbox2:client",
         "//sandboxed_api/sandbox2:comms",
@@ -112,6 +115,7 @@ cc_binary(
     testonly = True,
     srcs = ["pidcomms.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
     deps = [
         "//sandboxed_api/sandbox2:client",
         "//sandboxed_api/sandbox2:comms",
@@ -135,6 +139,7 @@ cc_binary(
     testonly = True,
     srcs = ["sanitizer.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
 )
 
 cc_binary(
@@ -176,6 +181,7 @@ cc_binary(
     testonly = True,
     srcs = ["tsync.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
     deps = [
         "//sandboxed_api/sandbox2:client",
         "//sandboxed_api/sandbox2:comms",
@@ -187,6 +193,7 @@ cc_binary(
     testonly = True,
     srcs = ["starve.cc"],
     copts = sapi_platform_copts(),
+    features = ["fully_static_link"],
 )
 
 cc_binary(

sandboxed_api/sandbox2/testcases/CMakeLists.txt

@@ -20,6 +20,7 @@ set_target_properties(sandbox2_testcase_abort PROPERTIES
   OUTPUT_NAME abort)
 add_executable(sandbox2::testcase_abort ALIAS sandbox2_testcase_abort)
 target_link_libraries(sandbox2_testcase_abort PRIVATE
+  -static
   sapi::base
   sapi::raw_logging
 )
@@ -47,6 +48,7 @@ set_target_properties(sandbox2_testcase_buffer PROPERTIES
   OUTPUT_NAME buffer
 )
 target_link_libraries(sandbox2_testcase_buffer PRIVATE
+  -static
   absl::str_format
   sandbox2::buffer
   sandbox2::comms
@@ -62,6 +64,7 @@ set_target_properties(sandbox2_testcase_ipc PROPERTIES
 )
 add_executable(sandbox2::testcase_ipc ALIAS sandbox2_testcase_ipc)
 target_link_libraries(sandbox2_testcase_ipc PRIVATE
+  -static
   absl::strings
   sandbox2::client
   sandbox2::comms
@@ -132,6 +135,7 @@ set_target_properties(sandbox2_testcase_pidcomms PROPERTIES
   OUTPUT_NAME pidcomms
 )
 target_link_libraries(sandbox2_testcase_pidcomms PRIVATE
+  -static
   sandbox2::client
   sandbox2::comms
   sapi::base
@@ -161,6 +165,7 @@ set_target_properties(sandbox2_testcase_sanitizer PROPERTIES
   OUTPUT_NAME sanitizer
 )
 target_link_libraries(sandbox2_testcase_sanitizer PRIVATE
+  -static
   sapi::base
 )
 
@@ -220,6 +225,7 @@ set_target_properties(sandbox2_testcase_starve PROPERTIES
   OUTPUT_NAME starve
 )
 target_link_libraries(sandbox2_testcase_starve PRIVATE
+  -static
   sapi::base
 )
 
@@ -232,6 +238,7 @@ set_target_properties(sandbox2_testcase_tsync PROPERTIES
   OUTPUT_NAME tsync
 )
 target_link_libraries(sandbox2_testcase_tsync PRIVATE
+  -static
   sandbox2::client
   sandbox2::comms
   sapi::base

sandboxed_api/sandbox2/util_test.cc

@@ -21,7 +21,6 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "sandboxed_api/testing.h"
-#include "sandboxed_api/util/path.h"
 
 namespace sandbox2::util {
 namespace {