Automated rollback of commit 2c6a392.

PiperOrigin-RevId: 907728147
Change-Id: I12162497cb7fbeee2cdd98195f0aff1514db6ded

Author: Sandboxed API Team

sandboxed_api/sandbox2/BUILD

@@ -357,7 +357,6 @@ cc_library(
     deps = [
         ":buffer",
         ":client",
-        ":client_cc_proto",
         ":comms",
         ":executor",
         ":fork_client",
@@ -443,7 +442,6 @@ cc_library(
     copts = sapi_platform_copts(),
     deps = [
         ":client",
-        ":client_cc_proto",
         ":comms",
         ":executor",
         ":flags",
@@ -485,7 +483,6 @@ cc_library(
     deps = [
         ":bpf_evaluator",
         ":client",
-        ":client_cc_proto",
         ":executor",
         ":flags",
         ":forkserver_cc_proto",
@@ -523,7 +520,6 @@ cc_library(
     copts = sapi_platform_copts(),
     deps = [
         ":client",
-        ":client_cc_proto",
         ":comms",
         ":executor",
         ":flags",
@@ -597,11 +593,6 @@ 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(
@@ -612,7 +603,6 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":buffer",
-        ":client_cc_proto",
         ":comms",
         ":logsink",
         ":policy",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -438,7 +438,6 @@ target_link_libraries(sandbox2_monitor_base
   PUBLIC  absl::status
           absl::statusor
           absl::synchronization
-          sandbox2::client_proto
           sandbox2::comms
           sandbox2::executor
           sandbox2::fork_client
@@ -475,7 +474,6 @@ target_link_libraries(sandbox2_monitor_ptrace
           sapi::fileops
           sapi::status
           sandbox2::client
-          sandbox2::client_proto
           sandbox2::comms
           sandbox2::result
           sandbox2::sanitizer
@@ -517,7 +515,6 @@ target_link_libraries(sandbox2_monitor_unotify
           sapi::base
           sandbox2::bpf_evaluator
           sandbox2::client
-          sandbox2::client_proto
           sandbox2::flags
           sandbox2::forkserver_proto
           sandbox2::seccomp_unotify
@@ -572,20 +569,6 @@ 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
@@ -608,7 +591,6 @@ 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,7 +50,6 @@
 #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"
@@ -190,7 +189,6 @@ std::string Client::GetFdMapEnvVar() const {
 }
 
 void Client::PrepareEnvironment(int* preserved_fd) {
-  ReceiveClientConfig();
   SetUpIPC(preserved_fd);
   SetUpCwd();
 }
@@ -206,19 +204,41 @@ void Client::SandboxMeHere() {
 }
 
 void Client::SetUpCwd() {
-  // Receive the user-supplied current working directory and change into it.
-  if (client_config_.cwd().empty()) {
-    return;
+  {
+    // 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");
+    }
   }
-  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());
+
+  // 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());
+    }
   }
 }
 
@@ -294,12 +314,9 @@ void Client::SetUpIPC(int* preserved_fd) {
 }
 
 void Client::ReceivePolicy() {
-  SAPI_RAW_CHECK(comms_->RecvBytes(&policy_), "receive bytes");
-}
-
-void Client::ReceiveClientConfig() {
-  SAPI_RAW_CHECK(comms_->RecvProtoBuf(&client_config_),
-                 "receive client config");
+  std::vector<uint8_t> bytes;
+  SAPI_RAW_CHECK(comms_->RecvBytes(&bytes), "receive bytes");
+  policy_ = std::move(bytes);
 }
 
 void Client::ApplyPolicyAndBecomeTracee() {
@@ -308,13 +325,18 @@ void Client::ApplyPolicyAndBecomeTracee() {
   // this function does nothing.
   sanitizer::WaitForSanitizer();
 
-  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");
-    }
+  // 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");
   }
 
   SAPI_RAW_CHECK(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == 0,
@@ -342,9 +364,7 @@ void Client::ApplyPolicyAndBecomeTracee() {
   uint32_t message;  // wait for confirmation
   SAPI_RAW_CHECK(comms_->RecvUint32(&message),
                  "receving confirmation from executor");
-  SAPI_RAW_CHECK(message == kSandbox2MonitorReady,
-                 "invalid confirmation from executor");
-  bool allow_speculation = client_config_.seccomp_allow_speculation();
+  bool allow_speculation = message & kAllowSpeculationBit;
   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,
@@ -361,13 +381,12 @@ void Client::ApplyPolicyAndBecomeTracee() {
   }
   uint32_t seccomp_extra_flags =
       allow_speculation ? SECCOMP_FILTER_FLAG_SPEC_ALLOW : 0;
-  if (client_config_.monitor_type() == ClientConfig::CLIENT_MONITOR_UNOTIFY) {
+  uint32_t monitor_type = message & kMonitorTypeMask;
+  if (monitor_type == kSandbox2ClientUnotify) {
     InitSeccompUnotify(prog, comms_, seccomp_extra_flags);
   } else {
-    SAPI_RAW_CHECK(
-        client_config_.monitor_type() == ClientConfig::CLIENT_MONITOR_PTRACE,
-        absl::StrCat("invalid monitor type: ", client_config_.monitor_type())
-            .c_str());
+    SAPI_RAW_CHECK(monitor_type == kSandbox2ClientPtrace,
+                   "invalid confirmation from executor");
     InitSeccompRegular(prog, seccomp_extra_flags);
   }
 }

sandboxed_api/sandbox2/client.h

@@ -27,7 +27,6 @@
 #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"
@@ -39,8 +38,12 @@ class Client {
   // Client is ready to be sandboxed.
   static constexpr uint32_t kClient2SandboxReady = 0x0A0B0C01;
 
-  // Sandbox is ready to monitor the sandboxee.
-  static constexpr uint32_t kSandbox2MonitorReady = 0x0A0B0D01;
+  // 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;
 
   // Sandboxee can proceed after seccomp_unotify setup as limits have been
   // applied by the monitor.
@@ -94,9 +97,6 @@ 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,9 +130,6 @@ 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

@@ -220,14 +220,14 @@ void MonitorBase::Launch() {
     SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_NOTIFY);
     return;
   }
-  if (!InitSendClientConfig()) {
-    SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_CLIENT_CONFIG);
-    return;
-  }
   if (!InitSendIPC()) {
     SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_IPC);
     return;
   }
+  if (!InitSendCwd()) {
+    SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_CWD);
+    return;
+  }
   if (!InitSendPolicy()) {
     SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_POLICY);
     return;
@@ -267,8 +267,12 @@ absl::Status MonitorBase::SendPolicy(const std::vector<sock_filter>& policy) {
   return absl::OkStatus();
 }
 
-bool MonitorBase::SendMonitorReadyMessage() {
-  return comms_->SendUint32(Client::kSandbox2MonitorReady);
+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::InitSendPolicy() {
@@ -283,6 +287,15 @@ 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;
@@ -322,21 +335,6 @@ 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() {