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

1 files changed, 60 insertions, 9 deletions

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