Add functions to allow update over cellular (including tethered connection)
am: 4b0d6032cb
Change-Id: I48b24ce353d2cf46d2face063de2fd5c8c704c25
diff --git a/UpdateEngine.conf b/UpdateEngine.conf
index 58cca09..192e6ab 100644
--- a/UpdateEngine.conf
+++ b/UpdateEngine.conf
@@ -53,6 +53,9 @@
send_member="SetUpdateOverCellularPermission"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
+ send_member="SetUpdateOverCellularTarget"/>
+ <allow send_destination="org.chromium.UpdateEngine"
+ send_interface="org.chromium.UpdateEngineInterface"
send_member="GetUpdateOverCellularPermission"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
diff --git a/binder_bindings/android/brillo/IUpdateEngine.aidl b/binder_bindings/android/brillo/IUpdateEngine.aidl
index b1a1b4f..7893548 100644
--- a/binder_bindings/android/brillo/IUpdateEngine.aidl
+++ b/binder_bindings/android/brillo/IUpdateEngine.aidl
@@ -33,6 +33,8 @@
void SetP2PUpdatePermission(in boolean enabled);
boolean GetP2PUpdatePermission();
void SetUpdateOverCellularPermission(in boolean enabled);
+ void SetUpdateOverCellularTarget(in String target_version,
+ in long target_size);
boolean GetUpdateOverCellularPermission();
long GetDurationSinceUpdate();
String GetPrevVersion();
diff --git a/binder_service_brillo.cc b/binder_service_brillo.cc
index 5e74159..14df725 100644
--- a/binder_service_brillo.cc
+++ b/binder_service_brillo.cc
@@ -151,6 +151,14 @@
&UpdateEngineService::SetUpdateOverCellularPermission, enabled);
}
+Status BinderUpdateEngineBrilloService::SetUpdateOverCellularTarget(
+ const String16& target_version,
+ int64_t target_size) {
+ return CallCommonHandler(
+ &UpdateEngineService::SetUpdateOverCellularTarget,
+ NormalString(target_version), target_size);
+}
+
Status BinderUpdateEngineBrilloService::GetUpdateOverCellularPermission(
bool* out_cellular_permission) {
return CallCommonHandler(
diff --git a/binder_service_brillo.h b/binder_service_brillo.h
index 982c7b1..4cc007b 100644
--- a/binder_service_brillo.h
+++ b/binder_service_brillo.h
@@ -76,6 +76,9 @@
bool* out_p2p_permission) override;
android::binder::Status SetUpdateOverCellularPermission(
bool enabled) override;
+ android::binder::Status SetUpdateOverCellularTarget(
+ const android::String16& target_version,
+ int64_t target_size) override;
android::binder::Status GetUpdateOverCellularPermission(
bool* out_cellular_permission) override;
android::binder::Status GetDurationSinceUpdate(
diff --git a/client_library/include/update_engine/update_status.h b/client_library/include/update_engine/update_status.h
index 3e9af5b..2f6322a 100644
--- a/client_library/include/update_engine/update_status.h
+++ b/client_library/include/update_engine/update_status.h
@@ -23,6 +23,9 @@
IDLE = 0,
CHECKING_FOR_UPDATE,
UPDATE_AVAILABLE,
+ // Broadcast this state when an update aborts because user preferences does
+ // not allow update over cellular.
+ NEED_PERMISSION_TO_UPDATE,
DOWNLOADING,
VERIFYING,
FINALIZING,
diff --git a/common/constants.cc b/common/constants.cc
index 88d0445..d6d1c6d 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -74,6 +74,10 @@
const char kPrefsUpdateFirstSeenAt[] = "update-first-seen-at";
const char kPrefsUpdateOverCellularPermission[] =
"update-over-cellular-permission";
+const char kPrefsUpdateOverCellularTargetVersion[] =
+ "update-over-cellular-target-version";
+const char kPrefsUpdateOverCellularTargetSize[] =
+ "update-over-cellular-target-size";
const char kPrefsUpdateServerCertificate[] = "update-server-cert";
const char kPrefsUpdateStateNextDataLength[] = "update-state-next-data-length";
const char kPrefsUpdateStateNextDataOffset[] = "update-state-next-data-offset";
diff --git a/common/constants.h b/common/constants.h
index ab66921..c571403 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -75,6 +75,8 @@
extern const char kPrefsUpdateDurationUptime[];
extern const char kPrefsUpdateFirstSeenAt[];
extern const char kPrefsUpdateOverCellularPermission[];
+extern const char kPrefsUpdateOverCellularTargetVersion[];
+extern const char kPrefsUpdateOverCellularTargetSize[];
extern const char kPrefsUpdateServerCertificate[];
extern const char kPrefsUpdateStateNextDataLength[];
extern const char kPrefsUpdateStateNextDataOffset[];
diff --git a/common/error_code.h b/common/error_code.h
index e08ec46..bcf541c 100644
--- a/common/error_code.h
+++ b/common/error_code.h
@@ -73,6 +73,7 @@
kFilesystemVerifierError = 47,
kUserCanceled = 48,
kNonCriticalUpdateInOOBE = 49,
+ kOmahaUpdateIgnoredOverCellular = 50,
// VERY IMPORTANT! When adding new error codes:
//
diff --git a/common/error_code_utils.cc b/common/error_code_utils.cc
index ad4aeeb..0169ab8 100644
--- a/common/error_code_utils.cc
+++ b/common/error_code_utils.cc
@@ -144,6 +144,8 @@
return "ErrorCode::kUserCanceled";
case ErrorCode::kNonCriticalUpdateInOOBE:
return "ErrorCode::kNonCriticalUpdateInOOBE";
+ case ErrorCode::kOmahaUpdateIgnoredOverCellular:
+ return "ErrorCode::kOmahaUpdateIgnoredOverCellular";
// Don't add a default case to let the compiler warn about newly added
// error codes which should be added here.
}
diff --git a/common_service.cc b/common_service.cc
index 1a41b63..ee890e1 100644
--- a/common_service.cc
+++ b/common_service.cc
@@ -16,7 +16,6 @@
#include "update_engine/common_service.h"
-#include <set>
#include <string>
#include <base/location.h>
@@ -41,7 +40,6 @@
using base::StringPrintf;
using brillo::ErrorPtr;
using brillo::string_utils::ToString;
-using std::set;
using std::string;
namespace chromeos_update_engine {
@@ -248,24 +246,12 @@
bool UpdateEngineService::SetUpdateOverCellularPermission(ErrorPtr* error,
bool in_allowed) {
- set<string> allowed_types;
- const policy::DevicePolicy* device_policy = system_state_->device_policy();
-
- // The device_policy is loaded in a lazy way before an update check. Load it
- // now from the libbrillo cache if it wasn't already loaded.
- if (!device_policy) {
- UpdateAttempter* update_attempter = system_state_->update_attempter();
- if (update_attempter) {
- update_attempter->RefreshDevicePolicy();
- device_policy = system_state_->device_policy();
- }
- }
+ ConnectionManagerInterface* connection_manager =
+ system_state_->connection_manager();
// Check if this setting is allowed by the device policy.
- if (device_policy &&
- device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
- LogAndSetError(error,
- FROM_HERE,
+ if (connection_manager->IsAllowedConnectionTypesForUpdateSet()) {
+ LogAndSetError(error, FROM_HERE,
"Ignoring the update over cellular setting since there's "
"a device policy enforcing this setting.");
return false;
@@ -276,9 +262,9 @@
PrefsInterface* prefs = system_state_->prefs();
- if (!prefs->SetBoolean(kPrefsUpdateOverCellularPermission, in_allowed)) {
- LogAndSetError(error,
- FROM_HERE,
+ if (!prefs ||
+ !prefs->SetBoolean(kPrefsUpdateOverCellularPermission, in_allowed)) {
+ LogAndSetError(error, FROM_HERE,
string("Error setting the update over cellular to ") +
(in_allowed ? "true" : "false"));
return false;
@@ -286,24 +272,63 @@
return true;
}
-bool UpdateEngineService::GetUpdateOverCellularPermission(ErrorPtr* /* error */,
- bool* out_allowed) {
- ConnectionManagerInterface* cm = system_state_->connection_manager();
+bool UpdateEngineService::SetUpdateOverCellularTarget(
+ brillo::ErrorPtr* error, const std::string &target_version,
+ int64_t target_size) {
+ ConnectionManagerInterface* connection_manager =
+ system_state_->connection_manager();
- // The device_policy is loaded in a lazy way before an update check and is
- // used to determine if an update is allowed over cellular. Load the device
- // policy now from the libbrillo cache if it wasn't already loaded.
- if (!system_state_->device_policy()) {
- UpdateAttempter* update_attempter = system_state_->update_attempter();
- if (update_attempter)
- update_attempter->RefreshDevicePolicy();
+ // Check if this setting is allowed by the device policy.
+ if (connection_manager->IsAllowedConnectionTypesForUpdateSet()) {
+ LogAndSetError(error, FROM_HERE,
+ "Ignoring the update over cellular setting since there's "
+ "a device policy enforcing this setting.");
+ return false;
}
- // Return the current setting based on the same logic used while checking for
- // updates. A log message could be printed as the result of this test.
- LOG(INFO) << "Checking if updates over cellular networks are allowed:";
- *out_allowed = cm->IsUpdateAllowedOver(ConnectionType::kCellular,
- ConnectionTethering::kUnknown);
+ // If the policy wasn't loaded yet, then it is still OK to change the local
+ // setting because the policy will be checked again during the update check.
+
+ PrefsInterface* prefs = system_state_->prefs();
+
+ if (!prefs ||
+ !prefs->SetString(kPrefsUpdateOverCellularTargetVersion,
+ target_version) ||
+ !prefs->SetInt64(kPrefsUpdateOverCellularTargetSize, target_size)) {
+ LogAndSetError(error, FROM_HERE,
+ "Error setting the target for update over cellular.");
+ return false;
+ }
+ return true;
+}
+
+bool UpdateEngineService::GetUpdateOverCellularPermission(ErrorPtr* error,
+ bool* out_allowed) {
+ ConnectionManagerInterface* connection_manager =
+ system_state_->connection_manager();
+
+ if (connection_manager->IsAllowedConnectionTypesForUpdateSet()) {
+ // We have device policy, so ignore the user preferences.
+ *out_allowed = connection_manager->IsUpdateAllowedOver(
+ ConnectionType::kCellular, ConnectionTethering::kUnknown);
+ } else {
+ PrefsInterface* prefs = system_state_->prefs();
+
+ if (!prefs || !prefs->Exists(kPrefsUpdateOverCellularPermission)) {
+ // Update is not allowed as user preference is not set or not available.
+ *out_allowed = false;
+ return true;
+ }
+
+ bool is_allowed;
+
+ if (!prefs->GetBoolean(kPrefsUpdateOverCellularPermission, &is_allowed)) {
+ LogAndSetError(error, FROM_HERE,
+ "Error getting the update over cellular preference.");
+ return false;
+ }
+ *out_allowed = is_allowed;
+ }
return true;
}
diff --git a/common_service.h b/common_service.h
index 69368fb..4320d09 100644
--- a/common_service.h
+++ b/common_service.h
@@ -115,6 +115,12 @@
bool SetUpdateOverCellularPermission(brillo::ErrorPtr* error,
bool in_allowed);
+ // If there's no device policy installed, sets the update over cellular
+ // target. Otherwise, this method returns with an error.
+ bool SetUpdateOverCellularTarget(brillo::ErrorPtr* error,
+ const std::string& target_version,
+ int64_t target_size);
+
// Returns the current value of the update over cellular network setting,
// either forced by the device policy if the device is enrolled or the current
// user preference otherwise.
diff --git a/connection_manager.cc b/connection_manager.cc
index f72d9e8..f0a2c92 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -30,6 +30,7 @@
#include "update_engine/connection_utils.h"
#include "update_engine/shill_proxy.h"
#include "update_engine/system_state.h"
+#include "update_engine/update_attempter.h"
using org::chromium::flimflam::ManagerProxyInterface;
using org::chromium::flimflam::ServiceProxyInterface;
@@ -58,15 +59,23 @@
case ConnectionType::kCellular: {
set<string> allowed_types;
+
const policy::DevicePolicy* device_policy =
system_state_->device_policy();
- // A device_policy is loaded in a lazy way right before an update check,
- // so the device_policy should be already loaded at this point. If it's
- // not, return a safe value for this setting.
+ // The device_policy is loaded in a lazy way before an update check. Load
+ // it now from the libbrillo cache if it wasn't already loaded.
if (!device_policy) {
- LOG(INFO) << "Disabling updates over cellular networks as there's no "
- "device policy loaded yet.";
+ UpdateAttempter* update_attempter = system_state_->update_attempter();
+ if (update_attempter) {
+ update_attempter->RefreshDevicePolicy();
+ device_policy = system_state_->device_policy();
+ }
+ }
+
+ if (!device_policy) {
+ LOG(INFO) << "Disabling updates over cellular as device policy "
+ "fails to be loaded.";
return false;
}
@@ -74,38 +83,17 @@
// The update setting is enforced by the device policy.
if (!ContainsKey(allowed_types, shill::kTypeCellular)) {
- LOG(INFO) << "Disabling updates over cellular connection as it's not "
- "allowed in the device policy.";
+ LOG(INFO) << "Disabling updates over cellular per device policy.";
return false;
}
LOG(INFO) << "Allowing updates over cellular per device policy.";
- return true;
- } else {
- // There's no update setting in the device policy, using the local user
- // setting.
- PrefsInterface* prefs = system_state_->prefs();
-
- if (!prefs || !prefs->Exists(kPrefsUpdateOverCellularPermission)) {
- LOG(INFO) << "Disabling updates over cellular connection as there's "
- "no device policy setting nor user preference present.";
- return false;
- }
-
- bool stored_value;
- if (!prefs->GetBoolean(kPrefsUpdateOverCellularPermission,
- &stored_value)) {
- return false;
- }
-
- if (!stored_value) {
- LOG(INFO) << "Disabling updates over cellular connection per user "
- "setting.";
- return false;
- }
- LOG(INFO) << "Allowing updates over cellular per user setting.";
- return true;
}
+
+ // If there's no update setting in the device policy, we do not check
+ // the local user setting here, which should be checked by
+ // |OmahaRequestAction| during checking for update.
+ return true;
}
default:
@@ -120,6 +108,22 @@
}
}
+bool ConnectionManager::IsAllowedConnectionTypesForUpdateSet() const {
+ const policy::DevicePolicy* device_policy = system_state_->device_policy();
+ if (!device_policy) {
+ LOG(INFO) << "There's no device policy loaded yet.";
+ return false;
+ }
+
+ set<string> allowed_types;
+ if (!device_policy->GetAllowedConnectionTypesForUpdate(
+ &allowed_types)) {
+ return false;
+ }
+
+ return true;
+}
+
bool ConnectionManager::GetConnectionProperties(
ConnectionType* out_type, ConnectionTethering* out_tethering) {
dbus::ObjectPath default_service_path;
diff --git a/connection_manager.h b/connection_manager.h
index e5a9d49..864985e 100644
--- a/connection_manager.h
+++ b/connection_manager.h
@@ -43,6 +43,7 @@
ConnectionTethering* out_tethering) override;
bool IsUpdateAllowedOver(ConnectionType type,
ConnectionTethering tethering) const override;
+ bool IsAllowedConnectionTypesForUpdateSet() const override;
private:
// Returns (via out_path) the default network path, or empty string if
diff --git a/connection_manager_android.cc b/connection_manager_android.cc
index 2dd824a..c594b53 100644
--- a/connection_manager_android.cc
+++ b/connection_manager_android.cc
@@ -34,5 +34,9 @@
ConnectionType type, ConnectionTethering tethering) const {
return true;
}
+bool ConnectionManagerAndroid::IsAllowedConnectionTypesForUpdateSet() const {
+ return false;
+}
+
} // namespace chromeos_update_engine
diff --git a/connection_manager_android.h b/connection_manager_android.h
index 0cd5e73..006f4ea 100644
--- a/connection_manager_android.h
+++ b/connection_manager_android.h
@@ -34,6 +34,7 @@
ConnectionTethering* out_tethering) override;
bool IsUpdateAllowedOver(ConnectionType type,
ConnectionTethering tethering) const override;
+ bool IsAllowedConnectionTypesForUpdateSet() const override;
DISALLOW_COPY_AND_ASSIGN(ConnectionManagerAndroid);
};
diff --git a/connection_manager_interface.h b/connection_manager_interface.h
index df8eb4b..2faeb80 100644
--- a/connection_manager_interface.h
+++ b/connection_manager_interface.h
@@ -46,6 +46,10 @@
virtual bool IsUpdateAllowedOver(ConnectionType type,
ConnectionTethering tethering) const = 0;
+ // Returns true if the allowed connection types for update is set in the
+ // device policy. Otherwise, returns false.
+ virtual bool IsAllowedConnectionTypesForUpdateSet() const = 0;
+
protected:
ConnectionManagerInterface() = default;
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index 0bb5547..f814667 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -277,16 +277,24 @@
ConnectionTethering::kConfirmed));
}
-TEST_F(ConnectionManagerTest, BlockUpdatesOverCellularByDefaultTest) {
- EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
- ConnectionTethering::kUnknown));
+TEST_F(ConnectionManagerTest, AllowUpdatesOverCellularByDefaultTest) {
+ policy::MockDevicePolicy device_policy;
+ // Set an empty device policy.
+ fake_system_state_.set_device_policy(&device_policy);
+
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
+ ConnectionTethering::kUnknown));
}
-TEST_F(ConnectionManagerTest, BlockUpdatesOverTetheredNetworkByDefaultTest) {
- EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kWifi,
- ConnectionTethering::kConfirmed));
- EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kEthernet,
- ConnectionTethering::kConfirmed));
+TEST_F(ConnectionManagerTest, AllowUpdatesOverTetheredNetworkByDefaultTest) {
+ policy::MockDevicePolicy device_policy;
+ // Set an empty device policy.
+ fake_system_state_.set_device_policy(&device_policy);
+
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kWifi,
+ ConnectionTethering::kConfirmed));
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kEthernet,
+ ConnectionTethering::kConfirmed));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kWifi,
ConnectionTethering::kSuspected));
}
@@ -311,62 +319,20 @@
ConnectionTethering::kUnknown));
}
-TEST_F(ConnectionManagerTest, BlockUpdatesOver3GIfErrorInPolicyFetchTest) {
- policy::MockDevicePolicy allow_3g_policy;
+TEST_F(ConnectionManagerTest, AllowUpdatesOver3GIfPolicyIsNotSet) {
+ policy::MockDevicePolicy device_policy;
- fake_system_state_.set_device_policy(&allow_3g_policy);
-
- set<string> allowed_set;
- allowed_set.insert(StringForConnectionType(ConnectionType::kCellular));
+ fake_system_state_.set_device_policy(&device_policy);
// Return false for GetAllowedConnectionTypesForUpdate and see
- // that updates are still blocked for 3G despite the value being in
- // the string set above.
- EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
- .Times(1)
- .WillOnce(DoAll(SetArgPointee<0>(allowed_set), Return(false)));
-
- EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
- ConnectionTethering::kUnknown));
-}
-
-TEST_F(ConnectionManagerTest, UseUserPrefForUpdatesOverCellularIfNoPolicyTest) {
- policy::MockDevicePolicy no_policy;
- testing::NiceMock<MockPrefs>* prefs = fake_system_state_.mock_prefs();
-
- fake_system_state_.set_device_policy(&no_policy);
-
- // No setting enforced by the device policy, user prefs should be used.
- EXPECT_CALL(no_policy, GetAllowedConnectionTypesForUpdate(_))
- .Times(3)
- .WillRepeatedly(Return(false));
-
- // No user pref: block.
- EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
+ // that updates are allowed as device policy is not set. Further
+ // check is left to |OmahaRequestAction|.
+ EXPECT_CALL(device_policy, GetAllowedConnectionTypesForUpdate(_))
.Times(1)
.WillOnce(Return(false));
- EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
- ConnectionTethering::kUnknown));
- // Allow per user pref.
- EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
- .Times(1)
- .WillOnce(Return(true));
- EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
- .Times(1)
- .WillOnce(DoAll(SetArgPointee<1>(true), Return(true)));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
ConnectionTethering::kUnknown));
-
- // Block per user pref.
- EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
- .Times(1)
- .WillOnce(Return(true));
- EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
- .Times(1)
- .WillOnce(DoAll(SetArgPointee<1>(false), Return(true)));
- EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
- ConnectionTethering::kUnknown));
}
TEST_F(ConnectionManagerTest, StringForConnectionTypeTest) {
diff --git a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
index 848f775..a20f33f 100644
--- a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
+++ b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
@@ -67,6 +67,10 @@
<method name="SetUpdateOverCellularPermission">
<arg type="b" name="allowed" direction="in" />
</method>
+ <method name="SetUpdateOverCellularTarget">
+ <arg type="s" name="target_version" direction="in" />
+ <arg type="x" name="target_size" direction="in" />
+ </method>
<method name="GetUpdateOverCellularPermission">
<arg type="b" name="allowed" direction="out" />
</method>
diff --git a/dbus_service.cc b/dbus_service.cc
index 0a7ad5b..731d489 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -123,6 +123,14 @@
return common_->SetUpdateOverCellularPermission(error, in_allowed);
}
+bool DBusUpdateEngineService::SetUpdateOverCellularTarget(
+ brillo::ErrorPtr* error,
+ const std::string& target_version,
+ int64_t target_size) {
+ return common_->SetUpdateOverCellularTarget(error, target_version,
+ target_size);
+}
+
bool DBusUpdateEngineService::GetUpdateOverCellularPermission(
ErrorPtr* error, bool* out_allowed) {
return common_->GetUpdateOverCellularPermission(error, out_allowed);
diff --git a/dbus_service.h b/dbus_service.h
index 2b36ae9..644dcee 100644
--- a/dbus_service.h
+++ b/dbus_service.h
@@ -114,6 +114,12 @@
bool SetUpdateOverCellularPermission(brillo::ErrorPtr* error,
bool in_allowed) override;
+ // If there's no device policy installed, sets the update over cellular
+ // target. Otherwise, this method returns with an error.
+ bool SetUpdateOverCellularTarget(brillo::ErrorPtr* error,
+ const std::string& target_version,
+ int64_t target_size) override;
+
// Returns the current value of the update over cellular network setting,
// either forced by the device policy if the device is enrolled or the current
// user preference otherwise.
diff --git a/metrics_utils.cc b/metrics_utils.cc
index 263bacd..15e3a8a 100644
--- a/metrics_utils.cc
+++ b/metrics_utils.cc
@@ -109,6 +109,7 @@
case ErrorCode::kPostinstallPowerwashError:
case ErrorCode::kUpdateCanceledByChannelChange:
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
+ case ErrorCode::kOmahaUpdateIgnoredOverCellular:
return metrics::AttemptResult::kInternalError;
// Special flags. These can't happen (we mask them out above) but
@@ -209,6 +210,7 @@
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
case ErrorCode::kFilesystemVerifierError:
case ErrorCode::kUserCanceled:
+ case ErrorCode::kOmahaUpdateIgnoredOverCellular:
break;
// Special flags. These can't happen (we mask them out above) but
diff --git a/mock_connection_manager.h b/mock_connection_manager.h
index e37460b..2fff68c 100644
--- a/mock_connection_manager.h
+++ b/mock_connection_manager.h
@@ -36,6 +36,7 @@
MOCK_CONST_METHOD2(IsUpdateAllowedOver,
bool(ConnectionType type, ConnectionTethering tethering));
+ MOCK_CONST_METHOD0(IsAllowedConnectionTypesForUpdateSet, bool());
};
} // namespace chromeos_update_engine
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index b06de09..ed3c716 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1001,9 +1001,11 @@
output_object.update_exists = true;
SetOutputObject(output_object);
- if (ShouldIgnoreUpdate(output_object)) {
- output_object.update_exists = false;
- completer.set_code(ErrorCode::kOmahaUpdateIgnoredPerPolicy);
+ ErrorCode error = ErrorCode::kSuccess;
+ if (ShouldIgnoreUpdate(&error, output_object)) {
+ // No need to change output_object.update_exists here, since the value
+ // has been output to the pipe.
+ completer.set_code(error);
return;
}
@@ -1461,6 +1463,7 @@
break;
case ErrorCode::kOmahaUpdateIgnoredPerPolicy:
+ case ErrorCode::kOmahaUpdateIgnoredOverCellular:
result = metrics::CheckResult::kUpdateAvailable;
reaction = metrics::CheckReaction::kIgnored;
break;
@@ -1495,7 +1498,7 @@
}
bool OmahaRequestAction::ShouldIgnoreUpdate(
- const OmahaResponse& response) const {
+ ErrorCode* error, const OmahaResponse& response) const {
// Note: policy decision to not update to a version we rolled back from.
string rollback_version =
system_state_->payload_state()->GetRollbackVersion();
@@ -1503,11 +1506,12 @@
LOG(INFO) << "Detected previous rollback from version " << rollback_version;
if (rollback_version == response.version) {
LOG(INFO) << "Received version that we rolled back from. Ignoring.";
+ *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
return true;
}
}
- if (!IsUpdateAllowedOverCurrentConnection()) {
+ if (!IsUpdateAllowedOverCurrentConnection(error, response)) {
LOG(INFO) << "Update is not allowed over current connection.";
return true;
}
@@ -1522,7 +1526,59 @@
return false;
}
-bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection() const {
+bool OmahaRequestAction::IsUpdateAllowedOverCellularByPrefs(
+ const OmahaResponse& response) const {
+ PrefsInterface* prefs = system_state_->prefs();
+
+ if (!prefs) {
+ LOG(INFO) << "Disabling updates over cellular as the preferences are "
+ "not available.";
+ return false;
+ }
+
+ bool is_allowed;
+
+ if (prefs->Exists(kPrefsUpdateOverCellularPermission) &&
+ prefs->GetBoolean(kPrefsUpdateOverCellularPermission, &is_allowed) &&
+ is_allowed) {
+ LOG(INFO) << "Allowing updates over cellular as permission preference is "
+ "set to true.";
+ return true;
+ }
+
+ if (!prefs->Exists(kPrefsUpdateOverCellularTargetVersion) ||
+ !prefs->Exists(kPrefsUpdateOverCellularTargetSize)) {
+ LOG(INFO) << "Disabling updates over cellular as permission preference is "
+ "set to false or does not exist while target does not exist.";
+ return false;
+ }
+
+ std::string target_version;
+ int64_t target_size;
+
+ if (!prefs->GetString(kPrefsUpdateOverCellularTargetVersion,
+ &target_version) ||
+ !prefs->GetInt64(kPrefsUpdateOverCellularTargetSize,
+ &target_size)) {
+ LOG(INFO) << "Disabling updates over cellular as the target version or "
+ "size is not accessible.";
+ return false;
+ }
+
+ if (target_version == response.version && target_size == response.size) {
+ LOG(INFO) << "Allowing updates over cellular as the target matches the"
+ "omaha response.";
+ return true;
+ } else {
+ LOG(INFO) << "Disabling updates over cellular as the target does not"
+ "match the omaha response.";
+ return false;
+ }
+}
+
+bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection(
+ ErrorCode* error,
+ const OmahaResponse& response) const {
ConnectionType type;
ConnectionTethering tethering;
ConnectionManagerInterface* connection_manager =
@@ -1532,7 +1588,30 @@
<< "Defaulting to allow updates.";
return true;
}
+
bool is_allowed = connection_manager->IsUpdateAllowedOver(type, tethering);
+ bool is_device_policy_set = connection_manager->
+ IsAllowedConnectionTypesForUpdateSet();
+ // Treats tethered connection as if it is cellular connection.
+ bool is_over_cellular = type == ConnectionType::kCellular ||
+ tethering == ConnectionTethering::kConfirmed;
+
+ if (!is_over_cellular) {
+ // There's no need to further check user preferences as we are not over
+ // cellular connection.
+ if (!is_allowed)
+ *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
+ } else if (is_device_policy_set) {
+ // There's no need to further check user preferences as the device policy
+ // is set regarding updates over cellular.
+ if (!is_allowed)
+ *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
+ } else if (!IsUpdateAllowedOverCellularByPrefs(response)) {
+ // The user prefereces does not allow updates over cellular.
+ is_allowed = false;
+ *error = ErrorCode::kOmahaUpdateIgnoredOverCellular;
+ }
+
LOG(INFO) << "We are connected via "
<< connection_utils::StringForConnectionType(type)
<< ", Updates allowed: " << (is_allowed ? "Yes" : "No");
diff --git a/omaha_request_action.h b/omaha_request_action.h
index 2915a6a..7d35c23 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -299,11 +299,17 @@
void OnLookupPayloadViaP2PCompleted(const std::string& url);
// Returns true if the current update should be ignored.
- bool ShouldIgnoreUpdate(const OmahaResponse& response) const;
+ bool ShouldIgnoreUpdate(ErrorCode* error,
+ const OmahaResponse& response) const;
+
+ // Return true if updates are allowed by user preferences.
+ bool IsUpdateAllowedOverCellularByPrefs(const OmahaResponse& response) const;
// Returns true if updates are allowed over the current type of connection.
// False otherwise.
- bool IsUpdateAllowedOverCurrentConnection() const;
+ bool IsUpdateAllowedOverCurrentConnection(
+ ErrorCode* error,
+ const OmahaResponse& response) const;
// Global system context.
SystemState* system_state_;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 1c1d25c..cb3ef71 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -518,6 +518,185 @@
EXPECT_FALSE(response.update_exists);
}
+TEST_F(OmahaRequestActionTest, ValidUpdateOverCellularAllowedByDevicePolicy) {
+ // This test tests that update over cellular is allowed as device policy
+ // says yes.
+ OmahaResponse response;
+ MockConnectionManager mock_cm;
+
+ fake_system_state_.set_connection_manager(&mock_cm);
+
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
+ SetArgumentPointee<1>(ConnectionTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
+ .WillRepeatedly(Return(true));
+
+ ASSERT_TRUE(
+ TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_TRUE(response.update_exists);
+}
+
+TEST_F(OmahaRequestActionTest, ValidUpdateOverCellularBlockedByDevicePolicy) {
+ // This test tests that update over cellular is blocked as device policy
+ // says no.
+ OmahaResponse response;
+ MockConnectionManager mock_cm;
+
+ fake_system_state_.set_connection_manager(&mock_cm);
+
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
+ SetArgumentPointee<1>(ConnectionTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
+ .WillRepeatedly(Return(false));
+
+ ASSERT_FALSE(
+ TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kOmahaUpdateIgnoredPerPolicy,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kIgnored,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_FALSE(response.update_exists);
+}
+
+TEST_F(OmahaRequestActionTest,
+ ValidUpdateOverCellularAllowedByUserPermissionTrue) {
+ // This test tests that, when device policy is not set, update over cellular
+ // is allowed as permission for update over cellular is set to true.
+ OmahaResponse response;
+ MockConnectionManager mock_cm;
+
+ fake_prefs_.SetBoolean(kPrefsUpdateOverCellularPermission, true);
+ fake_system_state_.set_connection_manager(&mock_cm);
+
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
+ SetArgumentPointee<1>(ConnectionTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
+ .WillRepeatedly(Return(true));
+
+ ASSERT_TRUE(
+ TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_TRUE(response.update_exists);
+}
+
+TEST_F(OmahaRequestActionTest,
+ ValidUpdateOverCellularBlockedByUpdateTargetNotMatch) {
+ // This test tests that, when device policy is not set and permission for
+ // update over cellular is set to false or does not exist, update over
+ // cellular is blocked as update target does not match the omaha response.
+ OmahaResponse response;
+ MockConnectionManager mock_cm;
+ // A version different from the version in omaha response.
+ string diff_version = "99.99.99";
+ // A size different from the size in omaha response.
+ int64_t diff_size = 999;
+
+ fake_prefs_.SetString(kPrefsUpdateOverCellularTargetVersion, diff_version);
+ fake_prefs_.SetInt64(kPrefsUpdateOverCellularTargetSize, diff_size);
+ // This test tests cellular (3G) being the only connection type being allowed.
+ fake_system_state_.set_connection_manager(&mock_cm);
+
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
+ SetArgumentPointee<1>(ConnectionTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
+ .WillRepeatedly(Return(true));
+
+ ASSERT_FALSE(
+ TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kOmahaUpdateIgnoredOverCellular,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kIgnored,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_FALSE(response.update_exists);
+}
+
+TEST_F(OmahaRequestActionTest,
+ ValidUpdateOverCellularAllowedByUpdateTargetMatch) {
+ // This test tests that, when device policy is not set and permission for
+ // update over cellular is set to false or does not exist, update over
+ // cellular is allowed as update target matches the omaha response.
+ OmahaResponse response;
+ MockConnectionManager mock_cm;
+ // A version same as the version in omaha response.
+ string new_version = fake_update_response_.version;
+ // A size same as the size in omaha response.
+ int64_t new_size = fake_update_response_.size;
+
+ fake_prefs_.SetString(kPrefsUpdateOverCellularTargetVersion, new_version);
+ fake_prefs_.SetInt64(kPrefsUpdateOverCellularTargetSize, new_size);
+ fake_system_state_.set_connection_manager(&mock_cm);
+
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
+ SetArgumentPointee<1>(ConnectionTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
+ .WillRepeatedly(Return(true));
+
+ ASSERT_TRUE(
+ TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_TRUE(response.update_exists);
+}
+
TEST_F(OmahaRequestActionTest, ValidUpdateBlockedByRollback) {
string rollback_version = "1234.0.0";
OmahaResponse response;
diff --git a/payload_state.cc b/payload_state.cc
index 1da472f..846f901 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -348,6 +348,7 @@
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
case ErrorCode::kFilesystemVerifierError:
case ErrorCode::kUserCanceled:
+ case ErrorCode::kOmahaUpdateIgnoredOverCellular:
LOG(INFO) << "Not incrementing URL index or failure count for this error";
break;
diff --git a/update_attempter.cc b/update_attempter.cc
index d9a8d04..2cc5308 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -49,6 +49,7 @@
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
+#include "update_engine/connection_manager_interface.h"
#include "update_engine/libcurl_http_fetcher.h"
#include "update_engine/metrics.h"
#include "update_engine/omaha_request_action.h"
@@ -1005,9 +1006,20 @@
consecutive_failed_update_checks_ = 0;
}
+ const OmahaResponse& omaha_response =
+ omaha_request_action->GetOutputObject();
// Store the server-dictated poll interval, if any.
server_dictated_poll_interval_ =
- std::max(0, omaha_request_action->GetOutputObject().poll_interval);
+ std::max(0, omaha_response.poll_interval);
+
+ // This update is ignored by omaha request action because update over
+ // cellular connection is not allowed. Needs to ask for user's permissions
+ // to update.
+ if (code == ErrorCode::kOmahaUpdateIgnoredOverCellular) {
+ new_version_ = omaha_response.version;
+ new_payload_size_ = omaha_response.size;
+ SetStatusAndNotify(UpdateStatus::NEED_PERMISSION_TO_UPDATE);
+ }
}
}
if (code != ErrorCode::kSuccess) {
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index ffd378c..e3893d3 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -134,6 +134,7 @@
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
case ErrorCode::kFilesystemVerifierError:
case ErrorCode::kUserCanceled:
+ case ErrorCode::kOmahaUpdateIgnoredOverCellular:
LOG(INFO) << "Not changing URL index or failure count due to error "
<< chromeos_update_engine::utils::ErrorCodeToString(err_code)
<< " (" << static_cast<int>(err_code) << ")";
diff --git a/update_status_utils.cc b/update_status_utils.cc
index ff039b8..5de3381 100644
--- a/update_status_utils.cc
+++ b/update_status_utils.cc
@@ -30,6 +30,8 @@
return update_engine::kUpdateStatusCheckingForUpdate;
case UpdateStatus::UPDATE_AVAILABLE:
return update_engine::kUpdateStatusUpdateAvailable;
+ case UpdateStatus::NEED_PERMISSION_TO_UPDATE:
+ return update_engine::kUpdateStatusNeedPermissionToUpdate;
case UpdateStatus::DOWNLOADING:
return update_engine::kUpdateStatusDownloading;
case UpdateStatus::VERIFYING:
@@ -61,6 +63,9 @@
} else if (s == update_engine::kUpdateStatusUpdateAvailable) {
*status = UpdateStatus::UPDATE_AVAILABLE;
return true;
+ } else if (s == update_engine::kUpdateStatusNeedPermissionToUpdate) {
+ *status = UpdateStatus::NEED_PERMISSION_TO_UPDATE;
+ return true;
} else if (s == update_engine::kUpdateStatusDownloading) {
*status = UpdateStatus::DOWNLOADING;
return true;