Seccomp sandbox: Add a policy flag to allow file namespace access to be disabled

This allows file namespace access to be turned on for the purpose of
testing, and we use this in some of the tests, but it is disabled by
default.

This synchronises the Chromium copy with r88 in the non-Chromium copy of
seccomp-sandbox.

BUG=none
TEST=make test

Review URL: http://codereview.chromium.org/3248002

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

6 files changed, 103 insertions, 14 deletions

diff --git a/sandbox/linux/seccomp/access.cc b/sandbox/linux/seccomp/access.cc
index a318e92..fbe7e53 100644
--- a/sandbox/linux/seccomp/access.cc
+++ b/sandbox/linux/seccomp/access.cc
@@ -62,6 +62,20 @@ bool Sandbox::process_access(int parentMapsFd, int sandboxFd, int threadFdPub,
     }
     return false;
   }
+
+  if (!g_policy.allow_file_namespace) {
+    // After locking the mutex, we can no longer abandon the system call. So,
+    // perform checks before clobbering the securely shared memory.
+    char tmp[access_req.path_length];
+    if (read(sys, sandboxFd, tmp, access_req.path_length) !=
+        (ssize_t)access_req.path_length) {
+      goto read_parm_failed;
+    }
+    Debug::message(("Denying access to \"" + std::string(tmp) + "\"").c_str());
+    SecureMem::abandonSystemCall(threadFd, -EACCES);
+    return false;
+  }
+
   SecureMem::lockSystemCall(parentMapsFd, mem);
   if (read(sys, sandboxFd, mem->pathname, access_req.path_length) !=
       (ssize_t)access_req.path_length) {
diff --git a/sandbox/linux/seccomp/open.cc b/sandbox/linux/seccomp/open.cc
index 73263d1..8a9093c 100644
--- a/sandbox/linux/seccomp/open.cc
+++ b/sandbox/linux/seccomp/open.cc
@@ -63,7 +63,8 @@ bool Sandbox::process_open(int parentMapsFd, int sandboxFd, int threadFdPub,
     return false;
   }
 
-  if ((open_req.flags & O_ACCMODE) != O_RDONLY) {
+  if ((open_req.flags & O_ACCMODE) != O_RDONLY ||
+      !g_policy.allow_file_namespace) {
     // After locking the mutex, we can no longer abandon the system call. So,
     // perform checks before clobbering the securely shared memory.
     char tmp[open_req.path_length];
diff --git a/sandbox/linux/seccomp/sandbox_impl.h b/sandbox/linux/seccomp/sandbox_impl.h
index 4fe96cf..3e99a5510 100644
--- a/sandbox/linux/seccomp/sandbox_impl.h
+++ b/sandbox/linux/seccomp/sandbox_impl.h
@@ -696,6 +696,17 @@ class Sandbox {
   static std::vector<SecureMem::Args*> secureMemPool_;
 };
 
+// If this struct is extended to contain parameters that are read by
+// the trusted thread, we will have to mprotect() it to be read-only when
+// starting the sandbox.  However, currently it is read only by the
+// trusted process, and the sandboxed process cannot change the values
+// that the fork()'d trusted process sees.
+struct SandboxPolicy {
+  bool allow_file_namespace;  // Allow filename-based system calls.
+};
+
+extern struct SandboxPolicy g_policy;
+
 } // namespace
 
 using playground::Sandbox;
diff --git a/sandbox/linux/seccomp/stat.cc b/sandbox/linux/seccomp/stat.cc
index 9277663..cdf7e4c 100644
--- a/sandbox/linux/seccomp/stat.cc
+++ b/sandbox/linux/seccomp/stat.cc
@@ -151,17 +151,47 @@ bool Sandbox::process_stat(int parentMapsFd, int sandboxFd, int threadFdPub,
     }
     return false;
   }
+  if (stat_req.sysnum != __NR_stat && stat_req.sysnum != __NR_lstat
+    #ifdef __NR_stat64
+      && stat_req.sysnum != __NR_stat64
+    #endif
+    #ifdef __NR_lstat64
+      && stat_req.sysnum != __NR_lstat64
+    #endif
+     ) {
+    die("Corrupted stat() request");
+  }
 
-  // We implement a strict policy of denying all file accesses
-  char tmp[stat_req.path_length + 1];
-  if (read(sys, sandboxFd, tmp, stat_req.path_length) !=
+  if (!g_policy.allow_file_namespace) {
+    // After locking the mutex, we can no longer abandon the system call. So,
+    // perform checks before clobbering the securely shared memory.
+    char tmp[stat_req.path_length];
+    if (read(sys, sandboxFd, tmp, stat_req.path_length) !=
+        (ssize_t)stat_req.path_length) {
+      goto read_parm_failed;
+    }
+    Debug::message(("Denying access to \"" + std::string(tmp) + "\"").c_str());
+    SecureMem::abandonSystemCall(threadFd, -EACCES);
+    return false;
+  }
+
+  SecureMem::lockSystemCall(parentMapsFd, mem);
+  if (read(sys, sandboxFd, mem->pathname, stat_req.path_length) !=
       (ssize_t)stat_req.path_length) {
     goto read_parm_failed;
   }
-  tmp[stat_req.path_length] = '\000';
-  Debug::message(("Denying access to \"" + std::string(tmp) + "\"").c_str());
-  SecureMem::abandonSystemCall(threadFd, -EACCES);
-  return false;
+  mem->pathname[stat_req.path_length] = '\000';
+
+  // TODO(markus): Implement sandboxing policy
+  Debug::message(("Allowing access to \"" + std::string(mem->pathname) +
+                  "\"").c_str());
+
+  // Tell trusted thread to stat the file.
+  SecureMem::sendSystemCall(threadFdPub, true, parentMapsFd, mem,
+                            stat_req.sysnum,
+                            mem->pathname - (char*)mem + (char*)mem->self,
+                            stat_req.buf);
+  return true;
 }
 
 } // namespace
diff --git a/sandbox/linux/seccomp/tests/test_syscalls.cc b/sandbox/linux/seccomp/tests/test_syscalls.cc
index e67f27f..3e6acd5 100644
--- a/sandbox/linux/seccomp/tests/test_syscalls.cc
+++ b/sandbox/linux/seccomp/tests/test_syscalls.cc
@@ -88,6 +88,7 @@ static void *thread_func(void *x) {
 }
 
 TEST(test_thread) {
+  playground::g_policy.allow_file_namespace = true;  // To allow count_fds()
   StartSeccompSandbox();
   int fd_count1 = count_fds();
   pthread_t tid;
@@ -135,6 +136,7 @@ static void *get_tls_base() {
 }
 
 TEST(test_clone) {
+  playground::g_policy.allow_file_namespace = true;  // To allow count_fds()
   StartSeccompSandbox();
   int fd_count1 = count_fds();
   int stack_size = 0x1000;
@@ -345,7 +347,21 @@ TEST(test_socket) {
   assert(errno == EINVAL || errno == ENOSYS);
 }
 
-TEST(test_open) {
+TEST(test_open_disabled) {
+  StartSeccompSandbox();
+  int fd = open("/dev/null", O_RDONLY);
+  assert(fd == -1);
+  assert(errno == EACCES);
+
+  // Writing to the policy flag does not change this.
+  playground::g_policy.allow_file_namespace = true;
+  fd = open("/dev/null", O_RDONLY);
+  assert(fd == -1);
+  assert(errno == EACCES);
+}
+
+TEST(test_open_enabled) {
+  playground::g_policy.allow_file_namespace = true;
   StartSeccompSandbox();
   int fd = open("/dev/null", O_RDONLY);
   assert(fd >= 0);
@@ -356,7 +372,15 @@ TEST(test_open) {
   assert(errno == EACCES);
 }
 
-TEST(test_access) {
+TEST(test_access_disabled) {
+  StartSeccompSandbox();
+  int rc = access("/dev/null", R_OK);
+  assert(rc == -1);
+  assert(errno == EACCES);
+}
+
+TEST(test_access_enabled) {
+  playground::g_policy.allow_file_namespace = true;
   StartSeccompSandbox();
   int rc = access("/dev/null", R_OK);
   assert(rc == 0);
@@ -365,16 +389,23 @@ TEST(test_access) {
   assert(errno == ENOENT);
 }
 
-TEST(test_stat) {
+TEST(test_stat_disabled) {
   StartSeccompSandbox();
   struct stat st;
-  // All file accesses are denied. TODO: Update if the sandbox gets a more
-  // fine-grained policy
   int rc = stat("/dev/null", &st);
   assert(rc == -1);
+  assert(errno == EACCES);
+}
+
+TEST(test_stat_enabled) {
+  playground::g_policy.allow_file_namespace = true;
+  StartSeccompSandbox();
+  struct stat st;
+  int rc = stat("/dev/null", &st);
+  assert(rc == 0);
   rc = stat("path-that-does-not-exist", &st);
   assert(rc == -1);
-  assert(errno == EACCES);
+  assert(errno == ENOENT);
 }
 
 static int g_value;
diff --git a/sandbox/linux/seccomp/trusted_process.cc b/sandbox/linux/seccomp/trusted_process.cc
index 80adbf6..5c62b0f 100644
--- a/sandbox/linux/seccomp/trusted_process.cc
+++ b/sandbox/linux/seccomp/trusted_process.cc
@@ -11,6 +11,8 @@
 
 namespace playground {
 
+struct SandboxPolicy g_policy;
+
 struct Thread {
   int              fdPub, fd;
   SecureMem::Args* mem;