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

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

Index: sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
index 365092ed4052e21f011997a6912fa2211339ae63..021ec795058e6aa6c5d78137780fab8c65024d0b 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
@@ -7,11 +7,14 @@
 
 #include <ostream>
 
+#include "base/memory/scoped_ptr.h"
 #include "sandbox/linux/seccomp-bpf/bpf_tests.h"
 #include "sandbox/linux/seccomp-bpf/verifier.h"
+#include "sandbox/linux/services/broker_process.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using namespace playground2;
+using sandbox::BrokerProcess;
 
 namespace {
 
@@ -480,4 +483,79 @@ BPF_TEST(SandboxBpf, UnsafeTrapWithErrno, RedirectAllSyscallsPolicy) {
   BPF_ASSERT(errno == 0);
 }
 
+// Test a trap handler that makes use of a broker process to open().
+
+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 BrokerProcess(allowed_files));
+    BPF_ASSERT(broker_process() != NULL);
+    BPF_ASSERT(broker_process_->Init(NULL));
+
+    initialized_ = true;
+  }
+  bool initialized() { return initialized_; }
+  class BrokerProcess* broker_process() { return broker_process_.get(); }
+ private:
+  bool initialized_;
+  scoped_ptr<class BrokerProcess> broker_process_;
+  DISALLOW_COPY_AND_ASSIGN(InitializedOpenBroker);
+};
+
+intptr_t BrokerOpenTrapHandler(const struct arch_seccomp_data& args,
+                               void *aux) {

[Markus (顧孟勤), 2012/12/13 09:48:52]

I would probably have moved the TrapHandler inside of the class, but that's
really just a question of personal preference.

If you move it inside of the class, it would still need to be static, but you
could make "aux" be a pointer to "this".

+  BPF_ASSERT(aux);
+  BrokerProcess* broker_process = static_cast<BrokerProcess*>(aux);
+  return broker_process->Open(reinterpret_cast<const char*>(args.args[0]),
+                              static_cast<int>(args.args[1]));
+}
+
+ErrorCode DenyOpenPolicy(int sysno, void *aux) {
+  InitializedOpenBroker* iob = static_cast<InitializedOpenBroker*>(aux);

[Markus (顧孟勤), 2012/12/13 09:48:52]

I probably also would have moved the body of this method into the class. That
can potentially make some of the code a little cleaner.

See my pending CL for argument inspection for an example of how I envisioned the
use of classes in BPF_TESTs.

But as mentioned earlier, it really turns out to be a personal preference. If
you think of the class as mere container, then what you are doing makes sense.
If you think of the class as the object that is being tested, then it makes more
sense to move as much as possible into the class.

+  if (!Sandbox::isValidSyscallNumber(sysno)) {
+    return ErrorCode(ENOSYS);
+  }
+
+  switch (sysno) {
+    case __NR_open:
+    case __NR_openat:
+      // We get a InitializedOpenBroker class, but our trap handler wants
+      // the BrokerProcess object.
+      return ErrorCode(Sandbox::Trap(BrokerOpenTrapHandler,
+                       iob->broker_process()));

[Markus (顧孟勤), 2012/12/13 09:48:52]

Indentation is a little funny, I think

[jln (very slow on Chromium), 2012/12/13 19:27:17]

Done.

+    default:
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+  }
+}
+
+// We use a InitializedOpenBroker class since, so that we can run unsandboxed

[Markus (顧孟勤), 2012/12/13 09:48:52]

s/ since//

[jln (very slow on Chromium), 2012/12/13 19:27:17]

Done.

+// code in its constructor, which is the only way to do so in a BPF_TEST.
+BPF_TEST(SandboxBpf, UseOpenBroker, DenyOpenPolicy,
+         class InitializedOpenBroker /* BPF_AUX */) {

[Markus (顧孟勤), 2012/12/13 09:48:52]

The "class" is superfluous here, and I don't think our code typically adds it.
But feel free to correct me if the style guide says otherwise.

[jln (very slow on Chromium), 2012/12/13 19:27:17]

Done.

+  BPF_ASSERT(BPF_AUX.initialized());
+  class BrokerProcess* broker_process =  BPF_AUX.broker_process();
+  BPF_ASSERT(broker_process != NULL);
+
+  // First, use the broker "manually"
+  BPF_ASSERT(broker_process->Open("/proc/denied", O_RDONLY) == -EPERM);
+  BPF_ASSERT(broker_process->Open("/proc/allowed", O_RDONLY) == -ENOENT);
+
+  // Now use glibc's open() as an external library would.
+  BPF_ASSERT(open("/proc/denied", O_RDONLY) == -1);
+  BPF_ASSERT(errno == EPERM);
+
+  BPF_ASSERT(open("/proc/allowed", O_RDONLY) == -1);
+  BPF_ASSERT(errno == ENOENT);
+
+  // This is also white listed and does exist.
+  int cpu_info_fd = open("/proc/cpuinfo", O_RDONLY);
+  BPF_ASSERT(cpu_info_fd >= 0);
+  char buf[1024];
+  BPF_ASSERT(read(cpu_info_fd, buf, sizeof(buf)) > 0);
+}
+
 } // namespace