Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Save the client base JID for authentication in case it differs from t…
Browse files Browse the repository at this point in the history
…he email (for accounts non-Google email). Keep a separate email field for use in user-facing UIs, logs, etc. Using the existing host_owner field for the base JID guarantees the correct *functional* behavior (i.e. authentication-wise) if the new config is used with an older host (the only difference is aesthetic - in that older host some UI/logs may show an uglier address instead of the user email).

BUG=224742

Review URL: https://codereview.chromium.org/595063005

Cr-Commit-Position: refs/heads/master@{#297269}
  • Loading branch information
rmsousa authored and Commit bot committed Sep 29, 2014
1 parent 6570731 commit 665bf30
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 40 deletions.
5 changes: 3 additions & 2 deletions remoting/host/chromoting_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,14 @@ ChromotingHost::~ChromotingHost() {
FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnShutdown());
}

void ChromotingHost::Start(const std::string& host_owner) {
void ChromotingHost::Start(const std::string& host_owner_email) {
DCHECK(CalledOnValidThread());
DCHECK(!started_);

HOST_LOG << "Starting host";
started_ = true;
FOR_EACH_OBSERVER(HostStatusObserver, status_observers_, OnStart(host_owner));
FOR_EACH_OBSERVER(HostStatusObserver, status_observers_,
OnStart(host_owner_email));

// Start the SessionManager, supplying this ChromotingHost as the listener.
session_manager_->Init(signal_strategy_, this);
Expand Down
1 change: 1 addition & 0 deletions remoting/host/host_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ namespace remoting {

const char kHostEnabledConfigPath[] = "enabled";
const char kHostOwnerConfigPath[] = "host_owner";
const char kHostOwnerEmailConfigPath[] = "host_owner_email";
const char kXmppLoginConfigPath[] = "xmpp_login";
const char kXmppAuthTokenConfigPath[] = "xmpp_auth_token";
const char kOAuthRefreshTokenConfigPath[] = "oauth_refresh_token";
Expand Down
6 changes: 4 additions & 2 deletions remoting/host/host_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ namespace remoting {

// Status of the host, whether it is enabled or disabled.
extern const char kHostEnabledConfigPath[];
// Google account of the owner of this host.
// Base JID of the host owner (may not equal the email for non-gmail users).
extern const char kHostOwnerConfigPath[];
// Login used to authenticate in XMPP network.
// Email of the owner of this host.
extern const char kHostOwnerEmailConfigPath[];
// Login used to authenticate in XMPP network (could be a service account).
extern const char kXmppLoginConfigPath[];
// Auth token used to authenticate to XMPP network.
extern const char kXmppAuthTokenConfigPath[];
Expand Down
2 changes: 1 addition & 1 deletion remoting/host/host_status_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class HostStatusObserver {
const protocol::TransportRoute& route) {}

// Called when hosting is started for an account.
virtual void OnStart(const std::string& xmpp_login) {}
virtual void OnStart(const std::string& host_owner_email) {}

// Called when the host shuts down.
virtual void OnShutdown() {}
Expand Down
16 changes: 10 additions & 6 deletions remoting/host/mac/me2me_preference_pane.mm
Original file line number Diff line number Diff line change
Expand Up @@ -468,14 +468,18 @@ - (void)updateUI {

std::string email;
if (config_.get()) {
bool result = config_->GetString(remoting::kHostOwnerConfigPath, &email);
bool result =
config_->GetString(remoting::kHostOwnerEmailConfigPath, &email);
if (!result) {
result = config_->GetString(remoting::kXmppLoginConfigPath, &email);

// The config has already been checked by |IsConfigValid|.
result = config_->GetString(remoting::kHostOwnerConfigPath, &email);
if (!result) {
[self showError];
return;
result = config_->GetString(remoting::kXmppLoginConfigPath, &email);

// The config has already been checked by |IsConfigValid|.
if (!result) {
[self showError];
return;
}
}
}
}
Expand Down
28 changes: 27 additions & 1 deletion remoting/host/remoting_me2me_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ class HostProcess
std::string oauth_refresh_token_;
std::string serialized_config_;
std::string host_owner_;
std::string host_owner_email_;
bool use_service_account_;
bool enable_vp9_;
int64_t frame_recorder_buffer_size_;
Expand Down Expand Up @@ -869,6 +870,13 @@ bool HostProcess::ApplyConfig(scoped_ptr<JsonHostConfig> config) {
use_service_account_ = false;
}

// For non-Gmail Google accounts, the owner base JID differs from the email.
// host_owner_ contains the base JID (used for authenticating clients), while
// host_owner_email contains the account's email (used for UI and logs).
if (!config->GetString(kHostOwnerEmailConfigPath, &host_owner_email_)) {
host_owner_email_ = host_owner_;
}

// Allow offering of VP9 encoding to be overridden by the command-line.
if (CommandLine::ForCurrentProcess()->HasSwitch(kEnableVp9SwitchName)) {
enable_vp9_ = true;
Expand Down Expand Up @@ -926,6 +934,17 @@ void HostProcess::OnPolicyUpdate(scoped_ptr<base::DictionaryValue> policies) {

void HostProcess::ApplyHostDomainPolicy() {
HOST_LOG << "Policy sets host domain: " << host_domain_;

// If the user does not have a Google email, their client JID will not be
// based on their email. In that case, the username/host domain policies would
// be meaningless, since there is no way to check that the JID attempting to
// connect actually corresponds to the owner email in question.
if (host_owner_ != host_owner_email_) {
LOG(ERROR) << "The username and host domain policies cannot be enabled for "
<< "accounts with a non-Google email.";
ShutdownHost(kInvalidHostDomainExitCode);
}

if (!host_domain_.empty() &&
!EndsWith(host_owner_, std::string("@") + host_domain_, false)) {
LOG(ERROR) << "The host domain does not match the policy.";
Expand All @@ -947,6 +966,13 @@ bool HostProcess::OnHostDomainPolicyUpdate(base::DictionaryValue* policies) {
}

void HostProcess::ApplyUsernamePolicy() {
// See comment in ApplyHostDomainPolicy.
if (host_owner_ != host_owner_email_) {
LOG(ERROR) << "The username and host domain policies cannot be enabled for "
<< "accounts with a non-Google email.";
ShutdownHost(kUsernameMismatchExitCode);
}

if (host_username_match_required_) {
HOST_LOG << "Policy requires host username match.";
std::string username = GetUsername();
Expand Down Expand Up @@ -1310,7 +1336,7 @@ void HostProcess::StartHost() {
#endif // !defined(REMOTING_MULTI_PROCESS)

host_->SetEnableCurtaining(curtain_required_);
host_->Start(host_owner_);
host_->Start(host_owner_email_);

CreateAuthenticatorFactory();
}
Expand Down
24 changes: 13 additions & 11 deletions remoting/host/win/elevated_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/values.h"
#include "base/win/scoped_handle.h"
#include "remoting/host/branding.h"
#include "remoting/host/host_config.h"
#include "remoting/host/usage_stats_consent.h"
#include "remoting/host/verify_config_window_win.h"
#include "remoting/host/win/core_resource.h"
Expand Down Expand Up @@ -50,16 +51,15 @@ const char kUnprivilegedConfigFileSecurityDescriptor[] =
"O:BAG:BAD:(A;;GA;;;SY)(A;;GA;;;BA)(A;;GR;;;AU)";

// Configuration keys.
const char kHostId[] = "host_id";
const char kXmppLogin[] = "xmpp_login";
const char kHostOwner[] = "host_owner";
const char kHostSecretHash[] = "host_secret_hash";

// The configuration keys that cannot be specified in UpdateConfig().
const char* const kReadonlyKeys[] = { kHostId, kHostOwner, kXmppLogin };
const char* const kReadonlyKeys[] = {
kHostIdConfigPath, kHostOwnerConfigPath, kHostOwnerEmailConfigPath,
kXmppLoginConfigPath };

// The configuration keys whose values may be read by GetConfig().
const char* const kUnprivilegedConfigKeys[] = { kHostId, kXmppLogin };
const char* const kUnprivilegedConfigKeys[] = {
kHostIdConfigPath, kXmppLoginConfigPath };

// Determines if the client runs in the security context that allows performing
// administrative tasks (i.e. the user belongs to the adminstrators group and
Expand Down Expand Up @@ -220,14 +220,16 @@ HRESULT WriteConfig(const char* content, size_t length, HWND owner_window) {
return E_FAIL;
}
std::string email;
if (!config_dict->GetString(kHostOwner, &email)) {
if (!config_dict->GetString(kXmppLogin, &email)) {
return E_FAIL;
if (!config_dict->GetString(kHostOwnerEmailConfigPath, &email)) {
if (!config_dict->GetString(kHostOwnerConfigPath, &email)) {
if (!config_dict->GetString(kXmppLoginConfigPath, &email)) {
return E_FAIL;
}
}
}
std::string host_id, host_secret_hash;
if (!config_dict->GetString(kHostId, &host_id) ||
!config_dict->GetString(kHostSecretHash, &host_secret_hash)) {
if (!config_dict->GetString(kHostIdConfigPath, &host_id) ||
!config_dict->GetString(kHostSecretHashConfigPath, &host_secret_hash)) {
return E_FAIL;
}

Expand Down
86 changes: 79 additions & 7 deletions remoting/webapp/host_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,11 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone,
* @param {string} privateKey
* @param {string} xmppLogin
* @param {string} refreshToken
* @param {string} clientBaseJid
* @param {string} hostSecretHash
*/
function startHostWithHash(hostName, publicKey, privateKey,
xmppLogin, refreshToken, hostSecretHash) {
function startHostWithHash(hostName, publicKey, privateKey, xmppLogin,
refreshToken, clientBaseJid, hostSecretHash) {
var hostConfig = {
xmpp_login: xmppLogin,
oauth_refresh_token: refreshToken,
Expand All @@ -183,9 +184,13 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone,
host_secret_hash: hostSecretHash,
private_key: privateKey
};
var hostOwner = remoting.identity.getCachedEmail();
var hostOwner = clientBaseJid;
var hostOwnerEmail = remoting.identity.getCachedEmail();
if (hostOwner != xmppLogin) {
hostConfig['host_owner'] = hostOwner;
if (hostOwnerEmail != hostOwner) {
hostConfig['host_owner_email'] = hostOwnerEmail;
}
}
that.hostDaemonFacade_.startDaemon(
hostConfig, consent, onStarted.bind(null, hostName, publicKey),
Expand All @@ -198,16 +203,32 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone,
* @param {string} privateKey
* @param {string} email
* @param {string} refreshToken
* @param {string} clientBaseJid
*/
function onServiceAccountCredentials(
hostName, publicKey, privateKey, email, refreshToken) {
function onClientBaseJid(
hostName, publicKey, privateKey, email, refreshToken, clientBaseJid) {
that.hostDaemonFacade_.getPinHash(
newHostId, hostPin,
startHostWithHash.bind(
null, hostName, publicKey, privateKey, email, refreshToken),
startHostWithHash.bind(null, hostName, publicKey, privateKey,
email, refreshToken, clientBaseJid),
onError);
}

/**
* @param {string} hostName
* @param {string} publicKey
* @param {string} privateKey
* @param {string} email
* @param {string} refreshToken
*/
function onServiceAccountCredentials(
hostName, publicKey, privateKey, email, refreshToken) {
that.getClientBaseJid_(
onClientBaseJid.bind(
null, hostName, publicKey, privateKey, email, refreshToken),
onStartError);
}

/**
* @param {string} hostName
* @param {string} publicKey
Expand Down Expand Up @@ -499,5 +520,56 @@ remoting.HostController.prototype.clearPairedClients = function(
this.hostDaemonFacade_.clearPairedClients(onDone, onError);
};

/**
* Gets the host owner's base JID, used by the host for client authorization.
* In most cases this is the same as the owner's email address, but for
* non-Gmail accounts, it may be different.
*
* @private
* @param {function(string): void} onSuccess
* @param {function(remoting.Error): void} onError
*/
remoting.HostController.prototype.getClientBaseJid_ = function(
onSuccess, onError) {
var signalStrategy = null;

var onState = function(state) {
switch (state) {
case remoting.SignalStrategy.State.CONNECTED:
var jid = signalStrategy.getJid().split('/')[0].toLowerCase();
base.dispose(signalStrategy);
signalStrategy = null;
onSuccess(jid);
break;

case remoting.SignalStrategy.State.FAILED:
var error = signalStrategy.getError();
base.dispose(signalStrategy);
signalStrategy = null;
onError(error);
break;
}
};

signalStrategy = remoting.SignalStrategy.create(onState);

/** @param {string} token */
function connectSignalingWithToken(token) {
remoting.identity.getEmail(
connectSignalingWithTokenAndEmail.bind(null, token), onError);
}

/**
* @param {string} token
* @param {string} email
*/
function connectSignalingWithTokenAndEmail(token, email) {
signalStrategy.connect(
remoting.settings.XMPP_SERVER_ADDRESS, email, token);
}

remoting.identity.callWithToken(connectSignalingWithToken, onError);
};

/** @type {remoting.HostController} */
remoting.hostController = null;
12 changes: 2 additions & 10 deletions remoting/webapp/session_connector_impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,16 +361,8 @@ remoting.SessionConnectorImpl.prototype.connectSignaling_ = function() {
remoting.settings.XMPP_SERVER_ADDRESS, email, token);
}

// Only use XMPP when TCP API is available and TLS support is enabled. That's
// not the case for V1 app (socket API is available only to platform apps)
// and for Chrome releases before 38.
if (chrome.socket && chrome.socket.secure) {
this.signalStrategy_ = /** @type {remoting.SignalStrategy} */
(new remoting.XmppConnection(this.onSignalingState_.bind(this)));
} else {
this.signalStrategy_ = /** @type {remoting.SignalStrategy} */
(new remoting.WcsAdapter(this.onSignalingState_.bind(this)));
}
this.signalStrategy_ =
remoting.SignalStrategy.create(this.onSignalingState_.bind(this));

remoting.identity.callWithToken(connectSignalingWithToken, this.onError_);
};
Expand Down
16 changes: 16 additions & 0 deletions remoting/webapp/signal_strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,19 @@ remoting.SignalStrategy.prototype.getError = function() {};

/** @return {string} Current JID when in CONNECTED state. */
remoting.SignalStrategy.prototype.getJid = function() {};

/**
* Creates the appropriate signal strategy for the current environment.
* @param {function(remoting.SignalStrategy.State): void} onStateChangedCallback
* @return {remoting.SignalStrategy} New signal strategy object.
*/
remoting.SignalStrategy.create = function(onStateChangedCallback) {
// Only use XMPP when TCP API is available and TLS support is enabled. That's
// not the case for V1 app (socket API is available only to platform apps)
// and for Chrome releases before 38.
if (chrome.socket && chrome.socket.secure) {
return new remoting.XmppConnection(onStateChangedCallback);
} else {
return new remoting.WcsAdapter(onStateChangedCallback);
}
};

0 comments on commit 665bf30

Please sign in to comment.