diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 072ce02..e26235b 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -46,7 +46,6 @@ cc_library( ":violation_cc_proto", "//sandboxed_api:config", "//sandboxed_api/util:status", - "//sandboxed_api/util:strerror", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", @@ -197,6 +196,10 @@ cc_library( ":comms", ":result", ":syscall", + ":util", + "//sandboxed_api:config", + "@com_google_absl//absl/base:core_headers", + "@com_google_glog//:glog", ], ) @@ -347,7 +350,6 @@ cc_library( "//sandboxed_api/util:file_helpers", "//sandboxed_api/util:fileops", "//sandboxed_api/util:flags", - "//sandboxed_api/util:raw_logging", "//sandboxed_api/util:status", "//sandboxed_api/util:strerror", "//sandboxed_api/util:temp_file", diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index 2867512..8b2ea11 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -170,11 +170,15 @@ add_library(sandbox2_notify ${SAPI_LIB_TYPE} notify.h ) add_library(sandbox2::notify ALIAS sandbox2_notify) -target_link_libraries(sandbox2_notify PRIVATE - sandbox2::comms - sandbox2::result - sandbox2::syscall - sapi::base +target_link_libraries(sandbox2_notify + PUBLIC absl::core_headers + glog::glog + sandbox2::comms + sandbox2::result + sandbox2::syscall + sandbox2::util + PRIVATE sapi::base + sapi::config ) # sandboxed_api/sandbox2:limits @@ -296,7 +300,6 @@ add_library(sandbox2::sandbox2 ALIAS sandbox2_sandbox2) target_link_libraries(sandbox2_sandbox2 PRIVATE absl::core_headers absl::cleanup - absl::flat_hash_map absl::flat_hash_set absl::memory absl::optional @@ -305,7 +308,8 @@ target_link_libraries(sandbox2_sandbox2 absl::strings sapi::strerror sapi::base - PUBLIC absl::status + PUBLIC absl::flat_hash_map + absl::status absl::statusor absl::synchronization absl::time diff --git a/sandboxed_api/sandbox2/monitor.cc b/sandboxed_api/sandbox2/monitor.cc index efb4342..1c16c8f 100644 --- a/sandboxed_api/sandbox2/monitor.cc +++ b/sandboxed_api/sandbox2/monitor.cc @@ -127,6 +127,18 @@ void StopProcess(pid_t pid, int signo) { } } +void CompleteSyscall(pid_t pid, int signo) { + if (ptrace(PTRACE_SYSCALL, pid, 0, signo) == -1) { + if (errno == ESRCH) { + LOG(WARNING) << "Process " << pid + << " died while trying to PTRACE_SYSCALL it"; + } else { + PLOG(ERROR) << "ptrace(PTRACE_SYSCALL, pid=" << pid << ", sig=" << signo + << ")"; + } + } +} + void MaybeEnableTomoyoLsmWorkaround(Mounts& mounts, std::string& comms_fd_dev) { static auto tomoyo_active = []() -> bool { std::string lsm_list; @@ -768,14 +780,20 @@ void Monitor::ActionProcessSyscall(Regs* regs, const Syscall& syscall) { // Notify can decide whether we want to allow this syscall. It could be useful // for sandbox setups in which some syscalls might still need some logging, // but nonetheless be allowed ('permissible syscalls' in sandbox v1). - if (notify_->EventSyscallTrap(syscall)) { - LOG(WARNING) << "[PERMITTED]: SYSCALL ::: PID: " << regs->pid() - << ", PROG: '" << util::GetProgName(regs->pid()) - << "' : " << syscall.GetDescription(); - + auto trace_response = notify_->EventSyscallTrace(syscall); + if (trace_response == Notify::TraceAction::kAllow) { ContinueProcess(regs->pid(), 0); return; } + if (trace_response == Notify::TraceAction::kInspectAfterReturn) { + // Note that a process might die without an exit-stop before the syscall is + // completed (eg. a thread calls execve() and the thread group leader dies), + // so this entry might never get removed from the table. This may increase + // the monitor's memory usage by O(number-of-sandboxed-pids). + syscalls_in_progress_[regs->pid()] = syscall; + CompleteSyscall(regs->pid(), 0); + return; + } // TODO(wiktorg): Further clean that up, probably while doing monitor cleanup // log_file_ not null iff FLAGS_sandbox2_danger_danger_permit_all_and_log is @@ -856,11 +874,68 @@ void Monitor::EventPtraceSeccomp(pid_t pid, int event_msg) { ActionProcessSyscall(®s, syscall); } +void Monitor::EventSyscallExit(pid_t pid) { + // Check that the monitor wants to inspect the current syscall's return value. + auto index = syscalls_in_progress_.find(pid); + if (index == syscalls_in_progress_.end()) { + LOG(ERROR) << "Expected a syscall in progress in PID " << pid; + SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_INSPECT); + return; + } + Regs regs(pid); + auto status = regs.Fetch(); + if (!status.ok()) { + LOG(ERROR) << status; + SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_FETCH); + return; + } + int64_t return_value = regs.GetReturnValue(sapi::host_cpu::Architecture()); + notify_->EventSyscallReturn(index->second, return_value); + syscalls_in_progress_.erase(index); + ContinueProcess(pid, 0); +} + +void Monitor::EventPtraceNewProcess(pid_t pid, int event_msg) { + // ptrace doesn't issue syscall-exit-stops for successful fork/vfork/clone + // system calls. Check if the monitor wanted to inspect the syscall's return + // value, and call EventSyscallReturn for the parent process if so. + auto index = syscalls_in_progress_.find(pid); + if (index != syscalls_in_progress_.end()) { + auto syscall_nr = index->second.nr(); + if (syscall_nr != __NR_fork && syscall_nr != __NR_vfork && + syscall_nr != __NR_clone) { + LOG(ERROR) << "Expected a fork/vfork/clone syscall in progress in PID " + << pid << "; actual: " << index->second.GetDescription(); + SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_INSPECT); + return; + } + notify_->EventSyscallReturn(index->second, event_msg); + syscalls_in_progress_.erase(index); + } + ContinueProcess(pid, 0); +} + void Monitor::EventPtraceExec(pid_t pid, int event_msg) { if (!IsActivelyMonitoring()) { VLOG(1) << "PTRACE_EVENT_EXEC seen from PID: " << event_msg << ". SANDBOX ENABLED!"; SetActivelyMonitoring(); + } else { + // ptrace doesn't issue syscall-exit-stops for successful execve/execveat + // system calls. Check if the monitor wanted to inspect the syscall's return + // value, and call EventSyscallReturn if so. + auto index = syscalls_in_progress_.find(pid); + if (index != syscalls_in_progress_.end()) { + auto syscall_nr = index->second.nr(); + if (syscall_nr != __NR_execve && syscall_nr != __NR_execveat) { + LOG(ERROR) << "Expected an execve/execveat syscall in progress in PID " + << pid << "; actual: " << index->second.GetDescription(); + SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_INSPECT); + return; + } + notify_->EventSyscallReturn(index->second, 0); + syscalls_in_progress_.erase(index); + } } ContinueProcess(pid, 0); } @@ -932,7 +1007,10 @@ void Monitor::EventPtraceStop(pid_t pid, int stopsig) { void Monitor::StateProcessStopped(pid_t pid, int status) { int stopsig = WSTOPSIG(status); - if (__WPTRACEEVENT(status) == 0) { + // We use PTRACE_O_TRACESYSGOOD, so we can tell it's a syscall stop without + // calling PTRACE_GETSIGINFO by checking the value of the reported signal. + bool is_syscall_exit = stopsig == (SIGTRAP | 0x80); + if (__WPTRACEEVENT(status) == 0 && !is_syscall_exit) { // Must be a regular signal delivery. VLOG(2) << "PID: " << pid << " received signal: " << util::GetSignalName(stopsig); @@ -980,13 +1058,25 @@ void Monitor::StateProcessStopped(pid_t pid, int status) { #define PTRACE_EVENT_STOP 128 #endif + if (is_syscall_exit) { + VLOG(2) << "PID: " << pid << " syscall-exit-stop: " << event_msg; + EventSyscallExit(pid); + return; + } + switch (__WPTRACEEVENT(status)) { case PTRACE_EVENT_FORK: - /* fall through */ + VLOG(2) << "PID: " << pid << " PTRACE_EVENT_FORK, PID: " << event_msg; + EventPtraceNewProcess(pid, event_msg); + break; case PTRACE_EVENT_VFORK: - /* fall through */ + VLOG(2) << "PID: " << pid << " PTRACE_EVENT_VFORK, PID: " << event_msg; + EventPtraceNewProcess(pid, event_msg); + break; case PTRACE_EVENT_CLONE: - /* fall through */ + VLOG(2) << "PID: " << pid << " PTRACE_EVENT_CLONE, PID: " << event_msg; + EventPtraceNewProcess(pid, event_msg); + break; case PTRACE_EVENT_VFORK_DONE: ContinueProcess(pid, 0); break; diff --git a/sandboxed_api/sandbox2/monitor.h b/sandboxed_api/sandbox2/monitor.h index a985590..113a32f 100644 --- a/sandboxed_api/sandbox2/monitor.h +++ b/sandboxed_api/sandbox2/monitor.h @@ -28,6 +28,7 @@ #include #include +#include "absl/container/flat_hash_map.h" #include "absl/synchronization/notification.h" #include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/executor.h" @@ -135,12 +136,18 @@ class Monitor final { // Processes exit path. void EventPtraceExit(pid_t pid, int event_msg); - // Processes excution path. + // Processes fork/vfork/clone path. + void EventPtraceNewProcess(pid_t pid, int event_msg); + + // Processes execution path. void EventPtraceExec(pid_t pid, int event_msg); // Processes stop path. void EventPtraceStop(pid_t pid, int stopsig); + // Processes syscall exit. + void EventSyscallExit(pid_t pid); + // Enable network proxy server, this will start a thread in the sandbox // that waits for connection requests from the sandboxee. void EnableNetworkProxyServer(); @@ -196,6 +203,9 @@ class Monitor final { std::unique_ptr network_proxy_server_; std::thread network_proxy_thread_; + + // Syscalls that are running, whose result values we want to inspect. + absl::flat_hash_map syscalls_in_progress_; }; } // namespace sandbox2 diff --git a/sandboxed_api/sandbox2/notify.h b/sandboxed_api/sandbox2/notify.h index b35c01f..302a0a1 100644 --- a/sandboxed_api/sandbox2/notify.h +++ b/sandboxed_api/sandbox2/notify.h @@ -19,9 +19,13 @@ #include +#include +#include "absl/base/attributes.h" +#include "sandboxed_api/config.h" #include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/result.h" #include "sandboxed_api/sandbox2/syscall.h" +#include "sandboxed_api/sandbox2/util.h" namespace sandbox2 { @@ -50,11 +54,41 @@ class Notify { virtual void EventSyscallViolation(const Syscall& syscall, ViolationType type) {} - // Called when a policy called TRACE. The syscall is allowed if this method - // returns true. - // This allows for implementing 'log, but allow' policies. + // Called when a policy called TRACE. The syscall is allowed and logged if + // this method returns true. This allows for implementing 'log, but allow' + // policies. + ABSL_DEPRECATED("Override EventSyscallTrace() instead") virtual bool EventSyscallTrap(const Syscall& syscall) { return false; } + // Actions to perform after calling EventSyscallTrace. + enum class TraceAction { + // Deny the syscall. + kDeny, + // Allow the syscall. + kAllow, + // Allow the syscall so its return value can be inspected through a + // subsequent call to EventSyscallReturn. + // Requires Linux kernel 4.8 or later. + kInspectAfterReturn + }; + + // Called when a policy called TRACE. The syscall is allowed or denied + // depending on the return value of this function. + virtual TraceAction EventSyscallTrace(const Syscall& syscall) { + if (EventSyscallTrap(syscall)) { + LOG(WARNING) << "[PERMITTED]: SYSCALL ::: PID: " << syscall.pid() + << ", PROG: '" << util::GetProgName(syscall.pid()) + << "' : " << syscall.GetDescription(); + return TraceAction::kAllow; + } + return TraceAction::kDeny; + } + + // Called when a policy called TRACE and EventSyscallTrace returned + // kInspectAfterReturn. + virtual void EventSyscallReturn(const Syscall& syscall, + int64_t return_value) {} + // Called when a process received a signal. virtual void EventSignal(pid_t pid, int sig_no) {} }; diff --git a/sandboxed_api/sandbox2/regs.cc b/sandboxed_api/sandbox2/regs.cc index 338772f..106a815 100644 --- a/sandboxed_api/sandbox2/regs.cc +++ b/sandboxed_api/sandbox2/regs.cc @@ -187,6 +187,30 @@ Syscall Regs::ToSyscall(sapi::cpu::Architecture syscall_arch) const { return Syscall(pid_); } +int64_t Regs::GetReturnValue(sapi::cpu::Architecture syscall_arch) const { +#if defined(SAPI_X86_64) + if (ABSL_PREDICT_TRUE(syscall_arch == sapi::cpu::kX8664)) { + return static_cast(user_regs_.rax); + } + if (syscall_arch == sapi::cpu::kX86) { + return static_cast(user_regs_.rax & 0xFFFFFFFF); + } +#elif defined(SAPI_PPC64_LE) + if (ABSL_PREDICT_TRUE(syscall_arch == sapi::cpu::kPPC64LE)) { + return static_cast(user_regs_.gpr[3]); + } +#elif defined(SAPI_ARM64) + if (ABSL_PREDICT_TRUE(syscall_arch == sapi::cpu::kArm64)) { + return static_cast(user_regs_.regs[0]); + } +#elif defined(SAPI_ARM) + if (ABSL_PREDICT_TRUE(syscall_arch == sapi::cpu::kArm)) { + return static_cast(user_regs_.regs[0]); + } +#endif + return -1; +} + void Regs::StoreRegisterValuesInProtobuf(RegisterValues* values) const { #if defined(SAPI_X86_64) RegisterX8664* regs = values->mutable_register_x86_64(); diff --git a/sandboxed_api/sandbox2/regs.h b/sandboxed_api/sandbox2/regs.h index 57a6747..a9dbd90 100644 --- a/sandboxed_api/sandbox2/regs.h +++ b/sandboxed_api/sandbox2/regs.h @@ -48,6 +48,9 @@ class Regs { // Converts raw register values obtained on syscall entry to syscall info Syscall ToSyscall(sapi::cpu::Architecture syscall_arch) const; + // Returns the content of the register that holds a syscall's return value + int64_t GetReturnValue(sapi::cpu::Architecture syscall_arch) const; + pid_t pid() const { return pid_; } // Stores register values in a protobuf structure. diff --git a/sandboxed_api/sandbox2/result.cc b/sandboxed_api/sandbox2/result.cc index 7aba1c7..255e491 100644 --- a/sandboxed_api/sandbox2/result.cc +++ b/sandboxed_api/sandbox2/result.cc @@ -189,6 +189,8 @@ std::string Result::ReasonCodeEnumToString(ReasonCodeEnum value) { return "FAILED_KILL"; case sandbox2::Result::FAILED_CHILD: return "FAILED_CHILD"; + case sandbox2::Result::FAILED_INSPECT: + return "FAILED_INSPECT"; case sandbox2::Result::VIOLATION_SYSCALL: return "VIOLATION_SYSCALL"; case sandbox2::Result::VIOLATION_ARCH: diff --git a/sandboxed_api/sandbox2/result.h b/sandboxed_api/sandbox2/result.h index cad5216..3a425a0 100644 --- a/sandboxed_api/sandbox2/result.h +++ b/sandboxed_api/sandbox2/result.h @@ -80,6 +80,7 @@ class Result { FAILED_MONITOR, FAILED_KILL, FAILED_CHILD, + FAILED_INSPECT, // TODO(wiktorg) not used currently (syscall number stored insted) - need to // fix clients first