From 20edaae54f783a70076125561f52c9ba90eaffce Mon Sep 17 00:00:00 2001 From: Wiktor Garbacz Date: Tue, 8 Mar 2022 08:00:46 -0800 Subject: [PATCH] Add an option to allow mount propagation PiperOrigin-RevId: 433211924 Change-Id: I653f000d44de10b668b375fd2dfff3c668cbf673 --- sandboxed_api/sandbox2/executor.cc | 1 + sandboxed_api/sandbox2/forkserver.cc | 2 +- sandboxed_api/sandbox2/forkserver.proto | 3 +++ sandboxed_api/sandbox2/namespace.cc | 17 ++++++++++++----- sandboxed_api/sandbox2/namespace.h | 8 ++++++-- sandboxed_api/sandbox2/policybuilder.cc | 3 ++- sandboxed_api/sandbox2/policybuilder.h | 7 +++++++ 7 files changed, 32 insertions(+), 9 deletions(-) diff --git a/sandboxed_api/sandbox2/executor.cc b/sandboxed_api/sandbox2/executor.cc index f6f9b23..828ff7d 100644 --- a/sandboxed_api/sandbox2/executor.cc +++ b/sandboxed_api/sandbox2/executor.cc @@ -141,6 +141,7 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns, clone_flags |= ns->GetCloneFlags(); *request.mutable_mount_tree() = ns->mounts().GetMountTree(); request.set_hostname(ns->hostname()); + request.set_allow_mount_propagation(ns->allow_mount_propagation()); } request.set_clone_flags(clone_flags); diff --git a/sandboxed_api/sandbox2/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index 755cb21..0244e60 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -605,7 +605,7 @@ void ForkServer::InitializeNamespaces(const ForkRequest& request, uid_t uid, Namespace::InitializeNamespaces( uid, gid, clone_flags, Mounts(request.mount_tree()), request.mode() != FORKSERVER_FORK_JOIN_SANDBOX_UNWIND, request.hostname(), - avoid_pivot_root); + avoid_pivot_root, request.allow_mount_propagation()); } } // namespace sandbox2 diff --git a/sandboxed_api/sandbox2/forkserver.proto b/sandboxed_api/sandbox2/forkserver.proto index 6846cf7..8f1ecbd 100644 --- a/sandboxed_api/sandbox2/forkserver.proto +++ b/sandboxed_api/sandbox2/forkserver.proto @@ -51,4 +51,7 @@ message ForkRequest { // Hostname in the network namespace optional bytes hostname = 7; + + // Changes mount propagation from MS_PRIVATE to MS_SLAVE if set + optional bool allow_mount_propagation = 8; } diff --git a/sandboxed_api/sandbox2/namespace.cc b/sandboxed_api/sandbox2/namespace.cc index 8cb08ab..9acc96d 100644 --- a/sandboxed_api/sandbox2/namespace.cc +++ b/sandboxed_api/sandbox2/namespace.cc @@ -197,11 +197,12 @@ void LogFilesystem(const std::string& dir) { } // namespace Namespace::Namespace(bool allow_unrestricted_networking, Mounts mounts, - std::string hostname) + std::string hostname, bool allow_mount_propagation) : clone_flags_(CLONE_NEWUSER | CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWIPC), mounts_(std::move(mounts)), - hostname_(std::move(hostname)) { + hostname_(std::move(hostname)), + allow_mount_propagation_(allow_mount_propagation) { if (!allow_unrestricted_networking) { clone_flags_ |= CLONE_NEWNET; } @@ -214,7 +215,8 @@ int32_t Namespace::GetCloneFlags() const { return clone_flags_; } void Namespace::InitializeNamespaces(uid_t uid, gid_t gid, int32_t clone_flags, const Mounts& mounts, bool mount_proc, const std::string& hostname, - bool avoid_pivot_root) { + bool avoid_pivot_root, + bool allow_mount_propagation) { if (clone_flags & CLONE_NEWUSER && !avoid_pivot_root) { SetupIDMaps(uid, gid); } @@ -325,8 +327,13 @@ void Namespace::InitializeNamespaces(uid_t uid, gid_t gid, int32_t clone_flags, SAPI_RAW_PCHECK(chdir("/") == 0, "changing cwd after mntns initialization failed"); - SAPI_RAW_PCHECK(mount("/", "/", "", MS_PRIVATE | MS_REC, nullptr) == 0, - "changing mount propagation to private failed"); + if (allow_mount_propagation) { + SAPI_RAW_PCHECK(mount("/", "/", "", MS_SLAVE | MS_REC, nullptr) == 0, + "changing mount propagation to slave failed"); + } else { + SAPI_RAW_PCHECK(mount("/", "/", "", MS_PRIVATE | MS_REC, nullptr) == 0, + "changing mount propagation to private failed"); + } if (SAPI_VLOG_IS_ON(2)) { SAPI_RAW_VLOG(2, "Dumping the sandboxee's filesystem:"); diff --git a/sandboxed_api/sandbox2/namespace.h b/sandboxed_api/sandbox2/namespace.h index 0f114fb..f1e2b9f 100644 --- a/sandboxed_api/sandbox2/namespace.h +++ b/sandboxed_api/sandbox2/namespace.h @@ -36,7 +36,8 @@ class Namespace final { static void InitializeNamespaces(uid_t uid, gid_t gid, int32_t clone_flags, const Mounts& mounts, bool mount_proc, const std::string& hostname, - bool avoid_pivot_root); + bool avoid_pivot_root, + bool allow_mount_propagation); static void InitializeInitialNamespaces(uid_t uid, gid_t gid); Namespace() = delete; @@ -44,7 +45,7 @@ class Namespace final { Namespace& operator=(const Namespace&) = delete; Namespace(bool allow_unrestricted_networking, Mounts mounts, - std::string hostname); + std::string hostname, bool allow_mount_propagation); void DisableUserNamespace(); @@ -59,12 +60,15 @@ class Namespace final { const std::string& hostname() const { return hostname_; } + bool allow_mount_propagation() const { return allow_mount_propagation_; } + private: friend class StackTracePeer; int32_t clone_flags_; Mounts mounts_; std::string hostname_; + bool allow_mount_propagation_; }; } // namespace sandbox2 diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index ea35fb5..622a7b1 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -898,7 +898,8 @@ absl::StatusOr> PolicyBuilder::TryBuild() { "Cannot set hostname without network namespaces."); } output->SetNamespace(absl::make_unique( - allow_unrestricted_networking_, std::move(mounts_), hostname_)); + allow_unrestricted_networking_, std::move(mounts_), hostname_, + allow_mount_propagation_)); } else { // Not explicitly disabling them here as this is a technical limitation in // our stack trace collection functionality. diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index eccff24..f249462 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/sandboxed_api/sandbox2/policybuilder.h @@ -577,6 +577,12 @@ class PolicyBuilder final { // Not recommended PolicyBuilder& SetRootWritable(); + // Changes mounts propagation from MS_PRIVATE to MS_SLAVE. + PolicyBuilder& DangerAllowMountPropagation() { + allow_mount_propagation_ = true; + return *this; + } + // Allows connections to this IP. PolicyBuilder& AllowIPv4(const std::string& ip_and_mask, uint32_t port = 0); PolicyBuilder& AllowIPv6(const std::string& ip_and_mask, uint32_t port = 0); @@ -613,6 +619,7 @@ class PolicyBuilder final { bool use_namespaces_ = true; bool requires_namespaces_ = false; bool allow_unrestricted_networking_ = false; + bool allow_mount_propagation_ = false; std::string hostname_ = std::string(kDefaultHostname); bool collect_stacktrace_on_violation_ = true;