Refactor comms around client config

Use protobuf for simplicity and extensibility.

PiperOrigin-RevId: 907573418
Change-Id: Ia7dc0a6e6097d1bd6c367010ad171029bdcc95db

Author: happyCoder92

sandboxed_api/sandbox2/BUILD

@@ -357,6 +357,7 @@ cc_library(
     deps = [
         ":buffer",
         ":client",
+        ":client_cc_proto",
         ":comms",
         ":executor",
         ":fork_client",
@@ -442,6 +443,7 @@ cc_library(
     copts = sapi_platform_copts(),
     deps = [
         ":client",
+        ":client_cc_proto",
         ":comms",
         ":executor",
         ":flags",
@@ -483,6 +485,7 @@ cc_library(
     deps = [
         ":bpf_evaluator",
         ":client",
+        ":client_cc_proto",
         ":executor",
         ":flags",
         ":forkserver_cc_proto",
@@ -520,6 +523,7 @@ cc_library(
     copts = sapi_platform_copts(),
     deps = [
         ":client",
+        ":client_cc_proto",
         ":comms",
         ":executor",
         ":flags",
@@ -593,6 +597,11 @@ cc_library(
     ],
 )
 
+sapi_proto_library(
+    name = "client_proto",
+    srcs = ["client.proto"],
+)
+
 # Should be used in sandboxee code instead of :sandbox2 if it uses just
 # sandbox2::Client::SandboxMeHere() and sandbox2::Comms
 cc_library(
@@ -603,6 +612,7 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":buffer",
+        ":client_cc_proto",
         ":comms",
         ":logsink",
         ":policy",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -438,6 +438,7 @@ target_link_libraries(sandbox2_monitor_base
   PUBLIC  absl::status
           absl::statusor
           absl::synchronization
+          sandbox2::client_proto
           sandbox2::comms
           sandbox2::executor
           sandbox2::fork_client
@@ -474,6 +475,7 @@ target_link_libraries(sandbox2_monitor_ptrace
           sapi::fileops
           sapi::status
           sandbox2::client
+          sandbox2::client_proto
           sandbox2::comms
           sandbox2::result
           sandbox2::sanitizer
@@ -515,6 +517,7 @@ target_link_libraries(sandbox2_monitor_unotify
           sapi::base
           sandbox2::bpf_evaluator
           sandbox2::client
+          sandbox2::client_proto
           sandbox2::flags
           sandbox2::forkserver_proto
           sandbox2::seccomp_unotify
@@ -569,6 +572,20 @@ target_link_libraries(sandbox2_policybuilder
          sandbox2::policy
 )
 
+# sandboxed_api/sandbox2:client_proto
+sapi_protobuf_generate_cpp(_sandbox2_client_pb_h _sandbox2_client_pb_cc
+  client.proto
+)
+add_library(sandbox2_client_proto ${SAPI_LIB_TYPE}
+  ${_sandbox2_client_pb_cc}
+  ${_sandbox2_client_pb_h}
+)
+add_library(sandbox2::client_proto ALIAS sandbox2_client_proto)
+target_link_libraries(sandbox2_client_proto PRIVATE
+  protobuf::libprotobuf
+  sapi::base
+)
+
 # sandboxed_api/sandbox2:client
 add_library(sandbox2_client ${SAPI_LIB_TYPE}
   client.cc
@@ -591,6 +608,7 @@ target_link_libraries(sandbox2_client
           sapi::status
   PUBLIC absl::flat_hash_map
          absl::status
+         sandbox2::client_proto
          sandbox2::comms
          sandbox2::logsink
          sandbox2::network_proxy_client

sandboxed_api/sandbox2/client.cc

@@ -50,6 +50,7 @@
 #include "absl/strings/string_view.h"
 #include "sandboxed_api/config.h"
 #include "sandboxed_api/sandbox2/buffer.h"
+#include "sandboxed_api/sandbox2/client.pb.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/logsink.h"
 #include "sandboxed_api/sandbox2/network_proxy/client.h"
@@ -189,6 +190,7 @@ std::string Client::GetFdMapEnvVar() const {
 }
 
 void Client::PrepareEnvironment(int* preserved_fd) {
+  ReceiveClientConfig();
   SetUpIPC(preserved_fd);
   SetUpCwd();
 }
@@ -204,41 +206,19 @@ void Client::SandboxMeHere() {
 }
 
 void Client::SetUpCwd() {
-  {
-    // Get the current working directory to check if we are in a mount
-    // namespace.
-    // Note: glibc 2.27 no longer returns a relative path in that case, but
-    //       fails with ENOENT and returns a nullptr instead. The code still
-    //       needs to run on lower version for the time being.
-    char cwd_buf[PATH_MAX + 1] = {0};
-    char* cwd = getcwd(cwd_buf, ABSL_ARRAYSIZE(cwd_buf));
-    SAPI_RAW_PCHECK(cwd != nullptr || errno == ENOENT,
-                    "no current working directory");
-
-    // Outside of the mount namespace, the path is of the form
-    // '(unreachable)/...'. Only check for the slash, since Linux might make up
-    // other prefixes in the future.
-    if (errno == ENOENT || cwd_buf[0] != '/') {
-      SAPI_RAW_VLOG(1, "chdir into mount namespace, cwd was '%s'", cwd_buf);
-      // If we are in a mount namespace but fail to chdir, then it can lead to a
-      // sandbox escape -- we need to fail with FATAL if the chdir fails.
-      SAPI_RAW_PCHECK(chdir("/") != -1, "corrective chdir");
-    }
-  }
-
   // Receive the user-supplied current working directory and change into it.
-  std::string cwd;
-  SAPI_RAW_CHECK(comms_->RecvString(&cwd), "receiving working directory");
-  if (!cwd.empty()) {
-    // On the other hand this chdir can fail without a sandbox escape. It will
-    // probably not have the intended behavior though.
-    if (chdir(cwd.c_str()) == -1 && SAPI_RAW_VLOG_IS_ON(1)) {
-      SAPI_RAW_PLOG(
-          INFO,
-          "chdir(%s) failed, falling back to previous cwd or / (with "
-          "namespaces). Use Executor::SetCwd() to set a working directory",
-          cwd.c_str());
-    }
+  if (client_config_.cwd().empty()) {
+    return;
+  }
+  std::string cwd(client_config_.cwd());
+  // This chdir can fail without a sandbox escape. It will probably not have
+  // the intended behavior though.
+  if (chdir(cwd.c_str()) == -1 && SAPI_RAW_VLOG_IS_ON(1)) {
+    SAPI_RAW_PLOG(
+        INFO,
+        "chdir(%s) failed, falling back to previous cwd or / (with "
+        "namespaces). Use Executor::SetCwd() to set a working directory",
+        cwd.c_str());
   }
 }
 
@@ -314,9 +294,12 @@ void Client::SetUpIPC(int* preserved_fd) {
 }
 
 void Client::ReceivePolicy() {
-  std::vector<uint8_t> bytes;
-  SAPI_RAW_CHECK(comms_->RecvBytes(&bytes), "receive bytes");
-  policy_ = std::move(bytes);
+  SAPI_RAW_CHECK(comms_->RecvBytes(&policy_), "receive bytes");
+}
+
+void Client::ReceiveClientConfig() {
+  SAPI_RAW_CHECK(comms_->RecvProtoBuf(&client_config_),
+                 "receive client config");
 }
 
 void Client::ApplyPolicyAndBecomeTracee() {
@@ -325,18 +308,13 @@ void Client::ApplyPolicyAndBecomeTracee() {
   // this function does nothing.
   sanitizer::WaitForSanitizer();
 
-  // Creds can be received w/o synchronization, once the connection is
-  // established.
-  pid_t cred_pid;
-  uid_t cred_uid ABSL_ATTRIBUTE_UNUSED;
-  gid_t cred_gid ABSL_ATTRIBUTE_UNUSED;
-  SAPI_RAW_CHECK(comms_->GetPeerCreds(&cred_pid, &cred_uid, &cred_gid),
-                 "receiving credentials");
-
-  SAPI_RAW_CHECK(prctl(PR_SET_DUMPABLE, 1) == 0,
-                 "setting PR_SET_DUMPABLE flag");
-  if (prctl(PR_SET_PTRACER, cred_pid) == -1) {
-    SAPI_RAW_VLOG(1, "No YAMA on this system. Continuing");
+  pid_t ptracer_tid = client_config_.ptracer_tid();
+  if (ptracer_tid > 0) {
+    SAPI_RAW_CHECK(prctl(PR_SET_DUMPABLE, 1) == 0,
+                   "setting PR_SET_DUMPABLE flag");
+    if (prctl(PR_SET_PTRACER, ptracer_tid) == -1) {
+      SAPI_RAW_VLOG(1, "No YAMA on this system. Continuing");
+    }
   }
 
   SAPI_RAW_CHECK(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == 0,
@@ -364,7 +342,9 @@ void Client::ApplyPolicyAndBecomeTracee() {
   uint32_t message;  // wait for confirmation
   SAPI_RAW_CHECK(comms_->RecvUint32(&message),
                  "receving confirmation from executor");
-  bool allow_speculation = message & kAllowSpeculationBit;
+  SAPI_RAW_CHECK(message == kSandbox2MonitorReady,
+                 "invalid confirmation from executor");
+  bool allow_speculation = client_config_.seccomp_allow_speculation();
   if (!allow_speculation) {
     if constexpr (sapi::host_cpu::IsX8664() || sapi::host_cpu::IsArm64()) {
       SAPI_RAW_PCHECK(prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS,
@@ -381,12 +361,13 @@ void Client::ApplyPolicyAndBecomeTracee() {
   }
   uint32_t seccomp_extra_flags =
       allow_speculation ? SECCOMP_FILTER_FLAG_SPEC_ALLOW : 0;
-  uint32_t monitor_type = message & kMonitorTypeMask;
-  if (monitor_type == kSandbox2ClientUnotify) {
+  if (client_config_.monitor_type() == ClientConfig::CLIENT_MONITOR_UNOTIFY) {
     InitSeccompUnotify(prog, comms_, seccomp_extra_flags);
   } else {
-    SAPI_RAW_CHECK(monitor_type == kSandbox2ClientPtrace,
-                   "invalid confirmation from executor");
+    SAPI_RAW_CHECK(
+        client_config_.monitor_type() == ClientConfig::CLIENT_MONITOR_PTRACE,
+        absl::StrCat("invalid monitor type: ", client_config_.monitor_type())
+            .c_str());
     InitSeccompRegular(prog, seccomp_extra_flags);
   }
 }

sandboxed_api/sandbox2/client.h

@@ -27,6 +27,7 @@
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "sandboxed_api/sandbox2/buffer.h"
+#include "sandboxed_api/sandbox2/client.pb.h"
 #include "sandboxed_api/sandbox2/comms.h"
 #include "sandboxed_api/sandbox2/logsink.h"
 #include "sandboxed_api/sandbox2/network_proxy/client.h"
@@ -38,12 +39,8 @@ class Client {
   // Client is ready to be sandboxed.
   static constexpr uint32_t kClient2SandboxReady = 0x0A0B0C01;
 
-  // Sandbox is ready to monitor the sandboxee. Used with PtraceMonitor.
-  static constexpr uint32_t kSandbox2ClientPtrace = 0x0A0B0C02;
-
-  // Sandboxee should setup seccomp_unotify and send back the FD. Used with
-  // UnotifyMonitor.
-  static constexpr uint32_t kSandbox2ClientUnotify = 0x0A0B0C03;
+  // Sandbox is ready to monitor the sandboxee.
+  static constexpr uint32_t kSandbox2MonitorReady = 0x0A0B0D01;
 
   // Sandboxee can proceed after seccomp_unotify setup as limits have been
   // applied by the monitor.
@@ -97,6 +94,9 @@ class Client {
 
   friend class ForkServer;
 
+  // Client configuration received from the monitor.
+  ClientConfig client_config_;
+
   // Seccomp-bpf policy received from the monitor.
   std::vector<uint8_t> policy_;
 
@@ -130,6 +130,9 @@ class Client {
   // Receives seccomp-bpf policy from the monitor.
   void ReceivePolicy();
 
+  // Receives client configuration from the monitor.
+  void ReceiveClientConfig();
+
   // Applies sandbox-bpf policy, have limits applied on us, and become ptrace'd.
   void ApplyPolicyAndBecomeTracee();
 

sandboxed_api/sandbox2/client.proto

@@ -0,0 +1,36 @@
+// Copyright 2026 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+edition = "2024";
+
+package sandbox2;
+
+message ClientConfig {
+  enum MonitorType {
+    // Default value
+    CLIENT_MONITOR_UNSPECIFIED = 0;
+    // Ptrace based monitor
+    CLIENT_MONITOR_PTRACE = 1;
+    // Seccomp_unotify based monitor
+    CLIENT_MONITOR_UNOTIFY = 2;
+  }
+  // Monitor type used by the sandbox
+  MonitorType monitor_type = 1;
+
+  string cwd = 2;
+
+  int32 ptracer_tid = 3;
+
+  bool seccomp_allow_speculation = 4;
+}

sandboxed_api/sandbox2/monitor_base.cc

@@ -220,12 +220,12 @@ void MonitorBase::Launch() {
     SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_NOTIFY);
     return;
   }
-  if (!InitSendIPC()) {
-    SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_IPC);
+  if (!InitSendClientConfig()) {
+    SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_CLIENT_CONFIG);
     return;
   }
-  if (!InitSendCwd()) {
-    SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_CWD);
+  if (!InitSendIPC()) {
+    SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_IPC);
     return;
   }
   if (!InitSendPolicy()) {
@@ -267,12 +267,8 @@ absl::Status MonitorBase::SendPolicy(const std::vector<sock_filter>& policy) {
   return absl::OkStatus();
 }
 
-bool MonitorBase::SendMonitorReadyMessageAndFlags(uint32_t monitor_type) {
-  uint32_t message = monitor_type;
-  if (policy_->allow_speculation_) {
-    message |= Client::kAllowSpeculationBit;
-  }
-  return comms_->SendUint32(message);
+bool MonitorBase::SendMonitorReadyMessage() {
+  return comms_->SendUint32(Client::kSandbox2MonitorReady);
 }
 
 bool MonitorBase::InitSendPolicy() {
@@ -287,15 +283,6 @@ bool MonitorBase::InitSendPolicy() {
   return true;
 }
 
-bool MonitorBase::InitSendCwd() {
-  if (!comms_->SendString(executor_->cwd_)) {
-    PLOG(ERROR) << "Couldn't send cwd";
-    return false;
-  }
-
-  return true;
-}
-
 bool MonitorBase::InitApplyLimit(pid_t pid, int resource,
                                  const rlimit64& rlim) const {
   using RlimitResource = __rlimit_resource;
@@ -335,6 +322,21 @@ bool MonitorBase::InitApplyLimits() {
          InitApplyLimit(process_.main_pid, RLIMIT_CORE, limits->rlimit_core());
 }
 
+ClientConfig MonitorBase::CreateClientConfig() const {
+  ClientConfig client_config;
+  client_config.set_cwd(executor_->cwd_);
+  client_config.set_seccomp_allow_speculation(policy_->allow_speculation_);
+  return client_config;
+}
+
+bool MonitorBase::InitSendClientConfig() {
+  if (!comms_->SendProtoBuf(CreateClientConfig())) {
+    LOG(ERROR) << "Couldn't send client config";
+    return false;
+  }
+  return true;
+}
+
 bool MonitorBase::InitSendIPC() { return ipc_->SendFdsOverComms(); }
 
 bool MonitorBase::WaitForSandboxReady() {