Bug 1289718 - Add support for link. draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Tue, 23 Aug 2016 20:00:09 +0200
changeset 409933 2f9e7af79ac69608b33d9500c122e051e784f60a
parent 409932 99dd3a730013fc0aa97be7792cd9e94ea3ea85d3
child 409934 8c08d3a0ba62a07cd0912af1dc725d6454ef05dd
push id28613
push usergpascutto@mozilla.com
push dateMon, 05 Sep 2016 18:00:21 +0000
bugs1289718
milestone51.0a1
Bug 1289718 - Add support for link. MozReview-Commit-ID: LmsTqv0GzC2
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
@@ -30,23 +30,25 @@ SandboxBrokerClient::SandboxBrokerClient
 
 SandboxBrokerClient::~SandboxBrokerClient()
 {
   close(mFileDesc);
 }
 
 int
 SandboxBrokerClient::DoCall(const Request* aReq, const char* aPath,
-                            struct stat* aStat, bool expectFd)
+                            const char* aPath2, struct stat* aStat,
+                            bool expectFd)
 {
   // Remap /proc/self to the actual pid, so that the broker can open
   // it.  This happens here instead of in the broker to follow the
   // principle of least privilege and keep the broker as simple as
   // possible.  (Note: when pid namespaces happen, this will also need
   // to remap the inner pid to the outer pid.)
+  // We only remap the first path.
   static const char kProcSelf[] = "/proc/self/";
   static const size_t kProcSelfLen = sizeof(kProcSelf) - 1;
   const char* path = aPath;
   // This buffer just needs to be large enough for any such path that
   // the policy would actually allow.  sizeof("/proc/2147483647/") == 18.
   char rewrittenPath[64];
   if (strncmp(aPath, kProcSelf, kProcSelfLen) == 0) {
     ssize_t len =
@@ -57,36 +59,48 @@ SandboxBrokerClient::DoCall(const Reques
         SANDBOX_LOG_ERROR("rewriting %s -> %s", aPath, rewrittenPath);
       }
       path = rewrittenPath;
     } else {
       SANDBOX_LOG_ERROR("not rewriting unexpectedly long path %s", aPath);
     }
   }
 
-  struct iovec ios[2];
-  int respFds[2];
+  struct iovec ios[3];
+  int respFds[3];
 
   // Set up iovecs for request + path.
   ios[0].iov_base = const_cast<Request*>(aReq);
   ios[0].iov_len = sizeof(*aReq);
   ios[1].iov_base = const_cast<char*>(path);
   ios[1].iov_len = strlen(path);
+  if (aPath2 != nullptr) {
+    ios[2].iov_base = const_cast<char*>(aPath2);
+    ios[2].iov_len = strlen(aPath2);
+  } else {
+    ios[2].iov_base = 0;
+    ios[2].iov_len = 0;
+  }
   if (ios[1].iov_len > kMaxPathLen) {
     return -ENAMETOOLONG;
   }
+  if (ios[2].iov_len > kMaxPathLen) {
+    return -ENAMETOOLONG;
+  }
 
   // Create response socket and send request.
   if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, respFds) < 0) {
     return -errno;
   }
-  const ssize_t sent = SendWithFd(mFileDesc, ios, 2, respFds[1]);
+  const ssize_t sent = SendWithFd(mFileDesc, ios, 3, respFds[1]);
   const int sendErrno = errno;
   MOZ_ASSERT(sent < 0 ||
-	     static_cast<size_t>(sent) == ios[0].iov_len + ios[1].iov_len);
+             static_cast<size_t>(sent) == ios[0].iov_len
+                                        + ios[1].iov_len
+                                        + ios[2].iov_len);
   close(respFds[1]);
   if (sent < 0) {
     close(respFds[0]);
     return -sendErrno;
   }
 
   // Set up iovecs for response.
   Response resp;
@@ -141,48 +155,55 @@ SandboxBrokerClient::DoCall(const Reques
   }
   return -resp.mError;
 }
 
 int
 SandboxBrokerClient::Open(const char* aPath, int aFlags)
 {
   Request req = { SANDBOX_FILE_OPEN, aFlags };
-  int maybeFd = DoCall(&req, aPath, nullptr, true);
+  int maybeFd = DoCall(&req, aPath, nullptr, nullptr, true);
   if (maybeFd >= 0) {
     // NSPR has opinions about file flags.  Fix O_CLOEXEC.
     if ((aFlags & O_CLOEXEC) == 0) {
       fcntl(maybeFd, F_SETFD, 0);
     }
   }
   return maybeFd;
 }
 
 int
 SandboxBrokerClient::Access(const char* aPath, int aMode)
 {
   Request req = { SANDBOX_FILE_ACCESS, aMode };
-  return DoCall(&req, aPath, nullptr, false);
+  return DoCall(&req, aPath, nullptr, nullptr, false);
 }
 
 int
 SandboxBrokerClient::Stat(const char* aPath, struct stat* aStat)
 {
   Request req = { SANDBOX_FILE_STAT, 0 };
-  return DoCall(&req, aPath, aStat, false);
+  return DoCall(&req, aPath, nullptr, aStat, false);
 }
 
 int
 SandboxBrokerClient::LStat(const char* aPath, struct stat* aStat)
 {
   Request req = { SANDBOX_FILE_STAT, O_NOFOLLOW };
-  return DoCall(&req, aPath, aStat, false);
+  return DoCall(&req, aPath, nullptr, aStat, false);
 }
 
 int
 SandboxBrokerClient::Chmod(const char* aPath, int aMode)
 {
   Request req = {SANDBOX_FILE_CHMOD, aMode};
-  return DoCall(&req, aPath, nullptr, false);
+  return DoCall(&req, aPath, nullptr, nullptr, false);
+}
+
+int
+SandboxBrokerClient::Link(const char* aOldPath, const char* aNewPath)
+{
+  Request req = {SANDBOX_FILE_LINK, 0};
+  return DoCall(&req, aOldPath, aNewPath, nullptr, false);
 }
 
 } // namespace mozilla
 
--- a/security/sandbox/linux/SandboxBrokerClient.h
+++ b/security/sandbox/linux/SandboxBrokerClient.h
@@ -29,19 +29,23 @@ class SandboxBrokerClient final : privat
   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);
+  int Link(const char* aPath, const char* aPath2);
 
  private:
   int mFileDesc;
 
-  int DoCall(const Request* aReq, const char* aPath, struct stat* aStat,
+  int DoCall(const Request* aReq,
+             const char* aPath,
+             const char* aPath2,
+             struct stat* aStat,
              bool expectFd);
 };
 
 } // namespace mozilla
 
 #endif // mozilla_SandboxBrokerClient_h
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -411,16 +411,23 @@ class ContentSandboxPolicy : public Sand
 
   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 LinkTrap(ArgsRef aArgs, void *aux) {
+    auto broker = static_cast<SandboxBrokerClient*>(aux);
+    auto path = reinterpret_cast<const char*>(aArgs.args[0]);
+    auto path2 = reinterpret_cast<const char*>(aArgs.args[1]);
+    return broker->Link(path, path2);
+  }
+
   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:
@@ -510,16 +517,18 @@ public:
       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);
+      case __NR_link:
+        return Trap(LinkTrap, mBroker);
       }
     } else {
       // No broker; allow the syscalls directly.  )-:
       switch(sysno) {
       case __NR_open:
       case __NR_openat:
       case __NR_access:
       case __NR_faccessat:
@@ -540,17 +549,16 @@ public:
     case __NR_mkdir:
     case __NR_rmdir:
     case __NR_getcwd:
     CASES_FOR_statfs:
     CASES_FOR_fstatfs:
     case __NR_rename:
     case __NR_symlink:
     case __NR_quotactl:
-    case __NR_link:
     case __NR_unlink:
     CASES_FOR_fchown:
     case __NR_fchmod:
     case __NR_flock:
 #endif
       return Allow();
 
     case __NR_readlink:
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -359,20 +359,25 @@ DoStat(const char* aPath, struct stat* a
 {
   if (aFlags & O_NOFOLLOW) {
     return lstat(aPath, aStat);
   }
   return stat(aPath, aStat);
 }
 
 static int
-DoChmod(const char* aPath, int* retVal, int aMode)
+DoChmod(const char* aPath, int aMode)
 {
-  *retVal = chmod(aPath, aMode);
-  return *retVal;
+  return chmod(aPath, aMode);
+}
+
+static int
+DoLink(const char* aPath, const char* aPath2)
+{
+  return link(aPath, aPath2);
 }
 
 size_t
 SandboxBroker::ConvertToRealPath(char* aPath, size_t aBufSize, size_t aPathLen)
 {
   if (strstr(aPath, "..") != NULL) {
     char* result = realpath(aPath, NULL);
     if (result != NULL) {
@@ -410,27 +415,34 @@ SandboxBroker::ThreadMain(void)
   if (syscall(nr_setregid, getgid(), AID_APP + mChildPid) != 0 ||
       syscall(nr_setreuid, getuid(), AID_APP + mChildPid) != 0) {
     MOZ_CRASH("SandboxBroker: failed to drop privileges");
   }
 #endif
 
   while (true) {
     struct iovec ios[2];
+    // We will receive the path strings in 1 buffer an split them back up.
+    char recvBuf[2 * (kMaxPathLen + 1)];
     char pathBuf[kMaxPathLen + 1];
+    char pathBuf2[kMaxPathLen + 1];
     size_t pathLen;
+    size_t pathLen2;
     struct stat statBuf;
     Request req;
     Response resp;
     int respfd;
 
+    // This makes our string handling below a bit less error prone.
+    memset(recvBuf, 0, sizeof(recvBuf));
+
     ios[0].iov_base = &req;
     ios[0].iov_len = sizeof(req);
-    ios[1].iov_base = pathBuf;
-    ios[1].iov_len = kMaxPathLen;
+    ios[1].iov_base = recvBuf;
+    ios[1].iov_len = sizeof(recvBuf);
 
     const ssize_t recvd = RecvWithFd(mFileDesc, ios, 2, &respfd);
     if (recvd == 0) {
       if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {
         SANDBOX_LOG_ERROR("EOF from pid %d", mChildPid);
       }
       break;
     }
@@ -459,26 +471,49 @@ 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;
+
+    // Clear permissions
+    int perms;
+
+    // Find end of first string
+    char* split = static_cast<char*>(memchr(recvBuf, 0, sizeof(recvBuf)));
+    size_t splitidx = split - recvBuf;
+    if (splitidx <= kMaxPathLen + 1) {
+      strcpy(pathBuf, recvBuf);
+      strncpy(pathBuf2, split + 1, kMaxPathLen + 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);
-    int perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
+      // Look up the first pathname.
+      // Force 0 termination.
+      pathLen = strnlen(pathBuf, kMaxPathLen + 1);
+      pathBuf[pathLen] = '\0';
+      // Translate relative paths
+      pathLen = ConvertToRealPath(pathBuf, sizeof(pathBuf), pathLen);
+      perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
+
+      // Same for second path
+      pathLen2 = strnlen(pathBuf2, kMaxPathLen + 1);
+      if (pathLen2 > 0) {
+        pathBuf[pathLen2] = '\0';
+        pathLen2 = ConvertToRealPath(pathBuf2, sizeof(pathBuf2), pathLen2);
+        int perms2 = mPolicy->Lookup(nsDependentCString(pathBuf2, pathLen2));
+
+        // Take the intersection of the permissions for both paths
+        perms &= perms2;
+      }
+    } else {
+      // Failed to receive intelligible paths.
+      perms = 0;
+    }
 
     // And now perform the operation if allowed.
     if (perms & CRASH_INSTEAD) {
       // This is somewhat nonmodular, but it works.
       resp.mError = ENOSYS;
     } else if (permissive || perms & MAY_ACCESS) {
       // If the operation was only allowed because of permissive mode, log it.
       if (permissive && !(perms & MAY_ACCESS)) {
@@ -532,17 +567,27 @@ SandboxBroker::ThreadMain(void)
           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) {
+          if (DoChmod(pathBuf,req.mFlags) == 0) {
+            resp.mError = 0;
+          } else {
+            resp.mError = errno;
+          }
+        }
+        break;
+
+      case SANDBOX_FILE_LINK:
+        if (permissive || AllowOperation(W_OK, perms)) {
+          if (DoLink(pathBuf, pathBuf2) == 0) {
             resp.mError = 0;
           } else {
             resp.mError = errno;
           }
         }
         break;
       }
     } else {
--- a/security/sandbox/linux/broker/SandboxBrokerCommon.h
+++ b/security/sandbox/linux/broker/SandboxBrokerCommon.h
@@ -25,16 +25,17 @@ namespace mozilla {
 
 class SandboxBrokerCommon {
 public:
   enum Operation {
     SANDBOX_FILE_OPEN,
     SANDBOX_FILE_ACCESS,
     SANDBOX_FILE_STAT,
     SANDBOX_FILE_CHMOD,
+    SANDBOX_FILE_LINK,
   };
 
   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.