Add DBus methods to allow/disallow updates over 3G
This fix adds a new DBus pair of methods to allow/disallow the
updates over cellular networks and get the current state of this
setting. The setting is overridden by the device policy and the
SetUpdateOverCellularPermission() method fails if called on an
enrolled device that has the autoupdate settings in the device
policy.
BUG=chromium:213401
TEST=unittests for connection_manager changes. Manual test for the DBus service, see below.
Manual test procedure.
======================
Run on a shell:
1. Test for the default setting.
$ update_engine_client -show_update_over_cellular
[0701/183633:INFO:update_engine_client.cc(371)] Current update over cellular network setting: DISABLED
[0701/183633:INFO:update_engine_client.cc(443)] Done.
2. Test that enable works.
$ update_engine_client -update_over_cellular=yes -show_update_over_cellular
[0701/183655:INFO:update_engine_client.cc(371)] Current update over cellular network setting: ENABLED
[0701/183655:INFO:update_engine_client.cc(443)] Done.
3. Test that disable works.
$ update_engine_client -update_over_cellular=no -show_update_over_cellular
[0701/183659:INFO:update_engine_client.cc(371)] Current update over cellular network setting: DISABLED
[0701/183659:INFO:update_engine_client.cc(443)] Done.
4. Enable again the update over cellular, connect the chromebook to a 3G
and perform an update check.
Change-Id: Ic234a3ef8898b1e60e26277208276a958b7e0d94
Reviewed-on: https://gerrit.chromium.org/gerrit/60716
Reviewed-by: Chris Sosa <[email protected]>
Commit-Queue: Alex Deymo <[email protected]>
Tested-by: Alex Deymo <[email protected]>
diff --git a/UpdateEngine.conf b/UpdateEngine.conf
index 9707402..5673bbe 100644
--- a/UpdateEngine.conf
+++ b/UpdateEngine.conf
@@ -40,6 +40,12 @@
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
send_member="GetChannel"/>
+ <allow send_destination="org.chromium.UpdateEngine"
+ send_interface="org.chromium.UpdateEngineInterface"
+ send_member="SetUpdateOverCellularPermission"/>
+ <allow send_destination="org.chromium.UpdateEngine"
+ send_interface="org.chromium.UpdateEngineInterface"
+ send_member="GetUpdateOverCellularPermission"/>
<allow send_interface="org.chromium.UpdateEngineLibcrosProxyResolvedInterface" />
</policy>
<policy context="default">
diff --git a/connection_manager.cc b/connection_manager.cc
index 3044130..185f3b5 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -12,6 +12,7 @@
#include <dbus/dbus-glib.h>
#include <glib.h>
+#include "update_engine/prefs.h"
#include "update_engine/system_state.h"
#include "update_engine/utils.h"
@@ -158,26 +159,51 @@
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.
if (!device_policy) {
- LOG(INFO) << "Disabling updates over cellular connection as there's no "
- "device policy object present";
+ LOG(INFO) << "Disabling updates over cellular networks as there's no "
+ "device policy loaded yet.";
return false;
}
- if (!device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
- LOG(INFO) << "Disabling updates over cellular connection as there's no "
- "allowed connection types from policy";
- return false;
- }
+ if (device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
+ // The update setting is enforced by the device policy.
- if (!ContainsKey(allowed_types, flimflam::kTypeCellular)) {
- LOG(INFO) << "Disabling updates over cellular connection as it's not "
- "allowed in the device policy.";
- return false;
- }
+ if ((type == kNetCellular &&
+ !ContainsKey(allowed_types, flimflam::kTypeCellular))) {
+ LOG(INFO) << "Disabling updates over cellular connection as it's not "
+ "allowed in the device policy.";
+ return false;
+ }
- LOG(INFO) << "Allowing updates over cellular per device policy";
- return true;
+ 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;
+ }
+
+ int64_t stored_value;
+ if (!prefs->GetInt64(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;
+ }
}
default:
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index e6a2ef4..724061b 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -201,7 +201,7 @@
.Times(1)
.WillOnce(Return(&allow_3g_policy));
- // This test tests 3G being the only connection type being allowed.
+ // This test tests cellular (3G) being the only connection type being allowed.
set<string> allowed_set;
allowed_set.insert(cmut_.StringForConnectionType(kNetCellular));
@@ -220,20 +220,24 @@
.WillOnce(Return(&allow_3g_policy));
// This test tests multiple connection types being allowed, with
- // 3G one among them.
+ // 3G one among them. Only Cellular is currently enforced by the policy
+ // setting, the others are ignored (see Bluetooth for example).
set<string> allowed_set;
- allowed_set.insert(cmut_.StringForConnectionType(kNetEthernet));
allowed_set.insert(cmut_.StringForConnectionType(kNetCellular));
- allowed_set.insert(cmut_.StringForConnectionType(kNetWifi));
+ allowed_set.insert(cmut_.StringForConnectionType(kNetBluetooth));
EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
.Times(1)
.WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetEthernet));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetCellular));
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWifi));
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWimax));
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetBluetooth));
}
-TEST_F(ConnectionManagerTest, BlockUpdatesOver3GByDefaultTest) {
+TEST_F(ConnectionManagerTest, BlockUpdatesOverCellularByDefaultTest) {
EXPECT_CALL(mock_system_state_, device_policy()).Times(1);
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
}
@@ -279,6 +283,44 @@
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
}
+TEST_F(ConnectionManagerTest, UseUserPrefForUpdatesOverCellularIfNoPolicyTest) {
+ policy::MockDevicePolicy no_policy;
+ testing::NiceMock<PrefsMock>* prefs = mock_system_state_.mock_prefs();
+
+ EXPECT_CALL(mock_system_state_, device_policy())
+ .Times(3)
+ .WillRepeatedly(Return(&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))
+ .Times(1)
+ .WillOnce(Return(false));
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
+
+ // Allow per user pref.
+ EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(*prefs, GetInt64(kPrefsUpdateOverCellularPermission, _))
+ .Times(1)
+ .WillOnce(DoAll(SetArgumentPointee<1>(1), Return(true)));
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetCellular));
+
+ // Block per user pref.
+ EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(*prefs, GetInt64(kPrefsUpdateOverCellularPermission, _))
+ .Times(1)
+ .WillOnce(DoAll(SetArgumentPointee<1>(0), Return(true)));
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
+}
+
TEST_F(ConnectionManagerTest, StringForConnectionTypeTest) {
EXPECT_STREQ(flimflam::kTypeEthernet,
cmut_.StringForConnectionType(kNetEthernet));
diff --git a/constants.cc b/constants.cc
index a85eb26..7878a85 100644
--- a/constants.cc
+++ b/constants.cc
@@ -48,6 +48,8 @@
const char kPrefsUpdateCheckResponseHash[] = "update-check-response-hash";
const char kPrefsUpdateDurationUptime[] = "update-duration-uptime";
const char kPrefsUpdateFirstSeenAt[] = "update-first-seen-at";
+const char kPrefsUpdateOverCellularPermission[] =
+ "update-over-cellular-permission";
const char kPrefsUpdateServerCertificate[] = "update-server-cert";
const char kPrefsUpdateStateNextDataOffset[] = "update-state-next-data-offset";
const char kPrefsUpdateStateNextOperation[] = "update-state-next-operation";
diff --git a/constants.h b/constants.h
index e930772..076aeae 100644
--- a/constants.h
+++ b/constants.h
@@ -51,6 +51,7 @@
extern const char kPrefsUpdateCheckResponseHash[];
extern const char kPrefsUpdateDurationUptime[];
extern const char kPrefsUpdateFirstSeenAt[];
+extern const char kPrefsUpdateOverCellularPermission[];
extern const char kPrefsUpdateServerCertificate[];
extern const char kPrefsUpdateStateNextDataOffset[];
extern const char kPrefsUpdateStateNextOperation[];
diff --git a/dbus_service.cc b/dbus_service.cc
index e75b2b2..1a209cd 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -4,15 +4,20 @@
#include "update_engine/dbus_service.h"
+#include <set>
#include <string>
#include <base/logging.h>
#include <policy/device_policy.h>
+#include "update_engine/connection_manager.h"
#include "update_engine/marshal.glibmarshal.h"
#include "update_engine/omaha_request_params.h"
+#include "update_engine/update_attempter.h"
+#include "update_engine/prefs.h"
#include "update_engine/utils.h"
+using std::set;
using std::string;
static const char kAUTestURLRequest[] = "autest";
@@ -219,6 +224,76 @@
return TRUE;
}
+gboolean update_engine_service_set_update_over_cellular_permission(
+ UpdateEngineService* self,
+ bool allowed,
+ GError **error) {
+ set<string> allowed_types;
+ const policy::DevicePolicy* device_policy =
+ self->system_state_->device_policy();
+
+ // The device_policy is loaded in a lazy way before an update check. Load it
+ // now from the libchromeos cache if it wasn't already loaded.
+ if (!device_policy) {
+ chromeos_update_engine::UpdateAttempter* update_attempter =
+ self->system_state_->update_attempter();
+ if (update_attempter) {
+ update_attempter->RefreshDevicePolicy();
+ device_policy = self->system_state_->device_policy();;
+ }
+ }
+
+ // Check if this setting is allowed by the device policy.
+ if (device_policy &&
+ device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
+ LOG(INFO) << "Ignoring the update over cellular setting since there's "
+ "a device policy enforcing this setting.";
+ *error = NULL;
+ return FALSE;
+ }
+
+ // 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.
+
+ chromeos_update_engine::PrefsInterface* prefs = self->system_state_->prefs();
+
+ if (!prefs->SetInt64(
+ chromeos_update_engine::kPrefsUpdateOverCellularPermission,
+ allowed ? 1 : 0)) {
+ LOG(ERROR) << "Error setting the update over cellular to "
+ << (allowed ? 1 : 0);
+ *error = NULL;
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+gboolean update_engine_service_get_update_over_cellular_permission(
+ UpdateEngineService* self,
+ bool* allowed,
+ GError **/*error*/) {
+ chromeos_update_engine::ConnectionManager* cm =
+ self->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 libchromeos cache if it wasn't already loaded.
+ if (!self->system_state_->device_policy()) {
+ chromeos_update_engine::UpdateAttempter* update_attempter =
+ self->system_state_->update_attempter();
+ if (update_attempter)
+ update_attempter->RefreshDevicePolicy();
+ }
+
+ // 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:";
+ *allowed = cm->IsUpdateAllowedOver(chromeos_update_engine::kNetCellular);
+
+ return TRUE;
+}
+
gboolean update_engine_service_emit_status_update(
UpdateEngineService* self,
gint64 last_checked_time,
diff --git a/dbus_service.h b/dbus_service.h
index d5e0240..b9bf08f 100644
--- a/dbus_service.h
+++ b/dbus_service.h
@@ -101,6 +101,22 @@
gchar** channel,
GError **error);
+// If there's no device policy installed, sets the update over cellular networks
+// permission to the |allowed| value. Otherwise, this method returns with an
+// error since this setting is overridden by the applied policy.
+gboolean update_engine_service_set_update_over_cellular_permission(
+ UpdateEngineService* self,
+ bool allowed,
+ GError **error);
+
+// 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.
+gboolean update_engine_service_get_update_over_cellular_permission(
+ UpdateEngineService* self,
+ bool* allowed,
+ GError **error);
+
gboolean update_engine_service_emit_status_update(
UpdateEngineService* self,
gint64 last_checked_time,
diff --git a/update_attempter.h b/update_attempter.h
index a98fcbf..d6015ee 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -174,6 +174,12 @@
// Called at update_engine startup to do various house-keeping.
void UpdateEngineStarted();
+ // Reloads the device policy from libchromeos. Note: This method doesn't
+ // cause a real-time policy fetch from the policy server. It just reloads the
+ // latest value that libchromeos has cached. libchromeos fetches the policies
+ // from the server asynchronously at its own frequency.
+ void RefreshDevicePolicy();
+
private:
// Update server URL for automated lab test.
static const char* const kTestUpdateUrl;
@@ -255,12 +261,6 @@
// update has been applied.
void PingOmaha();
- // Reloads the device policy from libchromeos. Note: This method doesn't
- // cause a real-time policy fetch from the policy server. It just reloads the
- // latest value that libchromeos has cached. libchromeos fetches the policies
- // from the server asynchronously at its own frequency.
- void RefreshDevicePolicy();
-
// Helper method of Update() to calculate the update-related parameters
// from various sources and set the appropriate state. Please refer to
// Update() method for the meaning of the parametes.
diff --git a/update_engine.xml b/update_engine.xml
index 195bde8..cafe26b 100644
--- a/update_engine.xml
+++ b/update_engine.xml
@@ -40,6 +40,12 @@
<arg type="b" name="get_current_channel" />
<arg type="s" name="channel" direction="out" />
</method>
+ <method name="SetUpdateOverCellularPermission">
+ <arg type="b" name="allowed" />
+ </method>
+ <method name="GetUpdateOverCellularPermission">
+ <arg type="b" name="allowed" direction="out" />
+ </method>
<signal name="StatusUpdate">
<arg type="x" name="last_checked_time" />
<arg type="d" name="progress" />
diff --git a/update_engine_client.cc b/update_engine_client.cc
index 87b1615..3455a52 100644
--- a/update_engine_client.cc
+++ b/update_engine_client.cc
@@ -39,6 +39,11 @@
"Exit status is 0 if the update succeeded, and 1 otherwise.");
DEFINE_bool(watch_for_updates, false,
"Listen for status updates and print them to the screen.");
+DEFINE_bool(show_update_over_cellular, false,
+ "Show the current setting for updates over cellular networks.");
+DEFINE_string(update_over_cellular, "",
+ "Enables (\"yes\") or disables (\"no\") the updates over "
+ "cellular networks.");
namespace {
@@ -260,6 +265,39 @@
return output;
}
+bool SetUpdateOverCellularPermission(gboolean allowed) {
+ DBusGProxy* proxy;
+ GError* error = NULL;
+
+ CHECK(GetProxy(&proxy));
+
+ gboolean rc =
+ org_chromium_UpdateEngineInterface_set_update_over_cellular_permission(
+ proxy,
+ allowed,
+ &error);
+ CHECK_EQ(rc, true) << "Error setting the update over cellular setting: "
+ << GetAndFreeGError(&error);
+ return true;
+}
+
+bool GetUpdateOverCellularPermission() {
+ DBusGProxy* proxy;
+ GError* error = NULL;
+
+ CHECK(GetProxy(&proxy));
+
+ gboolean allowed;
+ gboolean rc =
+ org_chromium_UpdateEngineInterface_get_update_over_cellular_permission(
+ proxy,
+ &allowed,
+ &error);
+ CHECK_EQ(rc, true) << "Error getting the update over cellular setting: "
+ << GetAndFreeGError(&error);
+ return allowed;
+}
+
static gboolean CompleteUpdateSource(gpointer data) {
string current_op;
if (!GetStatus(¤t_op) || current_op == "UPDATE_STATUS_IDLE") {
@@ -316,6 +354,24 @@
return 0;
}
+ // Changes the current update over cellular network setting.
+ if (!FLAGS_update_over_cellular.empty()) {
+ gboolean allowed = FLAGS_update_over_cellular == "yes";
+ if (!allowed && FLAGS_update_over_cellular != "no") {
+ LOG(ERROR) << "Unknown option: \"" << FLAGS_update_over_cellular
+ << "\". Please specify \"yes\" or \"no\".";
+ } else {
+ SetUpdateOverCellularPermission(allowed);
+ }
+ }
+
+ // Show the current update over cellular network setting.
+ if (FLAGS_show_update_over_cellular) {
+ bool allowed = GetUpdateOverCellularPermission();
+ LOG(INFO) << "Current update over cellular network setting: "
+ << (allowed ? "ENABLED" : "DISABLED");
+ }
+
// First, update the target channel if requested.
if (!FLAGS_channel.empty())
SetTargetChannel(FLAGS_channel);
