Skip to content

Commit

Permalink
This CL adds the "request pairing" implementation between the web-app…
Browse files Browse the repository at this point in the history
… and the host. Specifically:

* Adds a pairing registry to the Chromoting host.
* Checks the state of the "remember me" checkbox and sends a pairing request if needed.
* Adds the plumbing to get that request to host, and to get the response back to the web-app.
* Saves the pairing response to local storage, and uses it next time we connect.
* Uses Base64 throughout for the secret, since unencoded random strings can't be serialized via PPAPI.

Note that pairing is still disabled because we're still missing per-platform Delegate implementations, a UI for revoking pairings, and policy support for disabling the feature.

BUG=156182

Review URL: https://chromiumcodereview.appspot.com/16137004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203865 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jamiewalch@chromium.org committed Jun 4, 2013
1 parent de1bb9c commit 8835692
Show file tree
Hide file tree
Showing 23 changed files with 284 additions and 48 deletions.
16 changes: 11 additions & 5 deletions remoting/client/chromoting_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,12 @@ void ChromotingClient::Start(
scoped_ptr<protocol::TransportFactory> transport_factory) {
DCHECK(task_runner_->BelongsToCurrentThread());

// TODO(jamiewalch): Add the plumbing required to get the client id and
// shared secret from the web-app.
std::string client_id, shared_secret;
scoped_ptr<protocol::Authenticator> authenticator(
new protocol::NegotiatingClientAuthenticator(
client_id, shared_secret,
config_.authentication_tag, config_.fetch_secret_callback,
config_.client_pairing_id,
config_.client_paired_secret,
config_.authentication_tag,
config_.fetch_secret_callback,
user_interface_->GetTokenFetcher(config_.host_public_key),
config_.authentication_methods));

Expand Down Expand Up @@ -123,6 +122,13 @@ void ChromotingClient::SetCapabilities(
IntersectCapabilities(config_.capabilities, host_capabilities_));
}

void ChromotingClient::SetPairingResponse(
const protocol::PairingResponse& pairing_response) {
DCHECK(task_runner_->BelongsToCurrentThread());

user_interface_->SetPairingResponse(pairing_response);
}

void ChromotingClient::InjectClipboardEvent(
const protocol::ClipboardEvent& event) {
DCHECK(task_runner_->BelongsToCurrentThread());
Expand Down
2 changes: 2 additions & 0 deletions remoting/client/chromoting_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback,
// ClientStub implementation.
virtual void SetCapabilities(
const protocol::Capabilities& capabilities) OVERRIDE;
virtual void SetPairingResponse(
const protocol::PairingResponse& pairing_response) OVERRIDE;

// ClipboardStub implementation for receiving clipboard data from host.
virtual void InjectClipboardEvent(
Expand Down
8 changes: 8 additions & 0 deletions remoting/client/client_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ struct ClientConfig {

// The set of all capabilities supported by the webapp.
std::string capabilities;

// The host-generated id and secret for paired clients. Paired clients
// should set both of these in addition to fetch_secret_callback; the
// latter is used if the paired connection fails (for example, if the
// pairing has been revoked by the host) and the user needs to prompted
// to enter their PIN.
std::string client_pairing_id;
std::string client_paired_secret;
};

} // namespace remoting
Expand Down
5 changes: 5 additions & 0 deletions remoting/client/client_user_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace remoting {
namespace protocol {
class ClipboardStub;
class CursorShapeStub;
class PairingResponse;
} // namespace protocol

// ClientUserInterface is an interface that must be implemented by
Expand All @@ -37,6 +38,10 @@ class ClientUserInterface {
// to the application.
virtual void SetCapabilities(const std::string& capabilities) = 0;

// Passes a pairing response message to the client.
virtual void SetPairingResponse(
const protocol::PairingResponse& pairing_response) = 0;

// Get the view's ClipboardStub implementation.
virtual protocol::ClipboardStub* GetClipboardStub() = 0;

Expand Down
28 changes: 27 additions & 1 deletion remoting/client/plugin/chromoting_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ logging::LogMessageHandlerFunction g_logging_old_handler = NULL;
const char ChromotingInstance::kApiFeatures[] =
"highQualityScaling injectKeyEvent sendClipboardItem remapKey trapKey "
"notifyClientDimensions notifyClientResolution pauseVideo pauseAudio "
"asyncPin thirdPartyAuth";
"asyncPin thirdPartyAuth pinlessAuth";

const char ChromotingInstance::kRequestedCapabilities[] = "";
const char ChromotingInstance::kSupportedCapabilities[] = "";
Expand Down Expand Up @@ -289,6 +289,8 @@ void ChromotingInstance::HandleMessage(const pp::Var& message) {
LOG(ERROR) << "Invalid connect() data.";
return;
}
data->GetString("clientPairingId", &config.client_pairing_id);
data->GetString("clientPairedSecret", &config.client_paired_secret);
if (use_async_pin_dialog_) {
config.fetch_secret_callback =
base::Bind(&ChromotingInstance::FetchSecretFromDialog,
Expand Down Expand Up @@ -424,6 +426,13 @@ void ChromotingInstance::HandleMessage(const pp::Var& message) {
return;
}
OnThirdPartyTokenFetched(token, shared_secret);
} else if (method == "requestPairing") {
std::string client_name;
if (!data->GetString("clientName", &client_name)) {
LOG(ERROR) << "Invalid requestPairing";
return;
}
RequestPairing(client_name);
}
}

Expand Down Expand Up @@ -504,6 +513,14 @@ void ChromotingInstance::SetCapabilities(const std::string& capabilities) {
PostChromotingMessage("setCapabilities", data.Pass());
}

void ChromotingInstance::SetPairingResponse(
const protocol::PairingResponse& pairing_response) {
scoped_ptr<base::DictionaryValue> data(new base::DictionaryValue());
data->SetString("clientId", pairing_response.client_id());
data->SetString("sharedSecret", pairing_response.shared_secret());
PostChromotingMessage("pairingResponse", data.Pass());
}

void ChromotingInstance::FetchSecretFromDialog(
bool pairing_supported,
const protocol::SecretFetchedCallback& secret_fetched_callback) {
Expand Down Expand Up @@ -779,6 +796,15 @@ void ChromotingInstance::OnThirdPartyTokenFetched(
}
}

void ChromotingInstance::RequestPairing(const std::string& client_name) {
if (!IsConnected()) {
return;
}
protocol::PairingRequest pairing_request;
pairing_request.set_client_name(client_name);
host_connection_->host_stub()->RequestPairing(pairing_request);
}

ChromotingStats* ChromotingInstance::GetStats() {
if (!client_.get())
return NULL;
Expand Down
3 changes: 3 additions & 0 deletions remoting/client/plugin/chromoting_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ class ChromotingInstance :
protocol::ErrorCode error) OVERRIDE;
virtual void OnConnectionReady(bool ready) OVERRIDE;
virtual void SetCapabilities(const std::string& capabilities) OVERRIDE;
virtual void SetPairingResponse(
const protocol::PairingResponse& pairing_response) OVERRIDE;
virtual protocol::ClipboardStub* GetClipboardStub() OVERRIDE;
virtual protocol::CursorShapeStub* GetCursorShapeStub() OVERRIDE;
virtual scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
Expand Down Expand Up @@ -195,6 +197,7 @@ class ChromotingInstance :
void OnPinFetched(const std::string& pin);
void OnThirdPartyTokenFetched(const std::string& token,
const std::string& shared_secret);
void RequestPairing(const std::string& client_name);

// Helper method to post messages to the webapp.
void PostChromotingMessage(const std::string& method,
Expand Down
3 changes: 2 additions & 1 deletion remoting/host/chromoting_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,8 @@ void ChromotingHost::OnIncomingSession(
ui_task_runner_,
connection.Pass(),
desktop_environment_factory_,
max_session_duration_);
max_session_duration_,
pairing_registry_);
clients_.push_back(client);
}

Expand Down
16 changes: 15 additions & 1 deletion remoting/host/chromoting_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
#include "remoting/host/host_status_monitor.h"
#include "remoting/host/host_status_observer.h"
#include "remoting/protocol/authenticator.h"
#include "remoting/protocol/session_manager.h"
#include "remoting/protocol/connection_to_client.h"
#include "remoting/protocol/pairing_registry.h"
#include "remoting/protocol/session_manager.h"
#include "third_party/skia/include/core/SkSize.h"

namespace base {
Expand Down Expand Up @@ -139,6 +140,16 @@ class ChromotingHost : public base::NonThreadSafe,
return weak_factory_.GetWeakPtr();
}

// The host uses a pairing registry to generate and store pairing information
// for clients for PIN-less authentication.
scoped_refptr<protocol::PairingRegistry> pairing_registry() const {
return pairing_registry_;
}
void set_pairing_registry(
scoped_refptr<protocol::PairingRegistry> pairing_registry) {
pairing_registry_ = pairing_registry;
}

private:
friend class ChromotingHostTest;

Expand Down Expand Up @@ -190,6 +201,9 @@ class ChromotingHost : public base::NonThreadSafe,
// The maximum duration of any session.
base::TimeDelta max_session_duration_;

// The pairing registry for PIN-less authentication.
scoped_refptr<protocol::PairingRegistry> pairing_registry_;

base::WeakPtrFactory<ChromotingHost> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(ChromotingHost);
Expand Down
3 changes: 2 additions & 1 deletion remoting/host/chromoting_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ class ChromotingHostTest : public testing::Test {
task_runner_, // UI
connection.Pass(),
desktop_environment_factory_.get(),
base::TimeDelta()));
base::TimeDelta(),
NULL));

connection_ptr->set_host_stub(client.get());

Expand Down
19 changes: 17 additions & 2 deletions remoting/host/client_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "remoting/proto/event.pb.h"
#include "remoting/protocol/client_stub.h"
#include "remoting/protocol/clipboard_thread_proxy.h"
#include "remoting/protocol/pairing_registry.h"

// Default DPI to assume for old clients that use notifyClientDimensions.
const int kDefaultDPI = 96;
Expand All @@ -43,7 +44,8 @@ ClientSession::ClientSession(
scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
scoped_ptr<protocol::ConnectionToClient> connection,
DesktopEnvironmentFactory* desktop_environment_factory,
const base::TimeDelta& max_duration)
const base::TimeDelta& max_duration,
scoped_refptr<protocol::PairingRegistry> pairing_registry)
: event_handler_(event_handler),
connection_(connection.Pass()),
client_jid_(connection_->session()->jid()),
Expand All @@ -63,7 +65,8 @@ ClientSession::ClientSession(
video_capture_task_runner_(video_capture_task_runner),
video_encode_task_runner_(video_encode_task_runner),
network_task_runner_(network_task_runner),
ui_task_runner_(ui_task_runner) {
ui_task_runner_(ui_task_runner),
pairing_registry_(pairing_registry) {
connection_->SetEventHandler(this);

// TODO(sergeyu): Currently ConnectionToClient expects stubs to be
Expand Down Expand Up @@ -172,6 +175,18 @@ void ClientSession::SetCapabilities(
IntersectCapabilities(*client_capabilities_, host_capabilities_));
}

void ClientSession::RequestPairing(
const protocol::PairingRequest& pairing_request) {
if (pairing_request.has_client_name()) {
protocol::PairingRegistry::Pairing pairing =
pairing_registry_->CreatePairing(pairing_request.client_name());
protocol::PairingResponse pairing_response;
pairing_response.set_client_id(pairing.client_id);
pairing_response.set_shared_secret(pairing.shared_secret);
connection_->client_stub()->SetPairingResponse(pairing_response);
}
}

void ClientSession::OnConnectionAuthenticated(
protocol::ConnectionToClient* connection) {
DCHECK(CalledOnValidThread());
Expand Down
9 changes: 8 additions & 1 deletion remoting/host/client_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "remoting/protocol/input_event_tracker.h"
#include "remoting/protocol/input_filter.h"
#include "remoting/protocol/input_stub.h"
#include "remoting/protocol/pairing_registry.h"
#include "third_party/skia/include/core/SkPoint.h"
#include "third_party/skia/include/core/SkSize.h"

Expand Down Expand Up @@ -95,7 +96,8 @@ class ClientSession
scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
scoped_ptr<protocol::ConnectionToClient> connection,
DesktopEnvironmentFactory* desktop_environment_factory,
const base::TimeDelta& max_duration);
const base::TimeDelta& max_duration,
scoped_refptr<protocol::PairingRegistry> pairing_registry);
virtual ~ClientSession();

// protocol::HostStub interface.
Expand All @@ -107,6 +109,8 @@ class ClientSession
const protocol::AudioControl& audio_control) OVERRIDE;
virtual void SetCapabilities(
const protocol::Capabilities& capabilities) OVERRIDE;
virtual void RequestPairing(
const remoting::protocol::PairingRequest& pairing_request) OVERRIDE;

// protocol::ConnectionToClient::EventHandler interface.
virtual void OnConnectionAuthenticated(
Expand Down Expand Up @@ -224,6 +228,9 @@ class ClientSession
// Used to apply client-requested changes in screen resolution.
scoped_ptr<ScreenControls> screen_controls_;

// The pairing registry for PIN-less authentication.
scoped_refptr<protocol::PairingRegistry> pairing_registry_;

DISALLOW_COPY_AND_ASSIGN(ClientSession);
};

Expand Down
3 changes: 2 additions & 1 deletion remoting/host/client_session_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ void ClientSessionTest::SetUp() {
ui_task_runner, // UI thread.
connection.PassAs<protocol::ConnectionToClient>(),
desktop_environment_factory_.get(),
base::TimeDelta()));
base::TimeDelta(),
NULL));
}

void ClientSessionTest::TearDown() {
Expand Down
16 changes: 12 additions & 4 deletions remoting/host/remoting_me2me_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "base/single_thread_task_runner.h"
#include "base/string_number_conversions.h"
#include "base/string_util.h"
#include "base/string_util.h"
#include "base/synchronization/waitable_event.h"
#include "base/threading/thread.h"
#include "base/utf_string_conversions.h"
Expand Down Expand Up @@ -462,12 +461,19 @@ void HostProcess::CreateAuthenticatorFactory() {
ShutdownHost(kInitializationFailed);
return;
}

// TODO(jamiewalch): Add a pairing registry here once all the code
// is committed.
scoped_refptr<remoting::protocol::PairingRegistry> pairing_registry;
//scoped_refptr<protocol::PairingRegistry> pairing_registry(
// new protocol::PairingRegistry(
// scoped_ptr<protocol::PairingRegistry::Delegate>(
// new protocol::NotImplementedPairingRegistryDelegate),
// protocol::PairingRegistry::PairedClients()));

scoped_ptr<protocol::AuthenticatorFactory> factory;

if (token_url_.is_empty() && token_validation_url_.is_empty()) {
// TODO(jamiewalch): Add a pairing registry here once all the code
// is committed.
scoped_refptr<remoting::protocol::PairingRegistry> pairing_registry;
factory = protocol::Me2MeHostAuthenticatorFactory::CreateWithSharedSecret(
local_certificate, key_pair_, host_secret_hash_, pairing_registry);

Expand Down Expand Up @@ -495,6 +501,8 @@ void HostProcess::CreateAuthenticatorFactory() {
factory.reset(new PamAuthorizationFactory(factory.Pass()));
#endif
host_->SetAuthenticatorFactory(factory.Pass());

host_->set_pairing_registry(pairing_registry);
}

// IPC::Listener implementation.
Expand Down
4 changes: 1 addition & 3 deletions remoting/protocol/client_stub.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ class ClientStub : public ClipboardStub,
virtual void SetCapabilities(const Capabilities& capabilities) = 0;

// Passes a pairing response message to the client.
// TODO(jamiewalch): Make this pure virtual once the PIN-less authentication
// implementation CLs have landed.
virtual void SetPairingResponse(const PairingResponse& pairing_response) {}
virtual void SetPairingResponse(const PairingResponse& pairing_response) = 0;

private:
DISALLOW_COPY_AND_ASSIGN(ClientStub);
Expand Down
4 changes: 1 addition & 3 deletions remoting/protocol/host_stub.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ class HostStub {
virtual void SetCapabilities(const Capabilities& capabilities) = 0;

// Requests pairing between the host and client for PIN-less authentication.
// TODO(jamiewalch): Make this pure virtual once the PIN-less authentication
// implementation CLs have landed.
virtual void RequestPairing(const PairingRequest& pairing_request) {}
virtual void RequestPairing(const PairingRequest& pairing_request) = 0;

protected:
virtual ~HostStub() {}
Expand Down
1 change: 1 addition & 0 deletions remoting/protocol/negotiating_authenticator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "remoting/protocol/negotiating_authenticator_base.h"
#include "remoting/protocol/negotiating_client_authenticator.h"
#include "remoting/protocol/negotiating_host_authenticator.h"
#include "remoting/protocol/pairing_registry.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
Expand Down
7 changes: 5 additions & 2 deletions remoting/protocol/pairing_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ const PairingRegistry::Pairing& PairingRegistry::CreatePairing(
// Create a random shared secret to authenticate this client.
char buffer[kKeySize];
crypto::RandBytes(buffer, arraysize(buffer));
result.shared_secret = std::string(buffer, buffer+arraysize(buffer));
if (!base::Base64Encode(base::StringPiece(buffer, arraysize(buffer)),
&result.shared_secret)) {
LOG(FATAL) << "Base64Encode failed.";
}

// Save the result via the Delegate and return it to the caller.
paired_clients_[result.client_id] = result;
Expand All @@ -59,7 +62,7 @@ std::string PairingRegistry::GetSecret(const std::string& client_id) const {
void NotImplementedPairingRegistryDelegate::Save(
const PairingRegistry::PairedClients& paired_clients) {
NOTIMPLEMENTED();
};
}

} // namespace protocol
} // namespace remoting
Loading

0 comments on commit 8835692

Please sign in to comment.