Linux sandbox: add a new low-level broker process mechanism.

sandbox/linux/services/broker_process.cc

+#include "sandbox/linux/services/broker_process.h"
+
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <algorithm>
+#include <string>
+#include <vector>
+
+#include "base/logging.h"
+#include "base/pickle.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/posix/unix_domain_socket.h"
+
+namespace {
+
+// Check whether |requested_filename| is in |allowed_file_names|.
+// For paranoia, if file_to_open is not NULL, we will return the matching
+// string for the white list.

[Jorge Lucangeli Obes, 2012/12/13 02:14:54]

from?

[jln (very slow on Chromium), 2012/12/13 02:33:14]

Done.

+// Even if an attacker manager to fool the string comparison mechanism, we

[Jorge Lucangeli Obes, 2012/12/13 02:14:54]

managed?

[jln (very slow on Chromium), 2012/12/13 02:33:14]

Done.

+// would not open an attacker-controlled file name.
+bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names,
+                            const std::string& requested_filename,
+                            std::string* file_to_open) {
+  std::vector<std::string>::const_iterator it;
+  it = std::find(allowed_file_names.begin(), allowed_file_names.end(),
+                 requested_filename);
+  if (it < allowed_file_names.end()) {  // requested_filename was found?
+    if (file_to_open)
+      *file_to_open = *it;
+    return true;
+  }
+  return false;
+}
+
+bool HandleOpenRequest(int fd,
+                       const std::vector<std::string>& allowed_file_names,
+                       const Pickle& read_pickle, PickleIterator iter) {
+  std::string requested_filename;
+  int flags = 0;
+  if (!read_pickle.ReadString(&iter, &requested_filename) ||
+      !read_pickle.ReadInt(&iter, &flags)) {
+    return -1;
+  }
+
+  Pickle write_pickle;
+  std::vector<int> opened_files;
+  std::string file_to_open;
+  bool file_is_in_whitelist = GetFileNameInWhitelist(allowed_file_names,
+                                                     requested_filename,
+                                                     &file_to_open);
+  if (file_is_in_whitelist &&  // requested_filename is in the whitelist?
+      flags == O_RDONLY) {
+    int opened_fd = open(file_to_open.c_str(), O_RDONLY);
+    if (opened_fd < 0) {
+      write_pickle.WriteInt(-errno);
+    } else {
+      // Success.
+      opened_files.push_back(opened_fd);
+      write_pickle.WriteInt(0);
+    }
+  } else {
+    write_pickle.WriteInt(-EPERM);
+  }
+
+  const size_t kMaxMessageLength = sandbox::BrokerProcess::kMaxMessageLength;
+  CHECK_LE(write_pickle.size(), kMaxMessageLength);
+  ssize_t sent = UnixDomainSocket::SendMsg(fd, write_pickle.data(),
+                                           write_pickle.size(), opened_files);
+
+  // Close anything we have opened in this process.
+  for (std::vector<int>::iterator it = opened_files.begin();
+       it < opened_files.end(); ++it) {
+    (void) HANDLE_EINTR(close(*it));
+  }
+
+  if (sent <= 0) {
+    LOG(ERROR) << "Could not send IPC reply";
+    return false;
+  }
+  return true;
+}
+
+// Handle a request on the IPC channel FD.
+// A request should have a file descriptor attached on which we will reply and
+// that we will then close.
+// A request should start with an int that will be used as the command type.
+bool HandleRequest(int fd,
+                   const std::vector<std::string>& allowed_file_names) {
+
+  std::vector<int> fds;
+  char buf[sandbox::BrokerProcess::kMaxMessageLength];
+  errno = 0;
+  const ssize_t msg_len = UnixDomainSocket::RecvMsg(fd, buf,
+                                                    sizeof(buf), &fds);
+
+  if (msg_len == 0 || (msg_len == -1 && errno == ECONNRESET)) {
+    // EOF from our parent, or our parent died, we should die.
+    _exit(0);
+  }
+
+  // The parent should send exactly one file descriptor, on which we
+  // will write the reply.
+  if (msg_len < 0 || fds.size() != 1 || fds.at(0) < 0) {
+    PLOG(ERROR) << "Error reading message from the parent.";
+    return false;
+  }
+
+  Pickle pickle(buf, msg_len);
+  PickleIterator iter(pickle);
+  int command_type;
+  if (pickle.ReadInt(&iter, &command_type)) {
+    bool r = false;
+    // Go through all the possible IPC messages.
+    switch (command_type) {
+      case sandbox::BrokerProcess::kCommandOpen:
+        // We reply on the file descriptor sent to us via the IPC channel.
+        r = HandleOpenRequest(fds.at(0), allowed_file_names,
+                              pickle, iter);
+        // Close the temporary reply channel.
+        (void) HANDLE_EINTR(close(fds.at(0)));
+        return r;
+      default:
+        NOTREACHED();
+        return false;
+    }
+  }
+
+  LOG(ERROR) << "Error parsing Open request";

[Jorge Lucangeli Obes, 2012/12/13 02:14:54]

This should be a generic error right? It lives outside of the case: clause. E.g.
"Error parsing request"

[jln (very slow on Chromium), 2012/12/13 02:33:14]

Good point, done.

+  return false;
+}
+
+}  // namespace
+
+namespace sandbox {
+
+BrokerProcess::BrokerProcess(const std::vector<std::string>& allowed_file_names)
+    : initialized_(false),
+      is_child_(false),
+      fast_check_in_client_(true),
+      broker_pid_(-1),
+      allowed_file_names_(allowed_file_names),
+      ipc_socketpair_(-1) {
+}
+
+BrokerProcess::BrokerProcess(const std::vector<std::string>& allowed_file_names,
+                             bool fast_check_in_client)
+    : initialized_(false),
+      is_child_(false),
+      fast_check_in_client_(fast_check_in_client),
+      broker_pid_(-1),
+      allowed_file_names_(allowed_file_names),
+      ipc_socketpair_(-1) {
+}
+
+BrokerProcess::~BrokerProcess() {
+  if (initialized_ && ipc_socketpair_ != -1) {
+    void (HANDLE_EINTR(close(ipc_socketpair_)));
+  }
+}
+
+bool BrokerProcess::Init(void* sandbox_callback) {
+  CHECK(!initialized_);
+  CHECK_EQ(sandbox_callback, (void*) NULL) <<
+      "sandbox_callback is not implemented";
+  int socket_pair[2];
+  if (socketpair(AF_UNIX, SOCK_STREAM, 0, socket_pair)) {
+    LOG(ERROR) << "Failed to create socketpair";
+    return false;
+  }
+
+  int child_pid = fork();
+  if (child_pid == -1) {
+     (void) HANDLE_EINTR(close(socket_pair[0]));
+     (void) HANDLE_EINTR(close(socket_pair[1]));
+      return false;
+  }
+  if (child_pid) {
+    // We are the parent and we have just forked our broker process.
+    (void) HANDLE_EINTR(close(socket_pair[1]));
+    // We should only be able to write to the IPC channel. We'll always send
+    // a new file descriptor to receive the reply on.
+    shutdown(socket_pair[0], SHUT_RD);
+    ipc_socketpair_ = socket_pair[0];
+    initialized_ = true;
+    is_child_ = false;
+    broker_pid_ = child_pid;
+
+  } else {
+    // We are the broker.
+    (void) HANDLE_EINTR(close(socket_pair[0]));
+    // We should only be able to read from this IPC channel. We will send our
+    // replies on a new file descriptor attached to the requests.
+    shutdown(socket_pair[1], SHUT_WR);
+    ipc_socketpair_ = socket_pair[1];
+    initialized_ = true;
+    is_child_ = true;
+    // TODO(jln): activate a sandbox here.
+    for (;;) {
+      HandleRequest(ipc_socketpair_, allowed_file_names_);
+    }
+  }
+
+  initialized_ = true;
+  return true;
+}
+
+int BrokerProcess::Open(const char* pathname, int flags) const {
+  CHECK(initialized_);
+  // There is no point in forwarding a request that we know will be denied.
+  // Of course, the real security check needs to be on the other side of the
+  // IPC.
+  if (fast_check_in_client_) {
+    if(!GetFileNameInWhitelist(allowed_file_names_, pathname, NULL) ||
+       flags != O_RDONLY) {
+      return -EPERM;
+    }
+  }
+
+  Pickle write_pickle;
+  write_pickle.WriteInt(kCommandOpen);
+  write_pickle.WriteString(pathname);
+  write_pickle.WriteInt(flags);
+  const size_t kMaxMessageLength = sandbox::BrokerProcess::kMaxMessageLength;
+  CHECK_LE(write_pickle.size(), kMaxMessageLength);
+
+  int returned_fd = -1;
+  uint8_t reply_buf[kMaxMessageLength];
+  // Send a request (in write_pickle) as well as a new temporary socketpair.

[Jorge Lucangeli Obes, 2012/12/13 02:14:54]

Maybe clarify that SendRecvMsg takes care of creating the temp socketpair? It
was very confusing at first.

[jln (very slow on Chromium), 2012/12/13 02:33:14]

Done.

+  // Then read the reply on this new socketpair in reply_buf and put an
+  // eventual attached file descriptor in returned_fd.
+  // TODO(jln): this API needs some rewriting and documentation.
+  ssize_t msg_len = UnixDomainSocket::SendRecvMsg(ipc_socketpair_,
+                                                  reply_buf,
+                                                  sizeof(reply_buf),
+                                                  &returned_fd,
+                                                  write_pickle);
+  if (msg_len <= 0) {
+    LOG(ERROR) << "Could not make request to broker process";
+    return -ENOMEM;
+  }
+
+  Pickle read_pickle(reinterpret_cast<char*>(reply_buf), msg_len);
+  PickleIterator iter(read_pickle);
+  int return_value = -1;
+  // Now deserialize the return value and eventually return the file
+  // descriptor.
+  if (read_pickle.ReadInt(&iter, &return_value)) {
+    if (return_value < 0) {
+      CHECK_EQ(returned_fd, -1);
+      return return_value;
+    } else {
+      // We have a real file descriptor to return.
+      CHECK_GE(returned_fd, 0);
+      return returned_fd;
+    }
+  } else {
+    LOG(ERROR) << "Could not read pickle";
+    return -1;
+  }
+}
+
+}  // namespace sandbox.