Backed out 2 changesets (bug 1380701) for bustage in SandboxBroker a=backout
authorWes Kocher <wkocher@mozilla.com>
Wed, 16 Aug 2017 09:46:48 -0700
changeset 424593 bb2e59766bada3c9528e48c70b60ee0681c3db1e
parent 424592 06d739de4ad093cd8f0cbb37cc71b4859d59999c
child 424594 1d38626ba9686d119e489db538ce84f8f9854217
child 424639 d8d3540478ec7cba213884a4b55fe30a05685e69
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1380701
milestone57.0a1
backs out6cef83dd4d11538dfd27357d8e5fd21014ad82b8
4456ebfe5657fa3ab3d0e83f8d3494122588cb06
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 2 changesets (bug 1380701) for bustage in SandboxBroker a=backout Backed out changeset 6cef83dd4d11 (bug 1380701) Backed out changeset 4456ebfe5657 (bug 1380701) MozReview-Commit-ID: Cnfj7TZvCbv
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
security/sandbox/linux/gtest/TestBroker.cpp
--- a/security/sandbox/linux/SandboxBrokerClient.cpp
+++ b/security/sandbox/linux/SandboxBrokerClient.cpp
@@ -30,17 +30,18 @@ SandboxBrokerClient::SandboxBrokerClient
 
 SandboxBrokerClient::~SandboxBrokerClient()
 {
   close(mFileDesc);
 }
 
 int
 SandboxBrokerClient::DoCall(const Request* aReq, const char* aPath,
-                            void* aResponseBuff, bool expectFd)
+                            const char* aPath2, void* aResponseBuff,
+                            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/";
@@ -58,38 +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];
+  struct iovec ios[3];
   int respFds[2];
 
   // 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);
+  ios[1].iov_len = strlen(path) + 1;
+  if (aPath2 != nullptr) {
+    ios[2].iov_base = const_cast<char*>(aPath2);
+    ios[2].iov_len = strlen(aPath2) + 1;
+  } else {
+    ios[2].iov_base = nullptr;
+    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);
-
+                                        + 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;
@@ -140,76 +151,97 @@ SandboxBrokerClient::DoCall(const Reques
   }
   return resp.mError;
 }
 
 int
 SandboxBrokerClient::Open(const char* aPath, int aFlags)
 {
   Request req = { SANDBOX_FILE_OPEN, aFlags, 0 };
-  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, 0 };
-  return DoCall(&req, aPath, nullptr, false);
+  return DoCall(&req, aPath, nullptr, nullptr, false);
 }
 
 int
 SandboxBrokerClient::Stat(const char* aPath, statstruct* aStat)
 {
   Request req = { SANDBOX_FILE_STAT, 0, sizeof(statstruct) };
-  return DoCall(&req, aPath, (void*)aStat, false);
+  return DoCall(&req, aPath, nullptr, (void*)aStat, false);
 }
 
 int
 SandboxBrokerClient::LStat(const char* aPath, statstruct* aStat)
 {
   Request req = { SANDBOX_FILE_STAT, O_NOFOLLOW, sizeof(statstruct) };
-  return DoCall(&req, aPath, (void*)aStat, false);
+  return DoCall(&req, aPath, nullptr, (void*)aStat, false);
 }
 
 int
 SandboxBrokerClient::Chmod(const char* aPath, int aMode)
 {
   Request req = {SANDBOX_FILE_CHMOD, aMode, 0};
-  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, 0};
+  return DoCall(&req, aOldPath, aNewPath, nullptr, false);
+}
+
+int
+SandboxBrokerClient::Symlink(const char* aOldPath, const char* aNewPath)
+{
+  Request req = {SANDBOX_FILE_SYMLINK, 0, 0};
+  return DoCall(&req, aOldPath, aNewPath, nullptr, false);
+}
+
+int
+SandboxBrokerClient::Rename(const char* aOldPath, const char* aNewPath)
+{
+  Request req = {SANDBOX_FILE_RENAME, 0, 0};
+  return DoCall(&req, aOldPath, aNewPath, nullptr, false);
 }
 
 int
 SandboxBrokerClient::Mkdir(const char* aPath, int aMode)
 {
   Request req = {SANDBOX_FILE_MKDIR, aMode, 0};
-  return DoCall(&req, aPath, nullptr, false);
+  return DoCall(&req, aPath, nullptr, nullptr, false);
 }
 
 int
 SandboxBrokerClient::Unlink(const char* aPath)
 {
   Request req = {SANDBOX_FILE_UNLINK, 0, 0};
-  return DoCall(&req, aPath, nullptr, false);
+  return DoCall(&req, aPath, nullptr, nullptr, false);
 }
 
 int
 SandboxBrokerClient::Rmdir(const char* aPath)
 {
   Request req = {SANDBOX_FILE_RMDIR, 0, 0};
-  return DoCall(&req, aPath, nullptr, false);
+  return DoCall(&req, aPath, nullptr, nullptr, false);
 }
 
 int
 SandboxBrokerClient::Readlink(const char* aPath, void* aBuff, size_t aSize)
 {
   Request req = {SANDBOX_FILE_READLINK, 0, aSize};
-  return DoCall(&req, aPath, aBuff, false);
+  return DoCall(&req, aPath, nullptr, aBuff, false);
 }
 
 } // namespace mozilla
 
--- a/security/sandbox/linux/SandboxBrokerClient.h
+++ b/security/sandbox/linux/SandboxBrokerClient.h
@@ -30,25 +30,29 @@ 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, statstruct* aStat);
   int LStat(const char* aPath, statstruct* aStat);
   int Chmod(const char* aPath, int aMode);
+  int Link(const char* aPath, const char* aPath2);
   int Mkdir(const char* aPath, int aMode);
+  int Symlink(const char* aOldPath, const char* aNewPath);
+  int Rename(const char* aOldPath, const char* aNewPath);
   int Unlink(const char* aPath);
   int Rmdir(const char* aPath);
   int Readlink(const char* aPath, void* aBuf, size_t aBufSize);
 
  private:
   int mFileDesc;
 
   int DoCall(const Request* aReq,
              const char* aPath,
+             const char* aPath2,
              void *aReponseBuff,
              bool expectFd);
 };
 
 } // namespace mozilla
 
 #endif // mozilla_SandboxBrokerClient_h
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -441,16 +441,37 @@ private:
 
   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 SymlinkTrap(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->Symlink(path, path2);
+  }
+
+  static intptr_t RenameTrap(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->Rename(path, path2);
+  }
+
   static intptr_t MkdirTrap(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->Mkdir(path, mode);
   }
 
   static intptr_t RmdirTrap(ArgsRef aArgs, void* aux) {
@@ -589,18 +610,24 @@ 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);
       case __NR_mkdir:
         return Trap(MkdirTrap, mBroker);
+      case __NR_symlink:
+        return Trap(SymlinkTrap, mBroker);
+      case __NR_rename:
+        return Trap(RenameTrap, mBroker);
       case __NR_rmdir:
         return Trap(RmdirTrap, mBroker);
       case __NR_unlink:
         return Trap(UnlinkTrap, mBroker);
       case __NR_readlink:
         return Trap(ReadlinkTrap, mBroker);
       }
     } else {
@@ -609,17 +636,20 @@ public:
       case __NR_open:
       case __NR_openat:
       case __NR_access:
       case __NR_faccessat:
       CASES_FOR_stat:
       CASES_FOR_lstat:
       CASES_FOR_fstatat:
       case __NR_chmod:
+      case __NR_link:
       case __NR_mkdir:
+      case __NR_symlink:
+      case __NR_rename:
       case __NR_rmdir:
       case __NR_unlink:
       case __NR_readlink:
         return Allow();
       }
     }
 
     switch (sysno) {
--- a/security/sandbox/linux/broker/SandboxBroker.cpp
+++ b/security/sandbox/linux/broker/SandboxBroker.cpp
@@ -408,16 +408,29 @@ static int
 DoStat(const char* aPath, void* aBuff, int aFlags)
 {
  if (aFlags & O_NOFOLLOW) {
     return lstatsyscall(aPath, (statstruct*)aBuff);
   }
   return statsyscall(aPath, (statstruct*)aBuff);
 }
 
+static int
+DoLink(const char* aPath, const char* aPath2,
+       SandboxBrokerCommon::Operation aOper)
+{
+  if (aOper == SandboxBrokerCommon::Operation::SANDBOX_FILE_LINK) {
+    return link(aPath, aPath2);
+  }
+  if (aOper == SandboxBrokerCommon::Operation::SANDBOX_FILE_SYMLINK) {
+    return symlink(aPath, aPath2);
+  }
+  MOZ_CRASH("SandboxBroker: Unknown link operation");
+}
+
 size_t
 SandboxBroker::ConvertToRealPath(char* aPath, size_t aBufSize, size_t aPathLen)
 {
   if (strstr(aPath, "..") != nullptr) {
     char* result = realpath(aPath, nullptr);
     if (result != nullptr) {
       base::strlcpy(aPath, result, aBufSize);
       free(result);
@@ -496,29 +509,37 @@ SandboxBroker::ThreadMain(void)
 
   // Permissive mode can only be enabled through an environment variable,
   // therefore it is sufficient to fetch the value once
   // before the main thread loop starts
   bool permissive = SandboxInfo::Get().Test(SandboxInfo::kPermissive);
 
   while (true) {
     struct iovec ios[2];
-    char recvBuf[kMaxPathLen + 1];
+    // We will receive the path strings in 1 buffer and split them back up.
+    char recvBuf[2 * (kMaxPathLen + 1)];
+    char pathBuf[kMaxPathLen + 1];
+    char pathBuf2[kMaxPathLen + 1];
+    size_t pathLen = 0;
+    size_t pathLen2 = 0;
     char respBuf[kMaxPathLen + 1]; // Also serves as struct stat
     Request req;
     Response resp;
     int respfd;
 
     // Make sure stat responses fit in the response buffer
     MOZ_ASSERT((kMaxPathLen + 1) > sizeof(struct stat));
 
+    // 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 = recvBuf;
-    ios[1].iov_len = sizeof(recvBuf) - 1;
+    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;
     }
@@ -540,45 +561,80 @@ SandboxBroker::ThreadMain(void)
     if (respfd == -1) {
       SANDBOX_LOG_ERROR("no response fd from pid %d", mChildPid);
       shutdown(mFileDesc, SHUT_RD);
       break;
     }
 
     // Initialize the response with the default failure.
     memset(&resp, 0, sizeof(resp));
+    memset(&respBuf, 0, sizeof(respBuf));
     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;
 
-    size_t origPathLen = static_cast<size_t>(recvd) - sizeof(req);
-    // Null-terminate to get a C-style string.
-    MOZ_RELEASE_ASSERT(origPathLen < sizeof(recvBuf));
-    recvBuf[origPathLen] = '\0';
+    // Clear permissions
+    int perms;
+
+    // Find end of first string, make sure the buffer is still
+    // 0 terminated.
+    size_t recvBufLen = static_cast<size_t>(recvd) - sizeof(req);
+    if (recvBufLen > 0 && recvBuf[recvBufLen - 1] != 0) {
+      SANDBOX_LOG_ERROR("corrupted path buffer from pid %d", mChildPid);
+      shutdown(mFileDesc, SHUT_RD);
+      break;
+    }
+
+    // First path should fit in maximum path length buffer.
+    size_t first_len = strlen(recvBuf);
+    if (first_len <= kMaxPathLen) {
+      strcpy(pathBuf, recvBuf);
+      // Skip right over the terminating 0, and try to copy in the
+      // second path, if any. If there's no path, this will hit a
+      // 0 immediately (we nulled the buffer before receiving).
+      // We do not assume the second path is 0-terminated, this is
+      // enforced below.
+      strncpy(pathBuf2, recvBuf + first_len + 1, kMaxPathLen + 1);
+
+      // First string is guaranteed to be 0-terminated.
+      pathLen = first_len;
 
-    // Look up the pathname but first translate relative paths.
-    // (Make a copy so we can get back the original path if needed.)
-    char pathBuf[kMaxPathLen + 1];
-    base::strlcpy(pathBuf, recvBuf, sizeof(pathBuf));
-    size_t pathLen = ConvertToRealPath(pathBuf, sizeof(pathBuf), origPathLen);
-    int perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
+      // Look up the first pathname but first translate relative paths.
+      pathLen = ConvertToRealPath(pathBuf, sizeof(pathBuf), pathLen);
+      perms = mPolicy->Lookup(nsDependentCString(pathBuf, pathLen));
 
-    // We don't have read permissions on the requested dir.
-    // Did we arrive from a symlink in a path that is not writable?
-    // Then try to figure out the original path and see if that is readable.
-    if (!(perms & MAY_READ)) {
-      // Work on the original path,
-      // this reverses ConvertToRealPath above.
-      int symlinkPerms = SymlinkPermissions(recvBuf, origPathLen);
-      if (symlinkPerms > 0) {
-        perms = symlinkPerms;
+      // We don't have read permissions on the requested dir.
+      // Did we arrive from a symlink in a path that is not writable?
+      // Then try to figure out the original path and see if that is readable.
+      if (!(perms & MAY_READ)) {
+          // Work on the original path,
+          // this reverses ConvertToRealPath above.
+          int symlinkPerms = SymlinkPermissions(recvBuf, first_len);
+          if (symlinkPerms > 0) {
+            perms = symlinkPerms;
+          }
       }
+
+      // Same for the second path.
+      pathLen2 = strnlen(pathBuf2, kMaxPathLen);
+      if (pathLen2 > 0) {
+        // Force 0 termination.
+        pathBuf2[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.
@@ -633,16 +689,41 @@ SandboxBroker::ThreadMain(void)
           } else {
             resp.mError = -errno;
           }
         } else {
           AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
         }
         break;
 
+      case SANDBOX_FILE_LINK:
+      case SANDBOX_FILE_SYMLINK:
+        if (permissive || AllowOperation(W_OK, perms)) {
+          if (DoLink(pathBuf, pathBuf2, req.mOp) == 0) {
+            resp.mError = 0;
+          } else {
+            resp.mError = -errno;
+          }
+        } else {
+          AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
+        }
+        break;
+
+      case SANDBOX_FILE_RENAME:
+        if (permissive || AllowOperation(W_OK, perms)) {
+          if (rename(pathBuf, pathBuf2) == 0) {
+            resp.mError = 0;
+          } else {
+            resp.mError = -errno;
+          }
+        } else {
+          AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
+        }
+        break;
+
       case SANDBOX_FILE_MKDIR:
         if (permissive || AllowOperation(W_OK | X_OK, perms)) {
           if (mkdir(pathBuf, req.mFlags) == 0) {
             resp.mError = 0;
           } else {
             resp.mError = -errno;
           }
         } else {
@@ -678,22 +759,19 @@ SandboxBroker::ThreadMain(void)
           }
         } else {
           AuditDenial(req.mOp, req.mFlags, perms, pathBuf);
         }
         break;
 
       case SANDBOX_FILE_READLINK:
         if (permissive || AllowOperation(R_OK, perms)) {
-          ssize_t respSize = readlink(pathBuf, (char*)&respBuf, sizeof(respBuf) - 1);
+          ssize_t respSize = readlink(pathBuf, (char*)&respBuf, sizeof(respBuf));
           if (respSize >= 0) {
-            if (respSize > 0) {
-              // Null-terminate for nsDependentCString.
-              MOZ_RELEASE_ASSERT(respSize < sizeof(respBuf));
-              respBuf[respSize] = '\0';
+              if (respSize > 0) {
               // Record the mapping so we can invert the file to the original
               // symlink.
               nsDependentCString orig(pathBuf, pathLen);
               nsDependentCString xlat(respBuf, respSize);
               if (!orig.Equals(xlat) && xlat[0] == '/') {
                 if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {
                   SANDBOX_LOG_ERROR("Recording mapping %s -> %s",
                                     xlat.get(), orig.get());
--- a/security/sandbox/linux/broker/SandboxBrokerCommon.h
+++ b/security/sandbox/linux/broker/SandboxBrokerCommon.h
@@ -25,17 +25,20 @@ namespace mozilla {
 
 class SandboxBrokerCommon {
 public:
   enum Operation {
     SANDBOX_FILE_OPEN,
     SANDBOX_FILE_ACCESS,
     SANDBOX_FILE_STAT,
     SANDBOX_FILE_CHMOD,
+    SANDBOX_FILE_LINK,
+    SANDBOX_FILE_SYMLINK,
     SANDBOX_FILE_MKDIR,
+    SANDBOX_FILE_RENAME,
     SANDBOX_FILE_RMDIR,
     SANDBOX_FILE_UNLINK,
     SANDBOX_FILE_READLINK,
   };
   // String versions of the above
   static const char* OperationDescription[];
 
   struct Request {
--- a/security/sandbox/linux/gtest/TestBroker.cpp
+++ b/security/sandbox/linux/gtest/TestBroker.cpp
@@ -59,19 +59,28 @@ protected:
     return mClient->Stat(aPath, aStat);
   }
   int LStat(const char* aPath, statstruct* aStat) {
     return mClient->LStat(aPath, aStat);
   }
   int Chmod(const char* aPath, int aMode) {
     return mClient->Chmod(aPath, aMode);
   }
+  int Link(const char* aPath, const char* bPath) {
+    return mClient->Link(aPath, bPath);
+  }
   int Mkdir(const char* aPath, int aMode) {
     return mClient->Mkdir(aPath, aMode);
   }
+  int Symlink(const char* aPath, const char* bPath) {
+    return mClient->Symlink(aPath, bPath);
+  }
+  int Rename(const char* aPath, const char* bPath) {
+    return mClient->Rename(aPath, bPath);
+  }
   int Rmdir(const char* aPath) {
     return mClient->Rmdir(aPath);
   }
   int Unlink(const char* aPath) {
     return mClient->Unlink(aPath);
   }
   ssize_t Readlink(const char* aPath, char* aBuff, size_t aSize) {
     return mClient->Readlink(aPath, aBuff, aSize);
@@ -266,16 +275,53 @@ TEST_F(SandboxBrokerTest, Chmod)
   ASSERT_EQ(0, Chmod("/tmp/blublu", S_IRUSR | S_IWUSR));
   EXPECT_EQ(0, statsyscall("/tmp/blublu", &realStat));
   EXPECT_EQ((mode_t)(S_IRUSR | S_IWUSR), realStat.st_mode & 0777);
   EXPECT_EQ(0, unlink("/tmp/blublu"));
 
   PrePostTestCleanup();
 }
 
+TEST_F(SandboxBrokerTest, Link)
+{
+  PrePostTestCleanup();
+
+  int fd = Open("/tmp/blublu", O_WRONLY | O_CREAT);
+  ASSERT_GE(fd, 0) << "Opening /tmp/blublu for writing failed.";
+  close(fd);
+  ASSERT_EQ(0, Link("/tmp/blublu", "/tmp/blublublu"));
+  EXPECT_EQ(0, Access("/tmp/blublublu", F_OK));
+  // Not whitelisted target path
+  EXPECT_EQ(-EACCES, Link("/tmp/blublu", "/tmp/nope"));
+  EXPECT_EQ(0, unlink("/tmp/blublublu"));
+  EXPECT_EQ(0, unlink("/tmp/blublu"));
+
+  PrePostTestCleanup();
+}
+
+TEST_F(SandboxBrokerTest, Symlink)
+{
+  PrePostTestCleanup();
+
+  int fd = Open("/tmp/blublu", O_WRONLY | O_CREAT);
+  ASSERT_GE(fd, 0) << "Opening /tmp/blublu for writing failed.";
+  close(fd);
+  ASSERT_EQ(0, Symlink("/tmp/blublu", "/tmp/blublublu"));
+  EXPECT_EQ(0, Access("/tmp/blublublu", F_OK));
+  statstruct aStat;
+  ASSERT_EQ(0, lstatsyscall("/tmp/blublublu", &aStat));
+  EXPECT_EQ((mode_t)S_IFLNK, aStat.st_mode & S_IFMT);
+  // Not whitelisted target path
+  EXPECT_EQ(-EACCES, Symlink("/tmp/blublu", "/tmp/nope"));
+  EXPECT_EQ(0, unlink("/tmp/blublublu"));
+  EXPECT_EQ(0, unlink("/tmp/blublu"));
+
+  PrePostTestCleanup();
+}
+
 TEST_F(SandboxBrokerTest, Mkdir)
 {
   PrePostTestCleanup();
 
   ASSERT_EQ(0, mkdir("/tmp/blublu", 0600))
     << "Creating dir /tmp/blublu failed.";
   EXPECT_EQ(0, Access("/tmp/blublu", F_OK));
   // Not whitelisted target path
@@ -285,16 +331,34 @@ TEST_F(SandboxBrokerTest, Mkdir)
   EXPECT_EQ(-EEXIST, Mkdir("/proc/self", 0600))
     << "Creating uncreatable dir that already exists didn't fail correctly.";
   EXPECT_EQ(-EEXIST, Mkdir("/dev/zero", 0600))
     << "Creating uncreatable dir over preexisting file didn't fail correctly.";
 
   PrePostTestCleanup();
 }
 
+TEST_F(SandboxBrokerTest, Rename)
+{
+  PrePostTestCleanup();
+
+  ASSERT_EQ(0, mkdir("/tmp/blublu", 0600))
+    << "Creating dir /tmp/blublu failed.";
+  EXPECT_EQ(0, Access("/tmp/blublu", F_OK));
+  ASSERT_EQ(0, Rename("/tmp/blublu", "/tmp/blublublu"));
+  EXPECT_EQ(0, Access("/tmp/blublublu", F_OK));
+  EXPECT_EQ(-ENOENT , Access("/tmp/blublu", F_OK));
+  // Not whitelisted target path
+  EXPECT_EQ(-EACCES, Rename("/tmp/blublublu", "/tmp/nope"))
+    << "Renaming dir without write access succeed.";
+  EXPECT_EQ(0, rmdir("/tmp/blublublu"));
+
+  PrePostTestCleanup();
+}
+
 TEST_F(SandboxBrokerTest, Rmdir)
 {
   PrePostTestCleanup();
 
   ASSERT_EQ(0, mkdir("/tmp/blublu", 0600))
     << "Creating dir /tmp/blublu failed.";
   EXPECT_EQ(0, Access("/tmp/blublu", F_OK));
   ASSERT_EQ(0, Rmdir("/tmp/blublu"));
@@ -327,17 +391,17 @@ TEST_F(SandboxBrokerTest, Unlink)
 
 TEST_F(SandboxBrokerTest, Readlink)
 {
   PrePostTestCleanup();
 
   int fd = Open("/tmp/blublu", O_WRONLY | O_CREAT);
   ASSERT_GE(fd, 0) << "Opening /tmp/blublu for writing failed.";
   close(fd);
-  ASSERT_EQ(0, symlink("/tmp/blublu", "/tmp/blublublu"));
+  ASSERT_EQ(0, Symlink("/tmp/blublu", "/tmp/blublublu"));
   EXPECT_EQ(0, Access("/tmp/blublublu", F_OK));
   char linkBuff[256];
   EXPECT_EQ(11, Readlink("/tmp/blublublu", linkBuff, sizeof(linkBuff)));
   linkBuff[11] = '\0';
   EXPECT_EQ(0, strcmp(linkBuff, "/tmp/blublu"));
 
   PrePostTestCleanup();
 }