update_engine: Fall back to DeviceMinimumVersion
If the kiosk's required Chrome OS version can not be read several times
and DeviceMinimumVersionis set, update only if the current version is
below the DeviceMinimumVersion.
This a very conservative approach at preventing kiosks from updating
randomly:
- It only affects kiosk devices.
- It only affects devices that have DeviceMinimumVersion set, others
will still simply update once the kiosk version could not be fetched
several times.
BUG=chromium:1084453
TEST=FEATURES=test emerge-amd64-generic update_engine
TEST=Set policies on DUT and check behavior
Change-Id: I82caf4bf4969959e461a218a916f057ea73946ad
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2416628
Tested-by: Miriam Polzer <mpolzer@google.com>
Commit-Queue: Miriam Polzer <mpolzer@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/update_manager/enterprise_device_policy_impl.cc b/update_manager/enterprise_device_policy_impl.cc
index 8fc79ca..ce8150e 100644
--- a/update_manager/enterprise_device_policy_impl.cc
+++ b/update_manager/enterprise_device_policy_impl.cc
@@ -62,13 +62,37 @@
ec->GetValue(system_provider->var_kiosk_required_platform_version());
if (!kiosk_required_platform_version_p) {
LOG(INFO) << "Kiosk app required platform version is not fetched, "
- "blocking update checks";
+ "blocking update checks.";
return EvalStatus::kAskMeAgainLater;
+ } else if (kiosk_required_platform_version_p->empty()) {
+ // The platform version could not be fetched several times. Update
+ // based on |DeviceMinimumVersion| instead (crbug.com/1048931).
+ const base::Version* device_minimum_version_p =
+ ec->GetValue(dp_provider->var_device_minimum_version());
+ const base::Version* current_version_p(
+ ec->GetValue(system_provider->var_chromeos_version()));
+ if (device_minimum_version_p && device_minimum_version_p->IsValid() &&
+ current_version_p && current_version_p->IsValid() &&
+ *current_version_p > *device_minimum_version_p) {
+ // Do not update if the current version is newer than the minimum
+ // version.
+ LOG(INFO) << "Reading kiosk app required platform version failed "
+ "repeatedly but current version is newer than "
+ "DeviceMinimumVersion. Blocking update checks. "
+ "Current version: "
+ << *current_version_p
+ << " DeviceMinimumVersion: " << *device_minimum_version_p;
+ return EvalStatus::kAskMeAgainLater;
+ }
+ LOG(WARNING) << "Reading kiosk app required platform version failed "
+ "repeatedly. Attempting an update without it now.";
+ // An empty string for |target_version_prefix| allows arbitrary updates.
+ result->target_version_prefix = "";
+ } else {
+ result->target_version_prefix = *kiosk_required_platform_version_p;
+ LOG(INFO) << "Allow kiosk app to control Chrome version policy is set, "
+ << "target version is " << result->target_version_prefix;
}
-
- result->target_version_prefix = *kiosk_required_platform_version_p;
- LOG(INFO) << "Allow kiosk app to control Chrome version policy is set, "
- << "target version is " << result->target_version_prefix;
// TODO(hunyadym): Add support for allowing rollback using the manifest
// (if policy doesn't specify otherwise).
} else {
diff --git a/update_manager/enterprise_device_policy_impl_unittest.cc b/update_manager/enterprise_device_policy_impl_unittest.cc
index 5b25602..f27715e 100644
--- a/update_manager/enterprise_device_policy_impl_unittest.cc
+++ b/update_manager/enterprise_device_policy_impl_unittest.cc
@@ -36,6 +36,102 @@
}
};
+TEST_F(UmEnterpriseDevicePolicyImplTest, KioskAppVersionSet) {
+ fake_state_.device_policy_provider()->var_update_disabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()
+ ->var_allow_kiosk_app_control_chrome_version()
+ ->reset(new bool(true));
+
+ fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
+ new std::string("1234.5.6"));
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(
+ EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(result.target_version_prefix, "1234.5.6");
+}
+
+TEST_F(UmEnterpriseDevicePolicyImplTest, KioskAppVersionUnreadableNoUpdate) {
+ fake_state_.device_policy_provider()->var_update_disabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()
+ ->var_allow_kiosk_app_control_chrome_version()
+ ->reset(new bool(true));
+
+ fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
+ nullptr);
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(
+ EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+}
+
+TEST_F(UmEnterpriseDevicePolicyImplTest, KioskAppVersionUnreadableUpdate) {
+ fake_state_.device_policy_provider()->var_update_disabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()
+ ->var_allow_kiosk_app_control_chrome_version()
+ ->reset(new bool(true));
+
+ // The real variable returns an empty string after several unsuccessful
+ // reading attempts. Fake this by setting it directly to empty string.
+ fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
+ new std::string(""));
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(
+ EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(result.target_version_prefix, "");
+}
+
+TEST_F(UmEnterpriseDevicePolicyImplTest,
+ KioskAppVersionUnreadableUpdateWithMinVersion) {
+ fake_state_.device_policy_provider()->var_update_disabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()
+ ->var_allow_kiosk_app_control_chrome_version()
+ ->reset(new bool(true));
+
+ // The real variable returns an empty string after several unsuccessful
+ // reading attempts. Fake this by setting it directly to empty string.
+ fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
+ new std::string(""));
+ // Update if the minimum version is above the current OS version.
+ fake_state_.device_policy_provider()->var_device_minimum_version()->reset(
+ new base::Version("2.0.0"));
+ fake_state_.system_provider()->var_chromeos_version()->reset(
+ new base::Version("1.0.0"));
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(
+ EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(result.target_version_prefix, "");
+}
+
+TEST_F(UmEnterpriseDevicePolicyImplTest,
+ KioskAppVersionUnreadableNoUpdateWithMinVersion) {
+ fake_state_.device_policy_provider()->var_update_disabled()->reset(
+ new bool(true));
+ fake_state_.device_policy_provider()
+ ->var_allow_kiosk_app_control_chrome_version()
+ ->reset(new bool(true));
+
+ // The real variable returns an empty string after several unsuccessful
+ // reading attempts. Fake this by setting it directly to empty string.
+ fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
+ new std::string(""));
+ // Block update if the minimum version is below the current OS version.
+ fake_state_.device_policy_provider()->var_device_minimum_version()->reset(
+ new base::Version("1.0.0"));
+ fake_state_.system_provider()->var_chromeos_version()->reset(
+ new base::Version("2.0.0"));
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(
+ EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+}
+
TEST_F(UmEnterpriseDevicePolicyImplTest, ChannelDowngradeBehaviorNoRollback) {
fake_state_.device_policy_provider()->var_release_channel_delegated()->reset(
new bool(false));
diff --git a/update_manager/fake_system_provider.h b/update_manager/fake_system_provider.h
index f54951b..b320c01 100644
--- a/update_manager/fake_system_provider.h
+++ b/update_manager/fake_system_provider.h
@@ -50,6 +50,10 @@
return &var_kiosk_required_platform_version_;
}
+ FakeVariable<base::Version>* var_chromeos_version() override {
+ return &var_version_;
+ }
+
private:
FakeVariable<bool> var_is_normal_boot_mode_{"is_normal_boot_mode",
kVariableModeConst};
@@ -60,6 +64,8 @@
FakeVariable<unsigned int> var_num_slots_{"num_slots", kVariableModePoll};
FakeVariable<std::string> var_kiosk_required_platform_version_{
"kiosk_required_platform_version", kVariableModePoll};
+ FakeVariable<base::Version> var_version_{"chromeos_version",
+ kVariableModePoll};
DISALLOW_COPY_AND_ASSIGN(FakeSystemProvider);
};
diff --git a/update_manager/real_system_provider.cc b/update_manager/real_system_provider.cc
index 39eba22..9b5bc02 100644
--- a/update_manager/real_system_provider.cc
+++ b/update_manager/real_system_provider.cc
@@ -24,7 +24,10 @@
#include <kiosk-app/dbus-proxies.h>
#endif // USE_CHROME_KIOSK_APP
+#include "update_engine/common/boot_control_interface.h"
+#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/utils.h"
+#include "update_engine/omaha_request_params.h"
#include "update_engine/update_manager/generic_variables.h"
#include "update_engine/update_manager/variable.h"
@@ -97,19 +100,19 @@
bool RealSystemProvider::Init() {
var_is_normal_boot_mode_.reset(new ConstCopyVariable<bool>(
- "is_normal_boot_mode", hardware_->IsNormalBootMode()));
+ "is_normal_boot_mode", system_state_->hardware()->IsNormalBootMode()));
var_is_official_build_.reset(new ConstCopyVariable<bool>(
- "is_official_build", hardware_->IsOfficialBuild()));
+ "is_official_build", system_state_->hardware()->IsOfficialBuild()));
var_is_oobe_complete_.reset(new CallCopyVariable<bool>(
"is_oobe_complete",
base::Bind(&chromeos_update_engine::HardwareInterface::IsOOBEComplete,
- base::Unretained(hardware_),
+ base::Unretained(system_state_->hardware()),
nullptr)));
var_num_slots_.reset(new ConstCopyVariable<unsigned int>(
- "num_slots", boot_control_->GetNumSlots()));
+ "num_slots", system_state_->boot_control()->GetNumSlots()));
var_kiosk_required_platform_version_.reset(new RetryPollVariable<string>(
"kiosk_required_platform_version",
@@ -117,6 +120,10 @@
base::Bind(&RealSystemProvider::GetKioskAppRequiredPlatformVersion,
base::Unretained(this))));
+ var_chromeos_version_.reset(new ConstCopyVariable<base::Version>(
+ "chromeos_version",
+ base::Version(system_state_->request_params()->app_version())));
+
return true;
}
diff --git a/update_manager/real_system_provider.h b/update_manager/real_system_provider.h
index 114c6ea..0e68997 100644
--- a/update_manager/real_system_provider.h
+++ b/update_manager/real_system_provider.h
@@ -20,8 +20,9 @@
#include <memory>
#include <string>
-#include "update_engine/common/boot_control_interface.h"
-#include "update_engine/common/hardware_interface.h"
+#include <base/version.h>
+
+#include "update_engine/system_state.h"
#include "update_engine/update_manager/system_provider.h"
namespace org {
@@ -36,16 +37,13 @@
class RealSystemProvider : public SystemProvider {
public:
RealSystemProvider(
- chromeos_update_engine::HardwareInterface* hardware,
- chromeos_update_engine::BootControlInterface* boot_control,
+ chromeos_update_engine::SystemState* system_state,
org::chromium::KioskAppServiceInterfaceProxyInterface* kiosk_app_proxy)
- : hardware_(hardware),
#if USE_CHROME_KIOSK_APP
- boot_control_(boot_control),
- kiosk_app_proxy_(kiosk_app_proxy) {
+ : system_state_(system_state), kiosk_app_proxy_(kiosk_app_proxy) {
}
#else
- boot_control_(boot_control) {
+ system_state_(system_state) {
}
#endif // USE_CHROME_KIOSK_APP
@@ -72,6 +70,10 @@
return var_kiosk_required_platform_version_.get();
}
+ Variable<base::Version>* var_chromeos_version() override {
+ return var_chromeos_version_.get();
+ }
+
private:
bool GetKioskAppRequiredPlatformVersion(
std::string* required_platform_version);
@@ -81,9 +83,9 @@
std::unique_ptr<Variable<bool>> var_is_oobe_complete_;
std::unique_ptr<Variable<unsigned int>> var_num_slots_;
std::unique_ptr<Variable<std::string>> var_kiosk_required_platform_version_;
+ std::unique_ptr<Variable<base::Version>> var_chromeos_version_;
- chromeos_update_engine::HardwareInterface* const hardware_;
- chromeos_update_engine::BootControlInterface* const boot_control_;
+ chromeos_update_engine::SystemState* const system_state_;
#if USE_CHROME_KIOSK_APP
org::chromium::KioskAppServiceInterfaceProxyInterface* const kiosk_app_proxy_;
#endif // USE_CHROME_KIOSK_APP
diff --git a/update_manager/real_system_provider_unittest.cc b/update_manager/real_system_provider_unittest.cc
index 3996b65..9757146 100644
--- a/update_manager/real_system_provider_unittest.cc
+++ b/update_manager/real_system_provider_unittest.cc
@@ -24,6 +24,7 @@
#include "update_engine/common/fake_boot_control.h"
#include "update_engine/common/fake_hardware.h"
+#include "update_engine/fake_system_state.h"
#include "update_engine/update_manager/umtest_utils.h"
#if USE_CHROME_KIOSK_APP
#include "kiosk-app/dbus-proxies.h"
@@ -54,17 +55,15 @@
.WillByDefault(
DoAll(SetArgPointee<0>(kRequiredPlatformVersion), Return(true)));
- provider_.reset(new RealSystemProvider(
- &fake_hardware_, &fake_boot_control_, kiosk_app_proxy_mock_.get()));
+ provider_.reset(new RealSystemProvider(&fake_system_state_,
+ kiosk_app_proxy_mock_.get()));
#else
- provider_.reset(
- new RealSystemProvider(&fake_hardware_, &fake_boot_control_, nullptr));
+ provider_.reset(new RealSystemProvider(&fake_system_state, nullptr));
#endif // USE_CHROME_KIOSK_APP
EXPECT_TRUE(provider_->Init());
}
- chromeos_update_engine::FakeHardware fake_hardware_;
- chromeos_update_engine::FakeBootControl fake_boot_control_;
+ chromeos_update_engine::FakeSystemState fake_system_state_;
unique_ptr<RealSystemProvider> provider_;
#if USE_CHROME_KIOSK_APP
@@ -77,18 +76,29 @@
EXPECT_NE(nullptr, provider_->var_is_official_build());
EXPECT_NE(nullptr, provider_->var_is_oobe_complete());
EXPECT_NE(nullptr, provider_->var_kiosk_required_platform_version());
+ EXPECT_NE(nullptr, provider_->var_chromeos_version());
}
TEST_F(UmRealSystemProviderTest, IsOOBECompleteTrue) {
- fake_hardware_.SetIsOOBEComplete(base::Time());
+ fake_system_state_.fake_hardware()->SetIsOOBEComplete(base::Time());
UmTestUtils::ExpectVariableHasValue(true, provider_->var_is_oobe_complete());
}
TEST_F(UmRealSystemProviderTest, IsOOBECompleteFalse) {
- fake_hardware_.UnsetIsOOBEComplete();
+ fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
UmTestUtils::ExpectVariableHasValue(false, provider_->var_is_oobe_complete());
}
+TEST_F(UmRealSystemProviderTest, VersionFromRequestParams) {
+ fake_system_state_.request_params()->set_app_version("1.2.3");
+ // Call |Init| again to pick up the version.
+ EXPECT_TRUE(provider_->Init());
+
+ base::Version version("1.2.3");
+ UmTestUtils::ExpectVariableHasValue(version,
+ provider_->var_chromeos_version());
+}
+
#if USE_CHROME_KIOSK_APP
TEST_F(UmRealSystemProviderTest, KioskRequiredPlatformVersion) {
UmTestUtils::ExpectVariableHasValue(
diff --git a/update_manager/state_factory.cc b/update_manager/state_factory.cc
index 78cec6a..a0d8a63 100644
--- a/update_manager/state_factory.cc
+++ b/update_manager/state_factory.cc
@@ -69,8 +69,8 @@
unique_ptr<FakeShillProvider> shill_provider(new FakeShillProvider());
#endif // USE_SHILL
unique_ptr<RealRandomProvider> random_provider(new RealRandomProvider());
- unique_ptr<RealSystemProvider> system_provider(new RealSystemProvider(
- system_state->hardware(), system_state->boot_control(), kiosk_app_proxy));
+ unique_ptr<RealSystemProvider> system_provider(
+ new RealSystemProvider(system_state, kiosk_app_proxy));
unique_ptr<RealTimeProvider> time_provider(new RealTimeProvider(clock));
unique_ptr<RealUpdaterProvider> updater_provider(
diff --git a/update_manager/system_provider.h b/update_manager/system_provider.h
index 13e188b..8eb14e3 100644
--- a/update_manager/system_provider.h
+++ b/update_manager/system_provider.h
@@ -17,6 +17,10 @@
#ifndef UPDATE_ENGINE_UPDATE_MANAGER_SYSTEM_PROVIDER_H_
#define UPDATE_ENGINE_UPDATE_MANAGER_SYSTEM_PROVIDER_H_
+#include <string>
+
+#include <base/version.h>
+
#include "update_engine/update_manager/provider.h"
#include "update_engine/update_manager/variable.h"
@@ -46,6 +50,9 @@
// with zero delay kiosk app if any.
virtual Variable<std::string>* var_kiosk_required_platform_version() = 0;
+ // Chrome OS version number as provided by |ImagePropeties|.
+ virtual Variable<base::Version>* var_chromeos_version() = 0;
+
protected:
SystemProvider() {}