Revert 80602 - Limit access to named IPC channels with SO_PEERCRED, not file permissions.BUG=TEST=Run Chrome & configure a service, to get the service process to start. Close Chrome and run a copy as a different user, but from the same user-data-dir. New Chrome should not be able to communicate with the service process.Review URL: http://codereview.chromium.org/6631002

TBR=wez@chromium.org
Review URL: http://codereview.chromium.org/6806006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80604 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 17 insertions, 73 deletions

diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
index 1550d49..91d9f2b 100644
--- a/ipc/ipc_channel.h
+++ b/ipc/ipc_channel.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -67,10 +67,7 @@ class Channel : public Message::Sender {
     MODE_NO_FLAG = 0x0,
     MODE_SERVER_FLAG = 0x1,
     MODE_CLIENT_FLAG = 0x2,
-    MODE_NAMED_FLAG = 0x4,
-#if defined(OS_POSIX)
-    MODE_OPEN_ACCESS_FLAG = 0x8, // Don't restrict access based on client UID.
-#endif
+    MODE_NAMED_FLAG = 0x4
   };
 
   // Some Standard Modes
@@ -85,13 +82,6 @@ class Channel : public Message::Sender {
     // MODE_NAMED_CLIENT is equivalent to MODE_CLIENT.
     MODE_NAMED_SERVER = MODE_SERVER_FLAG | MODE_NAMED_FLAG,
     MODE_NAMED_CLIENT = MODE_CLIENT_FLAG | MODE_NAMED_FLAG,
-#if defined(OS_POSIX)
-    // An "open" named server accepts connections from ANY client.
-    // The caller must then implement their own access-control based on the
-    // client process' user Id.
-    MODE_OPEN_NAMED_SERVER = MODE_OPEN_ACCESS_FLAG | MODE_SERVER_FLAG |
-                             MODE_NAMED_FLAG
-#endif
   };
 
   enum {
@@ -162,14 +152,10 @@ class Channel : public Message::Sender {
   // currently connected.
   bool HasAcceptedConnection() const;
 
-  // Returns true if the peer process' effective user id can be determined, in
-  // which case the supplied client_euid is updated with it.
-  bool GetClientEuid(uid_t* client_euid) const;
-
   // Closes any currently connected socket, and returns to a listening state
   // for more connections.
   void ResetToAcceptingConnectionState();
-#endif  // defined(OS_POSIX) && !defined(OS_NACL)
+#endif  // defined(OS_POSIX)
 
  protected:
   // Used in Chrome by the TestSink to provide a dummy channel implementation
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index 22e4781..d9156d7 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -193,6 +193,19 @@ bool CreateServerUnixDomainSocket(const std::string& pipe_name,
     return false;
   }
 
+  // Explicitly set file system permissions on socket, mainly as a precaution
+  // for Chrome OS.
+  // Do not rely on these file permissions to provide security - the file is
+  // created during the above bind() call so there is still a window for
+  // malicious abuse because the file exists between bind() and chmod(). Also,
+  // the file permissions may not be enforced for unix sockets on all platforms.
+  if (chmod(pipe_name.c_str(), 0600)) {
+    PLOG(ERROR) << "chmod " << pipe_name;
+    if (HANDLE_EINTR(close(fd)) < 0)
+      PLOG(ERROR) << "close " << pipe_name;
+    return false;
+  }
+
   // Start listening on the socket.
   const int listen_queue_length = 1;
   if (listen(fd, listen_queue_length) != 0) {
@@ -925,33 +938,6 @@ bool Channel::ChannelImpl::HasAcceptedConnection() const {
   return AcceptsConnections() && pipe_ != -1;
 }
 
-bool Channel::ChannelImpl::GetClientEuid(uid_t* client_euid) const {
-  DCHECK(HasAcceptedConnection());
-#if defined(OS_MACOSX)
-  uid_t peer_euid;
-  gid_t peer_gid;
-  if (getpeereid(pipe_, &peer_euid, &peer_gid) != 0) {
-    PLOG(ERROR) << "getpeereid " << pipe_;
-    return false;
-  }
-  *client_euid = peer_euid;
-  return true;
-#else
-  struct ucred cred;
-  socklen_t cred_len = sizeof(cred);
-  if (getsockopt(pipe_, SOL_SOCKET, SO_PEERCRED, &cred, &cred_len) != 0) {
-    PLOG(ERROR) << "getsockopt " << pipe_;
-    return false;
-  }
-  if (cred_len < sizeof(cred)) {
-    NOTREACHED() << "Truncated ucred from SO_PEERCRED?";
-    return false;
-  }
-  *client_euid = cred.uid;
-  return true;
-#endif
-}
-
 void Channel::ChannelImpl::ResetToAcceptingConnectionState() {
   // Unregister libevent for the unix domain socket and close it.
   read_watcher_.StopWatchingFileDescriptor();
@@ -1011,21 +997,6 @@ void Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int fd) {
     }
     pipe_ = new_pipe;
 
-    if ((mode_ & MODE_OPEN_ACCESS_FLAG) == 0) {
-      // Verify that the IPC channel peer is running as the same user.
-      uid_t client_euid;
-      if (!GetClientEuid(&client_euid)) {
-        LOG(ERROR) << "Unable to query client euid";
-        ResetToAcceptingConnectionState();
-        return;
-      }
-      if (client_euid != geteuid()) {
-        LOG(WARNING) << "Client euid is not authorised";
-        ResetToAcceptingConnectionState();
-        return;
-      }
-    }
-
     if (!AcceptConnection()) {
       NOTREACHED() << "AcceptConnection should not fail on server";
     }
@@ -1190,10 +1161,6 @@ bool Channel::HasAcceptedConnection() const {
   return channel_impl_->HasAcceptedConnection();
 }
 
-bool Channel::GetClientEuid(uid_t* client_euid) const {
-  return channel_impl_->GetClientEuid(client_euid);
-}
-
 void Channel::ResetToAcceptingConnectionState() {
   channel_impl_->ResetToAcceptingConnectionState();
 }
diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
index f1cbd63..b1c4c3b 100644
--- a/ipc/ipc_channel_posix.h
+++ b/ipc/ipc_channel_posix.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2008 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.
 
@@ -53,7 +53,6 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   int GetClientFileDescriptor() const;
   bool AcceptsConnections() const;
   bool HasAcceptedConnection() const;
-  bool GetClientEuid(uid_t* client_euid) const;
   void ResetToAcceptingConnectionState();
 
  private:
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index c219829..a92d511 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -375,13 +375,6 @@ int ChannelProxy::GetClientFileDescriptor() const {
   DCHECK(channel) << context_.get()->channel_id_;
   return channel->GetClientFileDescriptor();
 }
-
-bool ChannelProxy::GetClientEuid(uid_t* client_euid) const {
-  Channel *channel = context_.get()->channel_.get();
-  // Channel must have been created first.
-  DCHECK(channel) << context_.get()->channel_id_;
-  return channel->GetClientEuid(client_euid);
-}
 #endif
 
 //-----------------------------------------------------------------------------
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 0c0176f..950edff 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -146,7 +146,6 @@ class ChannelProxy : public Message::Sender {
 #if defined(OS_POSIX)
   // Calls through to the underlying channel's methods.
   int GetClientFileDescriptor() const;
-  bool GetClientEuid(uid_t* client_euid) const;
 #endif  // defined(OS_POSIX)
 
  protected: