Linux sandbox: add access() brokering to the async-signal safe broker process.

We add support for access() that is consistent with our support for open().

BUG=232077,232068
TEST=sandbox_linux_unittests
NOTRY=true

Review URL: https://codereview.chromium.org/14301014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195945 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 305 insertions, 55 deletions

diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
index 2d775f4..b60cc14 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
@@ -693,6 +693,9 @@ intptr_t BrokerOpenTrapHandler(const struct arch_seccomp_data& args,
   BPF_ASSERT(aux);
   BrokerProcess* broker_process = static_cast<BrokerProcess*>(aux);
   switch(args.nr) {
+    case __NR_access:
+      return broker_process->Access(reinterpret_cast<const char*>(args.args[0]),
+          static_cast<int>(args.args[1]));
     case __NR_open:
       return broker_process->Open(reinterpret_cast<const char*>(args.args[0]),
           static_cast<int>(args.args[1]));
@@ -715,6 +718,7 @@ ErrorCode DenyOpenPolicy(Sandbox *sandbox, int sysno, void *aux) {
   }
 
   switch (sysno) {
+    case __NR_access:
     case __NR_open:
     case __NR_openat:
       // We get a InitializedOpenBroker class, but our trap handler wants
@@ -736,7 +740,9 @@ BPF_TEST(SandboxBpf, UseOpenBroker, DenyOpenPolicy,
 
   // First, use the broker "manually"
   BPF_ASSERT(broker_process->Open("/proc/denied", O_RDONLY) == -EPERM);
+  BPF_ASSERT(broker_process->Access("/proc/denied", R_OK) == -EPERM);
   BPF_ASSERT(broker_process->Open("/proc/allowed", O_RDONLY) == -ENOENT);
+  BPF_ASSERT(broker_process->Access("/proc/allowed", R_OK) == -ENOENT);
 
   // Now use glibc's open() as an external library would.
   BPF_ASSERT(open("/proc/denied", O_RDONLY) == -1);
@@ -753,8 +759,16 @@ BPF_TEST(SandboxBpf, UseOpenBroker, DenyOpenPolicy,
   BPF_ASSERT(openat(AT_FDCWD, "/proc/allowed", O_RDONLY) == -1);
   BPF_ASSERT(errno == ENOENT);
 
+  // And test glibc's access().
+  BPF_ASSERT(access("/proc/denied", R_OK) == -1);
+  BPF_ASSERT(errno == EPERM);
+
+  BPF_ASSERT(access("/proc/allowed", R_OK) == -1);
+  BPF_ASSERT(errno == ENOENT);
 
   // This is also white listed and does exist.
+  int cpu_info_access = access("/proc/cpuinfo", R_OK);
+  BPF_ASSERT(cpu_info_access == 0);
   int cpu_info_fd = open("/proc/cpuinfo", O_RDONLY);
   BPF_ASSERT(cpu_info_fd >= 0);
   char buf[1024];
diff --git a/sandbox/linux/services/broker_process.cc b/sandbox/linux/services/broker_process.cc
index ea71b8f..b486ca3 100644
--- a/sandbox/linux/services/broker_process.cc
+++ b/sandbox/linux/services/broker_process.cc
@@ -22,7 +22,6 @@
 
 namespace {
 
-static const int kCommandOpen = 'O';
 static const size_t kMaxMessageLength = 4096;
 
 // Some flags will need special treatment on the client side and are not
@@ -32,7 +31,7 @@ int ForCurrentProcessFlagsMask() {
 }
 
 // Check whether |requested_filename| is in |allowed_file_names|.
-// See GetFileNameIfAllowedAccess() for an explaination of |file_to_open|.
+// See GetFileNameIfAllowedToOpen() for an explanation of |file_to_open|.
 // async signal safe if |file_to_open| is NULL.
 // TODO(jln): assert signal safety.
 bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names,
@@ -164,21 +163,40 @@ bool BrokerProcess::Init(bool (*sandbox_callback)(void)) {
   NOTREACHED();
 }
 
-// This function needs to be async signal safe.
+int BrokerProcess::Access(const char* pathname, int mode) const {
+  return PathAndFlagsSyscall(kCommandAccess, pathname, mode);
+}
+
 int BrokerProcess::Open(const char* pathname, int flags) const {
+  return PathAndFlagsSyscall(kCommandOpen, pathname, flags);
+}
+
+// Make a remote system call over IPC for syscalls that take a path and flags
+// as arguments, currently open() and access().
+// Will return -errno like a real system call.
+// This function needs to be async signal safe.
+int BrokerProcess::PathAndFlagsSyscall(enum IPCCommands syscall_type,
+                                       const char* pathname, int flags) const {
   RAW_CHECK(initialized_);  // async signal safe CHECK().
+  RAW_CHECK(syscall_type == kCommandOpen || syscall_type == kCommandAccess);
   if (!pathname)
     return -EFAULT;
   // 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 (!GetFileNameIfAllowedAccess(pathname, flags, NULL))
+    if (syscall_type == kCommandOpen &&
+        !GetFileNameIfAllowedToOpen(pathname, flags, NULL)) {
+      return -EPERM;
+    }
+    if (syscall_type == kCommandAccess &&
+        !GetFileNameIfAllowedToAccess(pathname, flags, NULL)) {
       return -EPERM;
+    }
   }
 
   Pickle write_pickle;
-  write_pickle.WriteInt(kCommandOpen);
+  write_pickle.WriteInt(syscall_type);
   write_pickle.WriteString(pathname);
   write_pickle.WriteInt(flags);
   RAW_CHECK(write_pickle.size() <= kMaxMessageLength);
@@ -189,7 +207,6 @@ int BrokerProcess::Open(const char* pathname, int flags) const {
   // temporary socketpair (created internally by SendRecvMsg()).
   // 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),
@@ -207,17 +224,28 @@ int BrokerProcess::Open(const char* pathname, int flags) const {
   // Now deserialize the return value and eventually return the file
   // descriptor.
   if (read_pickle.ReadInt(&iter, &return_value)) {
-    if (return_value < 0) {
-      RAW_CHECK(returned_fd == -1);
-      return return_value;
-    } else {
-      // We have a real file descriptor to return.
-      RAW_CHECK(returned_fd >= 0);
-      return returned_fd;
+    switch (syscall_type) {
+      case kCommandAccess:
+        // We should never have a fd to return.
+        RAW_CHECK(returned_fd == -1);
+        return return_value;
+      case kCommandOpen:
+        if (return_value < 0) {
+          RAW_CHECK(returned_fd == -1);
+          return return_value;
+        } else {
+          // We have a real file descriptor to return.
+          RAW_CHECK(returned_fd >= 0);
+          return returned_fd;
+        }
+      default:
+        RAW_LOG(ERROR, "Unsupported command");
+        return -ENOSYS;
     }
   } else {
     RAW_LOG(ERROR, "Could not read pickle");
-    return -1;
+    NOTREACHED();
+    return -EPERM;
   }
 }
 
@@ -254,26 +282,33 @@ bool BrokerProcess::HandleRequest() const {
     bool r = false;
     // Go through all the possible IPC messages.
     switch (command_type) {
+      case kCommandAccess:
       case kCommandOpen:
         // We reply on the file descriptor sent to us via the IPC channel.
-        r = HandleOpenRequest(temporary_ipc, pickle, iter);
-        (void) HANDLE_EINTR(close(temporary_ipc));
-        return r;
+        r = HandleRemoteCommand(static_cast<IPCCommands>(command_type),
+                                temporary_ipc, pickle, iter);
+        break;
       default:
         NOTREACHED();
-        return false;
+        r = false;
+        break;
     }
+    int ret = HANDLE_EINTR(close(temporary_ipc));
+    DCHECK(!ret) << "Could not close temporary IPC channel";
+    return r;
   }
 
   LOG(ERROR) << "Error parsing IPC request";
   return false;
 }
 
-// Handle an open request contained in |read_pickle| and send the reply
+// Handle a |command_type| request contained in |read_pickle| and send the reply
 // on |reply_ipc|.
-bool BrokerProcess::HandleOpenRequest(int reply_ipc,
-                                      const Pickle& read_pickle,
-                                      PickleIterator iter) const {
+// Currently kCommandOpen and kCommandAccess are supported.
+bool BrokerProcess::HandleRemoteCommand(IPCCommands command_type, int reply_ipc,
+                                        const Pickle& read_pickle,
+                                        PickleIterator iter) const {
+  // Currently all commands have two arguments: filename and flags.
   std::string requested_filename;
   int flags = 0;
   if (!read_pickle.ReadString(&iter, &requested_filename) ||
@@ -284,8 +319,67 @@ bool BrokerProcess::HandleOpenRequest(int reply_ipc,
   Pickle write_pickle;
   std::vector<int> opened_files;
 
+  switch (command_type) {
+    case kCommandAccess:
+      AccessFileForIPC(requested_filename, flags, &write_pickle);
+      break;
+    case kCommandOpen:
+      OpenFileForIPC(requested_filename, flags, &write_pickle, &opened_files);
+      break;
+    default:
+      LOG(ERROR) << "Invalid IPC command";
+      break;
+  }
+
+  CHECK_LE(write_pickle.size(), kMaxMessageLength);
+  ssize_t sent = UnixDomainSocket::SendMsg(reply_ipc, 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) {
+    int ret = HANDLE_EINTR(close(*it));
+    DCHECK(!ret) << "Could not close file descriptor";
+  }
+
+  if (sent <= 0) {
+    LOG(ERROR) << "Could not send IPC reply";
+    return false;
+  }
+  return true;
+}
+
+// Perform access(2) on |requested_filename| with mode |mode| if allowed by our
+// policy. Write the syscall return value (-errno) to |write_pickle|.
+void BrokerProcess::AccessFileForIPC(const std::string& requested_filename,
+                                     int mode, Pickle* write_pickle) const {
+  DCHECK(write_pickle);
+  const char* file_to_access = NULL;
+  const bool safe_to_access_file = GetFileNameIfAllowedToAccess(
+      requested_filename.c_str(), mode, &file_to_access);
+  if (safe_to_access_file) {
+    CHECK(file_to_access);
+    int access_ret = access(file_to_access, mode);
+    int access_errno = errno;
+    if (!access_ret)
+      write_pickle->WriteInt(0);
+    else
+      write_pickle->WriteInt(-access_errno);
+  } else {
+    write_pickle->WriteInt(-EPERM);
+  }
+}
+
+// Open |requested_filename| with |flags| if allowed by our policy.
+// Write the syscall return value (-errno) to |write_pickle| and append
+// a file descriptor to |opened_files| if relevant.
+void BrokerProcess::OpenFileForIPC(const std::string& requested_filename,
+                                   int flags, Pickle* write_pickle,
+                                   std::vector<int>* opened_files) const {
+  DCHECK(write_pickle);
+  DCHECK(opened_files);
   const char* file_to_open = NULL;
-  const bool safe_to_open_file = GetFileNameIfAllowedAccess(
+  const bool safe_to_open_file = GetFileNameIfAllowedToOpen(
       requested_filename.c_str(), flags, &file_to_open);
 
   if (safe_to_open_file) {
@@ -297,40 +391,72 @@ bool BrokerProcess::HandleOpenRequest(int reply_ipc,
     // hurt to always pass a third argument though.
     int opened_fd = open(file_to_open, flags | O_CLOEXEC, 0);
     if (opened_fd < 0) {
-      write_pickle.WriteInt(-errno);
+      write_pickle->WriteInt(-errno);
     } else {
       // Success.
-      opened_files.push_back(opened_fd);
-      write_pickle.WriteInt(0);
+      opened_files->push_back(opened_fd);
+      write_pickle->WriteInt(0);
     }
   } else {
-    write_pickle.WriteInt(-EPERM);
+    write_pickle->WriteInt(-EPERM);
   }
+}
 
-  CHECK_LE(write_pickle.size(), kMaxMessageLength);
-  ssize_t sent = UnixDomainSocket::SendMsg(reply_ipc, 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";
+// Check if calling access() should be allowed on |requested_filename| with
+// mode |requested_mode|.
+// Note: access() being a system call to check permissions, this can get a bit
+// confusing. We're checking if calling access() should even be allowed with
+// the same policy we would use for open().
+// If |file_to_access| is not NULL, we will return the matching pointer from
+// the whitelist. For paranoia a caller should then use |file_to_access|. See
+// GetFileNameIfAllowedToOpen() fore more explanation.
+// return true if calling access() on this file should be allowed, false
+// otherwise.
+// Async signal safe if and only if |file_to_access| is NULL.
+bool BrokerProcess::GetFileNameIfAllowedToAccess(const char* requested_filename,
+    int requested_mode, const char** file_to_access) const {
+  // First, check if |requested_mode| is existence, ability to read or ability
+  // to write. We do not support X_OK.
+  if (requested_mode != F_OK &&
+      requested_mode & ~(R_OK | W_OK)) {
     return false;
   }
-  return true;
+  switch (requested_mode) {
+    case F_OK:
+      // We allow to check for file existence if we can either read or write.
+      return GetFileNameInWhitelist(allowed_r_files_, requested_filename,
+                                    file_to_access) ||
+             GetFileNameInWhitelist(allowed_w_files_, requested_filename,
+                                    file_to_access);
+    case R_OK:
+      return GetFileNameInWhitelist(allowed_r_files_, requested_filename,
+                                    file_to_access);
+    case W_OK:
+      return GetFileNameInWhitelist(allowed_w_files_, requested_filename,
+                                    file_to_access);
+    case R_OK | W_OK:
+    {
+      bool allowed_for_read_and_write =
+          GetFileNameInWhitelist(allowed_r_files_, requested_filename, NULL) &&
+          GetFileNameInWhitelist(allowed_w_files_, requested_filename,
+                                 file_to_access);
+      return allowed_for_read_and_write;
+    }
+    default:
+      return false;
+  }
 }
 
-// For paranoia, if |file_to_open| is not NULL, we will return the matching
-// string from the white list.
-// Async signal safe only if |file_to_open| is NULL.
-// Even if an attacker managed to fool the string comparison mechanism, we
-// would not open an attacker-controlled file name.
-// Return true if access should be allowed, false otherwise.
-bool BrokerProcess::GetFileNameIfAllowedAccess(const char* requested_filename,
+// Check if |requested_filename| can be opened with flags |requested_flags|.
+// If |file_to_open| is not NULL, we will return the matching pointer from the
+// whitelist. For paranoia, a caller should then use |file_to_open| rather
+// than |requested_filename|, so that it never attempts to open an
+// attacker-controlled file name, even if an attacker managed to fool the
+// string comparison mechanism.
+// Return true if opening should be allowed, false otherwise.
+// Async signal safe if and only if |file_to_open| is NULL.
+bool BrokerProcess::GetFileNameIfAllowedToOpen(const char* requested_filename,
     int requested_flags, const char** file_to_open) const {
   if (!IsAllowedOpenFlags(requested_flags)) {
     return false;
diff --git a/sandbox/linux/services/broker_process.h b/sandbox/linux/services/broker_process.h
index d04f703..b7bb239 100644
--- a/sandbox/linux/services/broker_process.h
+++ b/sandbox/linux/services/broker_process.h
@@ -41,6 +41,11 @@ class BrokerProcess {
   // sandbox in the broker.
   bool Init(bool (*sandbox_callback)(void));
 
+  // Can be used in place of access(). Will be async signal safe.
+  // X_OK will always EPERM in practice since the broker process doesn't support
+  // execute permissions.
+  // It's similar to the access() system call and will return -errno on errors.
+  int Access(const char* pathname, int mode) const;
   // Can be used in place of open(). Will be async signal safe.
   // The implementation only supports certain white listed flags and will
   // return -EPERM on other flags.
@@ -50,10 +55,24 @@ class BrokerProcess {
   int broker_pid() const { return broker_pid_; }
 
  private:
+  enum IPCCommands {
+    kCommandInvalid = 0,
+    kCommandOpen,
+    kCommandAccess,
+  };
+  int PathAndFlagsSyscall(enum IPCCommands command_type,
+                          const char* pathname, int flags) const;
   bool HandleRequest() const;
-  bool HandleOpenRequest(int reply_ipc, const Pickle& read_pickle,
-                         PickleIterator iter) const;
-  bool GetFileNameIfAllowedAccess(const char*, int, const char**) const;
+  bool HandleRemoteCommand(IPCCommands command_type, int reply_ipc,
+      const Pickle& read_pickle, PickleIterator iter) const;
+
+  void AccessFileForIPC(const std::string& requested_filename,
+                        int mode, Pickle* write_pickle) const;
+  void OpenFileForIPC(const std::string& requested_filename,
+                      int flags, Pickle* write_pickle,
+                      std::vector<int>* opened_files) const;
+  bool GetFileNameIfAllowedToAccess(const char*, int, const char**) const;
+  bool GetFileNameIfAllowedToOpen(const char*, int, const char**) const;
   bool initialized_;  // Whether we've been through Init() yet.
   bool is_child_;  // Whether we're the child (broker process).
   bool fast_check_in_client_;  // Whether to forward a request that we know
diff --git a/sandbox/linux/services/broker_process_unittest.cc b/sandbox/linux/services/broker_process_unittest.cc
index ab57035..ae14ad7 100644
--- a/sandbox/linux/services/broker_process_unittest.cc
+++ b/sandbox/linux/services/broker_process_unittest.cc
@@ -45,23 +45,30 @@ TEST(BrokerProcess, CreateAndDestroy) {
   ASSERT_EQ(WEXITSTATUS(status), 0);
 }
 
-TEST(BrokerProcess, TestOpenNull) {
+TEST(BrokerProcess, TestOpenAccessNull) {
   const std::vector<std::string> empty;
   BrokerProcess open_broker(empty, empty);
   ASSERT_TRUE(open_broker.Init(NULL));
 
   int fd = open_broker.Open(NULL, O_RDONLY);
   ASSERT_EQ(fd, -EFAULT);
+
+  int ret = open_broker.Access(NULL, F_OK);
+  ASSERT_EQ(ret, -EFAULT);
 }
 
 void TestOpenFilePerms(bool fast_check_in_client) {
   const char kR_WhiteListed[] = "/proc/DOESNOTEXIST1";
+  // We can't debug the init process, and shouldn't be able to access
+  // its auxv file.
+  const char kR_WhiteListedButDenied[] = "/proc/1/auxv";
   const char kW_WhiteListed[] = "/proc/DOESNOTEXIST2";
   const char kRW_WhiteListed[] = "/proc/DOESNOTEXIST3";
   const char k_NotWhitelisted[] = "/proc/DOESNOTEXIST4";
 
   std::vector<std::string> read_whitelist;
   read_whitelist.push_back(kR_WhiteListed);
+  read_whitelist.push_back(kR_WhiteListedButDenied);
   read_whitelist.push_back(kRW_WhiteListed);
 
   std::vector<std::string> write_whitelist;
@@ -80,6 +87,41 @@ void TestOpenFilePerms(bool fast_check_in_client) {
   ASSERT_EQ(fd, -EPERM);
   fd = open_broker.Open(kR_WhiteListed, O_RDWR);
   ASSERT_EQ(fd, -EPERM);
+  int ret = -1;
+  ret = open_broker.Access(kR_WhiteListed, F_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kR_WhiteListed, R_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kR_WhiteListed, W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kR_WhiteListed, R_OK | W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kR_WhiteListed, X_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kR_WhiteListed, R_OK | X_OK);
+  ASSERT_EQ(ret, -EPERM);
+
+  fd = open_broker.Open(kR_WhiteListedButDenied, O_RDONLY);
+  // The broker process will allow this, but the normal permission system
+  // won't.
+  ASSERT_EQ(fd, -EACCES);
+  fd = open_broker.Open(kR_WhiteListedButDenied, O_WRONLY);
+  ASSERT_EQ(fd, -EPERM);
+  fd = open_broker.Open(kR_WhiteListedButDenied, O_RDWR);
+  ASSERT_EQ(fd, -EPERM);
+  ret = open_broker.Access(kR_WhiteListedButDenied, F_OK);
+  // The normal permission system will let us check that the file exist.
+  ASSERT_EQ(ret, 0);
+  ret = open_broker.Access(kR_WhiteListedButDenied, R_OK);
+  ASSERT_EQ(ret, -EACCES);
+  ret = open_broker.Access(kR_WhiteListedButDenied, W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kR_WhiteListedButDenied, R_OK | W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kR_WhiteListedButDenied, X_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kR_WhiteListedButDenied, R_OK | X_OK);
+  ASSERT_EQ(ret, -EPERM);
 
   fd = open_broker.Open(kW_WhiteListed, O_RDONLY);
   ASSERT_EQ(fd, -EPERM);
@@ -87,6 +129,18 @@ void TestOpenFilePerms(bool fast_check_in_client) {
   ASSERT_EQ(fd, -ENOENT);
   fd = open_broker.Open(kW_WhiteListed, O_RDWR);
   ASSERT_EQ(fd, -EPERM);
+  ret = open_broker.Access(kW_WhiteListed, F_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kW_WhiteListed, R_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kW_WhiteListed, W_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kW_WhiteListed, R_OK | W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kW_WhiteListed, X_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kW_WhiteListed, R_OK | X_OK);
+  ASSERT_EQ(ret, -EPERM);
 
   fd = open_broker.Open(kRW_WhiteListed, O_RDONLY);
   ASSERT_EQ(fd, -ENOENT);
@@ -94,6 +148,18 @@ void TestOpenFilePerms(bool fast_check_in_client) {
   ASSERT_EQ(fd, -ENOENT);
   fd = open_broker.Open(kRW_WhiteListed, O_RDWR);
   ASSERT_EQ(fd, -ENOENT);
+  ret = open_broker.Access(kRW_WhiteListed, F_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kRW_WhiteListed, R_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kRW_WhiteListed, W_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kRW_WhiteListed, R_OK | W_OK);
+  ASSERT_EQ(ret, -ENOENT);
+  ret = open_broker.Access(kRW_WhiteListed, X_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(kRW_WhiteListed, R_OK | X_OK);
+  ASSERT_EQ(ret, -EPERM);
 
   fd = open_broker.Open(k_NotWhitelisted, O_RDONLY);
   ASSERT_EQ(fd, -EPERM);
@@ -101,6 +167,19 @@ void TestOpenFilePerms(bool fast_check_in_client) {
   ASSERT_EQ(fd, -EPERM);
   fd = open_broker.Open(k_NotWhitelisted, O_RDWR);
   ASSERT_EQ(fd, -EPERM);
+  ret = open_broker.Access(k_NotWhitelisted, F_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(k_NotWhitelisted, R_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(k_NotWhitelisted, W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(k_NotWhitelisted, R_OK | W_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(k_NotWhitelisted, X_OK);
+  ASSERT_EQ(ret, -EPERM);
+  ret = open_broker.Access(k_NotWhitelisted, R_OK | X_OK);
+  ASSERT_EQ(ret, -EPERM);
+
 
   // We have some extra sanity check for clearly wrong values.
   fd = open_broker.Open(kRW_WhiteListed, O_RDONLY|O_WRONLY|O_RDWR);
@@ -142,6 +221,13 @@ void TestOpenCpuinfo(bool fast_check_in_client) {
   fd = open_broker->Open(kFileCpuInfo, O_RDWR);
   ASSERT_EQ(fd, -EPERM);
 
+  // Check we can read /proc/cpuinfo.
+  int can_access = open_broker->Access(kFileCpuInfo, R_OK);
+  ASSERT_EQ(can_access, 0);
+  can_access = open_broker->Access(kFileCpuInfo, W_OK);
+  ASSERT_EQ(can_access, -EPERM);
+  // Check we can not write /proc/cpuinfo.
+
   // Open cpuinfo via the broker.
   int cpuinfo_fd = open_broker->Open(kFileCpuInfo, O_RDONLY);
   ASSERT_GE(cpuinfo_fd, 0);
@@ -222,6 +308,10 @@ TEST(BrokerProcess, DISABLE_ON_ANDROID(OpenFileRW)) {
   BrokerProcess open_broker(whitelist, whitelist);
   ASSERT_TRUE(open_broker.Init(NULL));
 
+  // Check we can access that file with read or write.
+  int can_access = open_broker.Access(tempfile_name, R_OK | W_OK);
+  ASSERT_EQ(can_access, 0);
+
   int tempfile2 = -1;
   tempfile2 = open_broker.Open(tempfile_name, O_RDWR);
   ASSERT_GE(tempfile2, 0);
@@ -274,9 +364,10 @@ SANDBOX_TEST(BrokerProcess, BrokerDied) {
   SANDBOX_ASSERT(WTERMSIG(status) == SIGKILL);
   // Hopefully doing Open with a dead broker won't SIGPIPE us.
   SANDBOX_ASSERT(open_broker.Open("/proc/cpuinfo", O_RDONLY) == -ENOMEM);
+  SANDBOX_ASSERT(open_broker.Access("/proc/cpuinfo", O_RDONLY) == -ENOMEM);
 }
 
-void TestComplexFlags(bool fast_check_in_client) {
+void TestOpenComplexFlags(bool fast_check_in_client) {
   std::vector<std::string> whitelist;
   whitelist.push_back("/proc/cpuinfo");
 
@@ -296,14 +387,14 @@ void TestComplexFlags(bool fast_check_in_client) {
   ASSERT_EQ(open_broker.Open("/proc/cpuinfo", O_RDONLY | O_NONBLOCK), -EPERM);
 }
 
-TEST(BrokerProcess, ComplexFlagsWithClientCheck) {
-  TestComplexFlags(true /* fast_check_in_client */);
+TEST(BrokerProcess, OpenComplexFlagsWithClientCheck) {
+  TestOpenComplexFlags(true /* fast_check_in_client */);
   // Don't do anything here, so that ASSERT works in the subfunction as
   // expected.
 }
 
-TEST(BrokerProcess, ComplexFlagsNoClientCheck) {
-  TestComplexFlags(false /* fast_check_in_client */);
+TEST(BrokerProcess, OpenComplexFlagsNoClientCheck) {
+  TestOpenComplexFlags(false /* fast_check_in_client */);
   // Don't do anything here, so that ASSERT works in the subfunction as
   // expected.
 }