diff options
| -rw-r--r-- | common/fake_hardware.h | 9 | ||||
| -rw-r--r-- | common/hardware_interface.h | 3 | ||||
| -rw-r--r-- | common/mock_hardware.h | 3 | ||||
| -rw-r--r-- | hardware_android.cc | 4 | ||||
| -rw-r--r-- | hardware_android.h | 1 | ||||
| -rw-r--r-- | hardware_chromeos.cc | 22 | ||||
| -rw-r--r-- | hardware_chromeos.h | 4 | ||||
| -rw-r--r-- | update_attempter.cc | 43 | ||||
| -rw-r--r-- | update_attempter.h | 7 | ||||
| -rw-r--r-- | update_attempter_unittest.cc | 54 |
10 files changed, 57 insertions, 93 deletions
diff --git a/common/fake_hardware.h b/common/fake_hardware.h index 2da12ad3..f7c02868 100644 --- a/common/fake_hardware.h +++ b/common/fake_hardware.h @@ -41,6 +41,10 @@ class FakeHardware : public HardwareInterface { bool IsNormalBootMode() const override { return is_normal_boot_mode_; } + bool AreDevFeaturesEnabled() const override { + return are_dev_features_enabled_; + } + bool IsOOBEEnabled() const override { return is_oobe_enabled_; } bool IsOOBEComplete(base::Time* out_time_of_oobe) const override { @@ -86,6 +90,10 @@ class FakeHardware : public HardwareInterface { is_normal_boot_mode_ = is_normal_boot_mode; } + void SetAreDevFeaturesEnabled(bool are_dev_features_enabled) { + are_dev_features_enabled_ = are_dev_features_enabled; + } + // Sets the SetIsOOBEEnabled to |is_oobe_enabled|. void SetIsOOBEEnabled(bool is_oobe_enabled) { is_oobe_enabled_ = is_oobe_enabled; @@ -120,6 +128,7 @@ class FakeHardware : public HardwareInterface { private: bool is_official_build_{true}; bool is_normal_boot_mode_{true}; + bool are_dev_features_enabled_{false}; bool is_oobe_enabled_{true}; bool is_oobe_complete_{true}; base::Time oobe_timestamp_{base::Time::FromTimeT(1169280000)}; // Jan 20, 2007 diff --git a/common/hardware_interface.h b/common/hardware_interface.h index f5f900e5..316ad3d5 100644 --- a/common/hardware_interface.h +++ b/common/hardware_interface.h @@ -44,6 +44,9 @@ class HardwareInterface { // features. virtual bool IsNormalBootMode() const = 0; + // Returns whether the developer features are enabled. + virtual bool AreDevFeaturesEnabled() const = 0; + // Returns whether the device has an OOBE flow that the user must go through // before getting non-critical updates. Use IsOOBEComplete() to determine if // that flow is complete. diff --git a/common/mock_hardware.h b/common/mock_hardware.h index 826b3b3f..1c4253a7 100644 --- a/common/mock_hardware.h +++ b/common/mock_hardware.h @@ -36,6 +36,9 @@ class MockHardware : public HardwareInterface { ON_CALL(*this, IsNormalBootMode()) .WillByDefault(testing::Invoke(&fake_, &FakeHardware::IsNormalBootMode)); + ON_CALL(*this, AreDevFeaturesEnabled()) + .WillByDefault(testing::Invoke(&fake_, + &FakeHardware::AreDevFeaturesEnabled)); ON_CALL(*this, IsOOBEEnabled()) .WillByDefault(testing::Invoke(&fake_, &FakeHardware::IsOOBEEnabled)); diff --git a/hardware_android.cc b/hardware_android.cc index 8de02c7f..653ccf90 100644 --- a/hardware_android.cc +++ b/hardware_android.cc @@ -122,6 +122,10 @@ bool HardwareAndroid::IsNormalBootMode() const { return property_get_bool("ro.debuggable", 0) != 1; } +bool HardwareAndroid::AreDevFeaturesEnabled() const { + return !IsNormalBootMode(); +} + bool HardwareAndroid::IsOOBEEnabled() const { // No OOBE flow blocking updates for Android-based boards. return false; diff --git a/hardware_android.h b/hardware_android.h index 88e00fac..78af871b 100644 --- a/hardware_android.h +++ b/hardware_android.h @@ -36,6 +36,7 @@ class HardwareAndroid final : public HardwareInterface { // HardwareInterface methods. bool IsOfficialBuild() const override; bool IsNormalBootMode() const override; + bool AreDevFeaturesEnabled() const override; bool IsOOBEEnabled() const override; bool IsOOBEComplete(base::Time* out_time_of_oobe) const override; std::string GetHardwareClass() const override; diff --git a/hardware_chromeos.cc b/hardware_chromeos.cc index 591cb3c7..4b0b82f3 100644 --- a/hardware_chromeos.cc +++ b/hardware_chromeos.cc @@ -23,6 +23,7 @@ #include <base/strings/string_util.h> #include <brillo/key_value_store.h> #include <brillo/make_unique_ptr.h> +#include <debugd/dbus-constants.h> #include <vboot/crossystem.h> extern "C" { @@ -35,6 +36,7 @@ extern "C" { #include "update_engine/common/platform_constants.h" #include "update_engine/common/subprocess.h" #include "update_engine/common/utils.h" +#include "update_engine/dbus_connection.h" using std::string; using std::vector; @@ -84,6 +86,8 @@ std::unique_ptr<HardwareInterface> CreateHardware() { void HardwareChromeOS::Init() { LoadConfig("" /* root_prefix */, IsNormalBootMode()); + debugd_proxy_.reset( + new org::chromium::debugdProxy(DBusConnection::Get()->GetDBus())); } bool HardwareChromeOS::IsOfficialBuild() const { @@ -95,6 +99,24 @@ bool HardwareChromeOS::IsNormalBootMode() const { return !dev_mode; } +bool HardwareChromeOS::AreDevFeaturesEnabled() const { + // Even though the debugd tools are also gated on devmode, checking here can + // save us a D-Bus call so it's worth doing explicitly. + if (IsNormalBootMode()) + return false; + + int32_t dev_features = debugd::DEV_FEATURES_DISABLED; + brillo::ErrorPtr error; + // Some boards may not include debugd so it's expected that this may fail, + // in which case we treat it as disabled. + if (debugd_proxy_ && debugd_proxy_->QueryDevFeatures(&dev_features, &error) && + !(dev_features & debugd::DEV_FEATURES_DISABLED)) { + LOG(INFO) << "Debugd dev tools enabled."; + return true; + } + return false; +} + bool HardwareChromeOS::IsOOBEEnabled() const { return is_oobe_enabled_; } diff --git a/hardware_chromeos.h b/hardware_chromeos.h index bc0e891b..03ad750f 100644 --- a/hardware_chromeos.h +++ b/hardware_chromeos.h @@ -22,6 +22,7 @@ #include <base/macros.h> #include <base/time/time.h> +#include <debugd/dbus-proxies.h> #include "update_engine/common/hardware_interface.h" @@ -39,6 +40,7 @@ class HardwareChromeOS final : public HardwareInterface { // HardwareInterface methods. bool IsOfficialBuild() const override; bool IsNormalBootMode() const override; + bool AreDevFeaturesEnabled() const override; bool IsOOBEEnabled() const override; bool IsOOBEComplete(base::Time* out_time_of_oobe) const override; std::string GetHardwareClass() const override; @@ -60,6 +62,8 @@ class HardwareChromeOS final : public HardwareInterface { bool is_oobe_enabled_; + std::unique_ptr<org::chromium::debugdProxyInterface> debugd_proxy_; + DISALLOW_COPY_AND_ASSIGN(HardwareChromeOS); }; diff --git a/update_attempter.cc b/update_attempter.cc index 49f00a2f..a2125a69 100644 --- a/update_attempter.cc +++ b/update_attempter.cc @@ -32,9 +32,9 @@ #include <base/strings/string_util.h> #include <base/strings/stringprintf.h> #include <brillo/bind_lambda.h> +#include <brillo/errors/error_codes.h> #include <brillo/make_unique_ptr.h> #include <brillo/message_loops/message_loop.h> -#include <debugd/dbus-constants.h> #include <policy/device_policy.h> #include <policy/libpolicy.h> #include <update_engine/dbus-constants.h> @@ -50,9 +50,6 @@ #include "update_engine/common/prefs_interface.h" #include "update_engine/common/subprocess.h" #include "update_engine/common/utils.h" -#if USE_DBUS -#include "update_engine/dbus_connection.h" -#endif // USE_DBUS #include "update_engine/metrics.h" #include "update_engine/omaha_request_action.h" #include "update_engine/omaha_request_params.h" @@ -164,13 +161,6 @@ void UpdateAttempter::Init() { #if USE_LIBCROS chrome_proxy_resolver_.Init(); #endif // USE_LIBCROS - -#if USE_DBUS - // unittest can set this to a mock before calling Init(). - if (!debugd_proxy_) - debugd_proxy_.reset( - new org::chromium::debugdProxy(DBusConnection::Get()->GetDBus())); -#endif // USE_DBUS } void UpdateAttempter::ScheduleUpdates() { @@ -1090,11 +1080,7 @@ bool UpdateAttempter::OnTrackChannel(const string& channel, channel, false /* powerwash_allowed */, &error_message)) { brillo::Error::AddTo(error, FROM_HERE, -#if USE_DBUS brillo::errors::dbus::kDomain, -#else - "dbus", -#endif // USE_DBUS "set_target_error", error_message); return false; @@ -1592,30 +1578,13 @@ bool UpdateAttempter::IsAnyUpdateSourceAllowed() { return true; } - // Even though the debugd tools are also gated on devmode, checking here can - // save us a D-Bus call so it's worth doing explicitly. - if (system_state_->hardware()->IsNormalBootMode()) { - LOG(INFO) << "Not in devmode; disallowing custom update sources."; - return false; - } - -#if USE_DBUS - // Official images in devmode are allowed a custom update source iff the - // debugd dev tools are enabled. - if (!debugd_proxy_) - return false; - int32_t dev_features = debugd::DEV_FEATURES_DISABLED; - brillo::ErrorPtr error; - bool success = debugd_proxy_->QueryDevFeatures(&dev_features, &error); - - // Some boards may not include debugd so it's expected that this may fail, - // in which case we default to disallowing custom update sources. - if (success && !(dev_features & debugd::DEV_FEATURES_DISABLED)) { - LOG(INFO) << "Debugd dev tools enabled; allowing any update source."; + if (system_state_->hardware()->AreDevFeaturesEnabled()) { + LOG(INFO) << "Developer features enabled; allowing custom update sources."; return true; } -#endif // USE_DBUS - LOG(INFO) << "Debugd dev tools disabled; disallowing custom update sources."; + + LOG(INFO) + << "Developer features disabled; disallowing custom update sources."; return false; } diff --git a/update_attempter.h b/update_attempter.h index 78b35f0f..0b83d5b2 100644 --- a/update_attempter.h +++ b/update_attempter.h @@ -29,9 +29,6 @@ #include <base/time/time.h> #include <gtest/gtest_prod.h> // for FRIEND_TEST -#if USE_DBUS -#include "debugd/dbus-proxies.h" -#endif // USE_DBUS #if USE_LIBCROS #include "update_engine/chrome_browser_proxy_resolver.h" #endif // USE_LIBCROS @@ -510,10 +507,6 @@ class UpdateAttempter : public ActionProcessorDelegate, std::string forced_app_version_; std::string forced_omaha_url_; -#if USE_DBUS - std::unique_ptr<org::chromium::debugdProxyInterface> debugd_proxy_; -#endif // USE_DBUS - DISALLOW_COPY_AND_ASSIGN(UpdateAttempter); }; diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc index 6ef1c15e..dfb04652 100644 --- a/update_attempter_unittest.cc +++ b/update_attempter_unittest.cc @@ -27,11 +27,6 @@ #include <brillo/message_loops/base_message_loop.h> #include <brillo/message_loops/message_loop.h> #include <brillo/message_loops/message_loop_utils.h> -#if USE_DBUS -#include <debugd/dbus-constants.h> -#include <debugd/dbus-proxies.h> -#include <debugd/dbus-proxy-mocks.h> -#endif // USE_DBUS #include <gtest/gtest.h> #include <policy/libpolicy.h> #include <policy/mock_device_policy.h> @@ -118,9 +113,6 @@ class UpdateAttempterTest : public ::testing::Test { protected: UpdateAttempterTest() : -#if USE_DBUS - debugd_proxy_mock_(new org::chromium::debugdProxyMock()), -#endif // USE_DBUS #if USE_LIBCROS service_interface_mock_(new LibCrosServiceInterfaceProxyMock()), ue_proxy_resolved_interface_mock_( @@ -138,10 +130,7 @@ class UpdateAttempterTest : public ::testing::Test { certificate_checker_.Init(); -// Finish initializing the attempter. -#if USE_DBUS - attempter_.debugd_proxy_.reset(debugd_proxy_mock_); -#endif // USE_DBUS + // Finish initializing the attempter. attempter_.Init(); } @@ -203,9 +192,6 @@ class UpdateAttempterTest : public ::testing::Test { brillo::BaseMessageLoop loop_{&base_loop_}; FakeSystemState fake_system_state_; -#if USE_DBUS - org::chromium::debugdProxyMock* debugd_proxy_mock_; -#endif // USE_DBUS #if USE_LIBCROS LibCrosServiceInterfaceProxyMock* service_interface_mock_; UpdateEngineLibcrosProxyResolvedInterfaceProxyMock* @@ -963,58 +949,28 @@ TEST_F(UpdateAttempterTest, AnyUpdateSourceAllowedUnofficial) { EXPECT_TRUE(attempter_.IsAnyUpdateSourceAllowed()); } -#if USE_DBUS TEST_F(UpdateAttempterTest, AnyUpdateSourceAllowedOfficialDevmode) { fake_system_state_.fake_hardware()->SetIsOfficialBuild(true); - fake_system_state_.fake_hardware()->SetIsNormalBootMode(false); - EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _)) - .WillRepeatedly(DoAll(SetArgumentPointee<0>(0), Return(true))); + fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(true); EXPECT_TRUE(attempter_.IsAnyUpdateSourceAllowed()); } -#endif // USE_DBUS TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedOfficialNormal) { fake_system_state_.fake_hardware()->SetIsOfficialBuild(true); - fake_system_state_.fake_hardware()->SetIsNormalBootMode(true); - // debugd should not be queried in this case. -#if USE_DBUS - EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _)).Times(0); -#endif // USE_DBUS - EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed()); -} - -TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedDebugdDisabled) { - fake_system_state_.fake_hardware()->SetIsOfficialBuild(true); - fake_system_state_.fake_hardware()->SetIsNormalBootMode(false); -#if USE_DBUS - using debugd::DEV_FEATURES_DISABLED; - EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _)) - .WillRepeatedly( - DoAll(SetArgumentPointee<0>(DEV_FEATURES_DISABLED), Return(true))); -#endif // USE_DBUS - EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed()); -} - -TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedDebugdFailure) { - fake_system_state_.fake_hardware()->SetIsOfficialBuild(true); - fake_system_state_.fake_hardware()->SetIsNormalBootMode(false); -#if USE_DBUS - EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _)) - .WillRepeatedly(Return(false)); -#endif // USE_DBUS + fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false); EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed()); } TEST_F(UpdateAttempterTest, CheckForUpdateAUTest) { fake_system_state_.fake_hardware()->SetIsOfficialBuild(true); - fake_system_state_.fake_hardware()->SetIsNormalBootMode(true); + fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false); attempter_.CheckForUpdate("", "autest", true); EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url()); } TEST_F(UpdateAttempterTest, CheckForUpdateScheduledAUTest) { fake_system_state_.fake_hardware()->SetIsOfficialBuild(true); - fake_system_state_.fake_hardware()->SetIsNormalBootMode(true); + fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false); attempter_.CheckForUpdate("", "autest-scheduled", true); EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url()); } |
