Verify the child process with a secret hello

BUG=117627
TEST=IPCSyncChannelTest.Verified

Review URL: http://codereview.chromium.org/9692035

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

7 files changed, 227 insertions, 10 deletions

diff --git a/ipc/ipc.gypi b/ipc/ipc.gypi
index d43947a..0684216 100644
--- a/ipc/ipc.gypi
+++ b/ipc/ipc.gypi
@@ -14,6 +14,7 @@
           'file_descriptor_set_posix.cc',
           'file_descriptor_set_posix.h',
           'ipc_channel.h',
+          'ipc_channel.cc',
           'ipc_channel_handle.h',
           'ipc_channel_posix.cc',
           'ipc_channel_posix.h',
diff --git a/ipc/ipc_channel.cc b/ipc/ipc_channel.cc
new file mode 100644
index 0000000..5973f72
--- /dev/null
+++ b/ipc/ipc_channel.cc
@@ -0,0 +1,40 @@
+// 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 "ipc/ipc_channel.h"
+
+#include <limits>
+
+#include "base/atomic_sequence_num.h"
+#include "base/process_util.h"
+#include "base/rand_util.h"
+#include "base/stringprintf.h"
+
+namespace {
+
+// Global atomic used to guarantee channel IDs are unique.
+base::StaticAtomicSequenceNumber g_last_id;
+
+}  // namespace
+
+namespace IPC {
+
+// static
+std::string 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 base::StringPrintf("%d.%u.%d",
+      base::GetCurrentProcId(),
+      g_last_id.GetNext(),
+      base::RandInt(0, std::numeric_limits<int32>::max()));
+}
+
+}  // namespace IPC
+
diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
index 63beeee..4f1e75f 100644
--- a/ipc/ipc_channel.h
+++ b/ipc/ipc_channel.h
@@ -6,6 +6,8 @@
 #define IPC_IPC_CHANNEL_H_
 #pragma once
 
+#include <string>
+
 #include "base/compiler_specific.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_message.h"
@@ -190,6 +192,15 @@ class IPC_EXPORT Channel : public Message::Sender {
   // ID. Even if true, the server may have already accepted a connection.
   static bool IsNamedServerInitialized(const std::string& channel_id);
 
+  // Generates a channel ID that's non-predictable and unique.
+  static std::string 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 this is simply calls GenerateUniqueRandomChannelID().
+  // For portability the prefix should not include the \ character.
+  static std::string GenerateVerifiedChannelID(const std::string& prefix);
+
 #if defined(OS_LINUX)
   // Sandboxed processes live in a PID namespace, so when sending the IPC hello
   // message from client to server we need to send the PID from the global
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index 39a11be..1a1f77e 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -29,6 +29,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/process_util.h"
+#include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/synchronization/lock.h"
@@ -1160,6 +1161,19 @@ bool Channel::IsNamedServerInitialized(const std::string& channel_id) {
   return ChannelImpl::IsNamedServerInitialized(channel_id);
 }
 
+// static
+std::string Channel::GenerateVerifiedChannelID(const std::string& prefix) {
+  // A random name is sufficient validation on posix systems, so we don't need
+  // an additional shared secret.
+
+  std::string id = prefix;
+  if (!id.empty())
+    id.append(".");
+
+  return id.append(GenerateUniqueRandomChannelID());
+}
+
+
 #if defined(OS_LINUX)
 // static
 void Channel::SetGlobalPid(int pid) {
diff --git a/ipc/ipc_channel_win.cc b/ipc/ipc_channel_win.cc
index ef974ee..e2f0535 100644
--- a/ipc/ipc_channel_win.cc
+++ b/ipc/ipc_channel_win.cc
@@ -10,6 +10,9 @@
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/process_util.h"
+#include "base/rand_util.h"
+#include "base/string_number_conversions.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/scoped_handle.h"
@@ -36,7 +39,9 @@ Channel::ChannelImpl::ChannelImpl(const IPC::ChannelHandle &channel_handle,
       pipe_(INVALID_HANDLE_VALUE),
       waiting_connect_(mode & MODE_SERVER_FLAG),
       processing_incoming_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
+      client_secret_(0),
+      validate_client_(false) {
   CreatePipe(channel_handle, mode);
 }
 
@@ -97,14 +102,13 @@ bool Channel::ChannelImpl::Send(Message* message) {
 // static
 bool Channel::ChannelImpl::IsNamedServerInitialized(
     const std::string& channel_id) {
-  if (WaitNamedPipe(PipeName(channel_id).c_str(), 1))
+  if (WaitNamedPipe(PipeName(channel_id, NULL).c_str(), 1))
     return true;
   // If ERROR_SEM_TIMEOUT occurred, the pipe exists but is handling another
   // connection.
   return GetLastError() == ERROR_SEM_TIMEOUT;
 }
 
-
 Channel::ChannelImpl::ReadState Channel::ChannelImpl::ReadData(
     char* buffer,
     int buffer_len,
@@ -144,7 +148,16 @@ bool Channel::ChannelImpl::WillDispatchInputMessage(Message* msg) {
 
 void Channel::ChannelImpl::HandleHelloMessage(const Message& msg) {
   // The hello message contains one parameter containing the PID.
-  listener()->OnChannelConnected(MessageIterator(msg).NextInt());
+  MessageIterator it = MessageIterator(msg);
+  int32 claimed_pid =  it.NextInt();
+  if (validate_client_ && (it.NextInt() != client_secret_)) {
+    NOTREACHED();
+    // Something went wrong. Abort connection.
+    Close();
+    listener()->OnChannelError();
+    return;
+  }
+  listener()->OnChannelConnected(claimed_pid);
 }
 
 bool Channel::ChannelImpl::DidEmptyInputBuffers() {
@@ -153,9 +166,21 @@ bool Channel::ChannelImpl::DidEmptyInputBuffers() {
 }
 
 // static
-const std::wstring Channel::ChannelImpl::PipeName(
-    const std::string& channel_id) {
+const string16 Channel::ChannelImpl::PipeName(
+    const std::string& channel_id, int32* secret) {
   std::string name("\\\\.\\pipe\\chrome.");
+
+  // Prevent the shared secret from ending up in the pipe name.
+  size_t index = channel_id.find_first_of('\\');
+  if (index != std::string::npos) {
+    if (secret)  // Retrieve the secret if asked for.
+      base::StringToInt(channel_id.substr(index + 1), secret);
+    return ASCIIToWide(name.append(channel_id.substr(0, index - 1)));
+  }
+
+  // This case is here to support predictable named pipes in tests.
+  if (secret)
+    *secret = 0;
   return ASCIIToWide(name.append(channel_id));
 }
 
@@ -192,7 +217,8 @@ bool Channel::ChannelImpl::CreatePipe(const IPC::ChannelHandle &channel_handle,
     DCHECK(!channel_handle.pipe.handle);
     const DWORD open_mode = PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |
                             FILE_FLAG_FIRST_PIPE_INSTANCE;
-    pipe_name = PipeName(channel_handle.name);
+    pipe_name = PipeName(channel_handle.name, &client_secret_);
+    validate_client_ = !!client_secret_;
     pipe_ = CreateNamedPipeW(pipe_name.c_str(),
                              open_mode,
                              PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
@@ -203,7 +229,7 @@ bool Channel::ChannelImpl::CreatePipe(const IPC::ChannelHandle &channel_handle,
                              NULL);
   } else if (mode & MODE_CLIENT_FLAG) {
     DCHECK(!channel_handle.pipe.handle);
-    pipe_name = PipeName(channel_handle.name);
+    pipe_name = PipeName(channel_handle.name, &client_secret_);
     pipe_ = CreateFileW(pipe_name.c_str(),
                         GENERIC_READ | GENERIC_WRITE,
                         0,
@@ -228,7 +254,12 @@ bool Channel::ChannelImpl::CreatePipe(const IPC::ChannelHandle &channel_handle,
   scoped_ptr<Message> m(new Message(MSG_ROUTING_NONE,
                                     HELLO_MESSAGE_TYPE,
                                     IPC::Message::PRIORITY_NORMAL));
-  if (!m->WriteInt(GetCurrentProcessId())) {
+
+  // Don't send the secret to the untrusted process, and don't send a secret
+  // if the value is zero (for IPC backwards compatability).
+  int32 secret = validate_client_ ? 0 : client_secret_;
+  if (!m->WriteInt(GetCurrentProcessId()) ||
+      (secret && !m->WriteUInt32(secret))) {
     CloseHandle(pipe_);
     pipe_ = INVALID_HANDLE_VALUE;
     return false;
@@ -441,4 +472,24 @@ bool Channel::IsNamedServerInitialized(const std::string& channel_id) {
   return ChannelImpl::IsNamedServerInitialized(channel_id);
 }
 
+// static
+std::string Channel::GenerateVerifiedChannelID(const std::string& 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
+  // hijacking the pipe name to spoof the client.
+
+  std::string id = prefix;
+  if (!id.empty())
+    id.append(".");
+
+  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(base::StringPrintf("\\%d", secret));
+}
+
 }  // namespace IPC
diff --git a/ipc/ipc_channel_win.h b/ipc/ipc_channel_win.h
index 7a48695..d680fb7 100644
--- a/ipc/ipc_channel_win.h
+++ b/ipc/ipc_channel_win.h
@@ -43,7 +43,8 @@ class Channel::ChannelImpl : public internal::ChannelReader,
   bool DidEmptyInputBuffers() OVERRIDE;
   virtual void HandleHelloMessage(const Message& msg) OVERRIDE;
 
-  static const std::wstring PipeName(const std::string& channel_id);
+  static const string16 PipeName(const std::string& channel_id,
+                                 int32* secret);
   bool CreatePipe(const IPC::ChannelHandle &channel_handle, Mode mode);
 
   bool ProcessConnection();
@@ -79,6 +80,16 @@ class Channel::ChannelImpl : public internal::ChannelReader,
   // problems.  TODO(darin): make this unnecessary
   bool processing_incoming_;
 
+  // Determines if we should validate a client's secret on connection.
+  bool validate_client_;
+
+  // 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 fto preserve IPC
+  // compatability with existing clients that don't validate the channel.)
+  int32 client_secret_;
+
+
   base::WeakPtrFactory<ChannelImpl> weak_factory_;
 
   scoped_ptr<base::NonThreadSafe> thread_check_;
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index 2b74ff4..6cb7457 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -1594,4 +1594,93 @@ TEST_F(IPCSyncChannelTest, RestrictedDispatchDeadlock) {
   RunTest(workers);
 }
 
+//-----------------------------------------------------------------------------
+
+// Generate a validated channel ID using Channel::GenerateVerifiedChannelID().
+namespace {
+
+class VerifiedServer : public Worker {
+ public:
+  VerifiedServer(base::Thread* listener_thread,
+                 const std::string& channel_name,
+                 const std::string& reply_text)
+      : Worker(channel_name, Channel::MODE_SERVER),
+        reply_text_(reply_text) {
+    Worker::OverrideThread(listener_thread);
+  }
+
+  virtual void OnNestedTestMsg(Message* reply_msg) {
+    VLOG(1) << __FUNCTION__ << " Sending reply: " << reply_text_;
+    SyncChannelNestedTestMsg_String::WriteReplyParams(reply_msg, reply_text_);
+    Send(reply_msg);
+    Done();
+  }
+
+ private:
+  std::string reply_text_;
+};
+
+class VerifiedClient : public Worker {
+ public:
+  VerifiedClient(base::Thread* listener_thread,
+                 const std::string& channel_name,
+                 const std::string& expected_text)
+      : Worker(channel_name, Channel::MODE_CLIENT),
+        expected_text_(expected_text) {
+    Worker::OverrideThread(listener_thread);
+  }
+
+  virtual void Run() {
+    std::string response;
+    SyncMessage* msg = new SyncChannelNestedTestMsg_String(&response);
+    bool result = Send(msg);
+    DCHECK(result);
+    DCHECK_EQ(response, expected_text_);
+
+    VLOG(1) << __FUNCTION__ << " Received reply: " << response;
+    Done();
+  }
+
+ private:
+  bool pump_during_send_;
+  std::string expected_text_;
+};
+
+void Verified() {
+  std::vector<Worker*> workers;
+
+  // A shared worker thread for servers
+  base::Thread server_worker_thread("Verified_ServerListener");
+  ASSERT_TRUE(server_worker_thread.Start());
+
+  base::Thread client_worker_thread("Verified_ClientListener");
+  ASSERT_TRUE(client_worker_thread.Start());
+
+  std::string channel_id = Channel::GenerateVerifiedChannelID("Verified");
+  Worker* worker;
+
+  worker = new VerifiedServer(&server_worker_thread,
+                              channel_id,
+                              "Got first message");
+  workers.push_back(worker);
+
+  worker = new VerifiedClient(&client_worker_thread,
+                              channel_id,
+                              "Got first message");
+  workers.push_back(worker);
+
+  RunTest(workers);
+
+#if defined(OS_WIN)
+#endif
+}
+
+}  // namespace
+
+// Windows needs to send an out-of-band secret to verify the client end of the
+// channel. Test that we still connect correctly in that case.
+TEST_F(IPCSyncChannelTest, Verified) {
+  Verified();
+}
+
 }  // namespace IPC