update_engine: UM: Make DefaultPolicy::UpdateCheckAllowed stateful.
This adds an auxiliary state to DefaultPolicy, and makes
UpdateCheckAllowed use it for recording the last time an update check
was allowed. This is necessary for enforcing a minimum interval between
consecutive update checks, a necessary property in the unlikely case
that the main policy is badly screwed: with it, the update engine will
repeatedly check for updates, unnecessarily consuming local resources
and potentially DDoS-ing Omaha.
In order to track time, the DefaultPolicy now takes a ClockInterface
argument; for backward compatibility with existing unit testing code, we
allow this handle to be null, in which case time is not tracked and the
policy resorts to the previous default behavior (namely, updates are
always allowed). We do plug a clock when DefaultPolicy is used in the
UpdateManager production (backup policy) and fake (main policy)
implementations.
Note that the state is added as an external object, in order to work
around the constness of policy objects that's implied by the policy API
(const methods).
Finally, it should be noted that we use monotonic time in order to
ensure that the DefaultPolicy does not become an attack surface for
denying updates, or exhausting local resources and/or DoS-ing services.
BUG=chromium:394778
TEST=Unit tests.
Change-Id: I08628ea9b0067fa7abf6e457c55d4ffea276c463
Reviewed-on: https://chromium-review.googlesource.com/208732
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_engine.gyp b/update_engine.gyp
index 328ee69..3750673 100644
--- a/update_engine.gyp
+++ b/update_engine.gyp
@@ -179,6 +179,7 @@
'update_check_scheduler.cc',
'update_manager/boxed_value.cc',
'update_manager/chromeos_policy.cc',
+ 'update_manager/default_policy.cc',
'update_manager/evaluation_context.cc',
'update_manager/event_loop.cc',
'update_manager/policy.cc',
diff --git a/update_manager/default_policy.cc b/update_manager/default_policy.cc
new file mode 100644
index 0000000..2c75d13
--- /dev/null
+++ b/update_manager/default_policy.cc
@@ -0,0 +1,42 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/update_manager/default_policy.h"
+
+namespace {
+
+// A fixed minimum interval between consecutive allowed update checks. This
+// needs to be long enough to prevent busywork and/or DDoS attacks on Omaha, but
+// at the same time short enough to allow the machine to update itself
+// reasonably soon.
+const int kCheckIntervalInSeconds = 15 * 60;
+
+} // namespace
+
+namespace chromeos_update_manager {
+
+DefaultPolicy::DefaultPolicy(chromeos_update_engine::ClockInterface* clock)
+ : clock_(clock), aux_state_(new DefaultPolicyState()) {}
+
+EvalStatus DefaultPolicy::UpdateCheckAllowed(
+ EvaluationContext* ec, State* state, std::string* error,
+ UpdateCheckParams* result) const {
+ result->updates_enabled = true;
+ result->target_channel.clear();
+
+ // Ensure that the minimum interval is set. If there's no clock, this defaults
+ // to always allowing the update.
+ if (!aux_state_->IsLastCheckAllowedTimeSet() ||
+ ec->IsMonotonicTimeGreaterThan(
+ aux_state_->last_check_allowed_time() +
+ base::TimeDelta::FromSeconds(kCheckIntervalInSeconds))) {
+ if (clock_)
+ aux_state_->set_last_check_allowed_time(clock_->GetMonotonicTime());
+ return EvalStatus::kSucceeded;
+ }
+
+ return EvalStatus::kAskMeAgainLater;
+}
+
+} // namespace chromeos_update_manager
diff --git a/update_manager/default_policy.h b/update_manager/default_policy.h
index ba78724..2fb1a06 100644
--- a/update_manager/default_policy.h
+++ b/update_manager/default_policy.h
@@ -9,26 +9,52 @@
#include <base/time/time.h>
+#include "update_engine/clock_interface.h"
#include "update_engine/update_manager/policy.h"
namespace chromeos_update_manager {
+// Auxiliary state class for DefaultPolicy evaluations.
+//
+// IMPORTANT: The use of a state object in policies is generally forbidden, as
+// it was a design decision to keep policy calls side-effect free. We make an
+// exception here to ensure that DefaultPolicy indeed serves as a safe (and
+// secure) fallback option. This practice should be avoided when imlpementing
+// other policies.
+class DefaultPolicyState {
+ public:
+ DefaultPolicyState() {}
+
+ bool IsLastCheckAllowedTimeSet() const {
+ return last_check_allowed_time_ != base::Time::Max();
+ }
+
+ // Sets/returns the point time on the monotonic time scale when the latest
+ // check allowed was recorded.
+ void set_last_check_allowed_time(base::Time timestamp) {
+ last_check_allowed_time_ = timestamp;
+ }
+ base::Time last_check_allowed_time() const {
+ return last_check_allowed_time_;
+ }
+
+ private:
+ base::Time last_check_allowed_time_ = base::Time::Max();
+};
+
// The DefaultPolicy is a safe Policy implementation that doesn't fail. The
// values returned by this policy are safe default in case of failure of the
// actual policy being used by the UpdateManager.
class DefaultPolicy : public Policy {
public:
- DefaultPolicy() {}
+ explicit DefaultPolicy(chromeos_update_engine::ClockInterface* clock);
+ DefaultPolicy() : DefaultPolicy(nullptr) {}
virtual ~DefaultPolicy() {}
// Policy overrides.
virtual EvalStatus UpdateCheckAllowed(
EvaluationContext* ec, State* state, std::string* error,
- UpdateCheckParams* result) const override {
- result->updates_enabled = true;
- result->target_channel.clear();
- return EvalStatus::kSucceeded;
- }
+ UpdateCheckParams* result) const override;
virtual EvalStatus UpdateCanStart(
EvaluationContext* ec,
@@ -63,6 +89,12 @@
}
private:
+ // A clock interface.
+ chromeos_update_engine::ClockInterface* clock_;
+
+ // An auxiliary state object.
+ scoped_ptr<DefaultPolicyState> aux_state_;
+
DISALLOW_COPY_AND_ASSIGN(DefaultPolicy);
};
diff --git a/update_manager/fake_update_manager.h b/update_manager/fake_update_manager.h
index b246412..b509454 100644
--- a/update_manager/fake_update_manager.h
+++ b/update_manager/fake_update_manager.h
@@ -17,7 +17,7 @@
explicit FakeUpdateManager(chromeos_update_engine::ClockInterface* clock)
: UpdateManager(clock, base::TimeDelta::FromSeconds(5), new FakeState()) {
// The FakeUpdateManager uses a DefaultPolicy.
- set_policy(new DefaultPolicy());
+ set_policy(new DefaultPolicy(clock));
}
// UpdateManager overrides.
diff --git a/update_manager/update_manager.cc b/update_manager/update_manager.cc
index d6ffcee..fdedc29 100644
--- a/update_manager/update_manager.cc
+++ b/update_manager/update_manager.cc
@@ -11,7 +11,8 @@
UpdateManager::UpdateManager(chromeos_update_engine::ClockInterface* clock,
base::TimeDelta evaluation_timeout, State* state)
- : state_(state), clock_(clock), evaluation_timeout_(evaluation_timeout) {
+ : default_policy_(clock), state_(state), clock_(clock),
+ evaluation_timeout_(evaluation_timeout) {
// TODO(deymo): Make it possible to replace this policy with a different
// implementation with a build-time flag.
policy_.reset(new ChromeOSPolicy());