Bug 1087565: Verify the child process with a secret hello on Windows. r=dvander

ipc/chromium/moz.build

@@ -65,6 +65,7 @@ UNIFIED_SOURCES += [
     'src/chrome/common/child_thread.cc',
     'src/chrome/common/chrome_switches.cc',
     'src/chrome/common/env_vars.cc',
+    'src/chrome/common/ipc_channel.cc',
     'src/chrome/common/ipc_channel_proxy.cc',
     'src/chrome/common/ipc_message.cc',
     'src/chrome/common/ipc_sync_channel.cc',

ipc/chromium/src/base/atomic_sequence_num.h

@@ -0,0 +1,60 @@
+// Copyright (c) 2012 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.
+
+#ifndef BASE_ATOMIC_SEQUENCE_NUM_H_
+#define BASE_ATOMIC_SEQUENCE_NUM_H_
+
+#include "base/atomicops.h"
+#include "base/basictypes.h"
+
+namespace base {
+
+class AtomicSequenceNumber;
+
+// Static (POD) AtomicSequenceNumber that MUST be used in global scope (or
+// non-function scope) ONLY. This implementation does not generate any static
+// initializer.  Note that it does not implement any constructor which means
+// that its fields are not initialized except when it is stored in the global
+// data section (.data in ELF). If you want to allocate an atomic sequence
+// number on the stack (or heap), please use the AtomicSequenceNumber class
+// declared below.
+class StaticAtomicSequenceNumber {
+ public:
+  inline int GetNext() {
+    return static_cast<int>(
+        base::subtle::NoBarrier_AtomicIncrement(&seq_, 1) - 1);
+  }
+
+ private:
+  friend class AtomicSequenceNumber;
+
+  inline void Reset() {
+    base::subtle::Release_Store(&seq_, 0);
+  }
+
+  base::subtle::Atomic32 seq_;
+};
+
+// AtomicSequenceNumber that can be stored and used safely (i.e. its fields are
+// always initialized as opposed to StaticAtomicSequenceNumber declared above).
+// Please use StaticAtomicSequenceNumber if you want to declare an atomic
+// sequence number in the global scope.
+class AtomicSequenceNumber {
+ public:
+  AtomicSequenceNumber() {
+    seq_.Reset();
+  }
+
+  inline int GetNext() {
+    return seq_.GetNext();
+  }
+
+ private:
+  StaticAtomicSequenceNumber seq_;
+  DISALLOW_COPY_AND_ASSIGN(AtomicSequenceNumber);
+};
+
+}  // namespace base
+
+#endif  // BASE_ATOMIC_SEQUENCE_NUM_H_

ipc/chromium/src/chrome/common/child_process_host.cc

@@ -74,7 +74,7 @@ ChildProcessHost::~ChildProcessHost() {
 }
 
 bool ChildProcessHost::CreateChannel() {
-  channel_id_ = GenerateRandomChannelID(this);
+  channel_id_ = IPC::Channel::GenerateVerifiedChannelID(std::wstring());
   channel_.reset(new IPC::Channel(
       channel_id_, IPC::Channel::MODE_SERVER, &listener_));
   if (!channel_->Connect())

ipc/chromium/src/chrome/common/child_process_info.cc

@@ -7,9 +7,6 @@
 #include <limits>
 
 #include "base/logging.h"
-#include "base/process_util.h"
-#include "base/rand_util.h"
-#include "base/string_util.h"
 
 std::wstring ChildProcessInfo::GetTypeNameInEnglish(
     ChildProcessInfo::ProcessType type) {
@@ -44,16 +41,4 @@ ChildProcessInfo::ChildProcessInfo(ProcessType type) {
 
 
 ChildProcessInfo::~ChildProcessInfo() {
-}
-
-std::wstring ChildProcessInfo::GenerateRandomChannelID(void* instance) {
-  // Note: the string must start with the current process id, this is how
-  // child processes determine the pid of the parent.
-  // Build the channel ID.  This is composed of a unique identifier for the
-  // parent browser process, an identifier for the child instance, and a random
-  // component. We use a random component so that a hacked child process can't
-  // cause denial of service by causing future named pipe creation to fail.
-  return StringPrintf(L"%d.%x.%d",
-                      base::GetCurrentProcId(), instance,
-                      base::RandInt(0, std::numeric_limits<int>::max()));
-}
+}

ipc/chromium/src/chrome/common/child_process_info.h

@@ -78,10 +78,6 @@ class ChildProcessInfo {
     return process_.handle() == rhs.process_.handle();
   }
 
-  // Generates a unique channel name for a child renderer/plugin process.
-  // The "instance" pointer value is baked into the channel id.
-  static std::wstring GenerateRandomChannelID(void* instance);
-
  protected:
   void set_type(ProcessType type) { type_ = type; }
   void set_name(const std::wstring& name) { name_ = name; }

ipc/chromium/src/chrome/common/ipc_channel.cc

@@ -0,0 +1,39 @@
+// Copyright (c) 2012 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 "chrome/common/ipc_channel.h"
+
+#include <limits>
+
+#include "base/atomic_sequence_num.h"
+#include "base/process_util.h"
+#include "base/rand_util.h"
+#include "base/string_util.h"
+
+namespace {
+
+// Global atomic used to guarantee channel IDs are unique.
+base::StaticAtomicSequenceNumber g_last_id;
+
+}  // namespace
+
+namespace IPC {
+
+// static
+std::wstring Channel::GenerateUniqueRandomChannelID() {
+  // Note: the string must start with the current process id, this is how
+  // some child processes determine the pid of the parent.
+  //
+  // This is composed of a unique incremental identifier, the process ID of
+  // the creator, an identifier for the child instance, and a strong random
+  // component. The strong random component prevents other processes from
+  // hijacking or squatting on predictable channel names.
+
+  return StringPrintf(L"%d.%u.%d",
+      base::GetCurrentProcId(),
+      g_last_id.GetNext(),
+      base::RandInt(0, std::numeric_limits<int32_t>::max()));
+}
+
+}  // namespace IPC

ipc/chromium/src/chrome/common/ipc_channel.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_COMMON_IPC_CHANNEL_H_
 #define CHROME_COMMON_IPC_CHANNEL_H_
 
+#include <string>
+
 #include <queue>
 #include "chrome/common/ipc_message.h"
 
@@ -131,6 +133,15 @@ class Channel : public Message::Sender {
   void* GetServerPipeHandle() const;
 #endif  // defined(OS_POSIX)
 
+  // Generates a channel ID that's non-predictable and unique.
+  static std::wstring GenerateUniqueRandomChannelID();
+
+  // Generates a channel ID that, if passed to the client as a shared secret,
+  // will validate that the client's authenticity. On platforms that do not
+  // require additional validation this is simply calls GenerateUniqueRandomChannelID().
+  // For portability the prefix should not include the \ character.
+  static std::wstring GenerateVerifiedChannelID(const std::wstring& prefix);
+
  private:
   // PIMPL to which all channel calls are delegated.
   class ChannelImpl;

ipc/chromium/src/chrome/common/ipc_channel_posix.cc

@@ -1056,4 +1056,16 @@ uint32_t Channel::Unsound_NumQueuedMessages() const {
   return channel_impl_->Unsound_NumQueuedMessages();
 }
 
+// static
+std::wstring Channel::GenerateVerifiedChannelID(const std::wstring& prefix) {
+  // A random name is sufficient validation on posix systems, so we don't need
+  // an additional shared secret.
+
+  std::wstring id = prefix;
+  if (!id.empty())
+    id.append(L".");
+
+  return id.append(GenerateUniqueRandomChannelID());
+}
+
 }  // namespace IPC

ipc/chromium/src/chrome/common/ipc_channel_win.cc

@@ -9,6 +9,9 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/process_util.h"
+#include "base/rand_util.h"
+#include "base/string_util.h"
 #include "base/non_thread_safe.h"
 #include "base/win_util.h"
 #include "chrome/common/ipc_message_utils.h"
@@ -33,7 +36,9 @@ Channel::ChannelImpl::ChannelImpl(const std::wstring& channel_id, Mode mode,
                               Listener* listener)
     : ALLOW_THIS_IN_INITIALIZER_LIST(input_state_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(output_state_(this)),
-      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
+      shared_secret_(0),
+      waiting_for_shared_secret_(false) {
   Init(mode, listener);
 
   if (!CreatePipe(channel_id, mode)) {
@@ -48,7 +53,9 @@ Channel::ChannelImpl::ChannelImpl(const std::wstring& channel_id,
                                   Mode mode, Listener* listener)
     : ALLOW_THIS_IN_INITIALIZER_LIST(input_state_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(output_state_(this)),
-      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
+      shared_secret_(0),
+      waiting_for_shared_secret_(false) {
   Init(mode, listener);
 
   if (mode == MODE_SERVER) {
@@ -153,18 +160,31 @@ bool Channel::ChannelImpl::Send(Message* message) {
 }
 
 const std::wstring Channel::ChannelImpl::PipeName(
-    const std::wstring& channel_id) const {
+    const std::wstring& channel_id, int32_t* secret) const {
+  MOZ_ASSERT(secret);
+
   std::wostringstream ss;
-  // XXX(darin): get application name from somewhere else
-  ss << L"\\\\.\\pipe\\chrome." << channel_id;
+  ss << L"\\\\.\\pipe\\chrome.";
+
+  // Prevent the shared secret from ending up in the pipe name.
+  size_t index = channel_id.find_first_of(L'\\');
+  if (index != std::string::npos) {
+    StringToInt(channel_id.substr(index + 1), secret);
+    ss << channel_id.substr(0, index - 1);
+  } else {
+    // This case is here to support predictable named pipes in tests.
+    *secret = 0;
+    ss << channel_id;
+  }
   return ss.str();
 }
 
 bool Channel::ChannelImpl::CreatePipe(const std::wstring& channel_id,
                                       Mode mode) {
   DCHECK(pipe_ == INVALID_HANDLE_VALUE);
-  const std::wstring pipe_name = PipeName(channel_id);
+  const std::wstring pipe_name = PipeName(channel_id, &shared_secret_);
   if (mode == MODE_SERVER) {
+    waiting_for_shared_secret_ = !!shared_secret_;
     pipe_ = CreateNamedPipeW(pipe_name.c_str(),
                              PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |
                                 FILE_FLAG_FIRST_PIPE_INSTANCE,
@@ -200,7 +220,15 @@ bool Channel::ChannelImpl::EnqueueHelloMessage() {
   mozilla::UniquePtr<Message> m = mozilla::MakeUnique<Message>(MSG_ROUTING_NONE,
                                                                HELLO_MESSAGE_TYPE,
                                                                IPC::Message::PRIORITY_NORMAL);
-  if (!m->WriteInt(GetCurrentProcessId())) {
+
+  // If we're waiting for our shared secret from the other end's hello message
+  // then don't give the game away by sending it in ours.
+  int32_t secret = waiting_for_shared_secret_ ? 0 : shared_secret_;
+
+  // Also, don't send if the value is zero (for IPC backwards compatability).
+  if (!m->WriteInt(GetCurrentProcessId()) ||
+      (secret && !m->WriteUInt32(secret)))
+  {
     CloseHandle(pipe_);
     pipe_ = INVALID_HANDLE_VALUE;
     return false;
@@ -338,8 +366,19 @@ bool Channel::ChannelImpl::ProcessIncomingMessages(
 #endif
         if (m.routing_id() == MSG_ROUTING_NONE &&
             m.type() == HELLO_MESSAGE_TYPE) {
-          // The Hello message contains only the process id.
-          listener_->OnChannelConnected(MessageIterator(m).NextInt());
+          // The Hello message contains the process id and must include the
+          // shared secret, if we are waiting for it.
+          MessageIterator it = MessageIterator(m);
+          int32_t claimed_pid = it.NextInt();
+          if (waiting_for_shared_secret_ && (it.NextInt() != shared_secret_)) {
+            NOTREACHED();
+            // Something went wrong. Abort connection.
+            Close();
+            listener_->OnChannelError();
+            return false;
+          }
+          waiting_for_shared_secret_ = false;
+          listener_->OnChannelConnected(claimed_pid);
         } else {
           listener_->OnMessageReceived(m);
         }
@@ -502,4 +541,21 @@ uint32_t Channel::Unsound_NumQueuedMessages() const {
   return channel_impl_->Unsound_NumQueuedMessages();
 }
 
+// static
+std::wstring Channel::GenerateVerifiedChannelID(const std::wstring& prefix) {
+  // Windows pipes can be enumerated by low-privileged processes. So, we
+  // append a strong random value after the \ character. This value is not
+  // included in the pipe name, but sent as part of the client hello, to
+  // prevent hijacking the pipe name to spoof the client.
+  std::wstring id = prefix;
+  if (!id.empty())
+    id.append(L".");
+  int secret;
+  do {  // Guarantee we get a non-zero value.
+    secret = base::RandInt(0, std::numeric_limits<int>::max());
+  } while (secret == 0);
+  id.append(GenerateUniqueRandomChannelID());
+  return id.append(StringPrintf(L"\\%d", secret));
+}
+
 }  // namespace IPC

ipc/chromium/src/chrome/common/ipc_channel_win.h

@@ -49,7 +49,8 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
   void OutputQueuePush(Message* msg);
   void OutputQueuePop();
 
-  const std::wstring PipeName(const std::wstring& channel_id) const;
+  const std::wstring PipeName(const std::wstring& channel_id,
+                              int32_t* secret) const;
   bool CreatePipe(const std::wstring& channel_id, Mode mode);
   bool EnqueueHelloMessage();
 
@@ -108,6 +109,16 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
 
   ScopedRunnableMethodFactory<ChannelImpl> factory_;
 
+  // This is a unique per-channel value used to authenticate the client end of
+  // a connection. If the value is non-zero, the client passes it in the hello
+  // and the host validates. (We don't send the zero value to preserve IPC
+  // compatibility with existing clients that don't validate the channel.)
+  int32_t shared_secret_;
+
+  // In server-mode, we wait for the channel at the other side of the pipe to
+  // send us back our shared secret, if we are using one.
+  bool waiting_for_shared_secret_;
+
   mozilla::UniquePtr<NonThreadSafe> thread_check_;
 
   DISALLOW_COPY_AND_ASSIGN(ChannelImpl);

ipc/glue/Transport_posix.cpp

@@ -25,10 +25,7 @@ bool
 CreateTransport(base::ProcessId /*unused*/,
                 TransportDescriptor* aOne, TransportDescriptor* aTwo)
 {
-  // Gecko doesn't care about this random ID, and the argument to this
-  // function isn't really necessary, it can be just any random
-  // pointer value
-  wstring id = ChildProcessInfo::GenerateRandomChannelID(aOne);
+  wstring id = IPC::Channel::GenerateVerifiedChannelID(std::wstring());
   // Use MODE_SERVER to force creation of the socketpair
   Transport t(id, Transport::MODE_SERVER, nullptr);
   int fd1 = t.GetFileDescriptor();

ipc/glue/Transport_win.cpp

@@ -22,9 +22,7 @@ bool
 CreateTransport(base::ProcessId aProcIdOne,
                 TransportDescriptor* aOne, TransportDescriptor* aTwo)
 {
-  // This id is used to name the IPC pipe.  The pointer passed to this
-  // function isn't significant.
-  wstring id = ChildProcessInfo::GenerateRandomChannelID(aOne);
+  wstring id = IPC::Channel::GenerateVerifiedChannelID(std::wstring());
   // Use MODE_SERVER to force creation of the pipe
   Transport t(id, Transport::MODE_SERVER, nullptr);
   HANDLE serverPipe = t.GetServerPipeHandle();