summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGilad Arnold <garnold@chromium.org>2014-07-17 11:40:43 -0700
committerchrome-internal-fetch <chrome-internal-fetch@google.com>2014-07-25 03:41:09 +0000
commita23e408368ad34e21ee90ebd0dcb55cd03417d22 (patch)
treeb2df3a675e9e81163eb67dd6979c2531d61b6781
parenta65fced5f4c2b551616b26ee90a800b44090735f (diff)
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>
-rw-r--r--update_engine.gyp1
-rw-r--r--update_manager/default_policy.cc42
-rw-r--r--update_manager/default_policy.h44
-rw-r--r--update_manager/fake_update_manager.h2
-rw-r--r--update_manager/update_manager.cc3
5 files changed, 84 insertions, 8 deletions
diff --git a/update_engine.gyp b/update_engine.gyp
index 328ee692..3750673d 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 00000000..2c75d134
--- /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 ba787247..2fb1a06b 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 @@ class DefaultPolicy : public Policy {
}
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 b246412f..b509454e 100644
--- a/update_manager/fake_update_manager.h
+++ b/update_manager/fake_update_manager.h
@@ -17,7 +17,7 @@ class FakeUpdateManager : public UpdateManager {
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 d6ffcee5..fdedc29f 100644
--- a/update_manager/update_manager.cc
+++ b/update_manager/update_manager.cc
@@ -11,7 +11,8 @@ namespace chromeos_update_manager {
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());