From 2d3a040f64ff975dca2f14d75a56cf57c8f5195b Mon Sep 17 00:00:00 2001 From: Christian Blichmann Date: Mon, 17 May 2021 04:06:39 -0700 Subject: [PATCH] Minor cleanup/formatting changes PiperOrigin-RevId: 374164136 Change-Id: I505cbc3ac9f899ed965cde66aaae1aba55a90c64 --- sandboxed_api/sandbox2/executor.cc | 2 +- sandboxed_api/sandbox2/forkserver.cc | 5 +- sandboxed_api/sandbox2/mounts.cc | 69 +++++++++++----------------- sandboxed_api/sandbox2/policy.h | 10 ++-- sandboxed_api/util/BUILD.bazel | 1 + sandboxed_api/util/CMakeLists.txt | 1 + sandboxed_api/util/temp_file.cc | 17 +++---- 7 files changed, 45 insertions(+), 60 deletions(-) diff --git a/sandboxed_api/sandbox2/executor.cc b/sandboxed_api/sandbox2/executor.cc index 0f0386e..b469162 100644 --- a/sandboxed_api/sandbox2/executor.cc +++ b/sandboxed_api/sandbox2/executor.cc @@ -84,7 +84,7 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns, } // Add LD_ORIGIN_PATH to envs, as it'll make the amount of syscalls invoked by - // ld.so smaller. See http://b/7626303 for more details on this behavior. + // ld.so smaller. if (!path_.empty()) { request.add_envs(absl::StrCat("LD_ORIGIN_PATH=", file_util::fileops::StripBasename(path_))); diff --git a/sandboxed_api/sandbox2/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index 73061e7..422b096 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -237,6 +237,7 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd, if (!sanitizer::GetListOfFDs(&open_fds)) { SAPI_RAW_LOG(WARNING, "Could not get list of current open FDs"); } + InitializeNamespaces(request, uid, gid, avoid_pivot_root); auto caps = cap_init(); @@ -320,9 +321,8 @@ pid_t ForkServer::ServeRequest() { if (comms_->IsTerminated()) { SAPI_RAW_VLOG(1, "ForkServer Comms closed. Exiting"); exit(0); - } else { - SAPI_RAW_LOG(FATAL, "Failed to receive ForkServer request"); } + SAPI_RAW_LOG(FATAL, "Failed to receive ForkServer request"); } int comms_fd; SAPI_RAW_CHECK(comms_->RecvFD(&comms_fd), "Failed to receive Comms FD"); @@ -575,6 +575,7 @@ void ForkServer::InitializeNamespaces(const ForkRequest& request, uid_t uid, SAPI_RAW_PCHECK(!unshare(clone_flags), "Could not create new namespaces for libunwind"); } + Namespace::InitializeNamespaces( uid, gid, clone_flags, Mounts(request.mount_tree()), request.mode() != FORKSERVER_FORK_JOIN_SANDBOX_UNWIND, request.hostname(), diff --git a/sandboxed_api/sandbox2/mounts.cc b/sandboxed_api/sandbox2/mounts.cc index 4c02032..5f22271 100644 --- a/sandboxed_api/sandbox2/mounts.cc +++ b/sandboxed_api/sandbox2/mounts.cc @@ -328,7 +328,7 @@ absl::Status Mounts::AddMappingsForBinary(const std::string& path, SAPI_RETURN_IF_ERROR(ValidateInterpreter(interpreter)); std::vector search_paths; - // 1. LD_LIBRARY_PRELOAD + // 1. LD_LIBRARY_PATH if (!ld_library_path.empty()) { std::vector ld_library_paths = absl::StrSplit(ld_library_path, absl::ByAnyChar(":;")); @@ -476,65 +476,52 @@ uint64_t GetMountFlagsFor(const std::string& path) { return 0; } - static constexpr struct { - const uint64_t mount_flag; - const uint64_t vfs_flag; - } mount_pairs[] = { - {MS_NOSUID, ST_NOSUID}, {MS_NODEV, ST_NODEV}, - {MS_NOEXEC, ST_NOEXEC}, {MS_SYNCHRONOUS, ST_SYNCHRONOUS}, - {MS_MANDLOCK, ST_MANDLOCK}, {MS_NOATIME, ST_NOATIME}, - {MS_NODIRATIME, ST_NODIRATIME}, {MS_RELATIME, ST_RELATIME}, - }; - uint64_t flags = 0; - for (const auto& i : mount_pairs) { - if (vfs.f_flag & i.vfs_flag) { - flags |= i.mount_flag; + using MountPair = std::pair; + for (const auto& [mount_flag, vfs_flag] : { + MountPair(MS_NOSUID, ST_NOSUID), + MountPair(MS_NODEV, ST_NODEV), + MountPair(MS_NOEXEC, ST_NOEXEC), + MountPair(MS_SYNCHRONOUS, ST_SYNCHRONOUS), + MountPair(MS_MANDLOCK, ST_MANDLOCK), + MountPair(MS_NOATIME, ST_NOATIME), + MountPair(MS_NODIRATIME, ST_NODIRATIME), + MountPair(MS_RELATIME, ST_RELATIME), + }) { + if (vfs.f_flag & vfs_flag) { + flags |= mount_flag; } } - return flags; } std::string MountFlagsToString(uint64_t flags) { #define SAPI_MAP(x) \ { x, #x } - static constexpr std::pair map[] = { - SAPI_MAP(MS_RDONLY), - SAPI_MAP(MS_NOSUID), - SAPI_MAP(MS_NODEV), - SAPI_MAP(MS_NOEXEC), - SAPI_MAP(MS_SYNCHRONOUS), - SAPI_MAP(MS_REMOUNT), - SAPI_MAP(MS_MANDLOCK), - SAPI_MAP(MS_DIRSYNC), - SAPI_MAP(MS_NOATIME), - SAPI_MAP(MS_NODIRATIME), - SAPI_MAP(MS_BIND), - SAPI_MAP(MS_MOVE), + static constexpr std::pair kMap[] = { + SAPI_MAP(MS_RDONLY), SAPI_MAP(MS_NOSUID), + SAPI_MAP(MS_NODEV), SAPI_MAP(MS_NOEXEC), + SAPI_MAP(MS_SYNCHRONOUS), SAPI_MAP(MS_REMOUNT), + SAPI_MAP(MS_MANDLOCK), SAPI_MAP(MS_DIRSYNC), + SAPI_MAP(MS_NOATIME), SAPI_MAP(MS_NODIRATIME), + SAPI_MAP(MS_BIND), SAPI_MAP(MS_MOVE), SAPI_MAP(MS_REC), #ifdef MS_VERBOSE - // MS_VERBOSE is deprecated - SAPI_MAP(MS_VERBOSE), + SAPI_MAP(MS_VERBOSE), // Deprecated #endif - SAPI_MAP(MS_SILENT), - SAPI_MAP(MS_POSIXACL), - SAPI_MAP(MS_UNBINDABLE), - SAPI_MAP(MS_PRIVATE), + SAPI_MAP(MS_SILENT), SAPI_MAP(MS_POSIXACL), + SAPI_MAP(MS_UNBINDABLE), SAPI_MAP(MS_PRIVATE), SAPI_MAP(MS_SLAVE), // Inclusive language: system constant - SAPI_MAP(MS_SHARED), - SAPI_MAP(MS_RELATIME), - SAPI_MAP(MS_KERNMOUNT), - SAPI_MAP(MS_I_VERSION), + SAPI_MAP(MS_SHARED), SAPI_MAP(MS_RELATIME), + SAPI_MAP(MS_KERNMOUNT), SAPI_MAP(MS_I_VERSION), SAPI_MAP(MS_STRICTATIME), #ifdef MS_LAZYTIME - // MS_LAZYTIME was added in Linux 4.0 - SAPI_MAP(MS_LAZYTIME), + SAPI_MAP(MS_LAZYTIME), // Added in Linux 4.0 #endif }; #undef SAPI_MAP std::vector flags_list; - for (auto [val, str] : map) { + for (const auto& [val, str] : kMap) { if ((flags & val) == val) { flags &= ~val; flags_list.push_back(str); diff --git a/sandboxed_api/sandbox2/policy.h b/sandboxed_api/sandbox2/policy.h index 7260b7d..1246916 100644 --- a/sandboxed_api/sandbox2/policy.h +++ b/sandboxed_api/sandbox2/policy.h @@ -58,6 +58,11 @@ class Policy final { void GetPolicyDescription(PolicyDescription* policy) const; private: + friend class Monitor; + friend class PolicyBuilder; + friend class PolicyBuilderPeer; // For testing + friend class StackTracePeer; + // Private constructor only called by the PolicyBuilder. Policy() = default; @@ -105,11 +110,6 @@ class Policy final { // Contains a list of hosts the sandboxee is allowed to connect to. absl::optional allowed_hosts_; - - friend class Monitor; - friend class PolicyBuilder; - friend class PolicyBuilderPeer; // For testing - friend class StackTracePeer; }; } // namespace sandbox2 diff --git a/sandboxed_api/util/BUILD.bazel b/sandboxed_api/util/BUILD.bazel index 761bcbe..28c144a 100644 --- a/sandboxed_api/util/BUILD.bazel +++ b/sandboxed_api/util/BUILD.bazel @@ -249,6 +249,7 @@ cc_library( copts = sapi_platform_copts(), deps = [ ":fileops", + ":status", ":strerror", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", diff --git a/sandboxed_api/util/CMakeLists.txt b/sandboxed_api/util/CMakeLists.txt index c7af144..ac90c78 100644 --- a/sandboxed_api/util/CMakeLists.txt +++ b/sandboxed_api/util/CMakeLists.txt @@ -151,6 +151,7 @@ add_library(sapi::temp_file ALIAS sapi_util_temp_file) target_link_libraries(sapi_util_temp_file PRIVATE absl::strings sapi::fileops + sapi::status sapi::strerror sapi::base PUBLIC absl::status diff --git a/sandboxed_api/util/temp_file.cc b/sandboxed_api/util/temp_file.cc index 5fef681..9a26054 100644 --- a/sandboxed_api/util/temp_file.cc +++ b/sandboxed_api/util/temp_file.cc @@ -25,6 +25,7 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "sandboxed_api/util/fileops.h" +#include "sandboxed_api/util/status_macros.h" #include "sandboxed_api/util/strerror.h" namespace sapi { @@ -38,28 +39,22 @@ absl::StatusOr> CreateNamedTempFile( std::string name_template = absl::StrCat(prefix, kMktempSuffix); int fd = mkstemp(&name_template[0]); if (fd < 0) { - return absl::UnknownError(absl::StrCat("mkstemp():", StrError(errno))); + return absl::UnknownError(absl::StrCat("mkstemp(): ", StrError(errno))); } return std::pair{std::move(name_template), fd}; } absl::StatusOr CreateNamedTempFileAndClose( absl::string_view prefix) { - auto result_or = CreateNamedTempFile(prefix); - if (result_or.ok()) { - std::string path; - int fd; - std::tie(path, fd) = std::move(result_or).value(); - close(fd); - return path; - } - return result_or.status(); + SAPI_ASSIGN_OR_RETURN(auto result, CreateNamedTempFile(prefix)); + close(result.second); + return std::move(result.first); } absl::StatusOr CreateTempDir(absl::string_view prefix) { std::string name_template = absl::StrCat(prefix, kMktempSuffix); if (mkdtemp(&name_template[0]) == nullptr) { - return absl::UnknownError(absl::StrCat("mkdtemp():", StrError(errno))); + return absl::UnknownError(absl::StrCat("mkdtemp(): ", StrError(errno))); } return name_template; }