Clean up the sandbox::Seatbelt API a little.

Remove some unused constants; make the error handling API easier.

Bug: 1315988
Change-Id: Iec911b76395febdff0cc22c67c39b2bbd2163414
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4067687
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Auto-Submit: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1078166}

chrome/utility/safe_browsing/mac/crdmg.cc

@@ -149,10 +149,9 @@ bool SafeDMG::EnableSandbox() {
         " (allow file-write* (subpath \"%s\"))", unpack_path);
   }
 
-  char* sbox_error;
-  if (sandbox::Seatbelt::Init(sbox_profile.c_str(), 0, &sbox_error) != 0) {
+  std::string sbox_error;
+  if (!sandbox::Seatbelt::Init(sbox_profile.c_str(), 0, &sbox_error)) {
     LOG(ERROR) << "Failed to initialize sandbox: " << sbox_error;
-    sandbox::Seatbelt::FreeError(sbox_error);
     return false;
   }
 

ipc/ipc_send_fds_test.cc

@@ -208,13 +208,10 @@ DEFINE_IPC_CHANNEL_MOJO_TEST_CLIENT_WITH_CUSTOM_FIXTURE(
   ASSERT_LE(0, IGNORE_EINTR(close(fd)));
 
   // Enable the sandbox.
-  char* error_buff = NULL;
-  int error = sandbox::Seatbelt::Init(
-      sandbox::Seatbelt::kProfilePureComputation, SANDBOX_NAMED, &error_buff);
-  ASSERT_EQ(0, error);
-  ASSERT_FALSE(error_buff);
-
-  sandbox::Seatbelt::FreeError(error_buff);
+  std::string error;
+  ASSERT_TRUE(sandbox::Seatbelt::Init(
+      sandbox::Seatbelt::kProfilePureComputation, SANDBOX_NAMED, &error))
+      << error;
 
   // Make sure sandbox is really enabled.
   ASSERT_EQ(-1, open(kDevZeroPath, O_RDONLY))

sandbox/mac/sandbox_compiler.cc

@@ -28,7 +28,6 @@ bool SandboxCompiler::InsertStringParam(const std::string& key,
 }
 
 bool SandboxCompiler::CompileAndApplyProfile(std::string* error) {
-  char* error_internal = nullptr;
   std::vector<const char*> params;
 
   for (const auto& kv : params_map_) {
@@ -38,13 +37,8 @@ bool SandboxCompiler::CompileAndApplyProfile(std::string* error) {
   // The parameters array must be null terminated.
   params.push_back(static_cast<const char*>(0));
 
-  if (sandbox::Seatbelt::InitWithParams(profile_str_.c_str(), 0, params.data(),
-                                        &error_internal)) {
-    error->assign(error_internal);
-    sandbox::Seatbelt::FreeError(error_internal);
-    return false;
-  }
-  return true;
+  return sandbox::Seatbelt::InitWithParams(profile_str_.c_str(), 0,
+                                           params.data(), error);
 }
 
 }  // namespace sandbox

sandbox/mac/seatbelt.cc

@@ -23,33 +23,50 @@ int sandbox_check(pid_t pid, const char* operation, int type, ...);
 
 namespace sandbox {
 
+namespace {
+
+bool HandleSandboxResult(int rv, char* errorbuf, std::string* error) {
+  if (rv == 0) {
+    if (error)
+      error->clear();
+    return true;
+  }
+
+  if (error)
+    *error = errorbuf;
+  Seatbelt::FreeError(errorbuf);
+  return false;
+}
+
+}  // namespace
+
 // Initialize the static member variables.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
-const char* Seatbelt::kProfileNoInternet = kSBXProfileNoInternet;
-const char* Seatbelt::kProfileNoNetwork = kSBXProfileNoNetwork;
-const char* Seatbelt::kProfileNoWrite = kSBXProfileNoWrite;
-const char* Seatbelt::kProfileNoWriteExceptTemporary =
-    kSBXProfileNoWriteExceptTemporary;
 const char* Seatbelt::kProfilePureComputation = kSBXProfilePureComputation;
 #pragma clang diagnostic pop
 
 // static
-int Seatbelt::Init(const char* profile, uint64_t flags, char** errorbuf) {
+bool Seatbelt::Init(const char* profile, uint64_t flags, std::string* error) {
 // OS X deprecated these functions, but did not provide a suitable replacement,
 // so ignore the deprecation warning.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
-  return ::sandbox_init(profile, flags, errorbuf);
+  char* errorbuf = nullptr;
+  int rv = ::sandbox_init(profile, flags, &errorbuf);
+  return HandleSandboxResult(rv, errorbuf, error);
 #pragma clang diagnostic pop
 }
 
 // static
-int Seatbelt::InitWithParams(const char* profile,
-                             uint64_t flags,
-                             const char* const parameters[],
-                             char** errorbuf) {
-  return ::sandbox_init_with_parameters(profile, flags, parameters, errorbuf);
+bool Seatbelt::InitWithParams(const char* profile,
+                              uint64_t flags,
+                              const char* const parameters[],
+                              std::string* error) {
+  char* errorbuf = nullptr;
+  int rv =
+      ::sandbox_init_with_parameters(profile, flags, parameters, &errorbuf);
+  return HandleSandboxResult(rv, errorbuf, error);
 }
 
 // static

sandbox/mac/seatbelt.h

@@ -6,6 +6,7 @@
 #define SANDBOX_MAC_SEATBELT_H_
 
 #include <cstdint>
+#include <string>
 
 #include "sandbox/mac/seatbelt_export.h"
 
@@ -16,36 +17,27 @@ namespace sandbox {
 // This class wraps the functions in deprecation warning supressions.
 class SEATBELT_EXPORT Seatbelt {
  public:
-  // Initializes the specified sandbox profile. Returns 0 on success, else -1
-  // and |errorbuf| is populated. |errorbuf| is allocated by the API and must be
-  // freed with FreeError().
-  static int Init(const char* profile, uint64_t flags, char** errorbuf);
+  // Initializes the specified sandbox profile. Returns true on success with
+  // the sandbox applied; otherwise, returns false and outputs the error in
+  // `error`.
+  static bool Init(const char* profile, uint64_t flags, std::string* error);
 
   // Initializes the specified sandbox profile and passes the parameters to the
-  // |profile|. |parameters| is a null terminated list containing key,value
-  // pairs in sequence. [key1,val1,key2,val2,nullptr]. |errorbuf| is allocated
-  // by the API and is set to a string description of the error. |errorbuf| must
-  // be freed with FreeError(). This function eturns 0 on success, else -1 and
-  // |errorbuf| is populated.
-  static int InitWithParams(const char* profile,
-                            uint64_t flags,
-                            const char* const parameters[],
-                            char** errorbuf);
+  // `profile`. `parameters` is a null terminated list containing key,value
+  // pairs in sequence. [key1,val1,key2,val2,nullptr]. Returns true on success
+  // with the sandbox applied; otherwise, returns false and outputs the
+  // error in `error`.
+  static bool InitWithParams(const char* profile,
+                             uint64_t flags,
+                             const char* const parameters[],
+                             std::string* error);
 
-  // Frees the |errorbuf| allocated and set by InitWithParams.
+  // Frees an error buffer allocated from libsandbox.dylib routines.
   static void FreeError(char* errorbuf);
 
   // Returns whether or not the process is currently sandboxed.
   static bool IsSandboxed();
 
-  static const char* kProfileNoInternet;
-
-  static const char* kProfileNoNetwork;
-
-  static const char* kProfileNoWrite;
-
-  static const char* kProfileNoWriteExceptTemporary;
-
   static const char* kProfilePureComputation;
 
   Seatbelt(const Seatbelt& other) = delete;

sandbox/mac/seatbelt_exec.cc

@@ -236,17 +236,14 @@ bool SeatbeltExecServer::ApplySandboxProfile(const mac::SandboxPolicy& policy) {
   }
   weak_params.push_back(nullptr);
 
-  char* error = nullptr;
-  int rv = Seatbelt::InitWithParams(policy.profile().c_str(), 0,
-                                    weak_params.data(), &error);
-  if (error) {
-    logging::Error("SeatbeltExecServer: Failed to initialize sandbox: %d %s",
-                   rv, error);
-    Seatbelt::FreeError(error);
-    return false;
+  std::string error;
+  bool ok = Seatbelt::InitWithParams(policy.profile().c_str(), 0,
+                                     weak_params.data(), &error);
+  if (!ok) {
+    logging::Error("SeatbeltExecServer: Failed to initialize sandbox: %s",
+                   error.c_str());
   }
-
-  return rv == 0;
+  return ok;
 }
 
 bool SeatbeltExecServer::ReadString(std::string* str) {