Update GnubbyAuthHandler to use the current session ID

remoting/host/security_key/gnubby_auth_handler.h

 // 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.
 
 #ifndef REMOTING_HOST_SECURITY_KEY_GNUBBY_AUTH_HANDLER_H_
 #define REMOTING_HOST_SECURITY_KEY_GNUBBY_AUTH_HANDLER_H_
 
+#include <cstdint>
 #include <memory>
 #include <string>
 
 #include "base/callback.h"
 #include "base/time/time.h"
 
 namespace base {
 class FilePath;
 }  // namespace base
 
 namespace remoting {
 
 // Class responsible for proxying authentication data between a local gnubbyd
 // and the client.
 class GnubbyAuthHandler {
  public:
   virtual ~GnubbyAuthHandler() {}
 
   // Used to send gnubby extension messages to the client.
   typedef base::Callback<void(int connection_id, const std::string& data)>
       SendMessageCallback;
 
+  // Returns the value of the current session being remoted.  Used to ensure
+  // global resources are only accessed from the remoted user session.
+  typedef base::Callback<uint32_t()> SessionIdCallback;
+
   // Creates a platform-specific GnubbyAuthHandler.
-  // All invocations of |callback| are guaranteed to occur before the underlying
-  // GnubbyAuthHandler object is destroyed.  It is not safe to destroy the
-  // GnubbyAuthHandler object within the callback.
+  // All invocations of the callbacks are guaranteed to occur before the
+  // underlying GnubbyAuthHandler object is destroyed.  It is not safe to
+  // destroy the GnubbyAuthHandler object within the callback.
   static std::unique_ptr<GnubbyAuthHandler> Create(
-      const SendMessageCallback& callback);
+      const SendMessageCallback& send_message_callback,
+      const SessionIdCallback& session_id_callback);

[Sergey Ulanov, 2016/06/27 23:51:58]

Why does the callback need to be passed to Create() given that you have
SetSessionIdCallback()?
Also do we need a callback at all? I.e. why SetSessionId(id) is not enough?

[joedow, 2016/06/28 01:27:34]

The session ID can change at any time (e.g. when a user signs into a Windows
session when in curtain mode, we will switch from the RD session into the logged
on user session).

I think that is a fair point about the callback being set in Create and in a
method.  I'll see if I can make this a bit cleaner.

[Sergey Ulanov, 2016/06/28 18:08:03]

Even if session ID may change it doesn't mean we need a callback here. With
SetSessionId() instead of the callback the handler implementation will need to
be notified whenever session-id changes by calling SetSessionId(). And if I
understand correctly the host knows when session id can change. But I'm not sure
which approach is simpler here.

[joedow, 2016/06/28 21:36:21]

The host definitely knows when the session id is changing, the complexity comes
from *which* object in the host knows.  AFAICT the DesktopSessionConnector is
the object which handles messages from the Daemon to attach/detach different
desktops.  This interface is implemented by the IpcDesktopEnvironmentFactory. 
The ClientSession (which owns the HostExtensionManager and HostExtensions) has
no knowledge of these events.  Plumbing the events to the ClientSession or
updating it when the session changes feels like the wrong approach. 

The session id is a property that may never be queried/needed unless security
key functionality is enabled and an attempt to connect to the IPC channel is
made so I feel like a lazy/JIT request to get the current id if needed is the
most efficient.

The main problem in this CL is the amount of churn needed due to the callback. 
If additional session details are needed in the future, then I think there is a
valid concern that another round of changes like this would be required.

I should be able to update the design without relying on the callback which
should make this cleaner and easier to maintain later on.

 
 #if defined(OS_LINUX)
   // Specify the name of the socket to listen to gnubby requests on.
   static void SetGnubbySocketName(const base::FilePath& gnubby_socket_name);
 #endif  // defined(OS_LINUX)
 
   // Sets the callback used to send messages to the client.
   virtual void SetSendMessageCallback(const SendMessageCallback& callback) = 0;
 
+  // Sets the callback used to retrieve the current session ID.
+  virtual void SetSessionIdCallback(const SessionIdCallback& callback) = 0;
+
   // Creates the platform specific connection to handle gnubby requests.
   virtual void CreateGnubbyConnection() = 0;
 
   // Returns true if |gnubby_connection_id| represents a valid connection.
   virtual bool IsValidConnectionId(int gnubby_connection_id) const = 0;
 
   // Sends the gnubby response from the client to the local gnubby agent.
   virtual void SendClientResponse(int gnubby_connection_id,
                                   const std::string& response) = 0;
 
   // Closes the gnubby connection represented by |gnubby_connection_id|.
   virtual void SendErrorAndCloseConnection(int gnubby_connection_id) = 0;
 
   // Returns the number of active gnubby connections.
   virtual size_t GetActiveConnectionCountForTest() const = 0;
 
   // Sets the timeout used when waiting for a gnubby response.
   virtual void SetRequestTimeoutForTest(base::TimeDelta timeout) = 0;
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_SECURITY_KEY_GNUBBY_AUTH_HANDLER_H_