Automated rollback of commit 1e26cd50dc.

PiperOrigin-RevId: 559102360
Change-Id: I5dd175d5f0b9ece602f5c26454ad1f1e2e3a60fc
This commit is contained in:
Sandboxed API Team 2023-08-22 07:11:27 -07:00 committed by Copybara-Service
parent c4660f8a6e
commit 41003aae83
5 changed files with 68 additions and 80 deletions

View File

@ -153,13 +153,13 @@ absl::Status SumTransaction::Main() {
sapi::v::Fd fd(fdesc); sapi::v::Fd fd(fdesc);
SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd)); SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd));
LOG(INFO) << "remote_fd = " << fd.GetRemoteFd(); LOG(INFO) << "remote_fd = " << fd.GetRemoteFd();
TRANSACTION_FAIL_IF_NOT(fd.GetRemoteFd() != -1, "remote_fd == -1"); TRANSACTION_FAIL_IF_NOT(fd.GetRemoteFd() == 3, "remote_fd != 3");
fdesc = open("/proc/self/comm", O_RDONLY); fdesc = open("/proc/self/comm", O_RDONLY);
sapi::v::Fd fd2(fdesc); sapi::v::Fd fd2(fdesc);
SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd2)); SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd2));
LOG(INFO) << "remote_fd2 = " << fd2.GetRemoteFd(); LOG(INFO) << "remote_fd2 = " << fd2.GetRemoteFd();
TRANSACTION_FAIL_IF_NOT(fd2.GetRemoteFd() != -1, "remote_fd2 == -1"); TRANSACTION_FAIL_IF_NOT(fd2.GetRemoteFd() == 4, "remote_fd2 != 4");
// Read from fd test. // Read from fd test.
char buffer[1024] = {0}; char buffer[1024] = {0};

View File

@ -565,6 +565,7 @@ target_link_libraries(sandbox2_forkserver
sandbox2::bpf_helper sandbox2::bpf_helper
sandbox2::client sandbox2::client
sandbox2::comms sandbox2::comms
sapi::fileops
sandbox2::fork_client sandbox2::fork_client
sandbox2::forkserver_proto sandbox2::forkserver_proto
sandbox2::namespace sandbox2::namespace
@ -577,7 +578,6 @@ target_link_libraries(sandbox2_forkserver
sapi::raw_logging sapi::raw_logging
PUBLIC absl::core_headers PUBLIC absl::core_headers
absl::log absl::log
sapi::fileops
) )
# sandboxed_api/sandbox2:fork_client # sandboxed_api/sandbox2:fork_client

View File

@ -38,9 +38,7 @@
#include <fstream> #include <fstream>
#include <memory> #include <memory>
#include <string> #include <string>
#include <utility>
#include "absl/base/attributes.h"
#include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_map.h"
#include "absl/container/flat_hash_set.h" #include "absl/container/flat_hash_set.h"
#include "absl/status/status.h" #include "absl/status/status.h"
@ -65,10 +63,12 @@
#include "sandboxed_api/util/strerror.h" #include "sandboxed_api/util/strerror.h"
namespace sandbox2 { namespace sandbox2 {
namespace file_util = ::sapi::file_util;
namespace { namespace {
using ::sapi::StrError; using ::sapi::StrError;
using ::sapi::file_util::fileops::FDCloser;
// "Moves" FDs in move_fds from current to target FD number while keeping FDs // "Moves" FDs in move_fds from current to target FD number while keeping FDs
// in keep_fds open - potentially moving them to another FD number as well in // in keep_fds open - potentially moving them to another FD number as well in
@ -113,11 +113,18 @@ void MoveFDs(std::initializer_list<std::pair<int*, int>> move_fds,
} }
} }
ABSL_ATTRIBUTE_NORETURN void RunInitProcess(pid_t main_pid, FDCloser pipe_fd) { void RunInitProcess(pid_t main_pid, int pipe_fd,
const absl::flat_hash_set<int>& open_fds) {
if (prctl(PR_SET_NAME, "S2-INIT-PROC", 0, 0, 0) != 0) { if (prctl(PR_SET_NAME, "S2-INIT-PROC", 0, 0, 0) != 0) {
SAPI_RAW_PLOG(WARNING, "prctl(PR_SET_NAME, 'S2-INIT-PROC')"); SAPI_RAW_PLOG(WARNING, "prctl(PR_SET_NAME, 'S2-INIT-PROC')");
} }
// Close all open fds (equals to CloseAllFDsExcept but does not require /proc
// to be available).
for (const auto& fd : open_fds) {
close(fd);
}
// Clear SA_NOCLDWAIT. // Clear SA_NOCLDWAIT.
struct sigaction sa; struct sigaction sa;
sa.sa_handler = SIG_DFL; sa.sa_handler = SIG_DFL;
@ -135,7 +142,7 @@ ABSL_ATTRIBUTE_NORETURN void RunInitProcess(pid_t main_pid, FDCloser pipe_fd) {
SYSCALL(__NR_waitid, ALLOW), SYSCALL(__NR_waitid, ALLOW),
SYSCALL(__NR_exit, ALLOW), SYSCALL(__NR_exit, ALLOW),
}; };
if (pipe_fd.get() >= 0) { if (pipe_fd >= 0) {
code.insert(code.end(), code.insert(code.end(),
{SYSCALL(__NR_getrusage, ALLOW), SYSCALL(__NR_write, ALLOW)}); {SYSCALL(__NR_getrusage, ALLOW), SYSCALL(__NR_write, ALLOW)});
} }
@ -162,13 +169,13 @@ ABSL_ATTRIBUTE_NORETURN void RunInitProcess(pid_t main_pid, FDCloser pipe_fd) {
} }
if (info.si_pid == main_pid) { if (info.si_pid == main_pid) {
if (pipe_fd.get() >= 0) { if (pipe_fd >= 0) {
write(pipe_fd.get(), &info.si_code, sizeof(info.si_code)); write(pipe_fd, &info.si_code, sizeof(info.si_code));
write(pipe_fd.get(), &info.si_status, sizeof(info.si_status)); write(pipe_fd, &info.si_status, sizeof(info.si_status));
rusage usage{}; rusage usage{};
getrusage(RUSAGE_CHILDREN, &usage); getrusage(RUSAGE_CHILDREN, &usage);
write(pipe_fd.get(), &usage, sizeof(usage)); write(pipe_fd, &usage, sizeof(usage));
} }
_exit(0); _exit(0);
} }
@ -274,8 +281,9 @@ void ForkServer::PrepareExecveArgs(const ForkRequest& request,
} }
void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd, void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
uid_t uid, gid_t gid, FDCloser signaling_fd, int client_fd, uid_t uid, gid_t gid,
FDCloser status_fd, bool avoid_pivot_root) const { int signaling_fd, int status_fd,
bool avoid_pivot_root) const {
SAPI_RAW_CHECK(request.mode() != FORKSERVER_FORK_UNSPECIFIED, SAPI_RAW_CHECK(request.mode() != FORKSERVER_FORK_UNSPECIFIED,
"Forkserver mode is unspecified"); "Forkserver mode is unspecified");
@ -289,6 +297,10 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
PrepareExecveArgs(request, &args, &envs); PrepareExecveArgs(request, &args, &envs);
} }
MoveFDs({{&execve_fd, Comms::kSandbox2TargetExecFD},
{&client_fd, Comms::kSandbox2ClientCommsFD}},
{&signaling_fd});
SanitizeEnvironment(); SanitizeEnvironment();
absl::StatusOr<absl::flat_hash_set<int>> open_fds = sanitizer::GetListOfFDs(); absl::StatusOr<absl::flat_hash_set<int>> open_fds = sanitizer::GetListOfFDs();
@ -312,23 +324,19 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
SAPI_RAW_PLOG(FATAL, "Could not spawn init process"); SAPI_RAW_PLOG(FATAL, "Could not spawn init process");
} }
if (child != 0) { if (child != 0) {
if (status_fd.get() >= 0) { if (status_fd >= 0) {
open_fds->erase(status_fd.get()); open_fds->erase(status_fd);
} }
// Close all open fds (equals to CloseAllFDsExcept but does not require RunInitProcess(child, status_fd, *open_fds);
// /proc to be available). }
for (const auto& fd : *open_fds) { if (status_fd >= 0) {
close(fd); close(status_fd);
}
RunInitProcess(child, std::move(status_fd));
} }
// Send sandboxee pid // Send sandboxee pid
auto status = SendPid(signaling_fd.get()); auto status = SendPid(signaling_fd);
SAPI_RAW_CHECK(status.ok(), SAPI_RAW_CHECK(status.ok(),
absl::StrCat("sending pid: ", status.message()).c_str()); absl::StrCat("sending pid: ", status.message()).c_str());
} }
signaling_fd.Close();
status_fd.Close();
if (request.mode() == FORKSERVER_FORK_EXECVE_SANDBOX) { if (request.mode() == FORKSERVER_FORK_EXECVE_SANDBOX) {
// Sandboxing can be enabled either here - just before execve, or somewhere // Sandboxing can be enabled either here - just before execve, or somewhere
@ -340,16 +348,17 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
// Create a Comms object here and not above, as we know we will execve and // Create a Comms object here and not above, as we know we will execve and
// therefore not call the Comms destructor, which would otherwise close the // therefore not call the Comms destructor, which would otherwise close the
// comms file descriptor, which we do not want for the general case. // comms file descriptor, which we do not want for the general case.
Client c(comms_); Comms client_comms(Comms::kDefaultConnection);
Client c(&client_comms);
// The following client calls are basically SandboxMeHere. We split it so // The following client calls are basically SandboxMeHere. We split it so
// that we can set up the envp after we received the file descriptors but // that we can set up the envp after we received the file descriptors but
// before we enable the syscall filter. // before we enable the syscall filter.
c.PrepareEnvironment(&execve_fd); c.PrepareEnvironment(&execve_fd);
if (comms_->GetConnectionFD() != Comms::kSandbox2ClientCommsFD) { if (client_comms.GetConnectionFD() != Comms::kSandbox2ClientCommsFD) {
envs.push_back(absl::StrCat(Comms::kSandbox2CommsFDEnvVar, "=", envs.push_back(absl::StrCat(Comms::kSandbox2CommsFDEnvVar, "=",
comms_->GetConnectionFD())); client_comms.GetConnectionFD()));
} }
envs.push_back(c.GetFdMapEnvVar()); envs.push_back(c.GetFdMapEnvVar());
// Convert args and envs before enabling sandbox (it'll allocate which might // Convert args and envs before enabling sandbox (it'll allocate which might
@ -396,14 +405,9 @@ pid_t ForkServer::ServeRequest() {
uid_t uid = getuid(); uid_t uid = getuid();
uid_t gid = getgid(); uid_t gid = getgid();
FDCloser pipe_fds[2]; int pfds[2] = {-1, -1};
{ if (fork_request.monitor_type() == FORKSERVER_MONITOR_UNOTIFY) {
int pfds[2] = {-1, -1}; SAPI_RAW_PCHECK(pipe(pfds) == 0, "creating status pipe");
if (fork_request.monitor_type() == FORKSERVER_MONITOR_UNOTIFY) {
SAPI_RAW_PCHECK(pipe(pfds) == 0, "creating status pipe");
}
pipe_fds[0] = FDCloser(pfds[0]);
pipe_fds[1] = FDCloser(pfds[1]);
} }
int socketpair_fds[2]; int socketpair_fds[2];
@ -417,8 +421,8 @@ pid_t ForkServer::ServeRequest() {
"setsockopt failed"); "setsockopt failed");
} }
FDCloser signaling_fds[] = {FDCloser(socketpair_fds[0]), file_util::fileops::FDCloser fd_closer0{socketpair_fds[0]};
FDCloser(socketpair_fds[1])}; file_util::fileops::FDCloser fd_closer1{socketpair_fds[1]};
// Note: init_pid will be overwritten with the actual init pid if the init // Note: init_pid will be overwritten with the actual init pid if the init
// process was started or stays at 0 if that is not needed - no pidns. // process was started or stays at 0 if that is not needed - no pidns.
@ -453,7 +457,7 @@ pid_t ForkServer::ServeRequest() {
_exit(0); _exit(0);
} }
// Send sandboxee pid // Send sandboxee pid
absl::Status status = SendPid(signaling_fds[1].get()); absl::Status status = SendPid(fd_closer1.get());
SAPI_RAW_CHECK(status.ok(), SAPI_RAW_CHECK(status.ok(),
absl::StrCat("sending pid: ", status.message()).c_str()); absl::StrCat("sending pid: ", status.message()).c_str());
} }
@ -470,29 +474,15 @@ pid_t ForkServer::ServeRequest() {
// Child. // Child.
if (sandboxee_pid == 0) { if (sandboxee_pid == 0) {
signaling_fds[0].Close(); LaunchChild(fork_request, exec_fd, comms_fd, uid, gid, fd_closer1.get(),
pipe_fds[0].Close(); pfds[1], avoid_pivot_root);
// Make sure we override the forkserver's comms fd
comms_->Terminate();
if (exec_fd != -1) {
int signaling_fd = signaling_fds[1].Release();
int pipe_fd = pipe_fds[1].Release();
MoveFDs({{&exec_fd, Comms::kSandbox2TargetExecFD},
{&comms_fd, Comms::kSandbox2ClientCommsFD}},
{&signaling_fd, &pipe_fd});
signaling_fds[1] = FDCloser(signaling_fd);
pipe_fds[1] = FDCloser(pipe_fd);
}
*comms_ = Comms(comms_fd);
LaunchChild(fork_request, exec_fd, uid, gid, std::move(signaling_fds[1]),
std::move(pipe_fds[1]), avoid_pivot_root);
return sandboxee_pid; return sandboxee_pid;
} }
signaling_fds[1].Close(); fd_closer1.Close();
if (avoid_pivot_root) { if (avoid_pivot_root) {
if (auto pid = ReceivePid(signaling_fds[0].get()); !pid.ok()) { if (auto pid = ReceivePid(fd_closer0.get()); !pid.ok()) {
SAPI_RAW_LOG(ERROR, "%s", std::string(pid.status().message()).c_str()); SAPI_RAW_LOG(ERROR, "%s", std::string(pid.status().message()).c_str());
} else { } else {
sandboxee_pid = pid.value(); sandboxee_pid = pid.value();
@ -506,7 +496,7 @@ pid_t ForkServer::ServeRequest() {
sandboxee_pid = -1; sandboxee_pid = -1;
// And the actual sandboxee is forked from the init process, so we need to // And the actual sandboxee is forked from the init process, so we need to
// receive the actual PID. // receive the actual PID.
if (auto pid_or = ReceivePid(signaling_fds[0].get()); !pid_or.ok()) { if (auto pid_or = ReceivePid(fd_closer0.get()); !pid_or.ok()) {
SAPI_RAW_LOG(ERROR, "%s", std::string(pid_or.status().message()).c_str()); SAPI_RAW_LOG(ERROR, "%s", std::string(pid_or.status().message()).c_str());
if (init_pid != -1) { if (init_pid != -1) {
kill(init_pid, SIGKILL); kill(init_pid, SIGKILL);
@ -518,7 +508,9 @@ pid_t ForkServer::ServeRequest() {
} }
// Parent. // Parent.
pipe_fds[1].Close(); if (pfds[1] >= 0) {
close(pfds[1]);
}
close(comms_fd); close(comms_fd);
if (exec_fd >= 0) { if (exec_fd >= 0) {
close(exec_fd); close(exec_fd);
@ -529,9 +521,9 @@ pid_t ForkServer::ServeRequest() {
comms_->SendInt32(sandboxee_pid), comms_->SendInt32(sandboxee_pid),
absl::StrCat("Failed to send sandboxee PID: ", sandboxee_pid).c_str()); absl::StrCat("Failed to send sandboxee PID: ", sandboxee_pid).c_str());
if (pipe_fds[0].get() >= 0) { if (pfds[0] >= 0) {
SAPI_RAW_CHECK(comms_->SendFD(pipe_fds[0].get()), SAPI_RAW_CHECK(comms_->SendFD(pfds[0]), "Failed to send status pipe");
"Failed to send status pipe"); close(pfds[0]);
} }
return sandboxee_pid; return sandboxee_pid;
} }
@ -570,9 +562,9 @@ void ForkServer::CreateInitialNamespaces() {
gid_t gid = getgid(); gid_t gid = getgid();
// Socket to synchronize so that we open ns fds before process dies // Socket to synchronize so that we open ns fds before process dies
FDCloser create_efd(eventfd(0, EFD_CLOEXEC)); file_util::fileops::FDCloser create_efd(eventfd(0, EFD_CLOEXEC));
SAPI_RAW_PCHECK(create_efd.get() != -1, "creating eventfd"); SAPI_RAW_PCHECK(create_efd.get() != -1, "creating eventfd");
FDCloser open_efd(eventfd(0, EFD_CLOEXEC)); file_util::fileops::FDCloser open_efd(eventfd(0, EFD_CLOEXEC));
SAPI_RAW_PCHECK(open_efd.get() != -1, "creating eventfd"); SAPI_RAW_PCHECK(open_efd.get() != -1, "creating eventfd");
pid_t pid = util::ForkWithFlags(CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); pid_t pid = util::ForkWithFlags(CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD);
if (pid == -1 && errno == EPERM && IsLikelyChrooted()) { if (pid == -1 && errno == EPERM && IsLikelyChrooted()) {
@ -606,11 +598,12 @@ void ForkServer::CreateInitialNamespaces() {
"synchronizing initial namespaces creation"); "synchronizing initial namespaces creation");
} }
void ForkServer::SanitizeEnvironment() const { void ForkServer::SanitizeEnvironment() {
// Mark all file descriptors, except the standard ones (needed // Mark all file descriptors, except the standard ones (needed
// for proper sandboxed process operations), as close-on-exec. // for proper sandboxed process operations), as close-on-exec.
absl::Status status = sanitizer::SanitizeCurrentProcess( absl::Status status = sanitizer::SanitizeCurrentProcess(
{STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO, comms_->GetConnectionFD()}, {STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO,
Comms::kSandbox2ClientCommsFD},
/* close_fds = */ false); /* close_fds = */ false);
SAPI_RAW_CHECK( SAPI_RAW_CHECK(
status.ok(), status.ok(),

View File

@ -25,7 +25,6 @@
#include "absl/base/attributes.h" #include "absl/base/attributes.h"
#include "absl/log/log.h" #include "absl/log/log.h"
#include "sandboxed_api/util/fileops.h"
namespace sandbox2 { namespace sandbox2 {
@ -54,9 +53,8 @@ class ForkServer {
private: private:
// Creates and launched the child process. // Creates and launched the child process.
void LaunchChild(const ForkRequest& request, int execve_fd, uid_t uid, void LaunchChild(const ForkRequest& request, int execve_fd, int client_fd,
gid_t gid, sapi::file_util::fileops::FDCloser signaling_fd, uid_t uid, gid_t gid, int signaling_fd, int status_fd,
sapi::file_util::fileops::FDCloser status_fd,
bool avoid_pivot_root) const; bool avoid_pivot_root) const;
// Prepares the Fork-Server (worker side, not the requester side) for work by // Prepares the Fork-Server (worker side, not the requester side) for work by
@ -75,7 +73,7 @@ class ForkServer {
std::vector<std::string>* envp); std::vector<std::string>* envp);
// Ensures that no unnecessary file descriptors are lingering after execve(). // Ensures that no unnecessary file descriptors are lingering after execve().
void SanitizeEnvironment() const; static void SanitizeEnvironment();
// Executes the sandboxee, or exit with Executor::kFailedExecve. // Executes the sandboxee, or exit with Executor::kFailedExecve.
static void ExecuteProcess(int execve_fd, const char* const* argv, static void ExecuteProcess(int execve_fd, const char* const* argv,

View File

@ -24,7 +24,6 @@
#include "sandboxed_api/examples/stringop/stringop-sapi.sapi.h" #include "sandboxed_api/examples/stringop/stringop-sapi.sapi.h"
#include "sandboxed_api/examples/stringop/stringop_params.pb.h" #include "sandboxed_api/examples/stringop/stringop_params.pb.h"
#include "sandboxed_api/examples/sum/sum-sapi.sapi.h" #include "sandboxed_api/examples/sum/sum-sapi.sapi.h"
#include "sandboxed_api/sandbox.h"
#include "sandboxed_api/testing.h" #include "sandboxed_api/testing.h"
#include "sandboxed_api/transaction.h" #include "sandboxed_api/transaction.h"
#include "sandboxed_api/util/status_matchers.h" #include "sandboxed_api/util/status_matchers.h"
@ -35,7 +34,6 @@ namespace {
using ::sapi::IsOk; using ::sapi::IsOk;
using ::sapi::StatusIs; using ::sapi::StatusIs;
using ::testing::Eq; using ::testing::Eq;
using ::testing::Gt;
using ::testing::HasSubstr; using ::testing::HasSubstr;
// Functions that will be used during the benchmarks: // Functions that will be used during the benchmarks:
@ -193,23 +191,22 @@ TEST(SandboxTest, RestartSandboxFD) {
TEST(SandboxTest, RestartTransactionSandboxFD) { TEST(SandboxTest, RestartTransactionSandboxFD) {
sapi::BasicTransaction st{std::make_unique<SumSandbox>()}; sapi::BasicTransaction st{std::make_unique<SumSandbox>()};
int fd_no = -1; EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status {
ASSERT_THAT(st.Run([&fd_no](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3));
fd_no = LeakFileDescriptor(sandbox, "/proc/self/exe");
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());
EXPECT_THAT(st.Run([fd_no](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Gt(fd_no)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(4));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());
EXPECT_THAT(st.Restart(), IsOk()); EXPECT_THAT(st.Restart(), IsOk());
EXPECT_THAT(st.Run([fd_no](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(fd_no)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());