diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 11bb0bc..1ee631b 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -220,16 +220,34 @@ cc_library( srcs = ["global_forkclient.cc"], hdrs = ["global_forkclient.h"], copts = sapi_platform_copts(), + visibility = ["//visibility:public"], deps = [ ":comms", ":fork_client", ":forkserver_bin_embed", - ":sanitizer", + ":forkserver_cc_proto", + ":util", "//sandboxed_api:embed_file", + "//sandboxed_api/sandbox2/util:fileops", "//sandboxed_api/sandbox2/util:strerror", + "//sandboxed_api/util:flags", "//sandboxed_api/util:raw_logging", - "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/memory", "@com_google_absl//absl/strings", + "@com_google_glog//:glog", + ], +) + +# Use only if Sandbox2 global forkserver has to be started very early on. +# By default the forkserver is started on demand. +cc_library( + name = "start_global_forkserver_lib_constructor", + srcs = ["global_forkclient_lib_ctor.cc"], + copts = sapi_platform_copts(), + visibility = ["//visibility:public"], + deps = [ + ":global_forkserver", + "@com_google_absl//absl/base:core_headers", ], ) diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index 5b64f9e..a1aab77 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -228,18 +228,36 @@ add_library(sandbox2_global_forkserver STATIC global_forkclient.h ) add_library(sandbox2::global_forkserver ALIAS sandbox2_global_forkserver) -target_link_libraries(sandbox2_global_forkserver PRIVATE +target_link_libraries(sandbox2_global_forkserver + PRIVATE absl::memory + absl::strings + glog::glog + sandbox2::client + sandbox2::fileops + sandbox2::forkserver_bin_embed + sandbox2::strerror + sandbox2::util + sapi::base + sapi::embed_file + sapi::flags + sapi::raw_logging + PUBLIC sandbox2::comms + sandbox2::fork_client + sandbox2::forkserver_proto +) + +# sandboxed_api/sandbox2:start_global_forkserver_lib_constructor +# Use only if Sandbox2 global forkserver has to be started very early on. +# By default the forkserver is started on demand. +add_library(sandbox2_start_global_forkserver_lib_constructor STATIC + global_forkclient_lib_ctor.cc +) +add_library(sandbox2::start_global_forkserver_lib_constructor ALIAS + sandbox2_start_global_forkserver_lib_constructor) +target_link_libraries(sandbox2_start_global_forkserver_lib_constructor PRIVATE absl::core_headers - absl::strings - sandbox2::client - sandbox2::comms - sandbox2::fork_client - sandbox2::forkserver_bin_embed - sandbox2::sanitizer - sandbox2::strerror sapi::base - sapi::embed_file - sapi::raw_logging + sandbox2::global_forkserver ) # sandboxed_api/sandbox2:executor diff --git a/sandboxed_api/sandbox2/executor.cc b/sandboxed_api/sandbox2/executor.cc index f2afb3a..d3e2d78 100644 --- a/sandboxed_api/sandbox2/executor.cc +++ b/sandboxed_api/sandbox2/executor.cc @@ -46,14 +46,13 @@ Executor::Executor(int exec_fd, const std::string& path, exec_fd_(exec_fd), path_(path), argv_(argv), - envp_(envp) { - if (fork_client != nullptr) { + envp_(envp), + fork_client_(fork_client) { + if (fork_client_ != nullptr) { CHECK(exec_fd == -1 && path.empty()); - fork_client_ = fork_client; } else { CHECK((exec_fd == -1 && (!path.empty() || libunwind_sbox_for_pid > 0)) || (exec_fd >= 0 && path.empty())); - fork_client_ = GetGlobalForkClient(); } SetUpServerSideCommsFd(); SetDefaultCwd(); @@ -78,10 +77,6 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns, LOG(ERROR) << "This executor has already been started"; return -1; } - if (fork_client_ == nullptr) { - LOG(ERROR) << "The ForkClient object is not instantiated"; - return -1; - } if (!path_.empty()) { exec_fd_ = open(path_.c_str(), O_PATH); @@ -155,8 +150,14 @@ pid_t Executor::StartSubProcess(int32_t clone_flags, const Namespace* ns, pid_t init_pid = -1; - pid_t sandboxee_pid = fork_client_->SendRequest( - request, exec_fd_, client_comms_fd_, ns_fd, &init_pid); + pid_t sandboxee_pid; + if (fork_client_) { + sandboxee_pid = fork_client_->SendRequest( + request, exec_fd_, client_comms_fd_, ns_fd, &init_pid); + } else { + sandboxee_pid = GlobalForkClient::SendRequest( + request, exec_fd_, client_comms_fd_, ns_fd, &init_pid); + } if (init_pid < 0) { LOG(ERROR) << "Could not obtain init PID"; @@ -191,10 +192,11 @@ std::unique_ptr Executor::StartForkServer() { // This flag is set explicitly to 'true' during object instantiation, and // custom fork-servers should never be sandboxed. set_enable_sandbox_before_exec(false); - if (StartSubProcess(0) == -1) { + pid_t pid = StartSubProcess(0); + if (pid == -1) { return nullptr; } - return absl::make_unique(ipc_.comms()); + return absl::make_unique(pid, ipc_.comms()); } void Executor::SetUpServerSideCommsFd() { diff --git a/sandboxed_api/sandbox2/fork_client.h b/sandboxed_api/sandbox2/fork_client.h index 36d8d24..60acc90 100644 --- a/sandboxed_api/sandbox2/fork_client.h +++ b/sandboxed_api/sandbox2/fork_client.h @@ -30,8 +30,7 @@ class ForkRequest; class ForkClient { public: - explicit ForkClient(Comms* comms) : comms_(comms) {} - + ForkClient(pid_t pid, Comms* comms) : pid_(pid), comms_(comms) {} ForkClient(const ForkClient&) = delete; ForkClient& operator=(const ForkClient&) = delete; @@ -39,7 +38,11 @@ class ForkClient { pid_t SendRequest(const ForkRequest& request, int exec_fd, int comms_fd, int user_ns_fd = -1, pid_t* init_pid = nullptr); + pid_t pid() { return pid_; } + private: + // Pid of the ForkServer. + pid_t pid_; // Comms channel connecting with the ForkServer. Not owned by the object. Comms* comms_ ABSL_GUARDED_BY(comms_mutex_); // Mutex locking transactions (requests) over the Comms channel. diff --git a/sandboxed_api/sandbox2/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index dc6a283..eb6dd17 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -453,12 +453,6 @@ pid_t ForkServer::ServeRequest() { } bool ForkServer::Initialize() { - // If the parent goes down, so should we. - if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0) != 0) { - SAPI_RAW_PLOG(ERROR, "prctl(PR_SET_PDEATHSIG, SIGKILL)"); - return false; - } - // All processes spawned by the fork'd/execute'd process will see this process // as /sbin/init. Therefore it will receive (and ignore) their final status // (see the next comment as well). PR_SET_CHILD_SUBREAPER is available since diff --git a/sandboxed_api/sandbox2/forkserver_test.cc b/sandboxed_api/sandbox2/forkserver_test.cc index 4affe4e..c64f747 100644 --- a/sandboxed_api/sandbox2/forkserver_test.cc +++ b/sandboxed_api/sandbox2/forkserver_test.cc @@ -59,7 +59,7 @@ pid_t TestSingleRequest(Mode mode, int exec_fd, int userns_fd) { fork_req.add_envs("FOO=1"); pid_t pid = - GetGlobalForkClient()->SendRequest(fork_req, exec_fd, sv[0], userns_fd); + GlobalForkClient::SendRequest(fork_req, exec_fd, sv[0], userns_fd); if (pid != -1) { VLOG(1) << "TestSingleRequest: Waiting for pid=" << pid; waitpid(pid, nullptr, 0); diff --git a/sandboxed_api/sandbox2/global_forkclient.cc b/sandboxed_api/sandbox2/global_forkclient.cc index 05063c1..9348a50 100644 --- a/sandboxed_api/sandbox2/global_forkclient.cc +++ b/sandboxed_api/sandbox2/global_forkclient.cc @@ -25,92 +25,99 @@ #include #include -#include "absl/base/attributes.h" -#include "absl/strings/str_cat.h" +#include +#include "sandboxed_api/util/flag.h" +#include "absl/memory/memory.h" #include "sandboxed_api/embed_file.h" #include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/forkserver_bin_embed.h" -#include "sandboxed_api/sandbox2/sanitizer.h" +#include "sandboxed_api/sandbox2/util.h" +#include "sandboxed_api/sandbox2/util/fileops.h" #include "sandboxed_api/sandbox2/util/strerror.h" #include "sandboxed_api/util/raw_logging.h" +ABSL_FLAG(bool, sandbox2_start_forkserver, true, + "Start Sandbox2 Forkserver process"); + namespace sandbox2 { -// Global ForkClient object linking with the global ForkServer. -static ForkClient* global_fork_client = nullptr; -static pid_t global_fork_server_pid = -1; +namespace { -ForkClient* GetGlobalForkClient() { - SAPI_RAW_CHECK(global_fork_client != nullptr, - "global fork client not initialized"); - return global_fork_client; -} - -pid_t GetGlobalForkServerPid() { return global_fork_server_pid; } - -static void StartGlobalForkServer() { - SAPI_RAW_CHECK(global_fork_client == nullptr, - "global fork server already initialized"); +std::unique_ptr StartGlobalForkServer() { if (getenv(kForkServerDisableEnv)) { SAPI_RAW_VLOG(1, "Start of the Global Fork-Server prevented by the '%s' " "environment variable present", kForkServerDisableEnv); - return; + return {}; } - sanitizer::WaitForTsan(); - - // We should be really single-threaded now, as it's the point of the whole - // exercise. - int num_threads = sanitizer::GetNumberOfThreads(getpid()); - if (num_threads != 1) { - SAPI_RAW_LOG(ERROR, - "BADNESS MAY HAPPEN. ForkServer::Init() created in a " - "multi-threaded context, %d threads present", - num_threads); + if (!absl::GetFlag(FLAGS_sandbox2_start_forkserver)) { + SAPI_RAW_VLOG( + 1, "Start of the Global Fork-Server prevented by commandline flag"); + return {}; } + file_util::fileops::FDCloser exec_fd( + sapi::EmbedFile::GetEmbedFileSingleton()->GetFdForFileToc( + forkserver_bin_embed_create())); + SAPI_RAW_CHECK(exec_fd.get() >= 0, "Getting FD for init binary failed"); + + std::string proc_name = "S2-FORK-SERV"; + int sv[2]; SAPI_RAW_CHECK(socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, sv) != -1, "creating socket pair"); // Fork the fork-server, and clean-up the resources (close remote sockets). - pid_t pid; - { - pid = fork(); - } + pid_t pid = util::ForkWithFlags(SIGCHLD); SAPI_RAW_PCHECK(pid != -1, "during fork"); - // Parent. - if (pid > 0) { - close(sv[0]); - global_fork_client = new ForkClient{new Comms{sv[1]}}; - global_fork_server_pid = pid; - return; + // Child. + if (pid == 0) { + // Move the comms FD to the proper, expected FD number. + // The new FD will not be CLOEXEC, which is what we want. + dup2(sv[0], Comms::kSandbox2ClientCommsFD); + + char* const args[] = {proc_name.data(), nullptr}; + char* const envp[] = {nullptr}; + syscall(__NR_execveat, exec_fd.get(), "", args, envp, AT_EMPTY_PATH); + SAPI_RAW_PLOG(FATAL, "Could not launch forkserver binary"); + abort(); } - // Move the comms FD to the proper, expected FD number. - // The new FD will not be CLOEXEC, which is what we want. - dup2(sv[0], Comms::kSandbox2ClientCommsFD); - - int exec_fd = sapi::EmbedFile::GetEmbedFileSingleton()->GetFdForFileToc( - forkserver_bin_embed_create()); - SAPI_RAW_CHECK(exec_fd >= 0, "Getting FD for init binary failed"); - - char* const args[] = {strdup("S2-FORK-SERV"), nullptr}; - char* const envp[] = {nullptr}; - syscall(__NR_execveat, exec_fd, "", args, envp, AT_EMPTY_PATH); - SAPI_RAW_PCHECK(false, "Could not launch forkserver binary"); + close(sv[0]); + return absl::make_unique(sv[1], pid); } +GlobalForkClient* GetGlobalForkClient() { + static GlobalForkClient* global_fork_client = + StartGlobalForkServer().release(); + return global_fork_client; +} + +} // namespace + +void GlobalForkClient::EnsureStarted() { GetGlobalForkClient(); } + +pid_t GlobalForkClient::SendRequest(const ForkRequest& request, int exec_fd, + int comms_fd, int user_ns_fd, + pid_t* init_pid) { + GlobalForkClient* global_fork_client = GetGlobalForkClient(); + SAPI_RAW_CHECK(global_fork_client != nullptr, + "global fork client not initialized"); + pid_t pid = global_fork_client->fork_client_.SendRequest( + request, exec_fd, comms_fd, user_ns_fd, init_pid); + if (global_fork_client->comms_.IsTerminated()) { + LOG(ERROR) << "Global forkserver connection terminated"; + } + return pid; +} + +pid_t GlobalForkClient::GetPid() { + GlobalForkClient* global_fork_client = GetGlobalForkClient(); + SAPI_RAW_CHECK(global_fork_client != nullptr, + "global fork client not initialized"); + return global_fork_client->fork_client_.pid(); +} } // namespace sandbox2 - -// Run the ForkServer from the constructor, when no other threads are present. -// Because it's possible to start thread-inducing initializers before -// RunInitializers() (base/googleinit.h) it's not enough to just register -// a 0000_ initializer instead. -ABSL_ATTRIBUTE_UNUSED -__attribute__((constructor)) static void StartSandbox2Forkserver() { - sandbox2::StartGlobalForkServer(); -} diff --git a/sandboxed_api/sandbox2/global_forkclient.h b/sandboxed_api/sandbox2/global_forkclient.h index 00ab359..b7f22b4 100644 --- a/sandboxed_api/sandbox2/global_forkclient.h +++ b/sandboxed_api/sandbox2/global_forkclient.h @@ -20,14 +20,28 @@ #include +#include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/fork_client.h" +#include "sandboxed_api/sandbox2/forkserver.pb.h" namespace sandbox2 { -ForkClient* GetGlobalForkClient(); +class GlobalForkClient { + public: + GlobalForkClient(int fd, pid_t pid) + : comms_(fd), fork_client_(pid, &comms_) {} -// Returns the fork server PID. Used in tests. -pid_t GetGlobalForkServerPid(); + static pid_t SendRequest(const ForkRequest& request, int exec_fd, + int comms_fd, int user_ns_fd = -1, + pid_t* init_pid = nullptr); + static pid_t GetPid(); + + static void EnsureStarted(); + + private: + Comms comms_; + ForkClient fork_client_; +}; } // namespace sandbox2 diff --git a/sandboxed_api/sandbox2/global_forkclient_lib_ctor.cc b/sandboxed_api/sandbox2/global_forkclient_lib_ctor.cc new file mode 100644 index 0000000..98e064c --- /dev/null +++ b/sandboxed_api/sandbox2/global_forkclient_lib_ctor.cc @@ -0,0 +1,25 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "absl/base/attributes.h" +#include "sandboxed_api/sandbox2/global_forkclient.h" + +// Run the ForkServer from the constructor, when no other threads are present. +// Because it's possible to start thread-inducing initializers before +// RunInitializers() (base/googleinit.h) it's not enough to just register +// a 0000_ initializer instead. +ABSL_ATTRIBUTE_UNUSED +__attribute__((constructor)) static void StartSandbox2Forkserver() { + sandbox2::GlobalForkClient::EnsureStarted(); +} diff --git a/sandboxed_api/sandbox2/stack_trace_test.cc b/sandboxed_api/sandbox2/stack_trace_test.cc index 0f0c600..0f89110 100644 --- a/sandboxed_api/sandbox2/stack_trace_test.cc +++ b/sandboxed_api/sandbox2/stack_trace_test.cc @@ -158,7 +158,7 @@ static size_t FileCountInDirectory(const std::string& path) { TEST(StackTraceTest, ForkEnterNsLibunwindDoesNotLeakFDs) { SKIP_SANITIZERS_AND_COVERAGE; // Get list of open FDs in the global forkserver. - pid_t forkserver_pid = GetGlobalForkServerPid(); + pid_t forkserver_pid = GlobalForkClient::GetPid(); std::string forkserver_fd_path = absl::StrCat("/proc/", forkserver_pid, "/fd"); size_t filecount_before = FileCountInDirectory(forkserver_fd_path);