sandbox: Extend Broker to support improved file permissions

Added BrokerFilePermission class which is used to specify whitelists for the Broker

BUG=432369
TEST=sandbox_linux_unittest, chrome on Ubuntu 14.04 and 14.10

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

Cr-Commit-Position: refs/heads/master@{#305894}

14 files changed, 892 insertions, 188 deletions

diff --git a/sandbox/linux/BUILD.gn b/sandbox/linux/BUILD.gn
index be3dc6a..7648753 100644
--- a/sandbox/linux/BUILD.gn
+++ b/sandbox/linux/BUILD.gn
@@ -71,6 +71,7 @@ test("sandbox_linux_unittests") {
     "services/syscall_wrappers_unittest.cc",
     "services/thread_helpers_unittests.cc",
     "services/yama_unittests.cc",
+    "syscall_broker/broker_file_permission_unittest.cc",
     "syscall_broker/broker_process_unittest.cc",
     "tests/main.cc",
     "tests/scoped_temporary_file.cc",
@@ -237,6 +238,8 @@ component("sandbox_services") {
     "syscall_broker/broker_client.cc",
     "syscall_broker/broker_client.h",
     "syscall_broker/broker_common.h",
+    "syscall_broker/broker_file_permission.cc",
+    "syscall_broker/broker_file_permission.h",
     "syscall_broker/broker_host.cc",
     "syscall_broker/broker_host.h",
     "syscall_broker/broker_policy.cc",
diff --git a/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc b/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc
index e39cf79..e8e8a00 100644
--- a/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc
+++ b/sandbox/linux/bpf_dsl/bpf_dsl_more_unittest.cc
@@ -42,6 +42,7 @@
 #include "sandbox/linux/seccomp-bpf/trap.h"
 #include "sandbox/linux/services/linux_syscalls.h"
 #include "sandbox/linux/services/syscall_wrappers.h"
+#include "sandbox/linux/syscall_broker/broker_file_permission.h"
 #include "sandbox/linux/syscall_broker/broker_process.h"
 #include "sandbox/linux/tests/scoped_temporary_file.h"
 #include "sandbox/linux/tests/unit_tests.h"
@@ -778,12 +779,14 @@ bool NoOpCallback() {
 class InitializedOpenBroker {
  public:
   InitializedOpenBroker() : initialized_(false) {
-    std::vector<std::string> allowed_files;
-    allowed_files.push_back("/proc/allowed");
-    allowed_files.push_back("/proc/cpuinfo");
-
-    broker_process_.reset(new syscall_broker::BrokerProcess(
-        EPERM, allowed_files, std::vector<std::string>()));
+    std::vector<syscall_broker::BrokerFilePermission> permissions;
+    permissions.push_back(
+        syscall_broker::BrokerFilePermission::ReadOnly("/proc/allowed"));
+    permissions.push_back(
+        syscall_broker::BrokerFilePermission::ReadOnly("/proc/cpuinfo"));
+
+    broker_process_.reset(
+        new syscall_broker::BrokerProcess(EPERM, permissions));
     BPF_ASSERT(broker_process() != NULL);
     BPF_ASSERT(broker_process_->Init(base::Bind(&NoOpCallback)));
 
diff --git a/sandbox/linux/sandbox_linux.gypi b/sandbox/linux/sandbox_linux.gypi
index 7fa934f..89c830e 100644
--- a/sandbox/linux/sandbox_linux.gypi
+++ b/sandbox/linux/sandbox_linux.gypi
@@ -234,6 +234,8 @@
         'syscall_broker/broker_client.cc',
         'syscall_broker/broker_client.h',
         'syscall_broker/broker_common.h',
+        'syscall_broker/broker_file_permission.cc',
+        'syscall_broker/broker_file_permission.h',
         'syscall_broker/broker_host.cc',
         'syscall_broker/broker_host.h',
         'syscall_broker/broker_policy.cc',
diff --git a/sandbox/linux/sandbox_linux_test_sources.gypi b/sandbox/linux/sandbox_linux_test_sources.gypi
index 607664e..fe8b1635 100644
--- a/sandbox/linux/sandbox_linux_test_sources.gypi
+++ b/sandbox/linux/sandbox_linux_test_sources.gypi
@@ -21,6 +21,7 @@
     'services/syscall_wrappers_unittest.cc',
     'services/thread_helpers_unittests.cc',
     'services/yama_unittests.cc',
+    'syscall_broker/broker_file_permission_unittest.cc',
     'syscall_broker/broker_process_unittest.cc',
     'tests/main.cc',
     'tests/scoped_temporary_file.cc',
diff --git a/sandbox/linux/syscall_broker/broker_client.cc b/sandbox/linux/syscall_broker/broker_client.cc
index e63205f..b82ecb3 100644
--- a/sandbox/linux/syscall_broker/broker_client.cc
+++ b/sandbox/linux/syscall_broker/broker_client.cc
@@ -54,7 +54,9 @@ int BrokerClient::PathAndFlagsSyscall(IPCCommand syscall_type,
   // IPC.
   if (fast_check_in_client_) {
     if (syscall_type == COMMAND_OPEN &&
-        !broker_policy_.GetFileNameIfAllowedToOpen(pathname, flags, NULL)) {
+        !broker_policy_.GetFileNameIfAllowedToOpen(
+            pathname, flags, NULL /* file_to_open */,
+            NULL /* unlink_after_open */)) {
       return -broker_policy_.denied_errno();
     }
     if (syscall_type == COMMAND_ACCESS &&
diff --git a/sandbox/linux/syscall_broker/broker_file_permission.cc b/sandbox/linux/syscall_broker/broker_file_permission.cc
new file mode 100644
index 0000000..beceda9
--- /dev/null
+++ b/sandbox/linux/syscall_broker/broker_file_permission.cc
@@ -0,0 +1,243 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/linux/syscall_broker/broker_file_permission.h"
+
+#include <fcntl.h>
+#include <string.h>
+
+#include <string>
+
+#include "base/logging.h"
+#include "sandbox/linux/syscall_broker/broker_common.h"
+
+namespace sandbox {
+
+namespace syscall_broker {
+
+// Async signal safe
+bool BrokerFilePermission::ValidatePath(const char* path) {
+  if (!path)
+    return false;
+
+  const size_t len = strlen(path);
+  // No empty paths
+  if (len == 0)
+    return false;
+  // Paths must be absolute and not relative
+  if (path[0] != '/')
+    return false;
+  // No trailing / (but "/" is valid)
+  if (len > 1 && path[len - 1] == '/')
+    return false;
+  // No trailing /..
+  if (len >= 3 && path[len - 3] == '/' && path[len - 2] == '.' &&
+      path[len - 1] == '.')
+    return false;
+  // No /../ anywhere
+  for (size_t i = 0; i < len; i++) {
+    if (path[i] == '/' && (len - i) > 3) {
+      if (path[i + 1] == '.' && path[i + 2] == '.' && path[i + 3] == '/') {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+// Async signal safe
+// Calls std::string::c_str(), strncmp and strlen. All these
+// methods are async signal safe in common standard libs.
+// TODO(leecam): remove dependency on std::string
+bool BrokerFilePermission::MatchPath(const char* requested_filename) const {
+  const char* path = path_.c_str();
+  if ((recursive_ && strncmp(requested_filename, path, strlen(path)) == 0)) {
+    // Note: This prefix match will allow any path under the whitelisted
+    // path, for any number of directory levels. E.g. if the whitelisted
+    // path is /good/ then the following will be permitted by the policy.
+    //   /good/file1
+    //   /good/folder/file2
+    //   /good/folder/folder2/file3
+    // If an attacker could make 'folder' a symlink to ../../ they would have
+    // access to the entire filesystem.
+    // Whitelisting with multiple depths is useful, e.g /proc/ but
+    // the system needs to ensure symlinks can not be created!
+    // That said if an attacker can convert any of the absolute paths
+    // to a symlink they can control any file on the system also.
+    return true;
+  } else if (strcmp(requested_filename, path) == 0) {
+    return true;
+  }
+  return false;
+}
+
+// Async signal safe.
+// External call to std::string::c_str() is
+// called in MatchPath.
+// TODO(leecam): remove dependency on std::string
+bool BrokerFilePermission::CheckAccess(const char* requested_filename,
+                                       int mode,
+                                       const char** file_to_access) const {
+  // First, check if |mode| is existence, ability to read or ability
+  // to write. We do not support X_OK.
+  if (mode != F_OK && mode & ~(R_OK | W_OK)) {
+    return false;
+  }
+
+  if (!ValidatePath(requested_filename))
+    return false;
+
+  if (!MatchPath(requested_filename)) {
+    return false;
+  }
+  bool allowed = false;
+  switch (mode) {
+    case F_OK:
+      if (allow_read_ || allow_write_)
+        allowed = true;
+      break;
+    case R_OK:
+      if (allow_read_)
+        allowed = true;
+      break;
+    case W_OK:
+      if (allow_write_)
+        allowed = true;
+      break;
+    case R_OK | W_OK:
+      if (allow_read_ && allow_write_)
+        allowed = true;
+      break;
+    default:
+      return false;
+  }
+
+  if (allowed && file_to_access) {
+    if (!recursive_)
+      *file_to_access = path_.c_str();
+    else
+      *file_to_access = requested_filename;
+  }
+  return allowed;
+}
+
+// Async signal safe.
+// External call to std::string::c_str() is
+// called in MatchPath.
+// TODO(leecam): remove dependency on std::string
+bool BrokerFilePermission::CheckOpen(const char* requested_filename,
+                                     int flags,
+                                     const char** file_to_open,
+                                     bool* unlink_after_open) const {
+  if (!ValidatePath(requested_filename))
+    return false;
+
+  if (!MatchPath(requested_filename)) {
+    return false;
+  }
+
+  // First, check the access mode is valid.
+  const int access_mode = flags & O_ACCMODE;
+  if (access_mode != O_RDONLY && access_mode != O_WRONLY &&
+      access_mode != O_RDWR) {
+    return false;
+  }
+
+  // Check if read is allowed
+  if (!allow_read_ && (access_mode == O_RDONLY || access_mode == O_RDWR)) {
+    return false;
+  }
+
+  // Check if write is allowed
+  if (!allow_write_ && (access_mode == O_WRONLY || access_mode == O_RDWR)) {
+    return false;
+  }
+
+  // Check if file creation is allowed.
+  if (!allow_create_ && (flags & O_CREAT)) {
+    return false;
+  }
+
+  // If O_CREAT is present, ensure O_EXCL
+  if ((flags & O_CREAT) && !(flags & O_EXCL)) {
+    return false;
+  }
+
+  // If this file is to be unlinked, ensure it's created.
+  if (unlink_ && !(flags & O_CREAT)) {
+    return false;
+  }
+
+  // Some flags affect the behavior of the current process. We don't support
+  // them and don't allow them for now.
+  if (flags & kCurrentProcessOpenFlagsMask) {
+    return false;
+  }
+
+  // Now check that all the flags are known to us.
+  const int creation_and_status_flags = flags & ~O_ACCMODE;
+
+  const int known_flags = O_APPEND | O_ASYNC | O_CLOEXEC | O_CREAT | O_DIRECT |
+                          O_DIRECTORY | O_EXCL | O_LARGEFILE | O_NOATIME |
+                          O_NOCTTY | O_NOFOLLOW | O_NONBLOCK | O_NDELAY |
+                          O_SYNC | O_TRUNC;
+
+  const int unknown_flags = ~known_flags;
+  const bool has_unknown_flags = creation_and_status_flags & unknown_flags;
+
+  if (has_unknown_flags)
+    return false;
+
+  if (file_to_open) {
+    if (!recursive_)
+      *file_to_open = path_.c_str();
+    else
+      *file_to_open = requested_filename;
+  }
+  if (unlink_after_open)
+    *unlink_after_open = unlink_;
+
+  return true;
+}
+
+const char* BrokerFilePermission::GetErrorMessageForTests() {
+  static char kInvalidBrokerFileString[] = "Invalid BrokerFilePermission";
+  return kInvalidBrokerFileString;
+}
+
+BrokerFilePermission::BrokerFilePermission(const std::string& path,
+                                           bool recursive,
+                                           bool unlink,
+                                           bool allow_read,
+                                           bool allow_write,
+                                           bool allow_create)
+    : path_(path),
+      recursive_(recursive),
+      unlink_(unlink),
+      allow_read_(allow_read),
+      allow_write_(allow_write),
+      allow_create_(allow_create) {
+  // Validate this permission and die if invalid!
+
+  // Must have enough length for a '/'
+  CHECK(path_.length() > 0) << GetErrorMessageForTests();
+  // Whitelisted paths must be absolute.
+  CHECK(path_[0] == '/') << GetErrorMessageForTests();
+
+  // Don't allow unlinking on creation without create permission
+  if (unlink_) {
+    CHECK(allow_create) << GetErrorMessageForTests();
+  }
+  const char last_char = *(path_.rbegin());
+  // Recursive paths must have a trailing slash
+  if (recursive_) {
+    CHECK(last_char == '/') << GetErrorMessageForTests();
+  } else {
+    CHECK(last_char != '/') << GetErrorMessageForTests();
+  }
+}
+
+}  // namespace syscall_broker
+
+}  // namespace sandbox
\ No newline at end of file
diff --git a/sandbox/linux/syscall_broker/broker_file_permission.h b/sandbox/linux/syscall_broker/broker_file_permission.h
new file mode 100644
index 0000000..03300d1
--- /dev/null
+++ b/sandbox/linux/syscall_broker/broker_file_permission.h
@@ -0,0 +1,119 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SANDBOX_LINUX_SYSCALL_BROKER_BROKER_FILE_PERMISSION_H_
+#define SANDBOX_LINUX_SYSCALL_BROKER_BROKER_FILE_PERMISSION_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "sandbox/sandbox_export.h"
+
+namespace sandbox {
+
+namespace syscall_broker {
+
+// BrokerFilePermission defines a path for whitelisting.
+// Pick the correct static factory method to create a permission.
+// CheckOpen and CheckAccess are async signal safe.
+// Constuction and Destruction are not async signal safe.
+// |path| is the path to be whitelisted.
+class SANDBOX_EXPORT BrokerFilePermission {
+ public:
+  ~BrokerFilePermission() {}
+  BrokerFilePermission(const BrokerFilePermission&) = default;
+  BrokerFilePermission& operator=(const BrokerFilePermission&) = default;
+
+  static BrokerFilePermission ReadOnly(const std::string& path) {
+    return BrokerFilePermission(path, false, false, true, false, false);
+  }
+
+  static BrokerFilePermission ReadOnlyRecursive(const std::string& path) {
+    return BrokerFilePermission(path, true, false, true, false, false);
+  }
+
+  static BrokerFilePermission WriteOnly(const std::string& path) {
+    return BrokerFilePermission(path, false, false, false, true, false);
+  }
+
+  static BrokerFilePermission ReadWrite(const std::string& path) {
+    return BrokerFilePermission(path, false, false, true, true, false);
+  }
+
+  static BrokerFilePermission ReadWriteCreate(const std::string& path) {
+    return BrokerFilePermission(path, false, false, true, true, true);
+  }
+
+  static BrokerFilePermission ReadWriteCreateUnlink(const std::string& path) {
+    return BrokerFilePermission(path, false, true, true, true, true);
+  }
+
+  static BrokerFilePermission ReadWriteCreateUnlinkRecursive(
+      const std::string& path) {
+    return BrokerFilePermission(path, true, true, true, true, true);
+  }
+
+  // Returns true if |requested_filename| is allowed to be opened
+  // by this permission.
+  // If |file_to_open| is not NULL it is set to point to either
+  // the |requested_filename| in the case of a recursive match,
+  // or a pointer the matched path in the whitelist if an absolute
+  // match.
+  // If not NULL |unlink_after_open| is set to point to true if the
+  // caller should unlink the path after openning.
+  // Async signal safe if |file_to_open| is NULL.
+  bool CheckOpen(const char* requested_filename,
+                 int flags,
+                 const char** file_to_open,
+                 bool* unlink_after_open) const;
+  // Returns true if |requested_filename| is allowed to be accessed
+  // by this permission as per access(2).
+  // If |file_to_open| is not NULL it is set to point to either
+  // the |requested_filename| in the case of a recursive match,
+  // or a pointer to the matched path in the whitelist if an absolute
+  // match.
+  // |mode| is per mode argument of access(2).
+  // Async signal safe if |file_to_access| is NULL
+  bool CheckAccess(const char* requested_filename,
+                   int mode,
+                   const char** file_to_access) const;
+
+ private:
+  friend class BrokerFilePermissionTester;
+  BrokerFilePermission(const std::string& path,
+                       bool recursive,
+                       bool unlink,
+                       bool allow_read,
+                       bool allow_write,
+                       bool allow_create);
+
+  // ValidatePath checks |path| and returns true if these conditions are met
+  // * Greater than 0 length
+  // * Is an absolute path
+  // * No trailing slash
+  // * No /../ path traversal
+  static bool ValidatePath(const char* path);
+
+  // MatchPath returns true if |requested_filename| is covered by this instance
+  bool MatchPath(const char* requested_filename) const;
+
+  // Used in by BrokerFilePermissionTester for tests.
+  static const char* GetErrorMessageForTests();
+
+  // These are not const as std::vector requires copy-assignment and this class
+  // is stored in vectors. All methods are marked const so
+  // the compiler will still enforce no changes outside of the constructor.
+  std::string path_;
+  bool recursive_;  // Allow everything under this path. |path| must be a dir.
+  bool unlink_;     // unlink after opening.
+  bool allow_read_;
+  bool allow_write_;
+  bool allow_create_;
+};
+
+}  // namespace syscall_broker
+
+}  // namespace sandbox
+
+#endif  //  SANDBOX_LINUX_SYSCALL_BROKER_BROKER_FILE_PERMISSION_H_
\ No newline at end of file
diff --git a/sandbox/linux/syscall_broker/broker_file_permission_unittest.cc b/sandbox/linux/syscall_broker/broker_file_permission_unittest.cc
new file mode 100644
index 0000000..2853021
--- /dev/null
+++ b/sandbox/linux/syscall_broker/broker_file_permission_unittest.cc
@@ -0,0 +1,262 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "sandbox/linux/syscall_broker/broker_file_permission.h"
+
+#include <fcntl.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "sandbox/linux/tests/test_utils.h"
+#include "sandbox/linux/tests/unit_tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace sandbox {
+
+namespace syscall_broker {
+
+class BrokerFilePermissionTester {
+ public:
+  static bool ValidatePath(const char* path) {
+    return BrokerFilePermission::ValidatePath(path);
+  }
+  static const char* GetErrorMessage() {
+    return BrokerFilePermission::GetErrorMessageForTests();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrokerFilePermissionTester);
+};
+
+namespace {
+
+// Creation tests are DEATH tests as a bad permission causes termination.
+SANDBOX_TEST(BrokerFilePermission, CreateGood) {
+  const char kPath[] = "/tmp/good";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnly(kPath);
+}
+
+SANDBOX_TEST(BrokerFilePermission, CreateGoodRecursive) {
+  const char kPath[] = "/tmp/good/";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnlyRecursive(kPath);
+}
+
+SANDBOX_DEATH_TEST(
+    BrokerFilePermission,
+    CreateBad,
+    DEATH_MESSAGE(BrokerFilePermissionTester::GetErrorMessage())) {
+  const char kPath[] = "/tmp/bad/";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnly(kPath);
+}
+
+SANDBOX_DEATH_TEST(
+    BrokerFilePermission,
+    CreateBadRecursive,
+    DEATH_MESSAGE(BrokerFilePermissionTester::GetErrorMessage())) {
+  const char kPath[] = "/tmp/bad";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnlyRecursive(kPath);
+}
+
+SANDBOX_DEATH_TEST(
+    BrokerFilePermission,
+    CreateBadNotAbs,
+    DEATH_MESSAGE(BrokerFilePermissionTester::GetErrorMessage())) {
+  const char kPath[] = "tmp/bad";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnly(kPath);
+}
+
+SANDBOX_DEATH_TEST(
+    BrokerFilePermission,
+    CreateBadEmpty,
+    DEATH_MESSAGE(BrokerFilePermissionTester::GetErrorMessage())) {
+  const char kPath[] = "";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnly(kPath);
+}
+
+// CheckPerm tests |path| against |perm| given |access_flags|.
+// If |create| is true then file creation is tested for success.
+void CheckPerm(const BrokerFilePermission& perm,
+               const char* path,
+               int access_flags,
+               bool create) {
+  const char* file_to_open = NULL;
+
+  ASSERT_FALSE(perm.CheckAccess(path, X_OK, NULL));
+  ASSERT_TRUE(perm.CheckAccess(path, F_OK, NULL));
+  // check bad perms
+  switch (access_flags) {
+    case O_RDONLY:
+      ASSERT_TRUE(perm.CheckOpen(path, O_RDONLY, &file_to_open, NULL));
+      ASSERT_FALSE(perm.CheckOpen(path, O_WRONLY, &file_to_open, NULL));
+      ASSERT_FALSE(perm.CheckOpen(path, O_RDWR, &file_to_open, NULL));
+      ASSERT_TRUE(perm.CheckAccess(path, R_OK, NULL));
+      ASSERT_FALSE(perm.CheckAccess(path, W_OK, NULL));
+      break;
+    case O_WRONLY:
+      ASSERT_FALSE(perm.CheckOpen(path, O_RDONLY, &file_to_open, NULL));
+      ASSERT_TRUE(perm.CheckOpen(path, O_WRONLY, &file_to_open, NULL));
+      ASSERT_FALSE(perm.CheckOpen(path, O_RDWR, &file_to_open, NULL));
+      ASSERT_FALSE(perm.CheckAccess(path, R_OK, NULL));
+      ASSERT_TRUE(perm.CheckAccess(path, W_OK, NULL));
+      break;
+    case O_RDWR:
+      ASSERT_TRUE(perm.CheckOpen(path, O_RDONLY, &file_to_open, NULL));
+      ASSERT_TRUE(perm.CheckOpen(path, O_WRONLY, &file_to_open, NULL));
+      ASSERT_TRUE(perm.CheckOpen(path, O_RDWR, &file_to_open, NULL));
+      ASSERT_TRUE(perm.CheckAccess(path, R_OK, NULL));
+      ASSERT_TRUE(perm.CheckAccess(path, W_OK, NULL));
+      break;
+    default:
+      // Bad test case
+      NOTREACHED();
+  }
+
+// O_SYNC can be defined as (__O_SYNC|O_DSYNC)
+#ifdef O_DSYNC
+  const int kSyncFlag = O_SYNC & ~O_DSYNC;
+#else
+  const int kSyncFlag = O_SYNC;
+#endif
+
+  const int kNumberOfBitsInOAccMode = 2;
+  COMPILE_ASSERT(O_ACCMODE == ((1 << kNumberOfBitsInOAccMode) - 1),
+                 number_of_bits);
+  // check every possible flag and act accordingly.
+  // Skipping AccMode bits as they are present in every case.
+  for (int i = kNumberOfBitsInOAccMode; i < 32; i++) {
+    int flag = 1 << i;
+    switch (flag) {
+      case O_APPEND:
+      case O_ASYNC:
+      case O_DIRECT:
+      case O_DIRECTORY:
+#ifdef O_DSYNC
+      case O_DSYNC:
+#endif
+      case O_EXCL:
+      case O_LARGEFILE:
+      case O_NOATIME:
+      case O_NOCTTY:
+      case O_NOFOLLOW:
+      case O_NONBLOCK:
+#if (O_NONBLOCK != O_NDELAY)
+      case O_NDELAY:
+#endif
+      case kSyncFlag:
+      case O_TRUNC:
+        ASSERT_TRUE(
+            perm.CheckOpen(path, access_flags | flag, &file_to_open, NULL));
+        break;
+      case O_CLOEXEC:
+      case O_CREAT:
+      default:
+        ASSERT_FALSE(
+            perm.CheckOpen(path, access_flags | flag, &file_to_open, NULL));
+    }
+  }
+  if (create) {
+    bool unlink;
+    ASSERT_TRUE(perm.CheckOpen(path, O_CREAT | O_EXCL | access_flags,
+                               &file_to_open, &unlink));
+    ASSERT_FALSE(unlink);
+  } else {
+    ASSERT_FALSE(perm.CheckOpen(path, O_CREAT | O_EXCL | access_flags,
+                                &file_to_open, NULL));
+  }
+}
+
+TEST(BrokerFilePermission, ReadOnly) {
+  const char kPath[] = "/tmp/good";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnly(kPath);
+  CheckPerm(perm, kPath, O_RDONLY, false);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerFilePermission, ReadOnlyRecursive) {
+  const char kPath[] = "/tmp/good/";
+  const char kPathFile[] = "/tmp/good/file";
+  BrokerFilePermission perm = BrokerFilePermission::ReadOnlyRecursive(kPath);
+  CheckPerm(perm, kPathFile, O_RDONLY, false);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerFilePermission, WriteOnly) {
+  const char kPath[] = "/tmp/good";
+  BrokerFilePermission perm = BrokerFilePermission::WriteOnly(kPath);
+  CheckPerm(perm, kPath, O_WRONLY, false);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerFilePermission, ReadWrite) {
+  const char kPath[] = "/tmp/good";
+  BrokerFilePermission perm = BrokerFilePermission::ReadWrite(kPath);
+  CheckPerm(perm, kPath, O_RDWR, false);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerFilePermission, ReadWriteCreate) {
+  const char kPath[] = "/tmp/good";
+  BrokerFilePermission perm = BrokerFilePermission::ReadWriteCreate(kPath);
+  CheckPerm(perm, kPath, O_RDWR, true);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+void CheckUnlink(BrokerFilePermission& perm,
+                 const char* path,
+                 int access_flags) {
+  bool unlink;
+  ASSERT_FALSE(perm.CheckOpen(path, access_flags, NULL, &unlink));
+  ASSERT_FALSE(perm.CheckOpen(path, access_flags | O_CREAT, NULL, &unlink));
+  ASSERT_TRUE(
+      perm.CheckOpen(path, access_flags | O_CREAT | O_EXCL, NULL, &unlink));
+  ASSERT_TRUE(unlink);
+}
+
+TEST(BrokerFilePermission, ReadWriteCreateUnlink) {
+  const char kPath[] = "/tmp/good";
+  BrokerFilePermission perm =
+      BrokerFilePermission::ReadWriteCreateUnlink(kPath);
+  CheckUnlink(perm, kPath, O_RDWR);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerFilePermission, ReadWriteCreateUnlinkRecursive) {
+  const char kPath[] = "/tmp/good/";
+  const char kPathFile[] = "/tmp/good/file";
+  BrokerFilePermission perm =
+      BrokerFilePermission::ReadWriteCreateUnlinkRecursive(kPath);
+  CheckUnlink(perm, kPathFile, O_RDWR);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerFilePermission, ValidatePath) {
+  EXPECT_TRUE(BrokerFilePermissionTester::ValidatePath("/path"));
+  EXPECT_TRUE(BrokerFilePermissionTester::ValidatePath("/"));
+  EXPECT_TRUE(BrokerFilePermissionTester::ValidatePath("/..path"));
+
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath(""));
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath("bad"));
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath("/bad/"));
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath("bad/"));
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath("/bad/.."));
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath("/bad/../bad"));
+  EXPECT_FALSE(BrokerFilePermissionTester::ValidatePath("/../bad"));
+}
+
+}  // namespace
+
+}  // namespace syscall_broker
+
+}  // namespace sandbox
diff --git a/sandbox/linux/syscall_broker/broker_host.cc b/sandbox/linux/syscall_broker/broker_host.cc
index 29300f7..7ebc785 100644
--- a/sandbox/linux/syscall_broker/broker_host.cc
+++ b/sandbox/linux/syscall_broker/broker_host.cc
@@ -38,8 +38,13 @@ bool IsRunningOnValgrind() {
 // make a direct system call since we want to keep in control of the broker
 // process' system calls profile to be able to loosely sandbox it.
 int sys_open(const char* pathname, int flags) {
-  // Always pass a defined |mode| in case flags mistakenly contains O_CREAT.
-  const int mode = 0;
+  // Hardcode mode to rw------- when creating files.
+  int mode;
+  if (flags & O_CREAT) {
+    mode = 0600;
+  } else {
+    mode = 0;
+  }
   if (IsRunningOnValgrind()) {
     // Valgrind does not support AT_FDCWD, just use libc's open() in this case.
     return open(pathname, flags, mode);
@@ -59,8 +64,9 @@ void OpenFileForIPC(const BrokerPolicy& policy,
   DCHECK(write_pickle);
   DCHECK(opened_files);
   const char* file_to_open = NULL;
+  bool unlink_after_open = false;
   const bool safe_to_open_file = policy.GetFileNameIfAllowedToOpen(
-      requested_filename.c_str(), flags, &file_to_open);
+      requested_filename.c_str(), flags, &file_to_open, &unlink_after_open);
 
   if (safe_to_open_file) {
     CHECK(file_to_open);
@@ -69,6 +75,9 @@ void OpenFileForIPC(const BrokerPolicy& policy,
       write_pickle->WriteInt(-errno);
     } else {
       // Success.
+      if (unlink_after_open) {
+        unlink(file_to_open);
+      }
       opened_files->push_back(opened_fd);
       write_pickle->WriteInt(0);
     }
diff --git a/sandbox/linux/syscall_broker/broker_policy.cc b/sandbox/linux/syscall_broker/broker_policy.cc
index ee9a658..d9f69e3 100644
--- a/sandbox/linux/syscall_broker/broker_policy.cc
+++ b/sandbox/linux/syscall_broker/broker_policy.cc
@@ -17,76 +17,19 @@
 namespace sandbox {
 namespace syscall_broker {
 
-namespace {
-
-// We maintain a list of flags that have been reviewed for "sanity" and that
-// we're ok to allow in the broker.
-// I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM.
-bool IsAllowedOpenFlags(int flags) {
-  // First, check the access mode.
-  const int access_mode = flags & O_ACCMODE;
-  if (access_mode != O_RDONLY && access_mode != O_WRONLY &&
-      access_mode != O_RDWR) {
-    return false;
-  }
-
-  // We only support a 2-parameters open, so we forbid O_CREAT.
-  if (flags & O_CREAT) {
-    return false;
-  }
-
-  // Some flags affect the behavior of the current process. We don't support
-  // them and don't allow them for now.
-  if (flags & kCurrentProcessOpenFlagsMask)
-    return false;
-
-  // Now check that all the flags are known to us.
-  const int creation_and_status_flags = flags & ~O_ACCMODE;
-
-  const int known_flags = O_APPEND | O_ASYNC | O_CLOEXEC | O_CREAT | O_DIRECT |
-                          O_DIRECTORY | O_EXCL | O_LARGEFILE | O_NOATIME |
-                          O_NOCTTY | O_NOFOLLOW | O_NONBLOCK | O_NDELAY |
-                          O_SYNC | O_TRUNC;
-
-  const int unknown_flags = ~known_flags;
-  const bool has_unknown_flags = creation_and_status_flags & unknown_flags;
-  return !has_unknown_flags;
-}
-
-// Needs to be async signal safe if |file_to_open| is NULL.
-// TODO(jln): assert signal safety.
-bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names,
-                            const char* requested_filename,
-                            const char** file_to_open) {
-  if (file_to_open && *file_to_open) {
-    // Make sure that callers never pass a non-empty string. In case callers
-    // wrongly forget to check the return value and look at the string
-    // instead, this could catch bugs.
-    RAW_LOG(FATAL, "*file_to_open should be NULL");
-    return false;
-  }
-
-  // Look for |requested_filename| in |allowed_file_names|.
-  // We don't use ::find() because it takes a std::string and
-  // the conversion allocates memory.
-  for (const auto& allowed_file_name : allowed_file_names) {
-    if (strcmp(requested_filename, allowed_file_name.c_str()) == 0) {
-      if (file_to_open)
-        *file_to_open = allowed_file_name.c_str();
-      return true;
-    }
-  }
-  return false;
-}
-
-}  // namespace
-
 BrokerPolicy::BrokerPolicy(int denied_errno,
-                           const std::vector<std::string>& allowed_r_files,
-                           const std::vector<std::string>& allowed_w_files)
+                           const std::vector<BrokerFilePermission>& permissions)
     : denied_errno_(denied_errno),
-      allowed_r_files_(allowed_r_files),
-      allowed_w_files_(allowed_w_files) {
+      permissions_(permissions),
+      num_of_permissions_(permissions.size()) {
+  // The spec guarantees vectors store their elements contiguously
+  // so set up a pointer to array of element so it can be used
+  // in async signal safe code instead of vector operations.
+  if (num_of_permissions_ > 0) {
+    permissions_array_ = &permissions_[0];
+  } else {
+    permissions_array_ = NULL;
+  }
 }
 
 BrokerPolicy::~BrokerPolicy() {
@@ -107,34 +50,20 @@ bool BrokerPolicy::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)) {
+  if (file_to_access && *file_to_access) {
+    // Make sure that callers never pass a non-empty string. In case callers
+    // wrongly forget to check the return value and look at the string
+    // instead, this could catch bugs.
+    RAW_LOG(FATAL, "*file_to_access should be NULL");
     return false;
   }
-  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;
+  for (size_t i = 0; i < num_of_permissions_; i++) {
+    if (permissions_array_[i].CheckAccess(requested_filename, requested_mode,
+                                          file_to_access)) {
+      return true;
     }
-    default:
-      return false;
   }
+  return false;
 }
 
 // Check if |requested_filename| can be opened with flags |requested_flags|.
@@ -147,27 +76,22 @@ bool BrokerPolicy::GetFileNameIfAllowedToAccess(
 // Async signal safe if and only if |file_to_open| is NULL.
 bool BrokerPolicy::GetFileNameIfAllowedToOpen(const char* requested_filename,
                                               int requested_flags,
-                                              const char** file_to_open) const {
-  if (!IsAllowedOpenFlags(requested_flags)) {
+                                              const char** file_to_open,
+                                              bool* unlink_after_open) const {
+  if (file_to_open && *file_to_open) {
+    // Make sure that callers never pass a non-empty string. In case callers
+    // wrongly forget to check the return value and look at the string
+    // instead, this could catch bugs.
+    RAW_LOG(FATAL, "*file_to_open should be NULL");
     return false;
   }
-  switch (requested_flags & O_ACCMODE) {
-    case O_RDONLY:
-      return GetFileNameInWhitelist(
-          allowed_r_files_, requested_filename, file_to_open);
-    case O_WRONLY:
-      return GetFileNameInWhitelist(
-          allowed_w_files_, requested_filename, file_to_open);
-    case O_RDWR: {
-      bool allowed_for_read_and_write =
-          GetFileNameInWhitelist(allowed_r_files_, requested_filename, NULL) &&
-          GetFileNameInWhitelist(
-              allowed_w_files_, requested_filename, file_to_open);
-      return allowed_for_read_and_write;
+  for (size_t i = 0; i < num_of_permissions_; i++) {
+    if (permissions_array_[i].CheckOpen(requested_filename, requested_flags,
+                                        file_to_open, unlink_after_open)) {
+      return true;
     }
-    default:
-      return false;
   }
+  return false;
 }
 
 }  // namespace syscall_broker
diff --git a/sandbox/linux/syscall_broker/broker_policy.h b/sandbox/linux/syscall_broker/broker_policy.h
index ef5bdfa..d5146ed 100644
--- a/sandbox/linux/syscall_broker/broker_policy.h
+++ b/sandbox/linux/syscall_broker/broker_policy.h
@@ -10,6 +10,8 @@
 
 #include "base/macros.h"
 
+#include "sandbox/linux/syscall_broker/broker_file_permission.h"
+
 namespace sandbox {
 namespace syscall_broker {
 
@@ -23,21 +25,21 @@ class BrokerPolicy {
   // |denied_errno| is the error code returned when IPC requests for system
   // calls such as open() or access() are denied because a file is not in the
   // whitelist. EACCESS would be a typical value.
-  // |allowed_r_files| and |allowed_w_files| are white lists of files that
-  // should be allowed for opening, respectively for reading and writing.
-  // A file available read-write should be listed in both.
+  // |permissions| is a list of BrokerPermission objects that define
+  // what the broker will allow.
   BrokerPolicy(int denied_errno,
-               const std::vector<std::string>& allowed_r_files,
-               const std::vector<std::string>& allowed_w_files_);
+               const std::vector<BrokerFilePermission>& permissions);
+
   ~BrokerPolicy();
 
   // 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
+  // If |file_to_open| is not NULL, a pointer to the path will be returned.
+  // In the case of a recursive match, this will be the requested_filename,
+  // otherwise it will return the matching pointer from the
+  // whitelist. For paranoia a caller should then use |file_to_access|. See
   // GetFileNameIfAllowedToOpen() for more explanation.
   // return true if calling access() on this file should be allowed, false
   // otherwise.
@@ -47,22 +49,34 @@ class BrokerPolicy {
                                     const char** file_to_access) const;
 
   // 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
+  // If |file_to_open| is not NULL, a pointer to the path will be returned.
+  // In the case of a recursive match, this will be the requested_filename,
+  // otherwise it 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.
+  // |unlink_after_open| if not NULL will be set to point to true if the
+  // policy requests the caller unlink the path after opening.
   // Return true if opening should be allowed, false otherwise.
   // Async signal safe if and only if |file_to_open| is NULL.
   bool GetFileNameIfAllowedToOpen(const char* requested_filename,
                                   int requested_flags,
-                                  const char** file_to_open) const;
+                                  const char** file_to_open,
+                                  bool* unlink_after_open) const;
   int denied_errno() const { return denied_errno_; }
 
  private:
   const int denied_errno_;
-  const std::vector<std::string> allowed_r_files_;
-  const std::vector<std::string> allowed_w_files_;
+  // The permissions_ vector is used as storage for the BrokerFilePermission
+  // objects but is not referenced outside of the constructor as
+  // vectors are unfriendly in async signal safe code.
+  const std::vector<BrokerFilePermission> permissions_;
+  // permissions_array_ is set up to point to the backing store of
+  // permissions_ and is used in async signal safe methods.
+  const BrokerFilePermission* permissions_array_;
+  const size_t num_of_permissions_;
+
   DISALLOW_COPY_AND_ASSIGN(BrokerPolicy);
 };
 
diff --git a/sandbox/linux/syscall_broker/broker_process.cc b/sandbox/linux/syscall_broker/broker_process.cc
index 9b92c49..81131cc 100644
--- a/sandbox/linux/syscall_broker/broker_process.cc
+++ b/sandbox/linux/syscall_broker/broker_process.cc
@@ -30,16 +30,16 @@ namespace sandbox {
 
 namespace syscall_broker {
 
-BrokerProcess::BrokerProcess(int denied_errno,
-                             const std::vector<std::string>& allowed_r_files,
-                             const std::vector<std::string>& allowed_w_files,
-                             bool fast_check_in_client,
-                             bool quiet_failures_for_tests)
+BrokerProcess::BrokerProcess(
+    int denied_errno,
+    const std::vector<syscall_broker::BrokerFilePermission>& permissions,
+    bool fast_check_in_client,
+    bool quiet_failures_for_tests)
     : initialized_(false),
       fast_check_in_client_(fast_check_in_client),
       quiet_failures_for_tests_(quiet_failures_for_tests),
       broker_pid_(-1),
-      policy_(denied_errno, allowed_r_files, allowed_w_files) {
+      policy_(denied_errno, permissions) {
 }
 
 BrokerProcess::~BrokerProcess() {
diff --git a/sandbox/linux/syscall_broker/broker_process.h b/sandbox/linux/syscall_broker/broker_process.h
index 50e7eee..c23ac3c 100644
--- a/sandbox/linux/syscall_broker/broker_process.h
+++ b/sandbox/linux/syscall_broker/broker_process.h
@@ -21,6 +21,7 @@ namespace sandbox {
 namespace syscall_broker {
 
 class BrokerClient;
+class BrokerFilePermission;
 
 // Create a new "broker" process to which we can send requests via an IPC
 // channel by forking the current process.
@@ -42,11 +43,13 @@ class SANDBOX_EXPORT BrokerProcess {
   // A file available read-write should be listed in both.
   // |fast_check_in_client| and |quiet_failures_for_tests| are reserved for
   // unit tests, don't use it.
-  BrokerProcess(int denied_errno,
-                const std::vector<std::string>& allowed_r_files,
-                const std::vector<std::string>& allowed_w_files,
-                bool fast_check_in_client = true,
-                bool quiet_failures_for_tests = false);
+
+  BrokerProcess(
+      int denied_errno,
+      const std::vector<syscall_broker::BrokerFilePermission>& permissions,
+      bool fast_check_in_client = true,
+      bool quiet_failures_for_tests = false);
+
   ~BrokerProcess();
   // Will initialize the broker process. There should be no threads at this
   // point, since we need to fork().
diff --git a/sandbox/linux/syscall_broker/broker_process_unittest.cc b/sandbox/linux/syscall_broker/broker_process_unittest.cc
index 997667c..ee284f2 100644
--- a/sandbox/linux/syscall_broker/broker_process_unittest.cc
+++ b/sandbox/linux/syscall_broker/broker_process_unittest.cc
@@ -54,11 +54,10 @@ bool NoOpCallback() {
 }  // namespace
 
 TEST(BrokerProcess, CreateAndDestroy) {
-  std::vector<std::string> read_whitelist;
-  read_whitelist.push_back("/proc/cpuinfo");
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadOnly("/proc/cpuinfo"));
 
-  scoped_ptr<BrokerProcess> open_broker(
-      new BrokerProcess(EPERM, read_whitelist, std::vector<std::string>()));
+  scoped_ptr<BrokerProcess> open_broker(new BrokerProcess(EPERM, permissions));
   ASSERT_TRUE(open_broker->Init(base::Bind(&NoOpCallback)));
 
   ASSERT_TRUE(TestUtils::CurrentProcessHasChildren());
@@ -68,8 +67,8 @@ TEST(BrokerProcess, CreateAndDestroy) {
 }
 
 TEST(BrokerProcess, TestOpenAccessNull) {
-  const std::vector<std::string> empty;
-  BrokerProcess open_broker(EPERM, empty, empty);
+  std::vector<BrokerFilePermission> empty;
+  BrokerProcess open_broker(EPERM, empty);
   ASSERT_TRUE(open_broker.Init(base::Bind(&NoOpCallback)));
 
   int fd = open_broker.Open(NULL, O_RDONLY);
@@ -88,17 +87,14 @@ void TestOpenFilePerms(bool fast_check_in_client, int denied_errno) {
   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;
-  write_whitelist.push_back(kW_WhiteListed);
-  write_whitelist.push_back(kRW_WhiteListed);
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadOnly(kR_WhiteListed));
+  permissions.push_back(
+      BrokerFilePermission::ReadOnly(kR_WhiteListedButDenied));
+  permissions.push_back(BrokerFilePermission::WriteOnly(kW_WhiteListed));
+  permissions.push_back(BrokerFilePermission::ReadWrite(kRW_WhiteListed));
 
-  BrokerProcess open_broker(
-      denied_errno, read_whitelist, write_whitelist, fast_check_in_client);
+  BrokerProcess open_broker(denied_errno, permissions, fast_check_in_client);
   ASSERT_TRUE(open_broker.Init(base::Bind(&NoOpCallback)));
 
   int fd = -1;
@@ -243,13 +239,78 @@ TEST(BrokerProcess, OpenOpenFilePermsNoClientCheckNoEnt) {
   // expected.
 }
 
-void TestOpenCpuinfo(bool fast_check_in_client) {
+void TestBadPaths(bool fast_check_in_client) {
+  const char kFileCpuInfo[] = "/proc/cpuinfo";
+  const char kNotAbsPath[] = "proc/cpuinfo";
+  const char kDotDotStart[] = "/../proc/cpuinfo";
+  const char kDotDotMiddle[] = "/proc/self/../cpuinfo";
+  const char kDotDotEnd[] = "/proc/..";
+  const char kTrailingSlash[] = "/proc/";
+
+  std::vector<BrokerFilePermission> permissions;
+
+  permissions.push_back(BrokerFilePermission::ReadOnlyRecursive("/proc/"));
+  scoped_ptr<BrokerProcess> open_broker(
+      new BrokerProcess(EPERM, permissions, fast_check_in_client));
+  ASSERT_TRUE(open_broker->Init(base::Bind(&NoOpCallback)));
+  // Open cpuinfo via the broker.
+  int cpuinfo_fd = open_broker->Open(kFileCpuInfo, O_RDONLY);
+  base::ScopedFD cpuinfo_fd_closer(cpuinfo_fd);
+  ASSERT_GE(cpuinfo_fd, 0);
+
+  int fd = -1;
+  int can_access;
+
+  can_access = open_broker->Access(kNotAbsPath, R_OK);
+  ASSERT_EQ(can_access, -EPERM);
+  fd = open_broker->Open(kNotAbsPath, O_RDONLY);
+  ASSERT_EQ(fd, -EPERM);
+
+  can_access = open_broker->Access(kDotDotStart, R_OK);
+  ASSERT_EQ(can_access, -EPERM);
+  fd = open_broker->Open(kDotDotStart, O_RDONLY);
+  ASSERT_EQ(fd, -EPERM);
+
+  can_access = open_broker->Access(kDotDotMiddle, R_OK);
+  ASSERT_EQ(can_access, -EPERM);
+  fd = open_broker->Open(kDotDotMiddle, O_RDONLY);
+  ASSERT_EQ(fd, -EPERM);
+
+  can_access = open_broker->Access(kDotDotEnd, R_OK);
+  ASSERT_EQ(can_access, -EPERM);
+  fd = open_broker->Open(kDotDotEnd, O_RDONLY);
+  ASSERT_EQ(fd, -EPERM);
+
+  can_access = open_broker->Access(kTrailingSlash, R_OK);
+  ASSERT_EQ(can_access, -EPERM);
+  fd = open_broker->Open(kTrailingSlash, O_RDONLY);
+  ASSERT_EQ(fd, -EPERM);
+}
+
+TEST(BrokerProcess, BadPathsClientCheck) {
+  TestBadPaths(true /* fast_check_in_client */);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerProcess, BadPathsNoClientCheck) {
+  TestBadPaths(false /* fast_check_in_client */);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+void TestOpenCpuinfo(bool fast_check_in_client, bool recursive) {
   const char kFileCpuInfo[] = "/proc/cpuinfo";
-  std::vector<std::string> read_whitelist;
-  read_whitelist.push_back(kFileCpuInfo);
+  const char kDirProc[] = "/proc/";
 
-  scoped_ptr<BrokerProcess> open_broker(new BrokerProcess(
-      EPERM, read_whitelist, std::vector<std::string>(), fast_check_in_client));
+  std::vector<BrokerFilePermission> permissions;
+  if (recursive)
+    permissions.push_back(BrokerFilePermission::ReadOnlyRecursive(kDirProc));
+  else
+    permissions.push_back(BrokerFilePermission::ReadOnly(kFileCpuInfo));
+
+  scoped_ptr<BrokerProcess> open_broker(
+      new BrokerProcess(EPERM, permissions, fast_check_in_client));
   ASSERT_TRUE(open_broker->Init(base::Bind(&NoOpCallback)));
 
   int fd = -1;
@@ -293,16 +354,28 @@ void TestOpenCpuinfo(bool fast_check_in_client) {
   ASSERT_FALSE(TestUtils::CurrentProcessHasChildren());
 }
 
-// Run the same thing twice. The second time, we make sure that no security
-// check is performed on the client.
+// Run this test 4 times. With and without the check in client
+// and using a recursive path.
 TEST(BrokerProcess, OpenCpuinfoWithClientCheck) {
-  TestOpenCpuinfo(true /* fast_check_in_client */);
+  TestOpenCpuinfo(true /* fast_check_in_client */, false /* not recursive */);
   // Don't do anything here, so that ASSERT works in the subfunction as
   // expected.
 }
 
 TEST(BrokerProcess, OpenCpuinfoNoClientCheck) {
-  TestOpenCpuinfo(false /* fast_check_in_client */);
+  TestOpenCpuinfo(false /* fast_check_in_client */, false /* not recursive */);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerProcess, OpenCpuinfoWithClientCheckRecursive) {
+  TestOpenCpuinfo(true /* fast_check_in_client */, true /* recursive */);
+  // Don't do anything here, so that ASSERT works in the subfunction as
+  // expected.
+}
+
+TEST(BrokerProcess, OpenCpuinfoNoClientCheckRecursive) {
+  TestOpenCpuinfo(false /* fast_check_in_client */, true /* recursive */);
   // Don't do anything here, so that ASSERT works in the subfunction as
   // expected.
 }
@@ -311,10 +384,10 @@ TEST(BrokerProcess, OpenFileRW) {
   ScopedTemporaryFile tempfile;
   const char* tempfile_name = tempfile.full_file_name();
 
-  std::vector<std::string> whitelist;
-  whitelist.push_back(tempfile_name);
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadWrite(tempfile_name));
 
-  BrokerProcess open_broker(EPERM, whitelist, whitelist);
+  BrokerProcess open_broker(EPERM, permissions);
   ASSERT_TRUE(open_broker.Init(base::Bind(&NoOpCallback)));
 
   // Check we can access that file with read or write.
@@ -344,13 +417,11 @@ TEST(BrokerProcess, OpenFileRW) {
 // SANDBOX_TEST because the process could die with a SIGPIPE
 // and we want this to happen in a subprocess.
 SANDBOX_TEST(BrokerProcess, BrokerDied) {
-  std::vector<std::string> read_whitelist;
-  read_whitelist.push_back("/proc/cpuinfo");
+  const char kCpuInfo[] = "/proc/cpuinfo";
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadOnly(kCpuInfo));
 
-  BrokerProcess open_broker(EPERM,
-                            read_whitelist,
-                            std::vector<std::string>(),
-                            true /* fast_check_in_client */,
+  BrokerProcess open_broker(EPERM, permissions, true /* fast_check_in_client */,
                             true /* quiet_failures_for_tests */);
   SANDBOX_ASSERT(open_broker.Init(base::Bind(&NoOpCallback)));
   const pid_t broker_pid = open_broker.broker_pid();
@@ -366,16 +437,16 @@ SANDBOX_TEST(BrokerProcess, BrokerDied) {
   SANDBOX_ASSERT(SIGKILL == process_info.si_status);
 
   // Check that 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);
+  SANDBOX_ASSERT(open_broker.Open(kCpuInfo, O_RDONLY) == -ENOMEM);
+  SANDBOX_ASSERT(open_broker.Access(kCpuInfo, O_RDONLY) == -ENOMEM);
 }
 
 void TestOpenComplexFlags(bool fast_check_in_client) {
   const char kCpuInfo[] = "/proc/cpuinfo";
-  std::vector<std::string> whitelist;
-  whitelist.push_back(kCpuInfo);
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadOnly(kCpuInfo));
 
-  BrokerProcess open_broker(EPERM, whitelist, whitelist, fast_check_in_client);
+  BrokerProcess open_broker(EPERM, permissions, fast_check_in_client);
   ASSERT_TRUE(open_broker.Init(base::Bind(&NoOpCallback)));
   // Test that we do the right thing for O_CLOEXEC and O_NONBLOCK.
   int fd = -1;
@@ -454,10 +525,10 @@ SANDBOX_TEST_ALLOW_NOISE(BrokerProcess, RecvMsgDescriptorLeak) {
   SANDBOX_ASSERT(0 == setrlimit(RLIMIT_NOFILE, &rlim));
 
   static const char kCpuInfo[] = "/proc/cpuinfo";
-  std::vector<std::string> read_whitelist;
-  read_whitelist.push_back(kCpuInfo);
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadOnly(kCpuInfo));
 
-  BrokerProcess open_broker(EPERM, read_whitelist, std::vector<std::string>());
+  BrokerProcess open_broker(EPERM, permissions);
   SANDBOX_ASSERT(open_broker.Init(base::Bind(&NoOpCallback)));
 
   const int ipc_fd = BrokerProcessTestHelper::GetIPCDescriptor(&open_broker);
@@ -498,16 +569,15 @@ bool WaitForClosedPipeWriter(int reader, int timeout_in_ms) {
 // Closing the broker client's IPC channel should terminate the broker
 // process.
 TEST(BrokerProcess, BrokerDiesOnClosedChannel) {
-  std::vector<std::string> read_whitelist;
-  read_whitelist.push_back("/proc/cpuinfo");
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadOnly("/proc/cpuinfo"));
 
   // Get the writing end of a pipe into the broker (child) process so
   // that we can reliably detect when it dies.
   int lifeline_fds[2];
   PCHECK(0 == pipe(lifeline_fds));
 
-  BrokerProcess open_broker(EPERM, read_whitelist, std::vector<std::string>(),
-                            true /* fast_check_in_client */,
+  BrokerProcess open_broker(EPERM, permissions, true /* fast_check_in_client */,
                             false /* quiet_failures_for_tests */);
   ASSERT_TRUE(open_broker.Init(base::Bind(&CloseFD, lifeline_fds[0])));
   // Make sure the writing end only exists in the broker process.
@@ -533,6 +603,55 @@ TEST(BrokerProcess, BrokerDiesOnClosedChannel) {
   EXPECT_EQ(1, process_info.si_status);
 }
 
+TEST(BrokerProcess, CreateFile) {
+  std::string temp_str;
+  {
+    ScopedTemporaryFile tmp_file;
+    temp_str = tmp_file.full_file_name();
+  }
+  const char* tempfile_name = temp_str.c_str();
+
+  std::vector<BrokerFilePermission> permissions;
+  permissions.push_back(BrokerFilePermission::ReadWriteCreate(tempfile_name));
+
+  BrokerProcess open_broker(EPERM, permissions);
+  ASSERT_TRUE(open_broker.Init(base::Bind(&NoOpCallback)));
+
+  int fd = -1;
+
+  // Try without O_EXCL
+  fd = open_broker.Open(tempfile_name, O_RDWR | O_CREAT);
+  ASSERT_EQ(fd, -EPERM);
+
+  const char kTestText[] = "TESTTESTTEST";
+  // Create a file
+  fd = open_broker.Open(tempfile_name, O_RDWR | O_CREAT | O_EXCL);
+  ASSERT_GE(fd, 0);
+  {
+    base::ScopedFD scoped_fd(fd);
+
+    // Confirm fail if file exists
+    int bad_fd = open_broker.Open(tempfile_name, O_RDWR | O_CREAT | O_EXCL);
+    ASSERT_EQ(bad_fd, -EEXIST);
+
+    // Write to the descriptor opened by the broker.
+
+    ssize_t len = HANDLE_EINTR(write(fd, kTestText, sizeof(kTestText)));
+    ASSERT_EQ(len, static_cast<ssize_t>(sizeof(kTestText)));
+  }
+
+  int fd_check = open(tempfile_name, O_RDONLY);
+  ASSERT_GE(fd_check, 0);
+  {
+    base::ScopedFD scoped_fd(fd_check);
+    char buf[1024];
+    ssize_t len = HANDLE_EINTR(read(fd_check, buf, sizeof(buf)));
+
+    ASSERT_EQ(len, static_cast<ssize_t>(sizeof(kTestText)));
+    ASSERT_EQ(memcmp(kTestText, buf, sizeof(kTestText)), 0);
+  }
+}
+
 }  // namespace syscall_broker
 
 }  // namespace sandbox