Implement IPC::ChannelFactory, a class that accept()s on a UNIX socket.

ipc/unix_domain_socket_util.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ipc/unix_domain_socket_util.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
+
+namespace IPC {
+
+// Verify that kMaxSocketNameLength is a decent size.
+COMPILE_ASSERT(sizeof(((sockaddr_un*)0)->sun_path) >= kMaxSocketNameLength,
+               BAD_SUN_PATH_LENGTH);
+
+namespace {
+
+// Returns fd (>= 0) on success, -1 on failure. If successful, fills in
+// |unix_addr| with the appropriate data for the socket, and sets
+// |unix_addr_len| to the length of the data therein.
+int MakeUnixAddrForPath(const std::string& socket_name,
+                        struct sockaddr_un* unix_addr,
+                        size_t* unix_addr_len) {
+  DCHECK_GT(socket_name.length(), 0u);
+  DCHECK_LT(socket_name.length(), kMaxSocketNameLength);
+
+  if (socket_name.length() == 0) {
+    LOG(ERROR) << "Empty socket name provided for unix socket address.";
+    return -1;
+  }
+  // We reject socket_name.length() == kMaxSocketNameLength to make room for

[Mark Mentovai, 2013/03/06 19:34:04]

Thanks.

+  // the NUL terminator at the end of the string.
+  if (socket_name.length() >= kMaxSocketNameLength) {
+    LOG(ERROR) << "Socket name too long: " << socket_name;
+    return -1;
+  }
+
+  // Create socket.
+  int fd = socket(AF_UNIX, SOCK_STREAM, 0);
+  if (fd < 0) {
+    PLOG(ERROR) << "socket";
+    return -1;
+  }
+  file_util::ScopedFD scoped_fd(&fd);
+
+  // Make socket non-blocking
+  if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
+    PLOG(ERROR) << "fcntl(O_NONBLOCK)";
+    return -1;
+  }
+
+  // Create unix_addr structure.
+  DCHECK(unix_addr);
+  memset(unix_addr, 0, sizeof(struct sockaddr_un));
+  unix_addr->sun_family = AF_UNIX;
+  unix_addr->sun_len = socket_name.length();
+  strncpy(unix_addr->sun_path, socket_name.c_str(), kMaxSocketNameLength);
+  *unix_addr_len =
+      offsetof(struct sockaddr_un, sun_path) + socket_name.length();
+  return *scoped_fd.release();
+}
+
+}  // namespace
+
+bool CreateServerUnixDomainSocket(const base::FilePath& socket_path,
+                                  int* server_listen_fd) {
+  DCHECK(server_listen_fd);
+
+  std::string socket_name = socket_path.value();
+  base::FilePath socket_dir = socket_path.DirName();
+
+  struct sockaddr_un unix_addr;
+  size_t unix_addr_len;
+  int fd = MakeUnixAddrForPath(socket_name, &unix_addr, &unix_addr_len);
+  if (fd < 0)
+    return false;
+  file_util::ScopedFD scoped_fd(&fd);
+
+  // Make sure the path we need exists.
+  if (!file_util::CreateDirectory(socket_dir)) {

[Mark Mentovai, 2013/03/06 19:34:04]

Are you going to add a ScopedAllowIO here?

If so, you should add a comment to the header that this does a blocking
operation and you have added a ScopedAllowIO.

If not, you might still want to add a comment to the header so that callers know
that they need to deal with this function potentially blocking on filesystem
operations.

[jeremya, 2013/03/06 21:46:01]

Given that this code path was previously only executed in the service process
(used for cloud print) which I imagine doesn't have distinct IO and FILE
threads, and the new code path I'm adding for app shims works around the problem
by creating the IPC::ChannelFactory on the FILE thread, I think it's safe to
leave it without a ScopedAllowIO for now. We may want to add one in the future.
For now I'll just add a comment to the header.

+    LOG(ERROR) << "Couldn't create directory: " << socket_dir.value();
+    return false;
+  }
+
+  // Delete any old FS instances.
+  if (unlink(socket_name.c_str()) < 0 && errno != ENOENT)
+    PLOG(ERROR) << "unlink " << socket_name;

[Mark Mentovai, 2013/03/06 19:34:04]

If you’re not going to “return false” for this, is it really worth PLOG(ERROR)?

[jeremya, 2013/03/06 21:46:01]

I think we should probably return false when unlink fails in a non-ENOENT way.
Updated.

+
+  // Bind the socket.
+  if (bind(fd, reinterpret_cast<const sockaddr*>(&unix_addr),
+           unix_addr_len) < 0) {
+    PLOG(ERROR) << "bind " << socket_path.value();
+    return false;
+  }
+
+  // Start listening on the socket.
+  if (listen(fd, SOMAXCONN) < 0) {
+    PLOG(ERROR) << "listen " << socket_path.value();
+    unlink(socket_name.c_str());
+    return false;
+  }
+
+  *server_listen_fd = *scoped_fd.release();
+  return true;
+}
+
+bool CreateClientUnixDomainSocket(const base::FilePath& socket_path,
+                                  int* client_socket) {
+  DCHECK(client_socket);
+
+  std::string socket_name = socket_path.value();
+  base::FilePath socket_dir = socket_path.DirName();
+
+  struct sockaddr_un unix_addr;
+  size_t unix_addr_len;
+  int fd = MakeUnixAddrForPath(socket_name, &unix_addr, &unix_addr_len);
+  if (fd < 0)
+    return false;
+  file_util::ScopedFD scoped_fd(&fd);
+
+  if (HANDLE_EINTR(connect(fd, reinterpret_cast<sockaddr*>(&unix_addr),
+                           unix_addr_len)) < 0) {
+    PLOG(ERROR) << "connect " << socket_path.value();
+    return false;
+  }
+
+  *client_socket = *scoped_fd.release();
+  return true;
+}
+
+bool GetPeerEuid(int fd, uid_t* client_euid) {
+#if defined(OS_MACOSX) || defined(OS_OPENBSD) || defined(OS_FREEBSD)
+  uid_t peer_euid;
+  gid_t peer_gid;
+  if (getpeereid(fd, &peer_euid, &peer_gid) < 0) {
+    PLOG(ERROR) << "getpeereid " << fd;
+    return false;
+  }
+  *client_euid = peer_euid;
+  return true;
+#else
+  struct ucred cred;
+  socklen_t cred_len = sizeof(cred);
+  if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cred, &cred_len) < 0) {
+    PLOG(ERROR) << "getsockopt " << fd;
+    return false;
+  }
+  if (static_cast<unsigned>(cred_len) < sizeof(cred)) {
+    NOTREACHED() << "Truncated ucred from SO_PEERCRED?";
+    return false;
+  }
+  *client_euid = cred.uid;
+  return true;
+#endif
+}
+
+bool IsPeerAuthorized(int peer_fd) {
+  uid_t peer_euid;
+  if (!GetPeerEuid(peer_fd, &peer_euid))
+    return false;
+  if (peer_euid != geteuid()) {
+    DLOG(ERROR) << "Client euid is not authorised";
+    return false;
+  }
+  return true;
+}
+
+bool IsRecoverableError(int err) {
+  return errno == ECONNABORTED || errno == EMFILE || errno == ENFILE ||

[Mark Mentovai, 2013/03/06 19:34:04]

I’m glad to see that you’ve picked out other recoverable errors in addition to
my ECONNABORTED example!

+         errno == ENOMEM;

[Mark Mentovai, 2013/03/06 19:34:04]

If you’re checking for ENOMEM, you should probably also tolerate ENOBUFS, which
is conceptually very similar. It doesn’t show up in “man 2 accept” on Mac, but
it does on Linux and in
http://pubs.opengroup.org/onlinepubs/9699919799/functions/accept.html .

[jeremya, 2013/03/06 21:46:01]

Ah, I was wondering if it existed at all on OSX and whether I would have to
#ifdef that bit. Seems to work fine though.

+}
+
+bool ServerAcceptConnection(int server_listen_fd, int* server_socket) {
+  DCHECK(server_socket);

[Mark Mentovai, 2013/03/06 19:34:04]

This is obsolete now that the next line dereferences server_socket.

(This file is a little inconsistent about whether it DCHECKs out-pointers like
this right off the bat. I think it’s generally unnecessary. This would favor
dumping the DCHECKs on lines 76 and 118 too. But if you really like the DCHECKs,
they’re missing on lines 33 and 141.)

[jeremya, 2013/03/06 21:46:01]

I like the DCHECKs, it's a clear indication to the reader of the code that the
function does not handle the possibility of the out ptr being NULL. I appreciate
not having to read the whole function to answer that sort of question :)

+  *server_socket = -1;
+
+  int accept_fd = HANDLE_EINTR(accept(server_listen_fd, NULL, 0));
+  if (accept_fd < 0)
+    return IsRecoverableError(errno);
+  file_util::ScopedFD scoped_fd(&accept_fd);
+  if (fcntl(accept_fd, F_SETFL, O_NONBLOCK) < 0) {
+    PLOG(ERROR) << "fcntl(O_NONBLOCK) " << accept_fd;
+    // It's safe to keep listening on |server_listen_fd| even if the attempt to
+    // set O_NONBLOCK failed on the client fd.
+    return true;
+  }
+
+  *server_socket = *scoped_fd.release();
+  return true;
+}
+
+}  // namespace IPC