update_engine: UM: Add P2P download limits to UpdateCanStart.
This adds two factors that are currently used for blocking P2P into the
Chrome OS policy implementation: a maximum number of P2P download
attempts and a maximum time period since the first attempt.
BUG=chromium:420732
TEST=Unit tests.
Change-Id: I430dec50cf07f37a0c3f14de3410d3c6bfb8ac78
Reviewed-on: https://chromium-review.googlesource.com/221735
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index 291a147..19f9dc8 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -156,6 +156,8 @@
const int ChromeOSPolicy::kTimeoutRegularFuzz = 10 * 60;
const int ChromeOSPolicy::kAttemptBackoffMaxIntervalInDays = 16;
const int ChromeOSPolicy::kAttemptBackoffFuzzInHours = 12;
+const int ChromeOSPolicy::kMaxP2PAttempts = 10;
+const int ChromeOSPolicy::kMaxP2PAttemptsPeriodInSeconds = 5 * 24 * 60 * 60;
EvalStatus ChromeOSPolicy::UpdateCheckAllowed(
EvaluationContext* ec, State* state, string* error,
@@ -315,6 +317,22 @@
return backoff_url_status;
}
+ // Check whether P2P has reached its number of attempts and/or time limits. In
+ // which case, set a flag preventing us from enabling P2P later on.
+ bool is_p2p_blocked = false;
+ if (update_state.p2p_num_attempts >= kMaxP2PAttempts) {
+ is_p2p_blocked = true;
+ LOG(INFO) << "Blocking P2P as it's been attempted too many times.";
+ } else if (!update_state.p2p_first_attempted.is_null()) {
+ Time p2p_expiry = (
+ update_state.p2p_first_attempted +
+ TimeDelta::FromSeconds(kMaxP2PAttemptsPeriodInSeconds));
+ if (ec->IsWallclockTimeGreaterThan(p2p_expiry)) {
+ is_p2p_blocked = true;
+ LOG(INFO) << "Blocking P2P as its usage timespan exceeds the limit.";
+ }
+ }
+
DevicePolicyProvider* const dp_provider = state->device_policy_provider();
const bool* device_policy_is_loaded_p = ec->GetValue(
@@ -363,20 +381,22 @@
// Determine whether use of P2P is allowed by policy. Even if P2P is not
// explicitly allowed, we allow it if the device is enterprise enrolled
// (that is, missing or empty owner string).
- const bool* policy_au_p2p_enabled_p = ec->GetValue(
- dp_provider->var_au_p2p_enabled());
- if (policy_au_p2p_enabled_p) {
- result->p2p_allowed = *policy_au_p2p_enabled_p;
- } else {
- const string* policy_owner_p = ec->GetValue(dp_provider->var_owner());
- if (!policy_owner_p || policy_owner_p->empty())
- result->p2p_allowed = true;
+ if (!is_p2p_blocked) {
+ const bool* policy_au_p2p_enabled_p = ec->GetValue(
+ dp_provider->var_au_p2p_enabled());
+ if (policy_au_p2p_enabled_p) {
+ result->p2p_allowed = *policy_au_p2p_enabled_p;
+ } else {
+ const string* policy_owner_p = ec->GetValue(dp_provider->var_owner());
+ if (!policy_owner_p || policy_owner_p->empty())
+ result->p2p_allowed = true;
+ }
}
}
// Enable P2P, if so mandated by the updater configuration. This is additive
// to whether or not P2P is allowed per device policy (see above).
- if (!result->p2p_allowed) {
+ if (!(is_p2p_blocked || result->p2p_allowed)) {
const bool* updater_p2p_enabled_p = ec->GetValue(
state->updater_provider()->var_p2p_enabled());
result->p2p_allowed = updater_p2p_enabled_p && *updater_p2p_enabled_p;
diff --git a/update_manager/chromeos_policy.h b/update_manager/chromeos_policy.h
index 17f2c29..e3e9c5f 100644
--- a/update_manager/chromeos_policy.h
+++ b/update_manager/chromeos_policy.h
@@ -85,6 +85,10 @@
FRIEND_TEST(UmChromeOSPolicyTest, UpdateCanStartAllowedScatteringSatisfied);
FRIEND_TEST(UmChromeOSPolicyTest,
UpdateCanStartAllowedInteractivePreventsScattering);
+ FRIEND_TEST(UmChromeOSPolicyTest,
+ UpdateCanStartAllowedP2PBlockedDueToNumAttempts);
+ FRIEND_TEST(UmChromeOSPolicyTest,
+ UpdateCanStartAllowedP2PBlockedDueToAttemptsPeriod);
// Auxiliary constant (zero by default).
const base::TimeDelta kZeroInterval;
@@ -101,6 +105,11 @@
static const int kAttemptBackoffMaxIntervalInDays;
static const int kAttemptBackoffFuzzInHours;
+ // Maximum number of times we'll allow using P2P for the same update payload.
+ static const int kMaxP2PAttempts;
+ // Maximum period of time allowed for download a payload via P2P, in seconds.
+ static const int kMaxP2PAttemptsPeriodInSeconds;
+
// A private policy implementation returning the wallclock timestamp when
// the next update check should happen.
// TODO(garnold) We should probably change that to infer a monotonic
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index ae741e7..10e6324 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -143,6 +143,9 @@
update_state.last_download_url_num_errors = 0;
// There were no download errors.
update_state.download_errors = vector<tuple<int, ErrorCode, Time>>();
+ // P2P was not attempted.
+ update_state.p2p_num_attempts = 0;
+ update_state.p2p_first_attempted = Time();
// No active backoff period, backoff is not disabled by Omaha.
update_state.backoff_expiry = Time();
update_state.is_backoff_disabled = false;
@@ -975,6 +978,55 @@
EXPECT_FALSE(result.do_increment_failures);
}
+TEST_F(UmChromeOSPolicyTest, UpdateCanStartAllowedP2PBlockedDueToNumAttempts) {
+ // The UpdateCanStart policy returns true; device policy permits HTTP but
+ // blocks P2P, because the max number of P2P downloads have been attempted.
+
+ SetUpdateCheckAllowed(false);
+
+ // Override specific device policy attributes.
+ fake_state_.device_policy_provider()->var_http_downloads_enabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()->var_au_p2p_enabled()->reset(
+ new bool(true));
+
+ // Check that the UpdateCanStart returns true.
+ UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
+ update_state.p2p_num_attempts = ChromeOSPolicy::kMaxP2PAttempts;
+ UpdateDownloadParams result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
+ update_state);
+ EXPECT_TRUE(result.update_can_start);
+ EXPECT_FALSE(result.p2p_allowed);
+}
+
+TEST_F(UmChromeOSPolicyTest,
+ UpdateCanStartAllowedP2PBlockedDueToAttemptsPeriod) {
+ // The UpdateCanStart policy returns true; device policy permits HTTP but
+ // blocks P2P, because the max period for attempt to download via P2P has
+ // elapsed.
+
+ SetUpdateCheckAllowed(false);
+
+ // Override specific device policy attributes.
+ fake_state_.device_policy_provider()->var_http_downloads_enabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()->var_au_p2p_enabled()->reset(
+ new bool(true));
+
+ // Check that the UpdateCanStart returns true.
+ UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
+ update_state.p2p_num_attempts = 1;
+ update_state.p2p_first_attempted =
+ fake_clock_.GetWallclockTime() -
+ TimeDelta::FromSeconds(ChromeOSPolicy::kMaxP2PAttemptsPeriodInSeconds + 1);
+ UpdateDownloadParams result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
+ update_state);
+ EXPECT_TRUE(result.update_can_start);
+ EXPECT_FALSE(result.p2p_allowed);
+}
+
TEST_F(UmChromeOSPolicyTest,
UpdateCanStartAllowedWithHttpUrlForUnofficialBuild) {
// The UpdateCanStart policy returns true; device policy forbids both HTTP and
diff --git a/update_manager/policy.h b/update_manager/policy.h
index a009c68..2f88996 100644
--- a/update_manager/policy.h
+++ b/update_manager/policy.h
@@ -86,6 +86,10 @@
// timestamp when it occurred.
std::vector<std::tuple<int, chromeos_update_engine::ErrorCode, base::Time>>
download_errors;
+ // The number of P2P download attempts and wallclock-based time when P2P
+ // download was first attempted.
+ int p2p_num_attempts;
+ base::Time p2p_first_attempted;
// Information pertaining to update backoff mechanism.
//
diff --git a/update_manager/update_manager_unittest.cc b/update_manager/update_manager_unittest.cc
index b263fc8..a9e4b22 100644
--- a/update_manager/update_manager_unittest.cc
+++ b/update_manager/update_manager_unittest.cc
@@ -173,6 +173,8 @@
update_state.failures_last_updated = Time();
update_state.download_urls = vector<string>{"http://fake/url/"};
update_state.download_errors_max = 10;
+ update_state.p2p_num_attempts = 0;
+ update_state.p2p_first_attempted = Time();
update_state.last_download_url_idx = -1;
update_state.last_download_url_num_errors = 0;
update_state.download_errors = vector<tuple<int, ErrorCode, Time>>();