UnotifyMonitor: allow only initial execveat

PiperOrigin-RevId: 734107977
Change-Id: I492002953c23021155e8b9596c828fb77888a42a

Author: happyCoder92

sandboxed_api/sandbox2/BUILD

@@ -203,6 +203,7 @@ cc_library(
         "@com_google_sandboxed_api//sandboxed_api:config",
         "@com_google_sandboxed_api//sandboxed_api/sandbox2/network_proxy:filtering",
         "@com_google_sandboxed_api//sandboxed_api/sandbox2/util:bpf_helper",
+        "@com_google_sandboxed_api//sandboxed_api/sandbox2/util:seccomp_unotify",
     ],
 )
 
@@ -469,6 +470,7 @@ cc_library(
         ":notify",
         ":policy",
         ":result",
+        ":util",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/cleanup",
         "@com_google_absl//absl/log",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -170,6 +170,7 @@ target_link_libraries(sandbox2_policy
          sandbox2::bpf_helper
          sandbox2::bpfdisassembler
          sandbox2::regs
+         sandbox2::seccomp_unotify
          sandbox2::syscall
          sapi::base
          sapi::config
@@ -488,6 +489,7 @@ target_link_libraries(sandbox2_monitor_unotify
           sandbox2::client
           sandbox2::forkserver_proto
           sandbox2::seccomp_unotify
+          sandbox2::util
           sapi::status
   PUBLIC sandbox2::executor
          sandbox2::monitor_base

sandboxed_api/sandbox2/executor.h

@@ -37,6 +37,11 @@
 
 namespace sandbox2 {
 
+// Forward declarations for friend declarations.
+class MonitorBase;
+class PtraceMonitor;
+class StackTracePeer;
+
 // The sandbox2::Executor class is responsible for both creating and executing
 // new processes which will be sandboxed.
 class Executor final {

sandboxed_api/sandbox2/monitor_base.cc

@@ -134,6 +134,7 @@ MonitorBase::MonitorBase(Executor* executor, Policy* policy, Notify* notify)
       comms_(executor_->ipc()->comms()),
       ipc_(executor_->ipc()),
       uses_custom_forkserver_(executor_->fork_client_ != nullptr) {
+  wait_for_execveat_ = executor->enable_sandboxing_pre_execve_;
   // It's a pre-connected Comms channel, no need to accept new connection.
   CHECK(comms_->IsConnected());
   std::string path =

sandboxed_api/sandbox2/monitor_base.h

@@ -80,6 +80,11 @@ class MonitorBase {
   virtual void SetWallTimeLimit(absl::Duration limit) = 0;
 
  protected:
+  bool wait_for_execveat() const { return wait_for_execveat_; }
+  void set_wait_for_execveat(bool wait_for_execve) {
+    wait_for_execveat_ = wait_for_execve;
+  }
+
   void OnDone();
   // Sets basic info status and reason code in the result object.
   void SetExitStatusCode(Result::StatusEnum final_status,
@@ -168,6 +173,9 @@ class MonitorBase {
 
   // Is the sandboxee forked from a custom forkserver?
   bool uses_custom_forkserver_;
+
+  // Are we waiting for the first execveat syscall?
+  bool wait_for_execveat_ = false;
 };
 
 }  // namespace sandbox2

sandboxed_api/sandbox2/monitor_ptrace.cc

@@ -138,8 +138,7 @@ void CompleteSyscall(pid_t pid, int signo) {
 }  // namespace
 
 PtraceMonitor::PtraceMonitor(Executor* executor, Policy* policy, Notify* notify)
-    : MonitorBase(executor, policy, notify),
-      wait_for_execve_(executor->enable_sandboxing_pre_execve_) {
+    : MonitorBase(executor, policy, notify) {
   if (executor_->limits()->wall_time_limit() != absl::ZeroDuration()) {
     auto deadline = absl::Now() + executor_->limits()->wall_time_limit();
     deadline_millis_.store(absl::ToUnixMillis(deadline),
@@ -151,13 +150,6 @@ PtraceMonitor::PtraceMonitor(Executor* executor, Policy* policy, Notify* notify)
       absl::GetFlag(FLAGS_sandbox2_monitor_ptrace_use_deadline_manager);
 }
 
-bool PtraceMonitor::IsActivelyMonitoring() {
-  // If we're still waiting for execve(), then we allow all syscalls.
-  return !wait_for_execve_;
-}
-
-void PtraceMonitor::SetActivelyMonitoring() { wait_for_execve_ = false; }
-
 void PtraceMonitor::SetAdditionalResultInfo(std::unique_ptr<Regs> regs) {
   pid_t pid = regs->pid();
   result_.SetRegs(std::move(regs));
@@ -346,7 +338,7 @@ void PtraceMonitor::Run() {
       // all remaining processes (if there are any) because of the
       // PTRACE_O_EXITKILL ptrace() flag.
       if (ret == process_.main_pid) {
-        if (IsActivelyMonitoring()) {
+        if (!wait_for_execveat()) {
           SetExitStatusCode(Result::OK, WEXITSTATUS(status));
         } else {
           SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_MONITOR);
@@ -632,7 +624,7 @@ bool PtraceMonitor::InitPtraceAttach() {
 
 void PtraceMonitor::ActionProcessSyscall(Regs* regs, const Syscall& syscall) {
   // If the sandboxing is not enabled yet, allow the first __NR_execveat.
-  if (syscall.nr() == __NR_execveat && !IsActivelyMonitoring()) {
+  if (syscall.nr() == __NR_execveat && wait_for_execveat()) {
     VLOG(1) << "[PERMITTED/BEFORE_EXECVEAT]: " << "SYSCALL ::: PID: "
             << regs->pid() << ", PROG: '" << util::GetProgName(regs->pid())
             << "' : " << syscall.GetDescription();
@@ -783,10 +775,10 @@ void PtraceMonitor::EventPtraceNewProcess(pid_t pid, int event_msg) {
 }
 
 void PtraceMonitor::EventPtraceExec(pid_t pid, int event_msg) {
-  if (!IsActivelyMonitoring()) {
+  if (wait_for_execveat()) {
     VLOG(1) << "PTRACE_EVENT_EXEC seen from PID: " << event_msg
             << ". SANDBOX ENABLED!";
-    SetActivelyMonitoring();
+    set_wait_for_execveat(false);
   } 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

sandboxed_api/sandbox2/monitor_ptrace.h

@@ -155,8 +155,6 @@ class PtraceMonitor : public MonitorBase {
   bool timed_out_ = false;
   // Should we dump the main sandboxed PID's stack?
   bool should_dump_stack_ = false;
-  // Is the sandboxee actively monitored, or maybe we're waiting for execve()?
-  bool wait_for_execve_;
   // Syscalls that are running, whose result values we want to inspect.
   absl::flat_hash_map<pid_t, Syscall> syscalls_in_progress_;
   sigset_t sset_;

sandboxed_api/sandbox2/monitor_unotify.cc

@@ -40,6 +40,7 @@
 #include "sandboxed_api/sandbox2/notify.h"
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/result.h"
+#include "sandboxed_api/sandbox2/util.h"
 #include "sandboxed_api/sandbox2/util/seccomp_unotify.h"
 #include "sandboxed_api/util/fileops.h"
 #include "sandboxed_api/util/status_macros.h"
@@ -189,13 +190,23 @@ void UnotifyMonitor::HandleUnotify() {
       return;
     }
   }
+  Syscall syscall(*req_data);
+  if (wait_for_execveat() && syscall.nr() == __NR_execveat &&
+      util::SeccompUnotify::IsContinueSupported()) {
+    VLOG(1) << "[PERMITTED/BEFORE_EXECVEAT]: " << "SYSCALL ::: PID: "
+            << syscall.pid() << ", PROG: '" << util::GetProgName(syscall.pid())
+            << "' : " << syscall.GetDescription();
+    set_wait_for_execveat(false);
+    AllowSyscallViaUnotify(*req_data);
+    return;
+  }
   absl::StatusOr<uint32_t> policy_ret =
       bpf::Evaluate(original_policy_, req_data->data);
   if (!policy_ret.ok()) {
     LOG(ERROR) << "Failed to evaluate policy: " << policy_ret.status();
     SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_NOTIFY);
   }
-  Syscall syscall(*req_data);
+
   const sock_filter trace_action = SANDBOX2_TRACE;
   bool should_trace = *policy_ret == trace_action.k;
   Notify::TraceAction trace_response = Notify::TraceAction::kDeny;

sandboxed_api/sandbox2/policy.cc

@@ -40,6 +40,7 @@
 #include "sandboxed_api/sandbox2/syscall.h"
 #include "sandboxed_api/sandbox2/util.h"
 #include "sandboxed_api/sandbox2/util/bpf_helper.h"
+#include "sandboxed_api/sandbox2/util/seccomp_unotify.h"
 
 #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER
 #define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3)
@@ -112,6 +113,10 @@ std::vector<sock_filter> Policy::GetDefaultPolicy(
 
   std::vector<sock_filter> policy;
   if (user_notif) {
+    sock_filter execve_action = ALLOW;
+    if (util::SeccompUnotify::IsContinueSupported()) {
+      execve_action = BPF_STMT(BPF_RET + BPF_K, SECCOMP_RET_USER_NOTIF);
+    }
     policy = {
         // If compiled arch is different from the runtime one, inform the
         // Monitor.
@@ -134,7 +139,7 @@ std::vector<sock_filter> Policy::GetDefaultPolicy(
               JNE32(AT_EMPTY_PATH, JUMP(&l, past_execveat_l)),
               ARG_32(5),
               JNE32(internal::kExecveMagic, JUMP(&l, past_execveat_l)),
-              ALLOW,
+              execve_action,
               LABEL(&l, past_execveat_l),
               LOAD_SYSCALL_NR,
           });

sandboxed_api/sandbox2/policy_test.cc

@@ -381,11 +381,11 @@ TEST_P(PolicyTest, DetectSandboxSyscall) {
   EXPECT_THAT(result.reason_code(), Eq(0));
 }
 
-TEST_P(PolicyTest, ExecveatAllowedByDefault) {
+TEST_P(PolicyTest, ExecveatNotAllowedByDefault) {
   const std::string path = GetTestSourcePath("sandbox2/testcases/execveat");
 
   std::unique_ptr<Sandbox2> s2 = CreateTestSandbox(
-      {path},
+      {path, "1"},
       CreateDefaultPermissiveTestPolicy(path).BlockSyscallWithErrno(
           __NR_execveat, EPERM),
       /*sandbox_pre_execve=*/false);
@@ -396,6 +396,20 @@ TEST_P(PolicyTest, ExecveatAllowedByDefault) {
   EXPECT_THAT(result.reason_code(), Eq(0));
 }
 
+TEST_P(PolicyTest, SecondExecveatNotAllowedByDefault) {
+  const std::string path = GetTestSourcePath("sandbox2/testcases/execveat");
+
+  std::unique_ptr<Sandbox2> s2 = CreateTestSandbox(
+      {path, "2"},
+      CreateDefaultPermissiveTestPolicy(path).BlockSyscallWithErrno(
+          __NR_execveat, EPERM));
+  Result result = s2->Run();
+
+  // The test binary should exit with success.
+  ASSERT_THAT(result.final_status(), Eq(Result::OK));
+  EXPECT_THAT(result.reason_code(), Eq(0));
+}
+
 INSTANTIATE_TEST_SUITE_P(Sandbox2, PolicyTest, ::testing::Values(false, true),
                          [](const ::testing::TestParamInfo<bool>& info) {
                            return info.param ? "UnotifyMonitor"

sandboxed_api/sandbox2/testcases/execveat.cc

@@ -24,9 +24,12 @@
 #include "sandboxed_api/sandbox2/util.h"
 
 int main(int argc, char** argv) {
-  sandbox2::Comms comms(sandbox2::Comms::kSandbox2ClientCommsFD);
-  sandbox2::Client client(&comms);
-  client.SandboxMeHere();
+  int testno = atoi(argv[1]);  // NOLINT
+  if (testno == 1) {
+    sandbox2::Comms comms(sandbox2::Comms::kSandbox2ClientCommsFD);
+    sandbox2::Client client(&comms);
+    client.SandboxMeHere();
+  }
   int result =
       sandbox2::util::Syscall(__NR_execveat, AT_EMPTY_PATH, 0, 0, 0, 0);
   if (result != -1 || errno != EPERM) {