Bug 1289718 - Add Chmod to the file broker. draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Tue, 23 Aug 2016 10:01:48 +0200
changeset 409932 99dd3a730013fc0aa97be7792cd9e94ea3ea85d3
parent 409931 bd265df759b4d7e3522090697a682f7530bfaaa8
child 409933 2f9e7af79ac69608b33d9500c122e051e784f60a
push id28613
push usergpascutto@mozilla.com
push dateMon, 05 Sep 2016 18:00:21 +0000
bugs1289718
milestone51.0a1
Bug 1289718 - Add Chmod to the file broker. MozReview-Commit-ID: BOYSSof3t7
security/sandbox/linux/SandboxBrokerClient.cpp
security/sandbox/linux/SandboxBrokerClient.h
security/sandbox/linux/SandboxFilter.cpp
security/sandbox/linux/broker/SandboxBroker.cpp
security/sandbox/linux/broker/SandboxBrokerCommon.h
--- a/security/sandbox/linux/SandboxBrokerClient.cpp
+++ b/security/sandbox/linux/SandboxBrokerClient.cpp
@@ -172,10 +172,17 @@ SandboxBrokerClient::Stat(const char* aP
 
 int
 SandboxBrokerClient::LStat(const char* aPath, struct stat* aStat)
 {
   Request req = { SANDBOX_FILE_STAT, O_NOFOLLOW };
   return DoCall(&req, aPath, aStat, false);
 }
 
+int
+SandboxBrokerClient::Chmod(const char* aPath, int aMode)
+{
+  Request req = {SANDBOX_FILE_CHMOD, aMode};
+  return DoCall(&req, aPath, nullptr, false);
+}
+
 } // namespace mozilla
 
--- a/security/sandbox/linux/SandboxBrokerClient.h
+++ b/security/sandbox/linux/SandboxBrokerClient.h
@@ -28,16 +28,17 @@ class SandboxBrokerClient final : privat
  public:
   explicit SandboxBrokerClient(int aFd);
   ~SandboxBrokerClient();
 
   int Open(const char* aPath, int aFlags);
   int Access(const char* aPath, int aMode);
   int Stat(const char* aPath, struct stat* aStat);
   int LStat(const char* aPath, struct stat* aStat);
+  int Chmod(const char* aPath, int aMode);
 
  private:
   int mFileDesc;
 
   int DoCall(const Request* aReq, const char* aPath, struct stat* aStat,
              bool expectFd);
 };
 
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -404,16 +404,23 @@ class ContentSandboxPolicy : public Sand
                         (flags & ~AT_SYMLINK_NOFOLLOW), fd, path, buf, flags);
       return BlockedSyscallTrap(aArgs, nullptr);
     }
     return (flags & AT_SYMLINK_NOFOLLOW) == 0
       ? broker->Stat(path, buf)
       : broker->LStat(path, buf);
   }
 
+  static intptr_t ChmodTrap(ArgsRef aArgs, void* aux) {
+    auto broker = static_cast<SandboxBrokerClient*>(aux);
+    auto path = reinterpret_cast<const char*>(aArgs.args[0]);
+    auto mode = static_cast<mode_t>(aArgs.args[1]);
+    return broker->Chmod(path, mode);
+  }
+
   static intptr_t GetPPidTrap(ArgsRef aArgs, void* aux) {
     // In a pid namespace, getppid() will return 0. We will return 0 instead
     // of the real parent pid to see what breaks when we introduce the
     // pid namespace (Bug 1151624).
     return 0;
   }
 
 public:
@@ -501,16 +508,18 @@ public:
       case __NR_faccessat:
         return Trap(AccessAtTrap, mBroker);
       CASES_FOR_stat:
         return Trap(StatTrap, mBroker);
       CASES_FOR_lstat:
         return Trap(LStatTrap, mBroker);
       CASES_FOR_fstatat:
         return Trap(StatAtTrap, mBroker);
+      case __NR_chmod:
+        return Trap(ChmodTrap, mBroker);
       }
     } else {
       // No broker; allow the syscalls directly.  )-:
       switch(sysno) {
       case __NR_open:
       case __NR_openat:
       case __NR_access:
       case __NR_faccessat:
@@ -528,17 +537,16 @@ public:
 
       // Filesystem syscalls that need more work to determine who's
       // using them, if they need to be, and what we intend to about it.
     case __NR_mkdir:
     case __NR_rmdir:
     case __NR_getcwd:
     CASES_FOR_statfs:
     CASES_FOR_fstatfs:
-    case __NR_chmod:
     case __NR_rename:
     case __NR_symlink:
     case __NR_quotactl:
     case __NR_link:
     case __NR_unlink:
     CASES_FOR_fchown:
     case __NR_fchmod:
     case __NR_flock:
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -277,16 +277,29 @@ SandboxBroker::Policy::Lookup(const nsAC
   }
 
   // Strip away the RECURSIVE flag as it doesn't
   // necessarily apply to aPath.
   return allPerms & ~RECURSIVE;
 }
 
 static bool
+AllowOperation(int aReqFlags, int aPerms)
+{
+  int needed = 0;
+  if (aReqFlags & R_OK) {
+    needed |= SandboxBroker::MAY_READ;
+  }
+  if (aReqFlags & W_OK) {
+    needed |= SandboxBroker::MAY_WRITE;
+  }
+  return (aPerms & needed) == needed;
+}
+
+static bool
 AllowAccess(int aReqFlags, int aPerms)
 {
   if (aReqFlags & ~(R_OK|W_OK|F_OK)) {
     return false;
   }
   int needed = 0;
   if (aReqFlags & R_OK) {
     needed |= SandboxBroker::MAY_READ;
@@ -345,16 +358,23 @@ static int
 DoStat(const char* aPath, struct stat* aStat, int aFlags)
 {
   if (aFlags & O_NOFOLLOW) {
     return lstat(aPath, aStat);
   }
   return stat(aPath, aStat);
 }
 
+static int
+DoChmod(const char* aPath, int* retVal, int aMode)
+{
+  *retVal = chmod(aPath, aMode);
+  return *retVal;
+}
+
 size_t
 SandboxBroker::ConvertToRealPath(char* aPath, size_t aBufSize, size_t aPathLen)
 {
   if (strstr(aPath, "..") != NULL) {
     char* result = realpath(aPath, NULL);
     if (result != NULL) {
       strncpy(aPath, result, aBufSize);
       aPath[aBufSize - 1] = '\0';
@@ -439,16 +459,17 @@ SandboxBroker::ThreadMain(void)
     memset(&resp, 0, sizeof(resp));
     memset(&statBuf, 0, sizeof(statBuf));
     resp.mError = EACCES;
     ios[0].iov_base = &resp;
     ios[0].iov_len = sizeof(resp);
     ios[1].iov_base = nullptr;
     ios[1].iov_len = 0;
     int openedFd = -1;
+    int returnVal = -1;
 
     // Look up the pathname.
     pathLen = recvd - sizeof(req);
     // It shouldn't be possible for recvmsg to violate this assertion,
     // but one more predictable branch shouldn't have much perf impact:
     MOZ_RELEASE_ASSERT(pathLen <= kMaxPathLen);
     pathBuf[pathLen] = '\0';
     pathLen = ConvertToRealPath(pathBuf, sizeof(pathBuf), pathLen);
@@ -508,16 +529,26 @@ SandboxBroker::ThreadMain(void)
         if (DoStat(pathBuf, &statBuf, req.mFlags) == 0) {
           resp.mError = 0;
           ios[1].iov_base = &statBuf;
           ios[1].iov_len = sizeof(statBuf);
         } else {
           resp.mError = errno;
         }
         break;
+
+      case SANDBOX_FILE_CHMOD:
+        if (permissive || AllowOperation(W_OK, perms)) {
+          if (DoChmod(pathBuf, &returnVal, req.mFlags) == 0) {
+            resp.mError = 0;
+          } else {
+            resp.mError = errno;
+          }
+        }
+        break;
       }
     } else {
       MOZ_ASSERT(perms == 0);
       AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
     }
 
     const size_t numIO = ios[1].iov_len > 0 ? 2 : 1;
     DebugOnly<const ssize_t> sent = SendWithFd(respfd, ios, numIO, openedFd);
--- a/security/sandbox/linux/broker/SandboxBrokerCommon.h
+++ b/security/sandbox/linux/broker/SandboxBrokerCommon.h
@@ -24,16 +24,17 @@ struct iovec;
 namespace mozilla {
 
 class SandboxBrokerCommon {
 public:
   enum Operation {
     SANDBOX_FILE_OPEN,
     SANDBOX_FILE_ACCESS,
     SANDBOX_FILE_STAT,
+    SANDBOX_FILE_CHMOD,
   };
 
   struct Request {
     Operation mOp;
     // For open, flags; for access, "mode"; for stat, O_NOFOLLOW for lstat.
     int mFlags;
     // The rest of the packet is the pathname.
     // SCM_RIGHTS for response socket attached.