From f0a9d1b2f35d1a0ae2c532b86fb3fd78b1c5465b Mon Sep 17 00:00:00 2001
From: "garykac@chromium.org"
 <garykac@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 4 Mar 2011 21:31:44 +0000
Subject: Block event processing on host/client until the client has
 authenticated.

Input events:
* Client will not send them
* Host will not process them

Control events:
* Client will only process BeginSessionResponse
* Host will only process BeginSessionRequest
All other control messages will be ignored.

BUG=72466
TEST=manual+tests

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76974 0039d316-1c4b-4281-b951-d872f2087c98
---
 remoting/protocol/client_message_dispatcher.cc | 34 ++++++++++++-----
 remoting/protocol/client_stub.cc               | 30 +++++++++++++++
 remoting/protocol/client_stub.h                | 18 ++++++++-
 remoting/protocol/connection_to_client.cc      | 20 ++++++++--
 remoting/protocol/connection_to_client.h       | 10 +++--
 remoting/protocol/connection_to_host.cc        | 18 ++++++++-
 remoting/protocol/connection_to_host.h         |  8 ++++
 remoting/protocol/host_message_dispatcher.cc   | 52 +++++++++++++++++---------
 remoting/protocol/host_stub.cc                 | 29 ++++++++++++++
 remoting/protocol/host_stub.h                  | 20 ++++++++--
 remoting/protocol/input_stub.cc                | 29 ++++++++++++++
 remoting/protocol/input_stub.h                 | 19 +++++++++-
 remoting/protocol/protocol_mock_objects.cc     |  7 +++-
 remoting/protocol/protocol_mock_objects.h      |  5 ++-
 14 files changed, 253 insertions(+), 46 deletions(-)
 create mode 100644 remoting/protocol/client_stub.cc
 create mode 100644 remoting/protocol/host_stub.cc
 create mode 100644 remoting/protocol/input_stub.cc

(limited to 'remoting/protocol')

diff --git a/remoting/protocol/client_message_dispatcher.cc b/remoting/protocol/client_message_dispatcher.cc
index e7b6dd6..657e30d 100644
--- a/remoting/protocol/client_message_dispatcher.cc
+++ b/remoting/protocol/client_message_dispatcher.cc
@@ -39,18 +39,32 @@ void ClientMessageDispatcher::Initialize(
 
 void ClientMessageDispatcher::OnControlMessageReceived(
     ControlMessage* message, Task* done_task) {
-  // TODO(sergeyu): Add message validation.
-  if (message->has_notify_resolution()) {
-    client_stub_->NotifyResolution(
-        &message->notify_resolution(), done_task);
-  } else if (message->has_begin_session_response()) {
-    client_stub_->BeginSessionResponse(
-        &message->begin_session_response().login_status(), done_task);
+  if (!client_stub_->authenticated()) {
+    // When the client has not authenticated with the host, we restrict the
+    // control messages that we support.
+    if (message->has_begin_session_response()) {
+      client_stub_->BeginSessionResponse(
+          &message->begin_session_response().login_status(), done_task);
+      return;
+    } else {
+      LOG(WARNING) << "Invalid control message received "
+                   << "(client not authenticated).";
+    }
   } else {
-    LOG(WARNING) << "Invalid control message received.";
-    done_task->Run();
-    delete done_task;
+    // TODO(sergeyu): Add message validation.
+    if (message->has_notify_resolution()) {
+      client_stub_->NotifyResolution(
+          &message->notify_resolution(), done_task);
+      return;
+    } else if (message->has_begin_session_response()) {
+      LOG(WARNING) << "BeginSessionResponse sent after client already "
+                   << "authorized.";
+    } else {
+      LOG(WARNING) << "Invalid control message received.";
+    }
   }
+  done_task->Run();
+  delete done_task;
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/client_stub.cc b/remoting/protocol/client_stub.cc
new file mode 100644
index 0000000..d766f5b
--- /dev/null
+++ b/remoting/protocol/client_stub.cc
@@ -0,0 +1,30 @@
+// Copyright (c) 2010 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.
+
+// Interface of a client that receives commands from a Chromoting host.
+//
+// This interface is responsible for a subset of control messages sent to
+// the Chromoting client.
+
+#include "remoting/protocol/client_stub.h"
+
+namespace remoting {
+namespace protocol {
+
+ClientStub::ClientStub() : authenticated_(false) {
+}
+
+ClientStub::~ClientStub() {
+}
+
+void ClientStub::OnAuthenticated() {
+  authenticated_ = true;
+}
+
+bool ClientStub::authenticated() {
+  return authenticated_;
+}
+
+}  // namespace protocol
+}  // namespace remoting
diff --git a/remoting/protocol/client_stub.h b/remoting/protocol/client_stub.h
index 4b73789..a2bd565 100644
--- a/remoting/protocol/client_stub.h
+++ b/remoting/protocol/client_stub.h
@@ -22,15 +22,29 @@ class NotifyResolutionRequest;
 
 class ClientStub {
  public:
-  ClientStub() {}
-  virtual ~ClientStub() {}
+  ClientStub();
+  virtual ~ClientStub();
 
   virtual void NotifyResolution(const NotifyResolutionRequest* msg,
                                 Task* done) = 0;
   virtual void BeginSessionResponse(const LocalLoginStatus* msg,
                                     Task* done) = 0;
 
+  // Called when the client has authenticated with the host to enable the
+  // host->client control channel.
+  // Before this is called, only a limited set of control messages will be
+  // processed.
+  void OnAuthenticated();
+
+  // Has the client successfully authenticated with the host?
+  // I.e., should we be processing control events?
+  bool authenticated();
+
  private:
+  // Initially false, this records whether the client has authenticated with
+  // the host.
+  bool authenticated_;
+
   DISALLOW_COPY_AND_ASSIGN(ClientStub);
 };
 
diff --git a/remoting/protocol/connection_to_client.cc b/remoting/protocol/connection_to_client.cc
index 3b4bd3f..383451b 100644
--- a/remoting/protocol/connection_to_client.cc
+++ b/remoting/protocol/connection_to_client.cc
@@ -8,6 +8,8 @@
 #include "net/base/io_buffer.h"
 #include "remoting/protocol/client_control_sender.h"
 #include "remoting/protocol/host_message_dispatcher.h"
+#include "remoting/protocol/host_stub.h"
+#include "remoting/protocol/input_stub.h"
 
 // TODO(hclam): Remove this header once MessageDispatcher is used.
 #include "remoting/base/compound_buffer.h"
@@ -23,7 +25,8 @@ ConnectionToClient::ConnectionToClient(MessageLoop* message_loop,
                                        EventHandler* handler,
                                        HostStub* host_stub,
                                        InputStub* input_stub)
-    : loop_(message_loop),
+    : client_authenticated_(false),
+      loop_(message_loop),
       handler_(handler),
       host_stub_(host_stub),
       input_stub_(input_stub) {
@@ -73,9 +76,6 @@ ClientStub* ConnectionToClient::client_stub() {
   return client_stub_.get();
 }
 
-ConnectionToClient::ConnectionToClient() {
-}
-
 void ConnectionToClient::OnSessionStateChange(protocol::Session::State state) {
   if (state == protocol::Session::CONNECTED) {
     client_stub_.reset(new ClientControlSender(session_->control_channel()));
@@ -123,5 +123,17 @@ void ConnectionToClient::StateChangeTask(protocol::Session::State state) {
 void ConnectionToClient::OnClosed() {
 }
 
+void ConnectionToClient::OnClientAuthenticated() {
+  client_authenticated_ = true;
+
+  // Enable/disable each of the channels.
+  if (input_stub_)
+    input_stub_->OnAuthenticated();
+  if (host_stub_)
+    host_stub_->OnAuthenticated();
+  if (client_stub_.get())
+    client_stub_->OnAuthenticated();
+}
+
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/connection_to_client.h b/remoting/protocol/connection_to_client.h
index b7aa052..d2721f3 100644
--- a/remoting/protocol/connection_to_client.h
+++ b/remoting/protocol/connection_to_client.h
@@ -72,9 +72,8 @@ class ConnectionToClient :
   // Return pointer to ClientStub.
   virtual ClientStub* client_stub();
 
- protected:
-  // Protected constructor used by unit test.
-  ConnectionToClient();
+  // Called when the host accepts the client authentication.
+  void OnClientAuthenticated();
 
  private:
   // Callback for protocol Session.
@@ -85,6 +84,11 @@ class ConnectionToClient :
 
   void OnClosed();
 
+  // Initially false, this is set to true once the client has authenticated
+  // properly. When this is false, many client messages (like input events)
+  // will be ignored.
+  bool client_authenticated_;
+
   // The libjingle channel used to send and receive data from the remote client.
   scoped_refptr<Session> session_;
 
diff --git a/remoting/protocol/connection_to_host.cc b/remoting/protocol/connection_to_host.cc
index 3d34997..2ef3a22 100644
--- a/remoting/protocol/connection_to_host.cc
+++ b/remoting/protocol/connection_to_host.cc
@@ -25,7 +25,8 @@ ConnectionToHost::ConnectionToHost(
     JingleThread* thread,
     talk_base::NetworkManager* network_manager,
     talk_base::PacketSocketFactory* socket_factory)
-    : thread_(thread),
+    : client_authenticated_(false),
+      thread_(thread),
       network_manager_(network_manager),
       socket_factory_(socket_factory),
       event_callback_(NULL),
@@ -190,7 +191,6 @@ void ConnectionToHost::OnSessionStateChange(
       // Initialize reader and writer.
       video_reader_.reset(VideoReader::Create(session_->config()));
       video_reader_->Init(session_, video_stub_);
-      input_stub_.reset(new InputSender(session_->event_channel()));
       host_stub_.reset(new HostControlSender(session_->control_channel()));
       dispatcher_->Initialize(session_.get(), client_stub_);
       event_callback_->OnConnectionOpened(this);
@@ -202,5 +202,19 @@ void ConnectionToHost::OnSessionStateChange(
   }
 }
 
+void ConnectionToHost::OnClientAuthenticated() {
+  client_authenticated_ = true;
+
+  // Create and enable the input stub now that we're authenticated.
+  input_stub_.reset(new InputSender(session_->event_channel()));
+  input_stub_->OnAuthenticated();
+
+  // Enable control channel stubs.
+  if (host_stub_.get())
+    host_stub_->OnAuthenticated();
+  if (client_stub_)
+    client_stub_->OnAuthenticated();
+}
+
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/connection_to_host.h b/remoting/protocol/connection_to_host.h
index 2f661ca..3962c32 100644
--- a/remoting/protocol/connection_to_host.h
+++ b/remoting/protocol/connection_to_host.h
@@ -85,6 +85,9 @@ class ConnectionToHost : public JingleClient::Callback {
   // Callback for chromotocol Session.
   void OnSessionStateChange(Session::State state);
 
+  // Called when the host accepts the client authentication.
+  void OnClientAuthenticated();
+
  private:
   // The message loop for the jingle thread this object works on.
   MessageLoop* message_loop();
@@ -101,6 +104,11 @@ class ConnectionToHost : public JingleClient::Callback {
   void OnDisconnected();
   void OnServerClosed();
 
+  // Initially false, this is set to true once the client has authenticated
+  // properly. When this is false, many messages to the host (like input events)
+  // will be suppressed.
+  bool client_authenticated_;
+
   JingleThread* thread_;
 
   scoped_ptr<talk_base::NetworkManager> network_manager_;
diff --git a/remoting/protocol/host_message_dispatcher.cc b/remoting/protocol/host_message_dispatcher.cc
index 1e1eea8..f4391a7 100644
--- a/remoting/protocol/host_message_dispatcher.cc
+++ b/remoting/protocol/host_message_dispatcher.cc
@@ -48,30 +48,46 @@ void HostMessageDispatcher::Initialize(
 
 void HostMessageDispatcher::OnControlMessageReceived(
     ControlMessage* message, Task* done_task) {
-  // TODO(sergeyu): Add message validation.
-  if (message->has_suggest_resolution()) {
-    host_stub_->SuggestResolution(&message->suggest_resolution(), done_task);
-  } else if (message->has_begin_session_request()) {
-    host_stub_->BeginSessionRequest(
-        &message->begin_session_request().credentials(), done_task);
+  if (!host_stub_->authenticated()) {
+    // When the client has not authenticated with the host, we restrict the
+    // control messages that we support.
+    if (message->has_begin_session_request()) {
+      host_stub_->BeginSessionRequest(
+          &message->begin_session_request().credentials(), done_task);
+      return;
+    } else {
+      LOG(WARNING) << "Invalid control message received "
+                   << "(client not authenticated).";
+    }
   } else {
-    LOG(WARNING) << "Invalid control message received.";
-    done_task->Run();
-    delete done_task;
+    // TODO(sergeyu): Add message validation.
+    if (message->has_suggest_resolution()) {
+      host_stub_->SuggestResolution(&message->suggest_resolution(), done_task);
+      return;
+    } else if (message->has_begin_session_request()) {
+      LOG(WARNING) << "BeginSessionRequest sent after client already "
+                   << "authorized.";
+    } else {
+      LOG(WARNING) << "Invalid control message received.";
+    }
   }
+  done_task->Run();
+  delete done_task;
 }
 
 void HostMessageDispatcher::OnEventMessageReceived(
     EventMessage* message, Task* done_task) {
-  // TODO(sergeyu): Add message validation.
-  if (message->has_key_event()) {
-    input_stub_->InjectKeyEvent(&message->key_event(), done_task);
-  } else if (message->has_mouse_event()) {
-    input_stub_->InjectMouseEvent(&message->mouse_event(), done_task);
-  } else {
-    LOG(WARNING) << "Invalid event message received.";
-    done_task->Run();
-    delete done_task;
+  if (input_stub_->authenticated()) {
+    // TODO(sergeyu): Add message validation.
+    if (message->has_key_event()) {
+      input_stub_->InjectKeyEvent(&message->key_event(), done_task);
+    } else if (message->has_mouse_event()) {
+      input_stub_->InjectMouseEvent(&message->mouse_event(), done_task);
+    } else {
+      LOG(WARNING) << "Invalid event message received.";
+      done_task->Run();
+      delete done_task;
+    }
   }
 }
 
diff --git a/remoting/protocol/host_stub.cc b/remoting/protocol/host_stub.cc
new file mode 100644
index 0000000..3ab1029
--- /dev/null
+++ b/remoting/protocol/host_stub.cc
@@ -0,0 +1,29 @@
+// Copyright (c) 2010 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.
+
+// Interface of a host that receives commands from a Chromoting client.
+//
+// This interface handles control messages defined in contro.proto.
+
+#include "remoting/protocol/host_stub.h"
+
+namespace remoting {
+namespace protocol {
+
+HostStub::HostStub() : authenticated_(false) {
+}
+
+HostStub::~HostStub() {
+}
+
+void HostStub::OnAuthenticated() {
+  authenticated_ = true;
+}
+
+bool HostStub::authenticated() {
+  return authenticated_;
+}
+
+}  // namespace protocol
+}  // namespace remoting
diff --git a/remoting/protocol/host_stub.h b/remoting/protocol/host_stub.h
index 1a26225..c8fa4ac 100644
--- a/remoting/protocol/host_stub.h
+++ b/remoting/protocol/host_stub.h
@@ -4,7 +4,7 @@
 
 // Interface of a host that receives commands from a Chromoting client.
 //
-// This interterface handles control messages defined in contro.proto.
+// This interface handles control messages defined in contro.proto.
 
 #ifndef REMOTING_PROTOCOL_HOST_STUB_H_
 #define REMOTING_PROTOCOL_HOST_STUB_H_
@@ -21,15 +21,29 @@ class LocalLoginCredentials;
 
 class HostStub {
  public:
-  HostStub() {}
-  virtual ~HostStub() {};
+  HostStub();
+  virtual ~HostStub();
 
   virtual void SuggestResolution(
       const SuggestResolutionRequest* msg, Task* done) = 0;
   virtual void BeginSessionRequest(
       const LocalLoginCredentials* credentials, Task* done) = 0;
 
+  // Called when the client has authenticated with the host to enable the
+  // client->host control channel.
+  // Before this is called, only a limited set of control messages will be
+  // processed.
+  void OnAuthenticated();
+
+  // Has the client successfully authenticated with the host?
+  // I.e., should we be processing control events?
+  bool authenticated();
+
  private:
+  // Initially false, this records whether the client has authenticated with
+  // the host.
+  bool authenticated_;
+
   DISALLOW_COPY_AND_ASSIGN(HostStub);
 };
 
diff --git a/remoting/protocol/input_stub.cc b/remoting/protocol/input_stub.cc
new file mode 100644
index 0000000..8bb1ffb
--- /dev/null
+++ b/remoting/protocol/input_stub.cc
@@ -0,0 +1,29 @@
+// Copyright (c) 2010 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.
+
+// Interface for a device that receives input events.
+// This interface handles event messages defined in event.proto.
+
+#include "remoting/protocol/input_stub.h"
+
+namespace remoting {
+namespace protocol {
+
+
+InputStub::InputStub() : authenticated_(false) {
+}
+
+InputStub::~InputStub() {
+}
+
+void InputStub::OnAuthenticated() {
+  authenticated_ = true;
+}
+
+bool InputStub::authenticated() {
+  return authenticated_;
+}
+
+}  // namespace protocol
+}  // namespace remoting
diff --git a/remoting/protocol/input_stub.h b/remoting/protocol/input_stub.h
index 8b89c7a..8c80d16 100644
--- a/remoting/protocol/input_stub.h
+++ b/remoting/protocol/input_stub.h
@@ -8,6 +8,8 @@
 #ifndef REMOTING_PROTOCOL_INPUT_STUB_H_
 #define REMOTING_PROTOCOL_INPUT_STUB_H_
 
+#include "base/basictypes.h"
+
 class Task;
 
 namespace remoting {
@@ -18,13 +20,26 @@ class MouseEvent;
 
 class InputStub {
  public:
-  InputStub() {}
-  virtual ~InputStub() {}
+  InputStub();
+  virtual ~InputStub();
 
   virtual void InjectKeyEvent(const KeyEvent* event, Task* done) = 0;
   virtual void InjectMouseEvent(const MouseEvent* event, Task* done) = 0;
 
+  // Called when the client has authenticated with the host to enable the
+  // input event channel.
+  // Before this is called, all input event will be ignored.
+  void OnAuthenticated();
+
+  // Has the client successfully authenticated with the host?
+  // I.e., should we be processing input events?
+  bool authenticated();
+
  private:
+  // Initially false, this records whether the client has authenticated with
+  // the host.
+  bool authenticated_;
+
   DISALLOW_COPY_AND_ASSIGN(InputStub);
 };
 
diff --git a/remoting/protocol/protocol_mock_objects.cc b/remoting/protocol/protocol_mock_objects.cc
index 8b38954..5deb40f 100644
--- a/remoting/protocol/protocol_mock_objects.cc
+++ b/remoting/protocol/protocol_mock_objects.cc
@@ -7,7 +7,12 @@
 namespace remoting {
 namespace protocol {
 
-MockConnectionToClient::MockConnectionToClient() {}
+MockConnectionToClient::MockConnectionToClient(MessageLoop* message_loop,
+                                               EventHandler* handler,
+                                               HostStub* host_stub,
+                                               InputStub* input_stub)
+    : ConnectionToClient(message_loop, handler, host_stub, input_stub) {
+}
 
 MockConnectionToClient::~MockConnectionToClient() {}
 
diff --git a/remoting/protocol/protocol_mock_objects.h b/remoting/protocol/protocol_mock_objects.h
index e9dd57e..57dad6a 100644
--- a/remoting/protocol/protocol_mock_objects.h
+++ b/remoting/protocol/protocol_mock_objects.h
@@ -21,7 +21,10 @@ class ChromotocolConnection;
 
 class MockConnectionToClient : public ConnectionToClient {
  public:
-  MockConnectionToClient();
+  MockConnectionToClient(MessageLoop* message_loop,
+                         EventHandler* handler,
+                         HostStub* host_stub,
+                         InputStub* input_stub);
   virtual ~MockConnectionToClient();
 
   MOCK_METHOD1(Init, void(Session* session));
-- 
cgit v1.1