sandbox: Extend BrokerPolicy to support file creation

sandbox/linux/syscall_broker/broker_file_permission.h

Index: sandbox/linux/syscall_broker/broker_file_permission.h
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 0000000000000000000000000000000000000000..f8738763e1328d11e43313a9e98a10aaa66225fe
--- /dev/null
+++ b/sandbox/linux/syscall_broker/broker_file_permission.h
@@ -0,0 +1,96 @@
+// 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"
+
+class Pickle;

[jln (very slow on Chromium), 2014/11/24 20:12:16]

Nit: remove

+
+namespace sandbox {
+
+namespace syscall_broker {
+

[jln (very slow on Chromium), 2014/11/24 20:12:16]

Please, add a general class description on top of this. Especially important is
the fact that construction is not async signal safe, but that outside of
construction / destruction, all methods are async signal safe and thread-safe.

[leecam, 2014/11/25 02:06:49]

Done.

+class BrokerFilePermission {
+ public:
+  ~BrokerFilePermission() {}
+  BrokerFilePermission(const BrokerFilePermission&) = default;
+  BrokerFilePermission& operator=(const BrokerFilePermission&) = default;
+
+  static BrokerFilePermission ReadOnly(std::string path) {

[jln (very slow on Chromium), 2014/11/24 20:12:16]

const std::string& path:

C++11 is better at optimizing this thanks to move semantics, but constructing an
object here could in theory incur 3 copies of the path.

Same remark for all the factory methods below.

[leecam, 2014/11/25 02:06:49]

Done.

+    return BrokerFilePermission(path, false, false, true, false, false);
+  }
+
+  static BrokerFilePermission ReadOnlyRecursive(std::string path) {
+    return BrokerFilePermission(path, true, false, true, false, false);
+  }
+
+  static BrokerFilePermission WriteOnly(std::string path) {
+    return BrokerFilePermission(path, false, false, false, true, false);
+  }
+
+  static BrokerFilePermission ReadWrite(std::string path) {
+    return BrokerFilePermission(path, false, false, true, true, false);
+  }
+
+  static BrokerFilePermission ReadWriteCreate(std::string path) {
+    return BrokerFilePermission(path, false, false, true, true, true);
+  }
+
+  static BrokerFilePermission ReadWriteCreateUnlink(std::string path) {
+    return BrokerFilePermission(path, false, true, true, true, true);
+  }
+
+  static BrokerFilePermission ReadWriteCreateUnlinkRecursive(std::string path) {
+    return BrokerFilePermission(path, true, true, true, true, true);
+  }
+
+  // Returns true if |requested_filename| is allowed to be open

[Jorge Lucangeli Obes, 2014/11/20 00:13:20]

"open" -> "opened"

[leecam, 2014/11/20 00:56:38]

Done.

+  // 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.
+  // 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;

[jln (very slow on Chromium), 2014/11/24 20:12:16]

Don't forget to describe unlink_after_open. In particular the fact that it call
be NULL.

+  // Returns true if |requested_filename| is allowed to be accessed
+  // by this permission.

[jln (very slow on Chromium), 2014/11/24 20:12:16]

Don't forget to mention |mode|. Also you should mention that this pertains to
access(2).

[leecam, 2014/11/25 02:06:49]

Done.

+  // If |file_to_open| is not NULL it is set to point to either

[jln (very slow on Chromium), 2014/11/24 20:12:16]

s/file_to_open/file_to_access/ in the comments.

[leecam, 2014/11/25 02:06:49]

Done.

+  // the |requested_filename| in the case of a recursive match,
+  // or a pointer the matched path in the whitelist if an absolute
+  // match.
+  // Async signal safe if |file_to_open| is NULL
+  bool CheckAccess(const char* requested_filename,
+                   int mode,
+                   const char** file_to_access) const;
+
+ private:
+  BrokerFilePermission(std::string path,

[jln (very slow on Chromium), 2014/11/24 20:12:16]

With C++11, this becomes less of an issue, but there is no reason for a copy
here.
Use const std::string path& instead.

[leecam, 2014/11/25 02:06:49]

Done.

+                       bool recursive,
+                       bool unlink,
+                       bool allow_read,
+                       bool allow_write,
+                       bool allow_create);
+  bool IsPathCoveredByThisPermission(const char* requested_filename) const;

[Jorge Lucangeli Obes, 2014/11/20 00:13:20]

Since this method lives in the Permission class, "ByThisPermission" is probably
redundant? Maybe something like "CoversPath"?

[leecam, 2014/11/20 00:56:39]

Done.

+
+  const std::string path_;
+  const bool
+      recursive_;  // Allow everything under this path. |path| must be a dir.
+  const bool unlink_;  // unlink after openning.

[Jorge Lucangeli Obes, 2014/11/20 00:13:20]

"opening"

[leecam, 2014/11/20 00:56:39]

Done.

+  const bool allow_read_;
+  const bool allow_write_;
+  const bool allow_create_;
+};
+
+}  // namespace syscall_broker
+
+}  // namespace sandbox
+
+#endif  //  SANDBOX_LINUX_SYSCALL_BROKER_BROKER_FILE_PERMISSION_H_