From 665bf309b93ea3c96c22ce75600beb40e2c083cb Mon Sep 17 00:00:00 2001 From: rmsousa Date: Mon, 29 Sep 2014 14:17:55 -0700 Subject: [PATCH] Save the client base JID for authentication in case it differs from the 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} --- remoting/host/chromoting_host.cc | 5 +- remoting/host/host_config.cc | 1 + remoting/host/host_config.h | 6 +- remoting/host/host_status_observer.h | 2 +- remoting/host/mac/me2me_preference_pane.mm | 16 ++-- remoting/host/remoting_me2me_host.cc | 28 ++++++- remoting/host/win/elevated_controller.cc | 24 +++--- remoting/webapp/host_controller.js | 86 ++++++++++++++++++++-- remoting/webapp/session_connector_impl.js | 12 +-- remoting/webapp/signal_strategy.js | 16 ++++ 10 files changed, 156 insertions(+), 40 deletions(-) diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc index 427ced6d8794b..346e887fc46c4 100644 --- a/remoting/host/chromoting_host.cc +++ b/remoting/host/chromoting_host.cc @@ -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); diff --git a/remoting/host/host_config.cc b/remoting/host/host_config.cc index 29a2720ca148e..e974e10592236 100644 --- a/remoting/host/host_config.cc +++ b/remoting/host/host_config.cc @@ -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"; diff --git a/remoting/host/host_config.h b/remoting/host/host_config.h index 19332b3064e0b..17e554fcd99ca 100644 --- a/remoting/host/host_config.h +++ b/remoting/host/host_config.h @@ -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[]; diff --git a/remoting/host/host_status_observer.h b/remoting/host/host_status_observer.h index 200b7d8d0f94b..eec5a5110a758 100644 --- a/remoting/host/host_status_observer.h +++ b/remoting/host/host_status_observer.h @@ -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() {} diff --git a/remoting/host/mac/me2me_preference_pane.mm b/remoting/host/mac/me2me_preference_pane.mm index 56cc235e847ee..d07fdb4da6fb0 100644 --- a/remoting/host/mac/me2me_preference_pane.mm +++ b/remoting/host/mac/me2me_preference_pane.mm @@ -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; + } } } } diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc index 837f848b6f6f7..d4eab21bbf892 100644 --- a/remoting/host/remoting_me2me_host.cc +++ b/remoting/host/remoting_me2me_host.cc @@ -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_; @@ -869,6 +870,13 @@ bool HostProcess::ApplyConfig(scoped_ptr 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; @@ -926,6 +934,17 @@ void HostProcess::OnPolicyUpdate(scoped_ptr 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."; @@ -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(); @@ -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(); } diff --git a/remoting/host/win/elevated_controller.cc b/remoting/host/win/elevated_controller.cc index 07b620e1160c5..2336df72ab7bd 100644 --- a/remoting/host/win/elevated_controller.cc +++ b/remoting/host/win/elevated_controller.cc @@ -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" @@ -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 @@ -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; } diff --git a/remoting/webapp/host_controller.js b/remoting/webapp/host_controller.js index 34278ac73b447..4a9d22139acb9 100644 --- a/remoting/webapp/host_controller.js +++ b/remoting/webapp/host_controller.js @@ -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, @@ -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), @@ -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 @@ -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; diff --git a/remoting/webapp/session_connector_impl.js b/remoting/webapp/session_connector_impl.js index 5557387e3fd91..c9ce6fd6a0399 100644 --- a/remoting/webapp/session_connector_impl.js +++ b/remoting/webapp/session_connector_impl.js @@ -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_); }; diff --git a/remoting/webapp/signal_strategy.js b/remoting/webapp/signal_strategy.js index b94acc35e3708..02c8acbd5f0cc 100644 --- a/remoting/webapp/signal_strategy.js +++ b/remoting/webapp/signal_strategy.js @@ -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); + } +};