From d74215d30d19537d9992e16b3b49cbf7fd3d3ced Mon Sep 17 00:00:00 2001 From: Wiktor Garbacz Date: Fri, 24 Jan 2020 02:37:32 -0800 Subject: [PATCH] Properly test read-only mounts PiperOrigin-RevId: 291337704 Change-Id: I806d0d09051ab205813d6626ea70e9e57a28a7a5 --- sandboxed_api/sandbox2/BUILD.bazel | 3 +- sandboxed_api/sandbox2/CMakeLists.txt | 2 ++ sandboxed_api/sandbox2/namespace_test.cc | 46 ++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 3c72e03..7a94c20 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -479,7 +479,8 @@ cc_test( ":namespace", ":sandbox2", ":testing", - "//sandboxed_api/sandbox2/util:bpf_helper", + "//sandboxed_api/sandbox2/util:fileops", + "//sandboxed_api/sandbox2/util:temp_file", "//sandboxed_api/util:status_matchers", "@com_google_absl//absl/memory", "@com_google_absl//absl/strings", diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index 92955c0..b2b58fd 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -600,9 +600,11 @@ if(SAPI_ENABLE_TESTS) absl::memory absl::strings sandbox2::comms + sandbox2::fileops sandbox2::namespace sandbox2::sandbox2 sandbox2::testing + sandbox2::temp_file sapi::status_matchers sapi::test_main ) diff --git a/sandboxed_api/sandbox2/namespace_test.cc b/sandboxed_api/sandbox2/namespace_test.cc index 6efd8a6..d2461f0 100644 --- a/sandboxed_api/sandbox2/namespace_test.cc +++ b/sandboxed_api/sandbox2/namespace_test.cc @@ -34,14 +34,15 @@ #include "sandboxed_api/sandbox2/result.h" #include "sandboxed_api/sandbox2/sandbox2.h" #include "sandboxed_api/sandbox2/testing.h" -#include "sandboxed_api/sandbox2/util/bpf_helper.h" +#include "sandboxed_api/sandbox2/util/fileops.h" +#include "sandboxed_api/sandbox2/util/temp_file.h" #include "sandboxed_api/util/status_matchers.h" namespace sandbox2 { namespace { TEST(NamespaceTest, FileNamespaceWorks) { - // Mount /binary_path RO and check that it actually is RO. + // Mount /binary_path RO and check that it exists and is readable. // /etc/passwd should not exist. const std::string path = GetTestSourcePath("sandbox2/testcases/namespace"); std::vector args = {path, "0", "/binary_path", "/etc/passwd"}; @@ -59,6 +60,47 @@ TEST(NamespaceTest, FileNamespaceWorks) { EXPECT_EQ(result.reason_code(), 2); } +TEST(NamespaceTest, ReadOnlyIsRespected) { + // Mount temporary file as RO and check that it actually is RO. + auto [name, fd] = + CreateNamedTempFile(GetTestTempPath("temp_file")).ValueOrDie(); + file_util::fileops::FDCloser temp_closer{fd}; + + const std::string path = GetTestSourcePath("sandbox2/testcases/namespace"); + { + // First check that it is readable + std::vector args = {path, "0", "/temp_file"}; + auto executor = absl::make_unique(path, args); + SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() + // Don't restrict the syscalls at all + .DangerDefaultAllowAll() + .AddFileAt(name, "/temp_file") + .TryBuild()); + + Sandbox2 sandbox(std::move(executor), std::move(policy)); + auto result = sandbox.Run(); + + ASSERT_EQ(result.final_status(), Result::OK); + EXPECT_EQ(result.reason_code(), 0); + } + { + // Then check that it is not writeable + std::vector args = {path, "1", "/temp_file"}; + auto executor = absl::make_unique(path, args); + SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() + // Don't restrict the syscalls at all + .DangerDefaultAllowAll() + .AddFileAt(name, "/temp_file") + .TryBuild()); + + Sandbox2 sandbox(std::move(executor), std::move(policy)); + auto result = sandbox.Run(); + + ASSERT_EQ(result.final_status(), Result::OK); + EXPECT_EQ(result.reason_code(), 1); + } +} + TEST(NamespaceTest, UserNamespaceWorks) { // Check that getpid() returns 2 (which is the case inside pid NS). const std::string path = GetTestSourcePath("sandbox2/testcases/namespace");