diff --git a/sandboxed_api/sandbox2/monitor.cc b/sandboxed_api/sandbox2/monitor.cc index 26528dc..942e7c2 100644 --- a/sandboxed_api/sandbox2/monitor.cc +++ b/sandboxed_api/sandbox2/monitor.cc @@ -360,6 +360,8 @@ bool Monitor::ShouldCollectStackTrace() { return policy_->collect_stacktrace_on_signal_; case Result::VIOLATION: return policy_->collect_stacktrace_on_violation_; + case Result::OK: + return policy_->collect_stacktrace_on_exit_; default: return false; } @@ -861,7 +863,9 @@ void Monitor::EventPtraceExec(pid_t pid, int event_msg) { void Monitor::EventPtraceExit(pid_t pid, int event_msg) { // A regular exit, let it continue (fast-path). - if (WIFEXITED(event_msg)) { + if (ABSL_PREDICT_TRUE( + WIFEXITED(event_msg) && + (!policy_->collect_stacktrace_on_exit_ || pid != pid_))) { ContinueProcess(pid, 0); return; } @@ -883,10 +887,11 @@ void Monitor::EventPtraceExit(pid_t pid, int event_msg) { return; } - // This can be reached in three cases: + // This can be reached in four cases: // 1) Process was killed from the sandbox. // 2) Process was killed because it hit a timeout. // 3) Regular signal/other exit cause. + // 4) Normal exit for which we want to obtain stack trace. if (pid == pid_) { VLOG(1) << "PID: " << pid << " main special exit"; if (network_violation_) { @@ -896,6 +901,8 @@ void Monitor::EventPtraceExit(pid_t pid, int event_msg) { SetExitStatusCode(Result::EXTERNAL_KILL, 0); } else if (timed_out_) { SetExitStatusCode(Result::TIMEOUT, 0); + } else if (WIFEXITED(event_msg)) { + SetExitStatusCode(Result::OK, WEXITSTATUS(event_msg)); } else { SetExitStatusCode(Result::SIGNALED, WTERMSIG(event_msg)); } diff --git a/sandboxed_api/sandbox2/policy.h b/sandboxed_api/sandbox2/policy.h index edd5cb8..1ac7a96 100644 --- a/sandboxed_api/sandbox2/policy.h +++ b/sandboxed_api/sandbox2/policy.h @@ -96,6 +96,7 @@ class Policy final { bool collect_stacktrace_on_signal_ = true; bool collect_stacktrace_on_timeout_ = true; bool collect_stacktrace_on_kill_ = true; + bool collect_stacktrace_on_exit_ = false; // The capabilities to keep in the sandboxee. std::vector capabilities_; diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index 54df4d0..5b9c9e1 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -807,6 +807,7 @@ absl::StatusOr> PolicyBuilder::TryBuild() { output->collect_stacktrace_on_violation_ = collect_stacktrace_on_violation_; output->collect_stacktrace_on_timeout_ = collect_stacktrace_on_timeout_; output->collect_stacktrace_on_kill_ = collect_stacktrace_on_kill_; + output->collect_stacktrace_on_exit_ = collect_stacktrace_on_exit_; output->user_policy_ = std::move(user_policy_); output->user_policy_handles_bpf_ = user_policy_handles_bpf_; @@ -959,6 +960,11 @@ PolicyBuilder& PolicyBuilder::CollectStacktracesOnKill(bool enable) { return *this; } +PolicyBuilder& PolicyBuilder::CollectStacktracesOnExit(bool enable) { + collect_stacktrace_on_exit_ = enable; + return *this; +} + PolicyBuilder& PolicyBuilder::AddNetworkProxyPolicy() { if (allowed_hosts_) { SetError(absl::FailedPreconditionError( diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index c4aa2ab..db2fea9 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/sandboxed_api/sandbox2/policybuilder.h @@ -540,6 +540,9 @@ class PolicyBuilder final { // monitor / the user. PolicyBuilder& CollectStacktracesOnKill(bool enable); + // Enables/disables stack trace collection on normal process exit. + PolicyBuilder& CollectStacktracesOnExit(bool enable); + // Appends an unconditional ALLOW action for all syscalls. // Do not use in environment with untrusted code and/or data, ask // sandbox-team@ first if unsure. @@ -590,6 +593,7 @@ class PolicyBuilder final { bool collect_stacktrace_on_signal_ = true; bool collect_stacktrace_on_timeout_ = true; bool collect_stacktrace_on_kill_ = false; + bool collect_stacktrace_on_exit_ = false; // Seccomp fields std::vector user_policy_; diff --git a/sandboxed_api/sandbox2/sandbox2_test.cc b/sandboxed_api/sandbox2/sandbox2_test.cc index 7601be8..b74545f 100644 --- a/sandboxed_api/sandbox2/sandbox2_test.cc +++ b/sandboxed_api/sandbox2/sandbox2_test.cc @@ -118,6 +118,26 @@ TEST(ExecutorTest, ExecutorFdConstructor) { ASSERT_EQ(result.final_status(), Result::OK); } +TEST(StackTraceTest, StackTraceOnExitWorks) { + SKIP_SANITIZERS_AND_COVERAGE; + + const std::string path = GetTestSourcePath("sandbox2/testcases/minimal"); + std::vector args = {path}; + auto executor = absl::make_unique(path, args); + + SAPI_ASSERT_OK_AND_ASSIGN(auto policy, + PolicyBuilder() + // Don't restrict the syscalls at all. + .DangerDefaultAllowAll() + .CollectStacktracesOnExit(true) + .TryBuild()); + Sandbox2 sandbox(std::move(executor), std::move(policy)); + auto result = sandbox.Run(); + + ASSERT_EQ(result.final_status(), Result::OK); + ASSERT_THAT(result.stack_trace(), Not(IsEmpty())); +} + // Tests that we return the correct state when the sandboxee was killed by an // external signal. Also make sure that we do not have the stack trace. TEST(RunAsyncTest, SandboxeeExternalKill) {