summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsergeyu@chromium.org <sergeyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-05-12 02:02:16 +0000
committersergeyu@chromium.org <sergeyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-05-12 02:02:16 +0000
commiteff1b5d80202924c20878c1c0bd17bfd8f5fd8f1 (patch)
treef6aaa6d323db874751d18aa7e937fc54bab89bbd
parent70adc1373596185e685facda48c605cb6987e923 (diff)
downloadchromium_src-eff1b5d80202924c20878c1c0bd17bfd8f5fd8f1.zip
chromium_src-eff1b5d80202924c20878c1c0bd17bfd8f5fd8f1.tar.gz
chromium_src-eff1b5d80202924c20878c1c0bd17bfd8f5fd8f1.tar.bz2
Verify that xmpp_login specified in the config matches auth token.
Previously the host would ignore the case when the oauth token doesn't match the xmpp_login specified in the config. That would lead to situations when host is connected with one account, but verifies incoming connections with a different account. Fix this by verifying that the JID that we receive from XMPP server matches the value in the config. Review URL: https://chromiumcodereview.appspot.com/10378110 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136745 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--remoting/host/remoting_me2me_host.cc8
-rw-r--r--remoting/host/signaling_connector.cc19
-rw-r--r--remoting/host/signaling_connector.h11
-rw-r--r--remoting/host/simple_host_process.cc43
-rw-r--r--remoting/jingle_glue/fake_signal_strategy.cc4
-rw-r--r--remoting/jingle_glue/fake_signal_strategy.h1
-rw-r--r--remoting/jingle_glue/javascript_signal_strategy.cc9
-rw-r--r--remoting/jingle_glue/javascript_signal_strategy.h1
-rw-r--r--remoting/jingle_glue/mock_objects.h1
-rw-r--r--remoting/jingle_glue/signal_strategy.h13
-rw-r--r--remoting/jingle_glue/xmpp_signal_strategy.cc46
-rw-r--r--remoting/jingle_glue/xmpp_signal_strategy.h4
12 files changed, 137 insertions, 23 deletions
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index bb7a7ea..195d91c 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -335,8 +335,9 @@ class HostProcess
new XmppSignalStrategy(context_->jingle_thread(), xmpp_login_,
xmpp_auth_token_, xmpp_auth_service_));
- signaling_connector_.reset(
- new SignalingConnector(signal_strategy_.get()));
+ signaling_connector_.reset(new SignalingConnector(
+ signal_strategy_.get(),
+ base::Bind(&HostProcess::OnAuthFailed, base::Unretained(this))));
if (!oauth_refresh_token_.empty()) {
OAuthClientInfo client_info = {
@@ -360,7 +361,6 @@ class HostProcess
xmpp_login_, oauth_refresh_token_, client_info));
signaling_connector_->EnableOAuth(
oauth_credentials.Pass(),
- base::Bind(&HostProcess::OnOAuthFailed, base::Unretained(this)),
context_->url_request_context_getter());
}
}
@@ -401,7 +401,7 @@ class HostProcess
CreateAuthenticatorFactory();
}
- void OnOAuthFailed() {
+ void OnAuthFailed() {
Shutdown(kInvalidOauthCredentialsExitCode);
}
diff --git a/remoting/host/signaling_connector.cc b/remoting/host/signaling_connector.cc
index cba384a..072eced 100644
--- a/remoting/host/signaling_connector.cc
+++ b/remoting/host/signaling_connector.cc
@@ -31,10 +31,14 @@ SignalingConnector::OAuthCredentials::OAuthCredentials(
client_info(client_info_value) {
}
-SignalingConnector::SignalingConnector(XmppSignalStrategy* signal_strategy)
+SignalingConnector::SignalingConnector(
+ XmppSignalStrategy* signal_strategy,
+ const base::Closure& auth_failed_callback)
: signal_strategy_(signal_strategy),
+ auth_failed_callback_(auth_failed_callback),
reconnect_attempts_(0),
refreshing_oauth_token_(false) {
+ DCHECK(!auth_failed_callback_.is_null());
net::NetworkChangeNotifier::AddOnlineStateObserver(this);
net::NetworkChangeNotifier::AddIPAddressObserver(this);
signal_strategy_->AddListener(this);
@@ -49,10 +53,8 @@ SignalingConnector::~SignalingConnector() {
void SignalingConnector::EnableOAuth(
scoped_ptr<OAuthCredentials> oauth_credentials,
- const base::Closure& oauth_failed_callback,
net::URLRequestContextGetter* url_context) {
oauth_credentials_ = oauth_credentials.Pass();
- oauth_failed_callback_ = oauth_failed_callback;
gaia_oauth_client_.reset(new GaiaOAuthClient(kGaiaOAuth2Url, url_context));
}
@@ -66,7 +68,14 @@ void SignalingConnector::OnSignalStrategyStateChange(
} else if (state == SignalStrategy::DISCONNECTED) {
LOG(INFO) << "Signaling disconnected.";
reconnect_attempts_++;
- ScheduleTryReconnect();
+
+ // If authentication failed then we have an invalid OAuth token,
+ // inform the upper layer about it.
+ if (signal_strategy_->GetError() == SignalStrategy::AUTHENTICATION_FAILED) {
+ auth_failed_callback_.Run();
+ } else {
+ ScheduleTryReconnect();
+ }
}
}
@@ -106,7 +115,7 @@ void SignalingConnector::OnOAuthError() {
LOG(ERROR) << "OAuth: invalid credentials.";
refreshing_oauth_token_ = false;
reconnect_attempts_++;
- oauth_failed_callback_.Run();
+ auth_failed_callback_.Run();
}
void SignalingConnector::OnNetworkError(int response_code) {
diff --git a/remoting/host/signaling_connector.h b/remoting/host/signaling_connector.h
index f73f744..a03ae64 100644
--- a/remoting/host/signaling_connector.h
+++ b/remoting/host/signaling_connector.h
@@ -50,13 +50,14 @@ class SignalingConnector
OAuthClientInfo client_info;
};
- // OAuth token is updated refreshed when |oauth_credentials| is
- // not NULL.
- SignalingConnector(XmppSignalStrategy* signal_strategy);
+ // The |auth_failed_callback| is called when authentication fails.
+ SignalingConnector(XmppSignalStrategy* signal_strategy,
+ const base::Closure& auth_failed_callback);
virtual ~SignalingConnector();
+ // May be called immediately after the constructor to enable OAuth
+ // access token updating.
void EnableOAuth(scoped_ptr<OAuthCredentials> oauth_credentials,
- const base::Closure& oauth_failed_callback,
net::URLRequestContextGetter* url_context);
// SignalStrategy::Listener interface.
@@ -83,9 +84,9 @@ class SignalingConnector
void RefreshOAuthToken();
XmppSignalStrategy* signal_strategy_;
+ base::Closure auth_failed_callback_;
scoped_ptr<OAuthCredentials> oauth_credentials_;
- base::Closure oauth_failed_callback_;
scoped_ptr<GaiaOAuthClient> gaia_oauth_client_;
// Number of times we tried to connect without success.
diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc
index 2548741..fd78413 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -35,6 +35,7 @@
#include "remoting/host/capturer_fake.h"
#include "remoting/host/chromoting_host.h"
#include "remoting/host/chromoting_host_context.h"
+#include "remoting/host/constants.h"
#include "remoting/host/desktop_environment.h"
#include "remoting/host/event_executor.h"
#include "remoting/host/heartbeat_sender.h"
@@ -94,7 +95,9 @@ class SimpleHost : public HeartbeatSender::Listener {
: message_loop_(MessageLoop::TYPE_UI),
context_(message_loop_.message_loop_proxy()),
fake_(false),
- is_it2me_(false) {
+ is_it2me_(false),
+ shutting_down_(false),
+ exit_code_(kSuccessExitCode) {
context_.Start();
network_change_notifier_.reset(net::NetworkChangeNotifier::Create());
}
@@ -102,7 +105,7 @@ class SimpleHost : public HeartbeatSender::Listener {
// Overridden from HeartbeatSender::Listener
virtual void OnUnknownHostIdError() OVERRIDE {
LOG(ERROR) << "Host ID not found.";
- Shutdown();
+ Shutdown(kInvalidHostIdExitCode);
}
int Run() {
@@ -153,7 +156,7 @@ class SimpleHost : public HeartbeatSender::Listener {
message_loop_.MessageLoop::Run();
- return 0;
+ return exit_code_;
}
void set_config_path(const FilePath& config_path) {
@@ -214,7 +217,9 @@ class SimpleHost : public HeartbeatSender::Listener {
signal_strategy_.reset(
new XmppSignalStrategy(context_.jingle_thread(), xmpp_login_,
xmpp_auth_token_, xmpp_auth_service_));
- signaling_connector_.reset(new SignalingConnector(signal_strategy_.get()));
+ signaling_connector_.reset(new SignalingConnector(
+ signal_strategy_.get(),
+ base::Bind(&SimpleHost::OnAuthFailed, base::Unretained(this))));
if (fake_) {
scoped_ptr<Capturer> capturer(new CapturerFake());
@@ -266,7 +271,32 @@ class SimpleHost : public HeartbeatSender::Listener {
}
}
- void Shutdown() {
+ void OnAuthFailed() {
+ Shutdown(kInvalidOauthCredentialsExitCode);
+ }
+
+ void Shutdown(int exit_code) {
+ DCHECK(context_.network_message_loop()->BelongsToCurrentThread());
+
+ if (shutting_down_)
+ return;
+
+ shutting_down_ = true;
+ exit_code_ = exit_code;
+ host_->Shutdown(base::Bind(
+ &SimpleHost::OnShutdownFinished, base::Unretained(this)));
+ }
+
+ void OnShutdownFinished() {
+ DCHECK(context_.network_message_loop()->BelongsToCurrentThread());
+
+ // Destroy networking objects while we are on the network thread.
+ host_ = NULL;
+ log_to_server_.reset();
+ heartbeat_sender_.reset();
+ signaling_connector_.reset();
+ signal_strategy_.reset();
+
message_loop_.PostTask(FROM_HERE, MessageLoop::QuitClosure());
}
@@ -296,6 +326,9 @@ class SimpleHost : public HeartbeatSender::Listener {
scoped_ptr<HeartbeatSender> heartbeat_sender_;
scoped_refptr<ChromotingHost> host_;
+
+ bool shutting_down_;
+ int exit_code_;
};
} // namespace remoting
diff --git a/remoting/jingle_glue/fake_signal_strategy.cc b/remoting/jingle_glue/fake_signal_strategy.cc
index a854502..1ddabda 100644
--- a/remoting/jingle_glue/fake_signal_strategy.cc
+++ b/remoting/jingle_glue/fake_signal_strategy.cc
@@ -52,6 +52,10 @@ SignalStrategy::State FakeSignalStrategy::GetState() const {
return CONNECTED;
}
+SignalStrategy::Error FakeSignalStrategy::GetError() const {
+ return OK;
+}
+
std::string FakeSignalStrategy::GetLocalJid() const {
DCHECK(CalledOnValidThread());
return jid_;
diff --git a/remoting/jingle_glue/fake_signal_strategy.h b/remoting/jingle_glue/fake_signal_strategy.h
index f7f8db2..9dc25cf 100644
--- a/remoting/jingle_glue/fake_signal_strategy.h
+++ b/remoting/jingle_glue/fake_signal_strategy.h
@@ -33,6 +33,7 @@ class FakeSignalStrategy : public SignalStrategy,
virtual void Connect() OVERRIDE;
virtual void Disconnect() OVERRIDE;
virtual State GetState() const OVERRIDE;
+ virtual Error GetError() const OVERRIDE;
virtual std::string GetLocalJid() const OVERRIDE;
virtual void AddListener(Listener* listener) OVERRIDE;
virtual void RemoveListener(Listener* listener) OVERRIDE;
diff --git a/remoting/jingle_glue/javascript_signal_strategy.cc b/remoting/jingle_glue/javascript_signal_strategy.cc
index 21cf3cc..6703c52 100644
--- a/remoting/jingle_glue/javascript_signal_strategy.cc
+++ b/remoting/jingle_glue/javascript_signal_strategy.cc
@@ -47,12 +47,21 @@ void JavascriptSignalStrategy::Disconnect() {
}
SignalStrategy::State JavascriptSignalStrategy::GetState() const {
+ DCHECK(CalledOnValidThread());
// TODO(sergeyu): Extend XmppProxy to provide status of the
// connection.
return CONNECTED;
}
+SignalStrategy::Error JavascriptSignalStrategy::GetError() const {
+ DCHECK(CalledOnValidThread());
+ // TODO(sergeyu): Extend XmppProxy to provide status of the
+ // connection.
+ return OK;
+}
+
std::string JavascriptSignalStrategy::GetLocalJid() const {
+ DCHECK(CalledOnValidThread());
return local_jid_;
}
diff --git a/remoting/jingle_glue/javascript_signal_strategy.h b/remoting/jingle_glue/javascript_signal_strategy.h
index a229f52..eaab7a2 100644
--- a/remoting/jingle_glue/javascript_signal_strategy.h
+++ b/remoting/jingle_glue/javascript_signal_strategy.h
@@ -33,6 +33,7 @@ class JavascriptSignalStrategy : public SignalStrategy,
virtual void Connect() OVERRIDE;
virtual void Disconnect() OVERRIDE;
virtual State GetState() const OVERRIDE;
+ virtual Error GetError() const OVERRIDE;
virtual std::string GetLocalJid() const OVERRIDE;
virtual void AddListener(Listener* listener) OVERRIDE;
virtual void RemoveListener(Listener* listener) OVERRIDE;
diff --git a/remoting/jingle_glue/mock_objects.h b/remoting/jingle_glue/mock_objects.h
index 01d7117..1804628 100644
--- a/remoting/jingle_glue/mock_objects.h
+++ b/remoting/jingle_glue/mock_objects.h
@@ -18,6 +18,7 @@ class MockSignalStrategy : public SignalStrategy {
MOCK_METHOD0(Connect, void());
MOCK_METHOD0(Disconnect, void());
MOCK_CONST_METHOD0(GetState, State());
+ MOCK_CONST_METHOD0(GetError, Error());
MOCK_CONST_METHOD0(GetLocalJid, std::string());
MOCK_METHOD1(AddListener, void(Listener* listener));
MOCK_METHOD1(RemoveListener, void(Listener* listener));
diff --git a/remoting/jingle_glue/signal_strategy.h b/remoting/jingle_glue/signal_strategy.h
index 8971267..0651272 100644
--- a/remoting/jingle_glue/signal_strategy.h
+++ b/remoting/jingle_glue/signal_strategy.h
@@ -30,6 +30,12 @@ class SignalStrategy {
DISCONNECTED,
};
+ enum Error {
+ OK,
+ AUTHENTICATION_FAILED,
+ NETWORK_ERROR,
+ };
+
// Callback interface for signaling event. Event handlers are not
// allowed to destroy SignalStrategy, but may add or remove other
// listeners.
@@ -37,7 +43,9 @@ class SignalStrategy {
public:
virtual ~Listener() {}
- // Called after state of the connection has changed.
+ // Called after state of the connection has changed. If the state
+ // is DISCONNECTED, then GetError() can be used to get the reason
+ // for the disconnection.
virtual void OnSignalStrategyStateChange(State state) {}
// Must return true if the stanza was handled, false
@@ -63,6 +71,9 @@ class SignalStrategy {
// Returns current state.
virtual State GetState() const = 0;
+ // Returns the last error. Set when state changes to DISCONNECT.
+ virtual Error GetError() const = 0;
+
// Returns local JID or an empty string when not connected.
virtual std::string GetLocalJid() const = 0;
diff --git a/remoting/jingle_glue/xmpp_signal_strategy.cc b/remoting/jingle_glue/xmpp_signal_strategy.cc
index 1aa7d9c..6ae4f1a 100644
--- a/remoting/jingle_glue/xmpp_signal_strategy.cc
+++ b/remoting/jingle_glue/xmpp_signal_strategy.cc
@@ -4,7 +4,9 @@
#include "remoting/jingle_glue/xmpp_signal_strategy.h"
+#include "base/bind.h"
#include "base/logging.h"
+#include "base/string_util.h"
#include "jingle/notifier/base/gaia_token_pre_xmpp_auth.h"
#include "remoting/jingle_glue/jingle_thread.h"
#include "remoting/jingle_glue/xmpp_socket_adapter.h"
@@ -20,6 +22,10 @@ const char kDefaultResourceName[] = "chromoting";
// connections that are idle for more than a minute.
const int kKeepAliveIntervalSeconds = 50;
+void DisconnectXmppClient(buzz::XmppClient* client) {
+ client->Disconnect();
+}
+
} // namespace
namespace remoting {
@@ -34,11 +40,11 @@ XmppSignalStrategy::XmppSignalStrategy(JingleThread* jingle_thread,
auth_token_service_(auth_token_service),
resource_name_(kDefaultResourceName),
xmpp_client_(NULL),
- state_(DISCONNECTED) {
+ state_(DISCONNECTED),
+ error_(OK) {
}
XmppSignalStrategy::~XmppSignalStrategy() {
- DCHECK_EQ(listeners_.size(), 0U);
Disconnect();
}
@@ -89,6 +95,11 @@ SignalStrategy::State XmppSignalStrategy::GetState() const {
return state_;
}
+SignalStrategy::Error XmppSignalStrategy::GetError() const {
+ DCHECK(CalledOnValidThread());
+ return error_;
+}
+
std::string XmppSignalStrategy::GetLocalJid() const {
DCHECK(CalledOnValidThread());
return xmpp_client_->jid().Str();
@@ -154,7 +165,26 @@ void XmppSignalStrategy::SetResourceName(const std::string &resource_name) {
void XmppSignalStrategy::OnConnectionStateChanged(
buzz::XmppEngine::State state) {
DCHECK(CalledOnValidThread());
+
if (state == buzz::XmppEngine::STATE_OPEN) {
+ // Verify that the JID that we've received matches the username
+ // that we have. If it doesn't, then the OAuth token was probably
+ // issued for a different account, so we treat is a an auth error.
+ //
+ // TODO(sergeyu): Some user accounts may not have associated
+ // e-mail address. The check below will fail for such
+ // accounts. Make sure we can handle this case proprely.
+ if (!StartsWithASCII(GetLocalJid(), username_, false)) {
+ LOG(ERROR) << "Received JID that is different from the expected value.";
+ error_ = AUTHENTICATION_FAILED;
+ xmpp_client_->SignalStateChange.disconnect(this);
+ MessageLoop::current()->PostTask(
+ FROM_HERE, base::Bind(&DisconnectXmppClient, xmpp_client_));
+ xmpp_client_ = NULL;
+ SetState(DISCONNECTED);
+ return;
+ }
+
keep_alive_timer_.Start(
FROM_HERE, base::TimeDelta::FromSeconds(kKeepAliveIntervalSeconds),
this, &XmppSignalStrategy::SendKeepAlive);
@@ -171,6 +201,18 @@ void XmppSignalStrategy::OnConnectionStateChanged(
// Client is destroyed by the TaskRunner after the client is
// closed. Reset the pointer so we don't try to use it later.
xmpp_client_ = NULL;
+
+ switch (error) {
+ case buzz::XmppEngine::ERROR_UNAUTHORIZED:
+ case buzz::XmppEngine::ERROR_AUTH:
+ case buzz::XmppEngine::ERROR_MISSING_USERNAME:
+ error_ = AUTHENTICATION_FAILED;
+ break;
+
+ default:
+ error_ = NETWORK_ERROR;
+ }
+
SetState(DISCONNECTED);
}
}
diff --git a/remoting/jingle_glue/xmpp_signal_strategy.h b/remoting/jingle_glue/xmpp_signal_strategy.h
index ec405a4..790b0bd 100644
--- a/remoting/jingle_glue/xmpp_signal_strategy.h
+++ b/remoting/jingle_glue/xmpp_signal_strategy.h
@@ -40,6 +40,7 @@ class XmppSignalStrategy : public base::NonThreadSafe,
virtual void Connect() OVERRIDE;
virtual void Disconnect() OVERRIDE;
virtual State GetState() const OVERRIDE;
+ virtual Error GetError() const OVERRIDE;
virtual std::string GetLocalJid() const OVERRIDE;
virtual void AddListener(Listener* listener) OVERRIDE;
virtual void RemoveListener(Listener* listener) OVERRIDE;
@@ -78,8 +79,9 @@ class XmppSignalStrategy : public base::NonThreadSafe,
buzz::XmppClient* xmpp_client_;
State state_;
+ Error error_;
- ObserverList<Listener> listeners_;
+ ObserverList<Listener, true> listeners_;
base::RepeatingTimer<XmppSignalStrategy> keep_alive_timer_;