Remount chroot as read-only

PiperOrigin-RevId: 280394655
Change-Id: I1490b7dfbbca3d91f5efb4dd5800397c9da57da8

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;

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

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;
     }
   }
 

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() {

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<std::string> args = {path, "4", "/tmp/testfile", "/testfile"};
+  auto executor = absl::make_unique<Executor>(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<std::string> args = {path, "4", "/testfile"};
+  auto executor = absl::make_unique<Executor>(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> policy) {

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);

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;

sandboxed_api/sandbox2/testcases/namespace.cc

@@ -15,17 +15,22 @@
 // Checks various things related to namespaces, depending on the first argument:
 // ./binary 0 <file1> <file2> ... <fileN>:
 //    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 <file1> <file2> ... <fileN>:
 //    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 <uid> <gid>
 //    Make sure getuid()/getgid() returns the provided uid/gid (User namespace).
 //    Returns 0 on OK.
+// ./binary 4 <file1> <file2> ... <fileN>:
+//    Create provided files, return 0 on OK.
+//    Returns the index of the first non-creatable file on failure.
 #include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 #include <unistd.h>
 
 #include <cstdlib>
@@ -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;
   }