Bug 1715182 - Sandbox broker abstract socket connect support. r=gcp

Differential Revision: https://phabricator.services.mozilla.com/D118716

security/sandbox/linux/SandboxBrokerClient.cpp

@@ -219,7 +219,7 @@ int SandboxBrokerClient::Readlink(const char* aPath, void* aBuff,
 
 int SandboxBrokerClient::Connect(const sockaddr_un* aAddr, size_t aLen,
                                  int aType) {
-  static const size_t maxLen = sizeof(aAddr->sun_path);
+  static constexpr size_t maxLen = sizeof(aAddr->sun_path);
   const char* path = aAddr->sun_path;
   const auto addrEnd = reinterpret_cast<const char*>(aAddr) + aLen;
   // Ensure that the length isn't impossibly small.
@@ -235,6 +235,25 @@ int SandboxBrokerClient::Connect(const sockaddr_un* aAddr, size_t aLen,
   if (bufLen > maxLen) {
     bufLen = maxLen;
   }
+
+  // Try to handle abstract addresses where the address (the part
+  // after the leading null byte) resembles a pathname: a leading
+  // slash and no embedded nulls.
+  //
+  // `DoCall` expects null-terminated strings, but in this case the
+  // "path" is terminated by the sockaddr length (without a null), so
+  // we need to make a copy.
+  if (bufLen >= 2 && path[0] == '\0' && path[1] == '/' &&
+      !memchr(path + 1, '\0', bufLen - 1)) {
+    char tmpBuf[maxLen];
+    MOZ_RELEASE_ASSERT(bufLen - 1 < maxLen);
+    memcpy(tmpBuf, path + 1, bufLen - 1);
+    tmpBuf[bufLen - 1] = '\0';
+
+    const Request req = {SANDBOX_SOCKET_CONNECT_ABSTRACT, aType, 0};
+    return DoCall(&req, tmpBuf, nullptr, nullptr, true);
+  }
+
   // Require null-termination.  (Linux doesn't require it, but
   // applications usually null-terminate for portability, and not
   // handling unterminated strings means we don't have to copy the path.)
@@ -242,9 +261,10 @@ int SandboxBrokerClient::Connect(const sockaddr_un* aAddr, size_t aLen,
   if (pathLen == bufLen) {
     return -ENAMETOOLONG;
   }
-  // Abstract addresses aren't handled (yet?).
+
+  // Abstract addresses are handled only in some specific case, error in others
   if (pathLen == 0) {
-    return -ECONNREFUSED;
+    return -ENETUNREACH;
   }
 
   const Request req = {SANDBOX_SOCKET_CONNECT, aType, 0};

security/sandbox/linux/broker/SandboxBroker.cpp

@@ -468,7 +468,8 @@ static int DoLink(const char* aPath, const char* aPath2,
   MOZ_CRASH("SandboxBroker: Unknown link operation");
 }
 
-static int DoConnect(const char* aPath, size_t aLen, int aType) {
+static int DoConnect(const char* aPath, size_t aLen, int aType,
+                     bool aIsAbstract) {
   // Deny SOCK_DGRAM for the same reason it's denied for socketpair.
   if (aType != SOCK_STREAM && aType != SOCK_SEQPACKET) {
     errno = EACCES;
@@ -478,27 +479,45 @@ static int DoConnect(const char* aPath, size_t aLen, int aType) {
   // resulting from an abstract address probably shouldn't have made
   // it past the policy check, but check explicitly just in case.)
   if (aPath[0] == '\0') {
-    errno = ECONNREFUSED;
+    errno = ENETUNREACH;
     return -1;
   }
 
   // Try to copy the name into a normal-sized sockaddr_un, with
-  // null-termination:
+  // null-termination. Specifically, from man page:
+  //
+  // When the address of an abstract socket is returned, the returned addrlen is
+  // greater than sizeof(sa_family_t) (i.e., greater than 2), and the name of
+  // the socket is contained in the first (addrlen - sizeof(sa_family_t)) bytes
+  // of sun_path.
+  //
+  // As mentionned in `SandboxBrokerClient::Connect()`, `DoCall` expects a
+  // null-terminated string while abstract socket are not. So we receive a copy
+  // here and we have to put things back correctly as a real abstract socket to
+  // perform the brokered `connect()` call.
   struct sockaddr_un sun;
   memset(&sun, 0, sizeof(sun));
   sun.sun_family = AF_UNIX;
-  if (aLen + 1 > sizeof(sun.sun_path)) {
+  char* sunPath = sun.sun_path;
+  size_t sunLen = sizeof(sun.sun_path);
+  size_t addrLen = sizeof(sun);
+  if (aIsAbstract) {
+    *sunPath++ = '\0';
+    sunLen--;
+    addrLen = offsetof(struct sockaddr_un, sun_path) + aLen + 1;
+  }
+  if (aLen + 1 > sunLen) {
     errno = ENAMETOOLONG;
     return -1;
   }
-  memcpy(&sun.sun_path, aPath, aLen);
+  memcpy(sunPath, aPath, aLen);
 
   // Finally, the actual socket connection.
   const int fd = socket(AF_UNIX, aType | SOCK_CLOEXEC, 0);
   if (fd < 0) {
     return -1;
   }
-  if (connect(fd, reinterpret_cast<struct sockaddr*>(&sun), sizeof(sun)) < 0) {
+  if (connect(fd, reinterpret_cast<struct sockaddr*>(&sun), addrLen) < 0) {
     close(fd);
     return -1;
   }
@@ -966,8 +985,10 @@ void SandboxBroker::ThreadMain(void) {
           break;
 
         case SANDBOX_SOCKET_CONNECT:
+        case SANDBOX_SOCKET_CONNECT_ABSTRACT:
           if (permissive || (perms & MAY_CONNECT) != 0) {
-            openedFd = DoConnect(pathBuf, pathLen, req.mFlags);
+            openedFd = DoConnect(pathBuf, pathLen, req.mFlags,
+                                 req.mOp == SANDBOX_SOCKET_CONNECT_ABSTRACT);
             if (openedFd >= 0) {
               resp.mError = 0;
             } else {

security/sandbox/linux/broker/SandboxBrokerCommon.cpp

@@ -31,8 +31,20 @@
 namespace mozilla {
 
 const char* SandboxBrokerCommon::OperationDescription[] = {
-    "open",  "access", "stat",  "chmod",  "link",     "symlink",
-    "mkdir", "rename", "rmdir", "unlink", "readlink", "connect"};
+    "open",
+    "access",
+    "stat",
+    "chmod",
+    "link",
+    "symlink",
+    "mkdir",
+    "rename",
+    "rmdir",
+    "unlink",
+    "readlink",
+    "connect",
+    "connect-abstract",
+};
 
 /* static */
 ssize_t SandboxBrokerCommon::RecvWithFd(int aFd, const iovec* aIO,

security/sandbox/linux/broker/SandboxBrokerCommon.h

@@ -38,6 +38,7 @@ class SandboxBrokerCommon {
     SANDBOX_FILE_UNLINK,
     SANDBOX_FILE_READLINK,
     SANDBOX_SOCKET_CONNECT,
+    SANDBOX_SOCKET_CONNECT_ABSTRACT,
   };
   // String versions of the above
   static const char* OperationDescription[];