Update GnubbyAuthHandler to use the current session ID

remoting/host/security_key/remote_security_key_ipc_server_impl.cc

 // Copyright 2016 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 "remoting/host/security_key/remote_security_key_ipc_server_impl.h"
 
+#include <cstdint>
 #include <memory>
 #include <string>
 
 #include "base/callback.h"
 #include "base/callback_helpers.h"
+#include "base/location.h"
 #include "base/threading/thread_checker.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/timer/timer.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
 #include "remoting/base/logging.h"
 #include "remoting/host/chromoting_messages.h"
 
 #if defined(OS_WIN)
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/win_util.h"
 #include "remoting/host/ipc_util.h"
 #endif  // defined(OS_WIN)
 
 namespace {
 
 // Returns the command code (the first byte of the data) if it exists, or -1 if
 // the data is empty.
 unsigned int GetCommandCode(const std::string& data) {
   return data.empty() ? -1 : static_cast<unsigned int>(data[0]);
 }
 
 }  // namespace
 
 namespace remoting {
 
 RemoteSecurityKeyIpcServerImpl::RemoteSecurityKeyIpcServerImpl(
     int connection_id,
+    uint32_t peer_session_id,
     base::TimeDelta initial_connect_timeout,
     const GnubbyAuthHandler::SendMessageCallback& message_callback,
     const base::Closure& done_callback)
     : connection_id_(connection_id),
+      peer_session_id_(peer_session_id),
       initial_connect_timeout_(initial_connect_timeout),
       done_callback_(done_callback),
-      message_callback_(message_callback) {
+      message_callback_(message_callback),
+      weak_factory_(this) {
   DCHECK_GT(connection_id_, 0);
   DCHECK(!done_callback_.is_null());
   DCHECK(!message_callback_.is_null());
 }
 
 RemoteSecurityKeyIpcServerImpl::~RemoteSecurityKeyIpcServerImpl() {}
 
 bool RemoteSecurityKeyIpcServerImpl::CreateChannel(
     const std::string& channel_name,
     base::TimeDelta request_timeout) {
@@ skipping 63 matching lines @@
                         OnSecurityKeyRequest)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
   CHECK(handled) << "Received unexpected IPC type: " << message.type();
   return handled;
 }
 
 void RemoteSecurityKeyIpcServerImpl::OnChannelConnected(int32_t peer_pid) {
   DCHECK(thread_checker_.CalledOnValidThread());
+
+#if defined(OS_WIN)
+  DWORD peer_session_id;
+  if (!ProcessIdToSessionId(peer_pid, &peer_session_id)) {
+    PLOG(ERROR) << "ProcessIdToSessionId() failed";
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(&RemoteSecurityKeyIpcServerImpl::OnChannelError,
+                              weak_factory_.GetWeakPtr()));
+    return;
+  }
+  if (peer_session_id != peer_session_id_) {
+    LOG(ERROR) << "Ignoring connection attempt from outside remoted session.";
+    base::ThreadTaskRunnerHandle::Get()->PostTask(

[Sergey Ulanov, 2016/06/29 22:36:46]

This duplicates the PostTask() call above. Structure this call to avoid
duplication?

bool close_connection = true;
if (!ProcessIdToSessionId()) {
  PLOG ...
} else if (peer_session_id != peer_session_id_) {
  LOG...
} else {
  close_connection = false;
}
if (close_connection) {
  PostTask
  return;
}

[Sergey Ulanov, 2016/06/29 22:36:46]

Should we also close the channel here? Otherwise there is a chance that
ChromotingRemoteSecurityKeyToNetworkMsg_Request will be executed before this
task.

[joedow, 2016/06/30 00:10:08]

I think that is a good idea, I'll add a member bool to track whether there is a
pending close operation and ignore IPC messages received if that is the case.

[joedow, 2016/06/30 00:10:08]

Done.

+        FROM_HERE, base::Bind(&RemoteSecurityKeyIpcServerImpl::OnChannelError,
+                              weak_factory_.GetWeakPtr()));
+    return;
+  }
+#else   // !defined(OS_WIN)
+  CHECK_EQ(peer_session_id_, UINT32_MAX);
+#endif  // !defined(OS_WIN)
+
   // Reset the timer to give the client a chance to send the request.
   timer_.Start(FROM_HERE, initial_connect_timeout_,
                base::Bind(&RemoteSecurityKeyIpcServerImpl::OnChannelError,
                           base::Unretained(this)));
 }
 
 void RemoteSecurityKeyIpcServerImpl::OnChannelError() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (ipc_channel_) {
+    ipc_channel_->Close();
+  }
 
   if (!done_callback_.is_null()) {
     // Note: This callback may result in this object being torn down.
     base::ResetAndReturn(&done_callback_).Run();
   }
 }
 
 void RemoteSecurityKeyIpcServerImpl::OnSecurityKeyRequest(
     const std::string& request_data) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Reset the timer to give the client a chance to send the response.
   timer_.Start(FROM_HERE, security_key_request_timeout_,
                base::Bind(&RemoteSecurityKeyIpcServerImpl::OnChannelError,
                           base::Unretained(this)));
 
   HOST_LOG << "Received gnubby request: " << GetCommandCode(request_data);
   message_callback_.Run(connection_id_, request_data);
 }
 
 }  // namespace remoting