Bug 1087565 - Verify the child process with a secret hello on Windows. r=dvander, a=sledru
authorBob Owen <bobowencode@gmail.com>
Thu, 23 Apr 2015 10:17:15 +0100
changeset 260300 c1f04200ed98
parent 260299 1bbb50c6a494
child 260301 a8fb9422ff13
push id741
push userryanvm@gmail.com
push date2015-04-27 20:01 +0000
treeherdermozilla-release@d10817faa571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander, sledru
bugs1087565
milestone38.0
Bug 1087565 - Verify the child process with a secret hello on Windows. r=dvander, a=sledru
ipc/chromium/moz.build
ipc/chromium/src/base/atomic_sequence_num.h
ipc/chromium/src/chrome/common/child_process_host.cc
ipc/chromium/src/chrome/common/child_process_info.cc
ipc/chromium/src/chrome/common/child_process_info.h
ipc/chromium/src/chrome/common/ipc_channel.cc
ipc/chromium/src/chrome/common/ipc_channel.h
ipc/chromium/src/chrome/common/ipc_channel_posix.cc
ipc/chromium/src/chrome/common/ipc_channel_win.cc
ipc/chromium/src/chrome/common/ipc_channel_win.h
ipc/glue/Transport_posix.cpp
ipc/glue/Transport_win.cpp
--- a/ipc/chromium/moz.build
+++ b/ipc/chromium/moz.build
@@ -60,16 +60,17 @@ UNIFIED_SOURCES += [
     'src/base/tracked.cc',
     'src/base/tracked_objects.cc',
     'src/chrome/common/child_process.cc',
     'src/chrome/common/child_process_host.cc',
     'src/chrome/common/child_process_info.cc',
     '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',
     'src/chrome/common/ipc_sync_message.cc',
     'src/chrome/common/message_router.cc',
     'src/chrome/common/notification_service.cc',
 ]
 
new file mode 100644
--- /dev/null
+++ b/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_
--- a/ipc/chromium/src/chrome/common/child_process_host.cc
+++ b/ipc/chromium/src/chrome/common/child_process_host.cc
@@ -69,17 +69,17 @@ ChildProcessHost::~ChildProcessHost() {
     // Above call took ownership, so don't want WaitableEvent to assert because
     // the handle isn't valid anymore.
     process_event_->Release();
 #endif
   }
 }
 
 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())
     return false;
 
   opening_channel_ = true;
 
   return true;
--- a/ipc/chromium/src/chrome/common/child_process_info.cc
+++ b/ipc/chromium/src/chrome/common/child_process_info.cc
@@ -2,19 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/child_process_info.h"
 
 #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) {
   switch (type) {
     case BROWSER_PROCESS:
       return L"Browser";
     case RENDER_PROCESS:
       return L"Tab";
@@ -39,21 +36,9 @@ ChildProcessInfo::ChildProcessInfo(Proce
   // just a simple object that contains information about it.  So add it to our
   // list of running processes.
   type_ = type;
   pid_ = -1;
 }
 
 
 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()));
-}
+}
\ No newline at end of file
--- a/ipc/chromium/src/chrome/common/child_process_info.h
+++ b/ipc/chromium/src/chrome/common/child_process_info.h
@@ -73,20 +73,16 @@ class ChildProcessInfo {
       return process_ .handle() < rhs.process_.handle();
     return false;
   }
 
   bool operator ==(const ChildProcessInfo& rhs) const {
     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; }
   void set_handle(base::ProcessHandle handle) {
     process_.set_handle(handle);
     pid_ = -1;
   }
 
new file mode 100644
--- /dev/null
+++ b/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
\ No newline at end of file
--- a/ipc/chromium/src/chrome/common/ipc_channel.h
+++ b/ipc/chromium/src/chrome/common/ipc_channel.h
@@ -1,15 +1,17 @@
 // Copyright (c) 2006-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.
 
 #ifndef CHROME_COMMON_IPC_CHANNEL_H_
 #define CHROME_COMMON_IPC_CHANNEL_H_
 
+#include <string>
+
 #include <queue>
 #include "chrome/common/ipc_message.h"
 
 namespace IPC {
 
 //------------------------------------------------------------------------------
 
 class Channel : public Message::Sender {
@@ -126,16 +128,25 @@ class Channel : public Message::Sender {
   // Close the client side of the socketpair.
   void CloseClientFileDescriptor();
 
 #elif defined(OS_WIN)
   // Return the server pipe handle.
   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;
   ChannelImpl *channel_impl_;
 
   enum {
 #if defined(OS_MACOSX)
     // If the channel receives a message that contains file descriptors, then
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
@@ -1018,9 +1018,21 @@ void Channel::CloseClientFileDescriptor(
 bool Channel::Unsound_IsClosed() const {
   return channel_impl_->Unsound_IsClosed();
 }
 
 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
--- a/ipc/chromium/src/chrome/common/ipc_channel_win.cc
+++ b/ipc/chromium/src/chrome/common/ipc_channel_win.cc
@@ -4,16 +4,19 @@
 
 #include "chrome/common/ipc_channel_win.h"
 
 #include <windows.h>
 #include <sstream>
 
 #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"
 #include "mozilla/ipc/ProtocolUtils.h"
 
 namespace IPC {
 //------------------------------------------------------------------------------
 
@@ -28,32 +31,36 @@ Channel::ChannelImpl::State::~State() {
 }
 
 //------------------------------------------------------------------------------
 
 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)) {
     // The pipe may have been closed already.
     CHROMIUM_LOG(WARNING) << "Unable to create pipe named \"" << channel_id <<
                              "\" in " << (mode == 0 ? "server" : "client") << " mode.";
   }
 }
 
 Channel::ChannelImpl::ChannelImpl(const std::wstring& channel_id,
                                   HANDLE server_pipe,
                                   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) {
     // Use the existing handle that was dup'd to us
     pipe_ = server_pipe;
     EnqueueHelloMessage();
   } else {
     // Take the normal init path to connect to the server pipe
@@ -148,28 +155,41 @@ bool Channel::ChannelImpl::Send(Message*
         return false;
     }
   }
 
   return true;
 }
 
 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,
                              PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
                              1,         // number of pipe instances
                              // output buffer size (XXX tune)
                              Channel::kReadBufferSize,
                              // input buffer size (XXX tune)
@@ -195,17 +215,25 @@ bool Channel::ChannelImpl::CreatePipe(co
   // Create the Hello message to be sent when Connect is called
   return EnqueueHelloMessage();
 }
 
 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;
   }
 
   OutputQueuePush(m.release());
   return true;
 }
@@ -333,18 +361,29 @@ bool Channel::ChannelImpl::ProcessIncomi
         int len = static_cast<int>(message_tail - p);
         const Message m(p, len);
 #ifdef IPC_MESSAGE_DEBUG_EXTRA
         DLOG(INFO) << "received message on channel @" << this <<
                       " with type " << m.type();
 #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);
         }
         p = message_tail;
       } else {
         // Last message is partial.
         break;
       }
@@ -497,9 +536,26 @@ bool Channel::Send(Message* message) {
 bool Channel::Unsound_IsClosed() const {
   return channel_impl_->Unsound_IsClosed();
 }
 
 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
--- a/ipc/chromium/src/chrome/common/ipc_channel_win.h
+++ b/ipc/chromium/src/chrome/common/ipc_channel_win.h
@@ -44,17 +44,18 @@ class Channel::ChannelImpl : public Mess
   uint32_t Unsound_NumQueuedMessages() const;
 
  private:
   void Init(Mode mode, Listener* listener);
 
   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();
 
   bool ProcessConnection();
   bool ProcessIncomingMessages(MessageLoopForIO::IOContext* context,
                                DWORD bytes_read);
   bool ProcessOutgoingMessages(MessageLoopForIO::IOContext* context,
                                DWORD bytes_written);
@@ -103,16 +104,26 @@ class Channel::ChannelImpl : public Mess
   // This variable is updated so it matches output_queue_.size(), except we can
   // read output_queue_length_ from any thread (if we're OK getting an
   // occasional out-of-date or bogus value).  We use output_queue_length_ to
   // implement Unsound_NumQueuedMessages.
   size_t output_queue_length_;
 
   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);
 };
 
 }  // namespace IPC
 
 #endif  // CHROME_COMMON_IPC_CHANNEL_WIN_H_
--- a/ipc/glue/Transport_posix.cpp
+++ b/ipc/glue/Transport_posix.cpp
@@ -20,20 +20,17 @@ using base::ProcessHandle;
 
 namespace mozilla {
 namespace ipc {
 
 bool
 CreateTransport(ProcessHandle /*unused*/, ProcessHandle /*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();
   int fd2, dontcare;
   t.GetClientFileDescriptorMapping(&fd2, &dontcare);
   if (fd1 < 0 || fd2 < 0) {
     return false;
   }
--- a/ipc/glue/Transport_win.cpp
+++ b/ipc/glue/Transport_win.cpp
@@ -16,19 +16,17 @@ using base::ProcessHandle;
 
 namespace mozilla {
 namespace ipc {
 
 bool
 CreateTransport(ProcessHandle aProcOne, ProcessHandle /*unused*/,
                 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();
   if (!serverPipe) {
     return false;
   }
 
   // NB: we create the server pipe immediately, instead of just