diff --git a/sandboxed_api/sandbox2/mounts.cc b/sandboxed_api/sandbox2/mounts.cc index 7f897c3..2cc437c 100644 --- a/sandboxed_api/sandbox2/mounts.cc +++ b/sandboxed_api/sandbox2/mounts.cc @@ -170,6 +170,8 @@ sapi::Status Mounts::Insert(absl::string_view path, } break; } + case MountTree::Node::kRootNode: + return sapi::InvalidArgumentError("Cannot insert a RootNode"); case MountTree::Node::kTmpfsNode: case MountTree::Node::NODE_NOT_SET: break; @@ -517,6 +519,7 @@ void CreateMounts(const MountTree& tree, const std::string& path, } case MountTree::Node::kDirNode: case MountTree::Node::kTmpfsNode: + case MountTree::Node::kRootNode: case MountTree::Node::NODE_NOT_SET: SAPI_RAW_VLOG(2, "Creating directory at %s", path); SAPI_RAW_PCHECK(mkdir(path.c_str(), 0700) == 0 || errno == EEXIST, ""); @@ -554,6 +557,7 @@ void CreateMounts(const MountTree& tree, const std::string& path, // A file node has to be a leaf so we can skip traversing here. return; } + case MountTree::Node::kRootNode: case MountTree::Node::NODE_NOT_SET: // Nothing to do, we already created the directory above. break; diff --git a/sandboxed_api/sandbox2/mounts.h b/sandboxed_api/sandbox2/mounts.h index 3c8454a..5e10ab1 100644 --- a/sandboxed_api/sandbox2/mounts.h +++ b/sandboxed_api/sandbox2/mounts.h @@ -27,9 +27,19 @@ namespace sandbox2 { class Mounts { public: - Mounts() = default; + Mounts() { + MountTree::Node root; + root.mutable_root_node()->set_is_ro(true); + *mount_tree_.mutable_node() = root; + } + explicit Mounts(MountTree mount_tree) : mount_tree_(std::move(mount_tree)) {} + Mounts(const Mounts&) = default; + Mounts(Mounts&&) = default; + Mounts& operator=(const Mounts&) = default; + Mounts& operator=(Mounts&&) = default; + sapi::Status AddFile(absl::string_view path, bool is_ro = true); sapi::Status AddFileAt(absl::string_view outside, absl::string_view inside, @@ -47,6 +57,15 @@ class Mounts { MountTree GetMountTree() const { return mount_tree_; } + void SetRootWritable() { + mount_tree_.mutable_node()->mutable_root_node()->set_is_ro(false); + } + + bool IsRootReadOnly() const { + return mount_tree_.has_node() && mount_tree_.node().has_root_node() && + mount_tree_.node().root_node().is_ro(); + } + // Lists the outside and inside entries of the input tree in the output // parameters, in an ls-like manner. Each entry is traversed in the // depth-first order. However, the entries on the same level of hierarchy are diff --git a/sandboxed_api/sandbox2/mounttree.proto b/sandboxed_api/sandbox2/mounttree.proto index 53568c3..ccf0625 100644 --- a/sandboxed_api/sandbox2/mounttree.proto +++ b/sandboxed_api/sandbox2/mounttree.proto @@ -15,6 +15,7 @@ // A proto for serializing the sandbox2::MountTree class syntax = "proto2"; + package sandbox2; // The MountTree maps path components to mount operations (bind/tmpfs). The path @@ -40,11 +41,17 @@ message MountTree { required string tmpfs_options = 1; } + // RootNode is as special node for root of the MountTree + message RootNode { + required bool is_ro = 3; + } + message Node { oneof node { FileNode file_node = 1; DirNode dir_node = 2; TmpfsNode tmpfs_node = 3; + RootNode root_node = 4; } } diff --git a/sandboxed_api/sandbox2/namespace.cc b/sandboxed_api/sandbox2/namespace.cc index 99916b0..f81b8dc 100644 --- a/sandboxed_api/sandbox2/namespace.cc +++ b/sandboxed_api/sandbox2/namespace.cc @@ -73,6 +73,13 @@ void PrepareChroot(const Mounts& mounts) { // Walk the tree and perform all the mount operations. mounts.CreateMounts(kSandbox2ChrootPath); + + if (mounts.IsRootReadOnly()) { + // Remount the chroot read-only + SAPI_RAW_PCHECK(mount(kSandbox2ChrootPath, kSandbox2ChrootPath, "", + MS_BIND | MS_REMOUNT | MS_RDONLY, nullptr) == 0, + "remounting chroot read-only failed"); + } } void TryDenySetgroups() { diff --git a/sandboxed_api/sandbox2/namespace_test.cc b/sandboxed_api/sandbox2/namespace_test.cc index bead3a8..265d440 100644 --- a/sandboxed_api/sandbox2/namespace_test.cc +++ b/sandboxed_api/sandbox2/namespace_test.cc @@ -133,6 +133,43 @@ TEST(NamespaceTest, UserNamespaceIDMapWritten) { } } +TEST(NamespaceTest, RootReadOnly) { + // Mount rw tmpfs at /tmp and check it is rw. + // Check also that / is ro. + const std::string path = GetTestSourcePath("sandbox2/testcases/namespace"); + std::vector args = {path, "4", "/tmp/testfile", "/testfile"}; + auto executor = absl::make_unique(path, args); + SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() + // Don't restrict the syscalls at all + .DangerDefaultAllowAll() + .AddTmpfs("/tmp") + .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(), 2); +} + +TEST(NamespaceTest, RootWritable) { + // Mount root rw and check it + const std::string path = GetTestSourcePath("sandbox2/testcases/namespace"); + std::vector args = {path, "4", "/testfile"}; + auto executor = absl::make_unique(path, args); + SAPI_ASSERT_OK_AND_ASSIGN(auto policy, PolicyBuilder() + // Don't restrict the syscalls at all + .DangerDefaultAllowAll() + .SetRootWritable() + .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); +} + class HostnameTest : public testing::Test { protected: void Try(std::string arg, std::unique_ptr policy) { diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index d8cfb24..62944bf 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -904,6 +904,13 @@ PolicyBuilder& PolicyBuilder::AddNetworkProxyHandlerPolicy() { return *this; } +PolicyBuilder& PolicyBuilder::SetRootWritable() { + EnableNamespaces(); + mounts_.SetRootWritable(); + + return *this; +} + void PolicyBuilder::StoreDescription(PolicyBuilderDescription* pb_description) { for (const auto& handled_syscall : handled_syscalls_) { pb_description->add_handled_syscalls(handled_syscall); diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index 418b900..a99c106 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/sandboxed_api/sandbox2/policybuilder.h @@ -497,6 +497,10 @@ class PolicyBuilder final { // the NetworkProxyHandler PolicyBuilder& AddNetworkProxyHandlerPolicy(); + // Makes root of the filesystem writeable + // Not recommended + PolicyBuilder& SetRootWritable(); + private: friend class PolicyBuilderPeer; // For testing friend class StackTracePeer; diff --git a/sandboxed_api/sandbox2/testcases/namespace.cc b/sandboxed_api/sandbox2/testcases/namespace.cc index 21e0d14..887cf08 100644 --- a/sandboxed_api/sandbox2/testcases/namespace.cc +++ b/sandboxed_api/sandbox2/testcases/namespace.cc @@ -15,17 +15,22 @@ // Checks various things related to namespaces, depending on the first argument: // ./binary 0 ... : // Make sure all provided files exist and are RO, return 0 on OK. -// Returns the index of the first non-existing file + 1 on failure. +// Returns the index of the first non-existing file on failure. // ./binary 1 ... : // Make sure all provided files exist and are RW, return 0 on OK. -// Returns the index of the first non-existing file + 1 on failure. +// Returns the index of the first non-existing file on failure. // ./binary 2 // Make sure that we run in a PID namespace (this implies getpid() == 1) // Returns 0 on OK. // ./binary 3 // Make sure getuid()/getgid() returns the provided uid/gid (User namespace). // Returns 0 on OK. +// ./binary 4 ... : +// Create provided files, return 0 on OK. +// Returns the index of the first non-creatable file on failure. #include +#include +#include #include #include @@ -72,6 +77,14 @@ int main(int argc, char* argv[]) { } } break; + case 4: + for (int i = 2; i < argc; ++i) { + if (open(argv[i], O_CREAT | O_WRONLY) == -1) { + return i - 1; + } + } + break; + default: return 1; }