StarMirror/sandboxed-api
1
0
Fork 0
mirror of https://github.com/google/sandboxed-api.git synced 2024-03-22 13:11:30 +08:00
Code Issues Projects Releases Wiki Activity

Always override forkservers comms_fd in sandboxee

Browse Source 
PiperOrigin-RevId: 561276110
Change-Id: I8bd1ce7e2f363b5e371a431b1e6db6534023e401
This commit is contained in:
Wiktor Garbacz 2023-08-30 02:20:25 -07:00 committed by Copybara-Service
parent 0150026d38
commit 4a6b0d4633
5 changed files with 80 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
sandboxed_api/examples/sum/main_sum.cc
View File

@ -156,13 +156,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() == 3, "remote_fd != 3"); TRANSACTION_FAIL_IF_NOT(fd.GetRemoteFd() != -1, "remote_fd == -1");
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() == 4, "remote_fd2 != 4"); TRANSACTION_FAIL_IF_NOT(fd2.GetRemoteFd() != -1, "remote_fd2 == -1");
// Read from fd test. // Read from fd test.
char buffer[1024] = {0}; char buffer[1024] = {0};

2
sandboxed_api/sandbox2/CMakeLists.txt
View File

@ -579,7 +579,6 @@ 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
@ -592,6 +591,7 @@ 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

112
sandboxed_api/sandbox2/forkserver.cc
View File

@ -37,9 +37,10 @@
#include <fstream> #include <fstream>
#include <initializer_list> #include <initializer_list>
#include <string> #include <string>
#include <utility>
#include <vector> #include <vector>
#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,12 +66,10 @@
#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
@ -115,18 +114,11 @@ void MoveFDs(std::initializer_list<std::pair<int*, int>> move_fds,
} }
} }
void RunInitProcess(pid_t main_pid, int pipe_fd, ABSL_ATTRIBUTE_NORETURN void RunInitProcess(pid_t main_pid, FDCloser 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;
@ -144,7 +136,7 @@ void RunInitProcess(pid_t main_pid, int pipe_fd,
SYSCALL(__NR_waitid, ALLOW), SYSCALL(__NR_waitid, ALLOW),
SYSCALL(__NR_exit, ALLOW), SYSCALL(__NR_exit, ALLOW),
}; };
if (pipe_fd >= 0) { if (pipe_fd.get() >= 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)});
} }
@ -171,13 +163,13 @@ void RunInitProcess(pid_t main_pid, int pipe_fd,
} }
if (info.si_pid == main_pid) { if (info.si_pid == main_pid) {
if (pipe_fd >= 0) { if (pipe_fd.get() >= 0) {
write(pipe_fd, &info.si_code, sizeof(info.si_code)); write(pipe_fd.get(), &info.si_code, sizeof(info.si_code));
write(pipe_fd, &info.si_status, sizeof(info.si_status)); write(pipe_fd.get(), &info.si_status, sizeof(info.si_status));
rusage usage{}; rusage usage{};
getrusage(RUSAGE_CHILDREN, &usage); getrusage(RUSAGE_CHILDREN, &usage);
write(pipe_fd, &usage, sizeof(usage)); write(pipe_fd.get(), &usage, sizeof(usage));
} }
_exit(0); _exit(0);
} }
@ -283,9 +275,8 @@ void ForkServer::PrepareExecveArgs(const ForkRequest& request,
} }
void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd, void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
int client_fd, uid_t uid, gid_t gid, uid_t uid, gid_t gid, FDCloser signaling_fd,
int signaling_fd, int status_fd, FDCloser status_fd, bool avoid_pivot_root) const {
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");
@ -299,10 +290,6 @@ 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();
@ -326,19 +313,23 @@ 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 >= 0) { if (status_fd.get() >= 0) {
open_fds->erase(status_fd); open_fds->erase(status_fd.get());
} }
RunInitProcess(child, status_fd, *open_fds); // Close all open fds (equals to CloseAllFDsExcept but does not require
// /proc to be available).
for (const auto& fd : *open_fds) {
close(fd);
} }
if (status_fd >= 0) { RunInitProcess(child, std::move(status_fd));
close(status_fd);
} }
// Send sandboxee pid // Send sandboxee pid
auto status = SendPid(signaling_fd); auto status = SendPid(signaling_fd.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());
} }
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
@ -350,17 +341,16 @@ 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.
Comms client_comms(Comms::kDefaultConnection); Client c(comms_);
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 (client_comms.GetConnectionFD() != Comms::kSandbox2ClientCommsFD) { if (comms_->GetConnectionFD() != Comms::kSandbox2ClientCommsFD) {
envs.push_back(absl::StrCat(Comms::kSandbox2CommsFDEnvVar, "=", envs.push_back(absl::StrCat(Comms::kSandbox2CommsFDEnvVar, "=",
client_comms.GetConnectionFD())); 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
@ -407,10 +397,15 @@ 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}; int pfds[2] = {-1, -1};
if (fork_request.monitor_type() == FORKSERVER_MONITOR_UNOTIFY) { if (fork_request.monitor_type() == FORKSERVER_MONITOR_UNOTIFY) {
SAPI_RAW_PCHECK(pipe(pfds) == 0, "creating status pipe"); 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];
SAPI_RAW_PCHECK( SAPI_RAW_PCHECK(
@ -423,8 +418,8 @@ pid_t ForkServer::ServeRequest() {
"setsockopt failed"); "setsockopt failed");
} }
file_util::fileops::FDCloser fd_closer0{socketpair_fds[0]}; FDCloser signaling_fds[] = {FDCloser(socketpair_fds[0]),
file_util::fileops::FDCloser fd_closer1{socketpair_fds[1]}; FDCloser(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.
@ -459,7 +454,7 @@ pid_t ForkServer::ServeRequest() {
_exit(0); _exit(0);
} }
// Send sandboxee pid // Send sandboxee pid
absl::Status status = SendPid(fd_closer1.get()); absl::Status status = SendPid(signaling_fds[1].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());
} }
@ -476,15 +471,29 @@ pid_t ForkServer::ServeRequest() {
// Child. // Child.
if (sandboxee_pid == 0) { if (sandboxee_pid == 0) {
LaunchChild(fork_request, exec_fd, comms_fd, uid, gid, fd_closer1.get(), signaling_fds[0].Close();
pfds[1], avoid_pivot_root); pipe_fds[0].Close();
// 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;
} }
fd_closer1.Close(); signaling_fds[1].Close();
if (avoid_pivot_root) { if (avoid_pivot_root) {
if (auto pid = ReceivePid(fd_closer0.get()); !pid.ok()) { if (auto pid = ReceivePid(signaling_fds[0].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();
@ -498,7 +507,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(fd_closer0.get()); !pid_or.ok()) { if (auto pid_or = ReceivePid(signaling_fds[0].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);
@ -510,9 +519,7 @@ pid_t ForkServer::ServeRequest() {
} }
// Parent. // Parent.
if (pfds[1] >= 0) { pipe_fds[1].Close();
close(pfds[1]);
}
close(comms_fd); close(comms_fd);
if (exec_fd >= 0) { if (exec_fd >= 0) {
close(exec_fd); close(exec_fd);
@ -523,9 +530,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 (pfds[0] >= 0) { if (pipe_fds[0].get() >= 0) {
SAPI_RAW_CHECK(comms_->SendFD(pfds[0]), "Failed to send status pipe"); SAPI_RAW_CHECK(comms_->SendFD(pipe_fds[0].get()),
close(pfds[0]); "Failed to send status pipe");
} }
return sandboxee_pid; return sandboxee_pid;
} }
@ -564,9 +571,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
file_util::fileops::FDCloser create_efd(eventfd(0, EFD_CLOEXEC)); 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");
file_util::fileops::FDCloser open_efd(eventfd(0, EFD_CLOEXEC)); 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()) {
@ -600,12 +607,11 @@ void ForkServer::CreateInitialNamespaces() {
"synchronizing initial namespaces creation"); "synchronizing initial namespaces creation");
} }
void ForkServer::SanitizeEnvironment() { void ForkServer::SanitizeEnvironment() const {
// 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, {STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO, comms_->GetConnectionFD()},
Comms::kSandbox2ClientCommsFD},
/* close_fds = */ false); /* close_fds = */ false);
SAPI_RAW_CHECK( SAPI_RAW_CHECK(
status.ok(), status.ok(),

8
sandboxed_api/sandbox2/forkserver.h
View File

@ -25,6 +25,7 @@
#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 {
@ -53,8 +54,9 @@ class ForkServer {
private: private:
// Creates and launched the child process. // Creates and launched the child process.
void LaunchChild(const ForkRequest& request, int execve_fd, int client_fd, void LaunchChild(const ForkRequest& request, int execve_fd, uid_t uid,
uid_t uid, gid_t gid, int signaling_fd, int status_fd, gid_t gid, sapi::file_util::fileops::FDCloser signaling_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
@ -73,7 +75,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().
static void SanitizeEnvironment(); void SanitizeEnvironment() const;
// 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,

15
sandboxed_api/sapi_test.cc
View File

@ -28,6 +28,7 @@
#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"
@ -38,6 +39,7 @@ 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:
@ -195,22 +197,23 @@ TEST(SandboxTest, RestartSandboxFD) {
TEST(SandboxTest, RestartTransactionSandboxFD) { TEST(SandboxTest, RestartTransactionSandboxFD) {
sapi::BasicTransaction st{std::make_unique<SumSandbox>()}; sapi::BasicTransaction st{std::make_unique<SumSandbox>()};
EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status { int fd_no = -1;
EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3)); ASSERT_THAT(st.Run([&fd_no](sapi::Sandbox* sandbox) -> absl::Status {
fd_no = LeakFileDescriptor(sandbox, "/proc/self/exe");
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());
EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([fd_no](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(4)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Gt(fd_no));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());
EXPECT_THAT(st.Restart(), IsOk()); EXPECT_THAT(st.Restart(), IsOk());
EXPECT_THAT(st.Run([](sapi::Sandbox* sandbox) -> absl::Status { EXPECT_THAT(st.Run([fd_no](sapi::Sandbox* sandbox) -> absl::Status {
EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(3)); EXPECT_THAT(LeakFileDescriptor(sandbox, "/proc/self/exe"), Eq(fd_no));
return absl::OkStatus(); return absl::OkStatus();
}), }),
IsOk()); IsOk());