Skip to content

GitLab

T
tangled
Commit fecb18b7 authored by lukasza's avatar lukasza Committed by Commit bot
Browse files
Options

Suspend (rather than shut down) the host upon policy errors. 

This changelist primarily changes HostProcess::OnPolicyError, so that it
doesn't anymore shut down the host, but instead restarts the host and prevents
the host from starting again until a valid policy has been read (the
"preventing" functionality has already been present in the form of
StartHostIfReady method for some time now).

This changelist also simplifies HostSignalingManager's functionality related to
sending host-offline-reason.  Previously HostProcess was destroyed while
HostSignalingManager was trying to send host-offline-reason.  Things are
simpler if HostProcess waits until HostSignalingManager sends
host-offline-reason or times out.  This synchronization is especially helpful
when restarting the host (to avoid having to deal with an old
HostSignalingManager from previous SendHostOfflineReason attempt, while we
construct a new HostSignalingManager when starting a host after a reset).

This changelist also introduces a separate host-offline-reason for policy
errors (and updates WebApp resources strings to reflect that).  Note that since
we are not shutting down the host, we probably cannot use one of HostExitCodes
as the host-offline-reason.

Testing done:
1. On Linux manually introduce policy errors at startup, verify host doesn't
   start and sends host-offline-reason, fix errors, verify host starts,
   reintroduce errors, verify host suspends and sends host-offline-reason, fix
   errors, verify host starts again.
2. Verify WebApp UI shows correct tooltip text for the new host-offline-reason.

BUG=455903

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

Cr-Commit-Position: refs/heads/master@{#317708}
parent 1e259c47
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 219 additions and 167 deletions
remoting/host/heartbeat_sender.h
View file @ fecb18b7
......@@ -48,8 +48,9 @@ class IqSender;
// accept new connections from a client, but the rem:heartbeat xml element can
// optionally include a rem:host-offline-reason attribute, which indicates that
// the host cannot accept connections from the client (and might possibly be
// shutting down). The value of the host-offline-reason attribute can be a
// string from host_exit_codes.cc (i.e. "INVALID_HOST_CONFIGURATION" string).
// shutting down). The value of the host-offline-reason attribute can be either
// a string from host_exit_codes.cc (i.e. "INVALID_HOST_CONFIGURATION" string)
// or one of kHostOfflineReasonXxx constants (i.e. "POLICY_READ_ERROR" string).
//
// The sequence-id attribute of the heartbeat is a zero-based incrementally
// increasing integer unique to each heartbeat from a single host.
......
remoting/host/host_signaling_manager.cc
View file @ fecb18b7
......@@ -19,21 +19,16 @@
namespace remoting {
HostSignalingManager::HostSignalingManager(
scoped_ptr<base::WeakPtrFactory<Listener>> weak_factory_for_listener,
const scoped_refptr<AutoThreadTaskRunner>& network_task_runner,
scoped_ptr<SignalStrategy> signal_strategy,
scoped_ptr<SignalingConnector> signaling_connector,
scoped_ptr<HeartbeatSender> heartbeat_sender)
: weak_factory_for_listener_(weak_factory_for_listener.Pass()),
network_task_runner_(network_task_runner),
signal_strategy_(signal_strategy.Pass()),
: signal_strategy_(signal_strategy.Pass()),
signaling_connector_(signaling_connector.Pass()),
heartbeat_sender_(heartbeat_sender.Pass()) {
}
scoped_ptr<HostSignalingManager> HostSignalingManager::Create(
Listener* listener,
const scoped_refptr<AutoThreadTaskRunner>& network_task_runner,
const scoped_refptr<net::URLRequestContextGetter>&
url_request_context_getter,
const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
......@@ -42,11 +37,6 @@ scoped_ptr<HostSignalingManager> HostSignalingManager::Create(
const scoped_refptr<const RsaKeyPair>& host_key_pair,
const std::string& directory_bot_jid,
scoped_ptr<OAuthTokenGetter::OAuthCredentials> oauth_credentials) {
DCHECK(network_task_runner->BelongsToCurrentThread());
scoped_ptr<base::WeakPtrFactory<Listener>> weak_factory(
new base::WeakPtrFactory<Listener>(listener));
scoped_ptr<XmppSignalStrategy> signal_strategy(
new XmppSignalStrategy(net::ClientSocketFactory::GetDefaultFactory(),
url_request_context_getter, xmpp_server_config));
......@@ -56,7 +46,7 @@ scoped_ptr<HostSignalingManager> HostSignalingManager::Create(
scoped_ptr<SignalingConnector> signaling_connector(new SignalingConnector(
signal_strategy.get(), dns_blackhole_checker.Pass(),
base::Bind(&Listener::OnAuthFailed, weak_factory->GetWeakPtr())));
base::Bind(&Listener::OnAuthFailed, base::Unretained(listener))));
if (!oauth_credentials->refresh_token.empty()) {
scoped_ptr<OAuthTokenGetter> oauth_token_getter(new OAuthTokenGetter(
......@@ -66,45 +56,27 @@ scoped_ptr<HostSignalingManager> HostSignalingManager::Create(
}
scoped_ptr<HeartbeatSender> heartbeat_sender(new HeartbeatSender(
base::Bind(&Listener::OnHeartbeatSuccessful, weak_factory->GetWeakPtr()),
base::Bind(&Listener::OnUnknownHostIdError, weak_factory->GetWeakPtr()),
base::Bind(&Listener::OnHeartbeatSuccessful, base::Unretained(listener)),
base::Bind(&Listener::OnUnknownHostIdError, base::Unretained(listener)),
host_id, signal_strategy.get(), host_key_pair, directory_bot_jid));
return scoped_ptr<HostSignalingManager>(new HostSignalingManager(
weak_factory.Pass(), network_task_runner, signal_strategy.Pass(),
signaling_connector.Pass(), heartbeat_sender.Pass()));
signal_strategy.Pass(), signaling_connector.Pass(),
heartbeat_sender.Pass()));
}
HostSignalingManager::~HostSignalingManager() {
DCHECK(network_task_runner_->BelongsToCurrentThread());
DCHECK(thread_checker_.CalledOnValidThread());
}
void HostSignalingManager::SendHostOfflineReasonAndDelete(
void HostSignalingManager::SendHostOfflineReason(
const std::string& host_offline_reason,
const base::TimeDelta& timeout) {
DCHECK(network_task_runner_->BelongsToCurrentThread());
HOST_LOG << "SendHostOfflineReason: sending " << host_offline_reason;
// Ensure that any subsequent callbacks from the HeartbeatSender or
// SignalingConnector don't touch the Listener.
weak_factory_for_listener_->InvalidateWeakPtrs();
heartbeat_sender_->SetHostOfflineReason(
host_offline_reason, timeout,
base::Bind(&HostSignalingManager::OnHostOfflineReasonAck,
base::Unretained(this)));
}
void HostSignalingManager::OnHostOfflineReasonAck(bool success) {
DCHECK(network_task_runner_->BelongsToCurrentThread());
if (success) {
HOST_LOG << "SendHostOfflineReason: succeeded";
} else {
HOST_LOG << "SendHostOfflineReason: timed out";
}
delete this;
const base::TimeDelta& timeout,
const base::Callback<void(bool success)>& ack_callback) {
DCHECK(thread_checker_.CalledOnValidThread());
HOST_LOG << "SendHostOfflineReason: sending " << host_offline_reason << ".";
heartbeat_sender_->SetHostOfflineReason(host_offline_reason, timeout,
ack_callback);
}
} // namespace remoting
remoting/host/host_signaling_manager.h
View file @ fecb18b7
......@@ -9,8 +9,7 @@
#include "base/callback.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "remoting/base/auto_thread_task_runner.h"
#include "base/threading/thread_checker.h"
#include "remoting/base/rsa_key_pair.h"
#include "remoting/host/oauth_token_getter.h"
#include "remoting/signaling/xmpp_signal_strategy.h"
......@@ -19,22 +18,15 @@ namespace base {
class TimeDelta;
}
namespace net {
class NetworkChangeNotifier;
}
namespace remoting {
class ChromotingHostContext;
class DnsBlackholeChecker;
class HeartbeatSender;
class OAuthTokenGetter;
class SignalStrategy;
class SignalingConnector;
// HostSignalingManager has 2 functions:
// 1. Keep sending regular heartbeats to the Chromoting Directory.
// 2. Keep the host process alive while sending host-offline-reason heartbeat.
// HostSignalingManager manages objects needed for sending regular heartbeats to
// the Chromoting Directory.
class HostSignalingManager {
public:
class Listener {
......@@ -55,12 +47,10 @@ class HostSignalingManager {
// Probably necessitates refactoring HostProcess to extract a new
// class to read and store config/policy/cmdline values.
//
// |listener| has to be valid until either
// 1) the returned HostSignalingManager is destroyed
// or 2) SendHostOfflineReasonAndDelete is called.
// |listener| has to be valid until the returned HostSignalingManager is
// destroyed.
static scoped_ptr<HostSignalingManager> Create(
Listener* listener,
const scoped_refptr<AutoThreadTaskRunner>& network_task_runner,
const scoped_refptr<net::URLRequestContextGetter>&
url_request_context_getter,
const XmppSignalStrategy::XmppServerConfig& xmpp_server_config,
......@@ -77,37 +67,23 @@ class HostSignalingManager {
SignalStrategy* signal_strategy() { return signal_strategy_.get(); }
// Kicks off sending a heartbeat containing a host-offline-reason attribute.
// Prevents future calls to the |listener| provided to the Create method.
// Will call |ack_callback| once either the bot acks receiving the
// |host_offline_reason|, or the |timeout| is reached.
//
// Will delete |this| once either the bot acks receiving the
// |host_offline_reason|, or the |timeout| is reached. Deleting
// |this| will release |network_task_runner_| and allow the host
// process to exit.
void SendHostOfflineReasonAndDelete(const std::string& host_offline_reason,
const base::TimeDelta& timeout);
// For discussion of allowed values for |host_offline_reason| argument,
// please see the description of rem:host-offline-reason xml attribute in
// the class-level comments for HeartbeatReasonSender.
void SendHostOfflineReason(
const std::string& host_offline_reason,
const base::TimeDelta& timeout,
const base::Callback<void(bool success)>& ack_callback);
private:
HostSignalingManager(
scoped_ptr<base::WeakPtrFactory<Listener>> weak_factory_for_listener,
const scoped_refptr<AutoThreadTaskRunner>& network_task_runner,
scoped_ptr<SignalStrategy> signal_strategy,
scoped_ptr<SignalingConnector> signaling_connector,
scoped_ptr<HeartbeatSender> heartbeat_sender);
void OnHostOfflineReasonAck(bool success);
// Used to bind HeartbeatSender and SignalingConnector callbacks to |listener|
// in a way that allows "detaching" the |listener| when either |this| is
// destroyed or when SendHostOfflineReasonAndDelete method is called.
scoped_ptr<base::WeakPtrFactory<Listener>> weak_factory_for_listener_;
// By holding a reference to |network_task_runner_|, HostSignalingManager is
// extending the lifetime of the host process. This is needed for the case
// where an instance of HostProcess has already been destroyed, but we want
// to keep the process running while we try to establish a connection and send
// host-offline-reason.
scoped_refptr<AutoThreadTaskRunner> network_task_runner_;
// |heartbeat_sender_| and |signaling_connector_| have to be destroyed before
// |signal_strategy_| because their destructors need to call
// signal_strategy_->RemoveListener(this)
......@@ -115,6 +91,9 @@ class HostSignalingManager {
scoped_ptr<SignalingConnector> signaling_connector_;
scoped_ptr<HeartbeatSender> heartbeat_sender_;
// Used to verify thread-safe usage.
base::ThreadChecker thread_checker_;
DISALLOW_COPY_AND_ASSIGN(HostSignalingManager);
};
......
remoting/host/policy_watcher.cc
View file @ fecb18b7
......@@ -145,6 +145,7 @@ void PolicyWatcher::UpdatePolicies(
}
void PolicyWatcher::SignalPolicyError() {
old_policies_->Clear();
policy_error_callback_.Run();
}
......
remoting/host/remoting_me2me_host.cc
View file @ fecb18b7
This diff is collapsed.
remoting/resources/remoting_strings.grd
View file @ fecb18b7
......@@ -886,6 +886,18 @@ For information about privacy, please see the Google Privacy Policy (http://goo.
name="IDS_OFFLINE_REASON_LOGIN_SCREEN_NOT_SUPPORTED">
Host running at the console logic screen has shutdown to support curtain mode by switching to a host running in a user-specific session.
</message>
<message desc="Error message indicating that the host is offline, because the host policy was unreadable (unparseable, inaccessible, mistyped or malformed)."
name="IDS_OFFLINE_REASON_POLICY_READ_ERROR">
Host failed to read the policy.
</message>
<message desc="Error message indicating that the host is temporarily offline, because the host is restarting due to a policy change."
name="IDS_OFFLINE_REASON_POLICY_CHANGE_REQUIRES_RESTART">
Host is restarting, to take into account a policy change.
</message>
<message desc="Error message indicating that the host is offline, because another process has requested the host to shut down (via a SIGTERM signal)."
name="IDS_OFFLINE_REASON_SUCCESS_EXIT">
Host has shut down.
</message>
<message desc="Error message indicating that the host is offline, because RemoteAccessHostMatchUsername policy is enabled and there is a mismatch between the name of the local user (that the host is associated with) and the name of the Google account registered as the host owner (i.e. 'johndoe' if the host is owned by 'johndoe@example.com' Google account)"
name="IDS_OFFLINE_REASON_USERNAME_MISMATCH">
Invalid host owner.
......
remoting/webapp/crd/js/host_table_entry.js
View file @ fecb18b7
......@@ -344,10 +344,13 @@ function formatHostOfflineReason(hostOfflineReason) {
var knownReasonTags = [
/*i18n-content*/ 'OFFLINE_REASON_INITIALIZATION_FAILED',
/*i18n-content*/ 'OFFLINE_REASON_INVALID_HOST_CONFIGURATION',
/*i18n-content*/ 'OFFLINE_REASON_INVALID_HOST_DOMAIN',
/*i18n-content*/ 'OFFLINE_REASON_INVALID_HOST_ID',
/*i18n-content*/ 'OFFLINE_REASON_INVALID_OAUTH_CREDENTIALS',
/*i18n-content*/ 'OFFLINE_REASON_INVALID_HOST_DOMAIN',
/*i18n-content*/ 'OFFLINE_REASON_LOGIN_SCREEN_NOT_SUPPORTED',
/*i18n-content*/ 'OFFLINE_REASON_POLICY_CHANGE_REQUIRES_RESTART',
/*i18n-content*/ 'OFFLINE_REASON_POLICY_READ_ERROR',
/*i18n-content*/ 'OFFLINE_REASON_SUCCESS_EXIT',
/*i18n-content*/ 'OFFLINE_REASON_USERNAME_MISMATCH'
];
var offlineReasonTag = 'OFFLINE_REASON_' + hostOfflineReason;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment