Support Notify::EventSyscallTrace in unotify monitor

The original policy is now saved before being modified to needs of unotify monitor.
This original policy is evaluated to decide whether a `USER_NOTIF` seccomp action is supposed to be a hard deny or `sandbox2::Notify` should be invoked.

PiperOrigin-RevId: 691728683
Change-Id: I17363d72952d66edcfd65bbdf59aa5e23389d455

sandboxed_api/sandbox2/BUILD

@@ -262,7 +262,6 @@ cc_library(
     copts = sapi_platform_copts(),
     deps = [
         ":bpfdisassembler",
-        ":comms",
         ":namespace",
         ":syscall",
         ":violation_cc_proto",
@@ -530,6 +529,7 @@ cc_library(
     hdrs = ["monitor_unotify.h"],
     copts = sapi_platform_copts(),
     deps = [
+        ":bpf_evaluator",
         ":client",
         ":executor",
         ":forkserver_cc_proto",
@@ -974,9 +974,11 @@ cc_test(
         ":sandbox2",
         ":trace_all_syscalls",
         "@com_google_absl//absl/log",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings",
         "@com_google_googletest//:gtest_main",
         "@com_google_sandboxed_api//sandboxed_api:testing",
+        "@com_google_sandboxed_api//sandboxed_api/util:status_matchers",
     ],
 )
 

sandboxed_api/sandbox2/CMakeLists.txt

@@ -204,7 +204,6 @@ target_link_libraries(sandbox2_policy
  PRIVATE absl::strings
          sandbox2::bpf_helper
          sandbox2::bpfdisassembler
-         sandbox2::comms
          sandbox2::regs
          sandbox2::syscall
          sapi::base
@@ -440,7 +439,6 @@ target_link_libraries(sandbox2_monitor_base
           absl::cleanup
           absl::flags
           absl::log
-          absl::status
           absl::strings
           absl::time
           absl::vlog_is_on
@@ -453,7 +451,8 @@ target_link_libraries(sandbox2_monitor_base
           sapi::file_helpers
           sapi::temp_file
           sapi::base
-  PUBLIC  absl::statusor
+  PUBLIC  absl::status
+          absl::statusor
           absl::synchronization
           sandbox2::comms
           sandbox2::executor
@@ -521,6 +520,7 @@ target_link_libraries(sandbox2_monitor_unotify
           absl::strings
           absl::time
           sapi::base
+          sandbox2::bpf_evaluator
           sandbox2::client
           sandbox2::forkserver_proto
           sapi::config
@@ -1029,11 +1029,13 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
     sandbox2::testcase_pidcomms
   )
   target_link_libraries(sandbox2_notify_test PRIVATE
+    absl::status
     absl::strings
     sandbox2::comms
     sandbox2::regs
     sandbox2::sandbox2
     sandbox2::trace_all_syscalls
+    sapi::status_matchers
     sapi::testing
     sapi::test_main
   )

sandboxed_api/sandbox2/monitor_base.cc

@@ -269,12 +269,22 @@ void MonitorBase::SetExitStatusCode(Result::StatusEnum final_status,
   result_.SetExitStatusCode(final_status, reason_code);
 }
 
+absl::Status MonitorBase::SendPolicy(const std::vector<sock_filter>& policy) {
+  if (!comms_->SendBytes(reinterpret_cast<const uint8_t*>(policy.data()),
+                         policy.size() * sizeof(sock_filter))) {
+    return absl::InternalError("Error while sending policy via comms");
+  }
+  return absl::OkStatus();
+}
+
 bool MonitorBase::InitSendPolicy() {
-  if (!policy_->SendPolicy(comms_, type_ == FORKSERVER_MONITOR_UNOTIFY)) {
-    LOG(ERROR) << "Couldn't send policy";
+  bool user_notif = type_ == FORKSERVER_MONITOR_UNOTIFY;
+  auto policy = policy_->GetPolicy(user_notif);
+  absl::Status status = SendPolicy(std::move(policy));
+  if (!status.ok()) {
+    LOG(ERROR) << "Couldn't send policy: " << status;
     return false;
   }
-
   return true;
 }
 

sandboxed_api/sandbox2/monitor_base.h

@@ -28,6 +28,7 @@
 #include <string>
 #include <vector>
 
+#include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/synchronization/notification.h"
 #include "absl/time/time.h"
@@ -115,8 +116,14 @@ class MonitorBase {
   // Monitor type
   MonitorType type_ = FORKSERVER_MONITOR_PTRACE;
 
- private:
+ protected:
   // Sends Policy to the Client.
+  // Can be overridden by subclasses to save/modify policy before sending.
+  // Returns success/failure status.
+  virtual absl::Status SendPolicy(const std::vector<sock_filter>& policy);
+
+ private:
+  // Instantiates and sends Policy to the Client.
   // Returns success/failure status.
   bool InitSendPolicy();
 

sandboxed_api/sandbox2/monitor_unotify.cc

@@ -37,6 +37,7 @@
 #include "absl/time/time.h"
 #include "absl/types/span.h"
 #include "sandboxed_api/config.h"
+#include "sandboxed_api/sandbox2/bpf_evaluator.h"
 #include "sandboxed_api/sandbox2/client.h"
 #include "sandboxed_api/sandbox2/executor.h"
 #include "sandboxed_api/sandbox2/forkserver.pb.h"
@@ -47,6 +48,16 @@
 #include "sandboxed_api/util/fileops.h"
 #include "sandboxed_api/util/status_macros.h"
 
+#ifndef SECCOMP_RET_USER_NOTIF
+#define SECCOMP_RET_USER_NOTIF 0x7fc00000U /* notifies userspace */
+#endif
+
+#ifndef SECCOMP_USER_NOTIF_FLAG_CONTINUE
+#define SECCOMP_USER_NOTIF_FLAG_CONTINUE 1
+#endif
+
+#define DO_USER_NOTIF BPF_STMT(BPF_RET + BPF_K, SECCOMP_RET_USER_NOTIF)
+
 #ifndef SECCOMP_GET_NOTIF_SIZES
 #define SECCOMP_GET_NOTIF_SIZES 3
 
@@ -66,6 +77,7 @@ struct seccomp_notif_sizes {
 
 // Flags for seccomp notification fd ioctl.
 #define SECCOMP_IOCTL_NOTIF_RECV SECCOMP_IOWR(0, struct seccomp_notif)
+#define SECCOMP_IOCTL_NOTIF_SEND SECCOMP_IOWR(1, struct seccomp_notif_resp)
 #endif
 
 namespace sandbox2 {
@@ -169,6 +181,52 @@ void UnotifyMonitor::RunInternal() {
   setup_notification_.WaitForNotification();
 }
 
+absl::Status UnotifyMonitor::SendPolicy(
+    const std::vector<sock_filter>& policy) {
+  original_policy_ = policy;
+  std::vector<sock_filter> modified_policy = policy;
+  const sock_filter trace_action = SANDBOX2_TRACE;
+  for (sock_filter& filter : modified_policy) {
+    if ((filter.code == BPF_RET + BPF_K && filter.k == SECCOMP_RET_KILL) ||
+        (filter.code == trace_action.code && filter.k == trace_action.k)) {
+      filter = DO_USER_NOTIF;
+    }
+  }
+  return MonitorBase::SendPolicy(modified_policy);
+}
+
+void UnotifyMonitor::HandleViolation(const Syscall& syscall) {
+  ViolationType violation_type = syscall.arch() == Syscall::GetHostArch()
+                                     ? ViolationType::kSyscall
+                                     : ViolationType::kArchitectureSwitch;
+  LogSyscallViolation(syscall);
+  notify_->EventSyscallViolation(syscall, violation_type);
+  MaybeGetStackTrace(req_->pid, Result::VIOLATION);
+  SetExitStatusCode(Result::VIOLATION, syscall.nr());
+  notify_->EventSyscallViolation(syscall, violation_type);
+  result_.SetSyscall(std::make_unique<Syscall>(syscall));
+  KillSandboxee();
+}
+
+void UnotifyMonitor::AllowSyscallViaUnotify() {
+  memset(resp_.get(), 0, resp_size_);
+  resp_->id = req_->id;
+  resp_->val = 0;
+  resp_->error = 0;
+  resp_->flags = SECCOMP_USER_NOTIF_FLAG_CONTINUE;
+  if (ioctl(seccomp_notify_fd_.get(), SECCOMP_IOCTL_NOTIF_SEND, resp_.get()) !=
+      0) {
+    if (errno == ENOENT) {
+      VLOG(1) << "Unotify send failed with ENOENT";
+    } else {
+      LOG_IF(ERROR, errno == EINVAL)
+          << "Unotify send failed with EINVAL. Likely "
+             "SECCOMP_USER_NOTIF_FLAG_CONTINUE unsupported by the kernel.";
+      SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_NOTIFY);
+    }
+  }
+}
+
 void UnotifyMonitor::HandleUnotify() {
   memset(req_.get(), 0, req_size_);
   if (ioctl(seccomp_notify_fd_.get(), SECCOMP_IOCTL_NOTIF_RECV, req_.get()) !=
@@ -184,16 +242,31 @@ void UnotifyMonitor::HandleUnotify() {
                   {req_->data.args[0], req_->data.args[1], req_->data.args[2],
                    req_->data.args[3], req_->data.args[4], req_->data.args[5]},
                   req_->pid, 0, req_->data.instruction_pointer);
-  ViolationType violation_type = syscall.arch() == Syscall::GetHostArch()
-                                     ? ViolationType::kSyscall
-                                     : ViolationType::kArchitectureSwitch;
-  LogSyscallViolation(syscall);
-  notify_->EventSyscallViolation(syscall, violation_type);
-  MaybeGetStackTrace(req_->pid, Result::VIOLATION);
-  SetExitStatusCode(Result::VIOLATION, syscall.nr());
-  notify_->EventSyscallViolation(syscall, violation_type);
-  result_.SetSyscall(std::make_unique<Syscall>(syscall));
-  KillSandboxee();
+  absl::StatusOr<uint32_t> policy_ret =
+      bpf::Evaluate(original_policy_, req_->data);
+  if (!policy_ret.ok()) {
+    LOG(ERROR) << "Failed to evaluate policy: " << policy_ret.status();
+    SetExitStatusCode(Result::INTERNAL_ERROR, Result::FAILED_NOTIFY);
+  }
+  const sock_filter trace_action = SANDBOX2_TRACE;
+  bool should_trace = *policy_ret == trace_action.k;
+  Notify::TraceAction trace_response = Notify::TraceAction::kDeny;
+  if (should_trace) {
+    trace_response = notify_->EventSyscallTrace(syscall);
+  }
+  switch (trace_response) {
+    case Notify::TraceAction::kAllow:
+      AllowSyscallViaUnotify();
+      return;
+    case Notify::TraceAction::kDeny:
+      HandleViolation(syscall);
+      return;
+    case Notify::TraceAction::kInspectAfterReturn:
+      LOG(FATAL) << "TraceAction::kInspectAfterReturn not supported by unotify "
+                    "monitor";
+    default:
+      LOG(FATAL) << "Unknown TraceAction: " << static_cast<int>(trace_response);
+  }
 }
 
 void UnotifyMonitor::Run() {
@@ -339,6 +412,8 @@ bool UnotifyMonitor::InitSetupUnotify() {
   }
   req_size_ = sizes.seccomp_notif;
   req_.reset(static_cast<seccomp_notif*>(malloc(req_size_)));
+  resp_size_ = sizes.seccomp_notif_resp;
+  resp_.reset(static_cast<seccomp_notif_resp*>(malloc(resp_size_)));
   return true;
 }
 

sandboxed_api/sandbox2/monitor_unotify.h

@@ -1,6 +1,8 @@
 #ifndef SANDBOXED_API_SANDBOX2_MONITOR_UNOTIFY_H_
 #define SANDBOXED_API_SANDBOX2_MONITOR_UNOTIFY_H_
 
+#include <linux/audit.h>
+#include <linux/filter.h>
 #include <linux/seccomp.h>
 #include <sys/sysinfo.h>
 #include <sys/types.h>
@@ -35,6 +37,13 @@ struct seccomp_notif {
   __u32 flags;
   struct seccomp_data data;
 };
+
+struct seccomp_notif_resp {
+  __u64 id;
+  __s64 val;
+  __s32 error;
+  __u32 flags;
+};
 #endif
 
 class UnotifyMonitor : public MonitorBase {
@@ -71,13 +80,16 @@ class UnotifyMonitor : public MonitorBase {
   void Join() override;
   void Run();
 
+  absl::Status SendPolicy(const std::vector<sock_filter>& policy) override;
   bool InitSetupUnotify();
   bool InitSetupNotifyEventFd();
   // Kills the main traced PID with SIGKILL.
   // Returns false if an error occurred and process could not be killed.
   bool KillSandboxee();
   void KillInit();
 
+  void AllowSyscallViaUnotify();
+  void HandleViolation(const Syscall& syscall);
   void HandleUnotify();
   void SetExitStatusFromStatusPipe();
 
@@ -90,6 +102,8 @@ class UnotifyMonitor : public MonitorBase {
   absl::Notification setup_notification_;
   sapi::file_util::fileops::FDCloser seccomp_notify_fd_;
   sapi::file_util::fileops::FDCloser monitor_notify_fd_;
+  // Original policy as configured by the user.
+  std::vector<sock_filter> original_policy_;
   // Deadline in Unix millis
   std::atomic<int64_t> deadline_millis_{0};
   // False iff external kill is requested
@@ -110,8 +124,12 @@ class UnotifyMonitor : public MonitorBase {
   // Synchronizes monitor thread deletion and notifying the monitor.
   absl::Mutex notify_mutex_;
 
+  static constexpr auto kFreeFn = [](void* ptr) { std::free(ptr); };
   size_t req_size_;
-  std::unique_ptr<seccomp_notif, decltype(std::free)*> req_{nullptr, std::free};
+  std::unique_ptr<seccomp_notif, decltype(kFreeFn)> req_{nullptr, kFreeFn};
+  size_t resp_size_;
+  std::unique_ptr<seccomp_notif_resp, decltype(kFreeFn)> resp_{nullptr,
+                                                               kFreeFn};
 };
 
 }  // namespace sandbox2