Make Policy a simple copyable type

PiperOrigin-RevId: 555146979
Change-Id: I83d7260d65d4291c418e6c8e80385cbdc8fbc758

sandboxed_api/sandbox2/BUILD.bazel

@@ -221,7 +221,6 @@ cc_library(
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/log",
-        "@com_google_absl//absl/types:optional",
     ],
 )
 
@@ -539,6 +538,7 @@ cc_library(
         ":mounts",
         ":namespace",
         ":policy",
+        ":violation_cc_proto",
         "//sandboxed_api:config",
         "//sandboxed_api/sandbox2/network_proxy:filtering",
         "//sandboxed_api/sandbox2/util:bpf_helper",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -167,7 +167,6 @@ add_library(sandbox2_policy ${SAPI_LIB_TYPE}
 add_library(sandbox2::policy ALIAS sandbox2_policy)
 target_link_libraries(sandbox2_policy PRIVATE
   absl::core_headers
-  absl::optional
   sandbox2::bpf_helper
   sandbox2::bpfdisassembler
   sandbox2::comms
@@ -494,6 +493,7 @@ target_link_libraries(sandbox2_policybuilder
           sapi::config
           sandbox2::bpf_helper
           sandbox2::namespace
+          sandbox2::violation_proto
           sapi::file_base
           sapi::status
   PUBLIC absl::check

sandboxed_api/sandbox2/monitor_base.cc

@@ -20,12 +20,12 @@
 #include <syscall.h>
 
 #include <cerrno>
-#include <csignal>
 #include <cstdio>
-#include <deque>
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "absl/cleanup/cleanup.h"
 #include "absl/flags/declare.h"
@@ -132,7 +132,7 @@ MonitorBase::MonitorBase(Executor* executor, Policy* policy, Notify* notify)
     PCHECK(log_file_ != nullptr) << "Failed to open log file '" << path << "'";
   }
 
-  if (auto* ns = policy_->GetNamespace(); ns) {
+  if (auto& ns = policy_->namespace_; ns) {
     // Check for the Tomoyo LSM, which is active by default in several common
     // distribution kernels (esp. Debian).
     MaybeEnableTomoyoLsmWorkaround(ns->mounts(), comms_fd_dev_);
@@ -172,7 +172,7 @@ void MonitorBase::Launch() {
   };
   absl::Cleanup monitor_done = [this] { OnDone(); };
 
-  Namespace* ns = policy_->GetNamespace();
+  const Namespace* ns = policy_->GetNamespaceOrNull();
   if (SAPI_VLOG_IS_ON(1) && ns != nullptr) {
     std::vector<std::string> outside_entries;
     std::vector<std::string> inside_entries;
@@ -397,7 +397,7 @@ bool MonitorBase::StackTraceCollectionPossible() const {
   }
   LOG(ERROR) << "Cannot collect stack trace. Unwind pid "
              << executor_->libunwind_sbox_for_pid_ << ", namespace "
-             << policy_->GetNamespace();
+             << policy_->GetNamespaceOrNull();
   return false;
 }
 
@@ -433,7 +433,7 @@ bool MonitorBase::ShouldCollectStackTrace(Result::StatusEnum status) const {
 
 absl::StatusOr<std::vector<std::string>> MonitorBase::GetStackTrace(
     const Regs* regs) {
-  return sandbox2::GetStackTrace(regs, policy_->GetNamespace(),
+  return sandbox2::GetStackTrace(regs, policy_->GetNamespaceOrNull(),
                                  uses_custom_forkserver_);
 }
 

sandboxed_api/sandbox2/monitor_ptrace.cc

@@ -806,7 +806,7 @@ void PtraceMonitor::EventPtraceExit(pid_t pid, int event_msg) {
 
   // A regular exit, let it continue (fast-path).
   if (ABSL_PREDICT_TRUE(WIFEXITED(event_msg) &&
-                        (!policy_->collect_stacktrace_on_exit_ ||
+                        (!policy_->collect_stacktrace_on_exit() ||
                          pid != process_.main_pid))) {
     ContinueProcess(pid, 0);
     return;

sandboxed_api/sandbox2/policy.h

@@ -21,15 +21,9 @@
 #include <asm/types.h>
 #include <linux/filter.h>
 
-#include <cstddef>
+#include <optional>
-#include <memory>
-#include <set>
-#include <utility>
 #include <vector>
 
-#include "absl/base/macros.h"
-#include "absl/types/optional.h"
-#include "sandboxed_api/config.h"
 #include "sandboxed_api/sandbox2/namespace.h"
 #include "sandboxed_api/sandbox2/network_proxy/filtering.h"
 #include "sandboxed_api/sandbox2/syscall.h"
@@ -46,24 +40,21 @@ inline constexpr uintptr_t kExecveMagic = 0x921c2c34;
 }  // namespace internal
 
 class Comms;
+class PolicyBuilder;
+class MonitorBase;
 
 class Policy final {
  public:
+  Policy(const Policy&) = default;
+  Policy& operator=(const Policy&) = default;
+
+  Policy(Policy&&) = delete;
+  Policy& operator=(Policy&&) = delete;
+
   // Stores information about the policy (and the policy builder if existing)
   // in the protobuf structure.
   void GetPolicyDescription(PolicyDescription* policy) const;
 
- private:
-  friend class Sandbox2;
-  friend class MonitorBase;
-  friend class PtraceMonitor;
-  friend class UnotifyMonitor;
-  friend class PolicyBuilder;
-  friend class StackTracePeer;
-
-  // Private constructor only called by the PolicyBuilder.
-  Policy() = default;
-
   // Sends the policy over the IPC channel.
   bool SendPolicy(Comms* comms, bool user_notif) const;
 
@@ -71,9 +62,9 @@ class Policy final {
   // requirements (message passing via Comms, Executor::WaitForExecve etc.).
   std::vector<sock_filter> GetPolicy(bool user_notif) const;
 
-  Namespace* GetNamespace() { return namespace_.get(); }
+  const std::optional<Namespace>& GetNamespace() const { return namespace_; }
-  void SetNamespace(std::unique_ptr<Namespace> ns) {
+  const Namespace* GetNamespaceOrNull() const {
-    namespace_ = std::move(ns);
+    return namespace_ ? &namespace_.value() : nullptr;
   }
 
   // Returns the default policy, which blocks certain dangerous syscalls and
@@ -82,8 +73,23 @@ class Policy final {
   // Returns a policy allowing the Monitor module to track all syscalls.
   std::vector<sock_filter> GetTrackingPolicy() const;
 
+  bool collect_stacktrace_on_signal() const {
+    return collect_stacktrace_on_signal_;
+  }
+
+  bool collect_stacktrace_on_exit() const {
+    return collect_stacktrace_on_exit_;
+  }
+
+ private:
+  friend class PolicyBuilder;
+  friend class MonitorBase;
+
+  // Private constructor only called by the PolicyBuilder.
+  Policy() = default;
+
   // The Namespace object, defines ways of putting sandboxee into namespaces.
-  std::unique_ptr<Namespace> namespace_;
+  std::optional<Namespace> namespace_;
 
   // Gather stack traces on violations, signals, timeouts or when getting
   // killed. See policybuilder.h for more information.
@@ -94,7 +100,7 @@ class Policy final {
   bool collect_stacktrace_on_exit_ = false;
 
   // Optional pointer to a PolicyBuilder description pb object.
-  std::unique_ptr<PolicyBuilderDescription> policy_builder_description_;
+  std::optional<PolicyBuilderDescription> policy_builder_description_;
 
   // The policy set by the user.
   std::vector<sock_filter> user_policy_;
@@ -102,7 +108,7 @@ class Policy final {
   bool user_policy_handles_ptrace_ = false;
 
   // Contains a list of hosts the sandboxee is allowed to connect to.
-  absl::optional<AllowedHosts> allowed_hosts_;
+  std::optional<AllowedHosts> allowed_hosts_;
 };
 
 }  // namespace sandbox2

sandboxed_api/sandbox2/policybuilder.cc

@@ -40,7 +40,6 @@
 #include "absl/memory/memory.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
-#include "absl/strings/escaping.h"
 #include "absl/strings/match.h"
 #include "absl/strings/string_view.h"
 #include "sandboxed_api/config.h"
@@ -49,6 +48,7 @@
 #include "sandboxed_api/sandbox2/namespace.h"
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/util/bpf_helper.h"
+#include "sandboxed_api/sandbox2/violation.pb.h"
 #include "sandboxed_api/util/path.h"
 #include "sandboxed_api/util/status_macros.h"
 
@@ -1188,8 +1188,9 @@ std::vector<sock_filter> PolicyBuilder::ResolveBpfFunc(BpfFunc f) {
 }
 
 absl::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() {
-  // Using `new` to access a non-public constructor.
+  if (!last_status_.ok()) {
-  auto output = absl::WrapUnique(new Policy());
+    return last_status_;
+  }
 
   if (user_policy_.size() > kMaxUserPolicyLength) {
     return absl::FailedPreconditionError(
@@ -1197,9 +1198,8 @@ absl::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() {
                      " > ", kMaxUserPolicyLength, ")."));
   }
 
-  if (!last_status_.ok()) {
+  // Using `new` to access a non-public constructor.
-    return last_status_;
+  auto output = absl::WrapUnique(new Policy());
-  }
 
   if (already_built_) {
     return absl::FailedPreconditionError("Can only build policy once.");
@@ -1210,9 +1210,9 @@ absl::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() {
       return absl::FailedPreconditionError(
           "Cannot set hostname without network namespaces.");
     }
-    output->SetNamespace(std::make_unique<Namespace>(
+    output->namespace_ =
-        allow_unrestricted_networking_, std::move(mounts_), hostname_,
+        Namespace(allow_unrestricted_networking_, std::move(mounts_), hostname_,
-        allow_mount_propagation_));
+                  allow_mount_propagation_);
   }
 
   output->collect_stacktrace_on_signal_ = collect_stacktrace_on_signal_;
@@ -1230,10 +1230,10 @@ absl::StatusOr<std::unique_ptr<Policy>> PolicyBuilder::TryBuild() {
   output->user_policy_handles_bpf_ = user_policy_handles_bpf_;
   output->user_policy_handles_ptrace_ = user_policy_handles_ptrace_;
 
-  auto pb_description = std::make_unique<PolicyBuilderDescription>();
+  PolicyBuilderDescription pb_description;
 
-  StoreDescription(pb_description.get());
+  StoreDescription(&pb_description);
-  output->policy_builder_description_ = std::move(pb_description);
+  output->policy_builder_description_ = pb_description;
   output->allowed_hosts_ = std::move(allowed_hosts_);
   already_built_ = true;
   return std::move(output);

sandboxed_api/sandbox2/sandbox2.cc

@@ -122,16 +122,16 @@ absl::Status Sandbox2::EnableUnotifyMonitor() {
                     "EventSyscallTrap/EventSyscallTrace, notifications about "
                     "signals via EventSignal will not work";
   }
-  if (policy_->GetNamespace() == nullptr) {
+  if (!policy_->GetNamespace()) {
     return absl::FailedPreconditionError(
         "Unotify monitor can only be used together with namespaces");
   }
-  if (policy_->collect_stacktrace_on_signal_) {
+  if (policy_->collect_stacktrace_on_signal()) {
     return absl::FailedPreconditionError(
         "Unotify monitor cannot collect stack traces on signal");
   }
 
-  if (policy_->collect_stacktrace_on_exit_) {
+  if (policy_->collect_stacktrace_on_exit()) {
     return absl::FailedPreconditionError(
         "Unotify monitor cannot collect stack traces on normal exit");
   }