PM: Policy for checking whether an update may use the current connection.
This is based on logic currently found in
ConnectionManager::IsUpdateAllowedOver() and
LibcurlHttpFetcher::IsUpdateAllowedOverCurrentConnection().
BUG=chromium:358323
TEST=Unit tests.
Change-Id: Ib1a73d3fbe603b8686294088e26bc2d04ee73877
Reviewed-on: https://chromium-review.googlesource.com/199540
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/policy_manager/chromeos_policy.cc b/policy_manager/chromeos_policy.cc
index df6b921..99e13e9 100644
--- a/policy_manager/chromeos_policy.cc
+++ b/policy_manager/chromeos_policy.cc
@@ -5,6 +5,7 @@
#include "update_engine/policy_manager/chromeos_policy.h"
#include <algorithm>
+#include <set>
#include <string>
#include <base/logging.h>
@@ -12,10 +13,12 @@
#include "update_engine/policy_manager/device_policy_provider.h"
#include "update_engine/policy_manager/policy_utils.h"
+#include "update_engine/policy_manager/shill_provider.h"
using base::Time;
using base::TimeDelta;
using std::min;
+using std::set;
using std::string;
namespace chromeos_policy_manager {
@@ -284,4 +287,97 @@
return ret;
}
+// TODO(garnold) Logic in this method is based on
+// ConnectionManager::IsUpdateAllowedOver(); be sure to deprecate the latter.
+//
+// TODO(garnold) The current logic generally treats the list of allowed
+// connections coming from the device policy as a whitelist, meaning that it
+// can only be used for enabling connections, but not disable them. Further,
+// certain connection types (like Bluetooth) cannot be enabled even by policy.
+// In effect, the only thing that device policy can change is to enable
+// updates over a cellular network (disabled by default). We may want to
+// revisit this semantics, allowing greater flexibility in defining specific
+// permissions over all types of networks.
+EvalStatus ChromeOSPolicy::UpdateCurrentConnectionAllowed(
+ EvaluationContext* ec,
+ State* state,
+ string* error,
+ bool* result) const {
+ // Get the current connection type.
+ ShillProvider* const shill_provider = state->shill_provider();
+ const ConnectionType* conn_type_p = ec->GetValue(
+ shill_provider->var_conn_type());
+ POLICY_CHECK_VALUE_AND_FAIL(conn_type_p, error);
+ ConnectionType conn_type = *conn_type_p;
+
+ // If we're tethering, treat it as a cellular connection.
+ if (conn_type != ConnectionType::kCellular) {
+ const ConnectionTethering* conn_tethering_p = ec->GetValue(
+ shill_provider->var_conn_tethering());
+ POLICY_CHECK_VALUE_AND_FAIL(conn_tethering_p, error);
+ if (*conn_tethering_p == ConnectionTethering::kConfirmed)
+ conn_type = ConnectionType::kCellular;
+ }
+
+ // By default, we allow updates for all connection types, with exceptions as
+ // noted below. This also determines whether a device policy can override the
+ // default.
+ *result = true;
+ bool device_policy_can_override = false;
+ switch (conn_type) {
+ case ConnectionType::kBluetooth:
+ *result = false;
+ break;
+
+ case ConnectionType::kCellular:
+ *result = false;
+ device_policy_can_override = true;
+ break;
+
+ case ConnectionType::kUnknown:
+ if (error)
+ *error = "Unknown connection type";
+ return EvalStatus::kFailed;
+
+ default:
+ break; // Nothing to do.
+ }
+
+ // If update is allowed, we're done.
+ if (*result)
+ return EvalStatus::kSucceeded;
+
+ // Check whether the device policy specifically allows this connection.
+ bool user_settings_can_override = false;
+ if (device_policy_can_override) {
+ DevicePolicyProvider* const dp_provider = state->device_policy_provider();
+ const bool* device_policy_is_loaded_p = ec->GetValue(
+ dp_provider->var_device_policy_is_loaded());
+ if (device_policy_is_loaded_p && *device_policy_is_loaded_p) {
+ const set<ConnectionType>* allowed_conn_types_p = ec->GetValue(
+ dp_provider->var_allowed_connection_types_for_update());
+ if (allowed_conn_types_p) {
+ if (allowed_conn_types_p->count(conn_type)) {
+ *result = true;
+ return EvalStatus::kSucceeded;
+ }
+ } else {
+ user_settings_can_override = true;
+ }
+ }
+ }
+
+ // Local user settings can allow updates iff a policy was loaded but no
+ // allowed connections were specified in it. In all other cases, we either
+ // stick with the default or use the values determined by the policy.
+ if (user_settings_can_override) {
+ const bool* update_over_cellular_allowed_p = ec->GetValue(
+ state->updater_provider()->var_cellular_enabled());
+ if (update_over_cellular_allowed_p && *update_over_cellular_allowed_p)
+ *result = true;
+ }
+
+ return EvalStatus::kSucceeded;
+}
+
} // namespace chromeos_policy_manager
diff --git a/policy_manager/chromeos_policy.h b/policy_manager/chromeos_policy.h
index ee3c16c..1542f9e 100644
--- a/policy_manager/chromeos_policy.h
+++ b/policy_manager/chromeos_policy.h
@@ -39,6 +39,12 @@
const bool interactive,
const UpdateState& update_state) const override;
+ virtual EvalStatus UpdateCurrentConnectionAllowed(
+ EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ bool* result) const override;
+
private:
friend class PmChromeOSPolicyTest;
FRIEND_TEST(PmChromeOSPolicyTest,
diff --git a/policy_manager/chromeos_policy_unittest.cc b/policy_manager/chromeos_policy_unittest.cc
index c13e3ba..69f092f 100644
--- a/policy_manager/chromeos_policy_unittest.cc
+++ b/policy_manager/chromeos_policy_unittest.cc
@@ -4,6 +4,7 @@
#include "update_engine/policy_manager/chromeos_policy.h"
+#include <set>
#include <string>
#include <base/time/time.h>
@@ -17,6 +18,7 @@
using base::Time;
using base::TimeDelta;
using chromeos_update_engine::FakeClock;
+using std::set;
using std::string;
namespace chromeos_policy_manager {
@@ -55,6 +57,12 @@
// For the purpose of the tests, this is an official build.
fake_state_.system_provider()->var_is_official_build()->reset(
new bool(true));
+
+ // Connection is wifi, untethered.
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kWifi));
+ fake_state_.shill_provider()->var_conn_tethering()->
+ reset(new ConnectionTethering(ConnectionTethering::kNotDetected));
}
// Sets up a default device policy that does not impose any restrictions, nor
@@ -512,4 +520,154 @@
EXPECT_EQ("foo-channel", result.target_channel);
}
+TEST_F(PmChromeOSPolicyTest, UpdateCurrentConnectionAllowedEthernetDefault) {
+ // Ethernet is always allowed.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kEthernet));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest, UpdateCurrentConnectionAllowedWifiDefault) {
+ // Wifi is allowed if not tethered.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kWifi));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCurrentConnectionNotAllowedWifiTetheredDefault) {
+ // Tethered wifi is not allowed by default.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kWifi));
+ fake_state_.shill_provider()->var_conn_tethering()->
+ reset(new ConnectionTethering(ConnectionTethering::kConfirmed));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_FALSE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCurrentConnectionAllowedWifiTetheredPolicyOverride) {
+ // Tethered wifi can be allowed by policy.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kWifi));
+ fake_state_.shill_provider()->var_conn_tethering()->
+ reset(new ConnectionTethering(ConnectionTethering::kConfirmed));
+ set<ConnectionType> allowed_connections;
+ allowed_connections.insert(ConnectionType::kCellular);
+ fake_state_.device_policy_provider()->
+ var_allowed_connection_types_for_update()->
+ reset(new set<ConnectionType>(allowed_connections));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest, UpdateCurrentConnectionAllowedWimaxDefault) {
+ // Wimax is always allowed.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kWifi));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCurrentConnectionNotAllowedBluetoothDefault) {
+ // Bluetooth is never allowed.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kBluetooth));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_FALSE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCurrentConnectionNotAllowedBluetoothPolicyCannotOverride) {
+ // Bluetooth cannot be allowed even by policy.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kBluetooth));
+ set<ConnectionType> allowed_connections;
+ allowed_connections.insert(ConnectionType::kBluetooth);
+ fake_state_.device_policy_provider()->
+ var_allowed_connection_types_for_update()->
+ reset(new set<ConnectionType>(allowed_connections));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_FALSE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest, UpdateCurrentConnectionNotAllowedCellularDefault) {
+ // Cellular is not allowed by default.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kCellular));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_FALSE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCurrentConnectionAllowedCellularPolicyOverride) {
+ // Update over cellular can be enabled by policy.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kCellular));
+ set<ConnectionType> allowed_connections;
+ allowed_connections.insert(ConnectionType::kCellular);
+ fake_state_.device_policy_provider()->
+ var_allowed_connection_types_for_update()->
+ reset(new set<ConnectionType>(allowed_connections));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCurrentConnectionAllowedCellularUserOverride) {
+ // Update over cellular can be enabled by user settings, but only if policy
+ // is present and does not determine allowed connections.
+
+ fake_state_.shill_provider()->var_conn_type()->
+ reset(new ConnectionType(ConnectionType::kCellular));
+ set<ConnectionType> allowed_connections;
+ allowed_connections.insert(ConnectionType::kCellular);
+ fake_state_.updater_provider()->var_cellular_enabled()->
+ reset(new bool(true));
+
+ bool result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCurrentConnectionAllowed, &result);
+ EXPECT_TRUE(result);
+}
+
} // namespace chromeos_policy_manager
diff --git a/policy_manager/default_policy.h b/policy_manager/default_policy.h
index 56eedf2..1e2a471 100644
--- a/policy_manager/default_policy.h
+++ b/policy_manager/default_policy.h
@@ -44,6 +44,15 @@
return EvalStatus::kSucceeded;
}
+ virtual EvalStatus UpdateCurrentConnectionAllowed(
+ EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ bool* result) const override {
+ *result = true;
+ return EvalStatus::kSucceeded;
+ }
+
private:
DISALLOW_COPY_AND_ASSIGN(DefaultPolicy);
};
diff --git a/policy_manager/mock_policy.h b/policy_manager/mock_policy.h
index ffa323d..dbee0c2 100644
--- a/policy_manager/mock_policy.h
+++ b/policy_manager/mock_policy.h
@@ -27,6 +27,10 @@
UpdateCanStartResult*,
const bool, const UpdateState&));
+ MOCK_CONST_METHOD4(UpdateCanStart,
+ EvalStatus(EvaluationContext*, State*, std::string*,
+ bool*));
+
private:
DISALLOW_COPY_AND_ASSIGN(MockPolicy);
};
diff --git a/policy_manager/policy.h b/policy_manager/policy.h
index 8ec3ace..22d651e 100644
--- a/policy_manager/policy.h
+++ b/policy_manager/policy.h
@@ -114,6 +114,17 @@
const bool interactive,
const UpdateState& update_state) const = 0;
+ // Checks whether updating is allowed over the current network connection
+ // Consults the shill provider as well as the device policy (if available).
+ // Returns |EvalStatus::kSucceeded|, setting |result| according to whether or
+ // not the current connection can be used; on failure, returns
+ // |EvalStatus::kFailed| and sets |error| accordingly.
+ virtual EvalStatus UpdateCurrentConnectionAllowed(
+ EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ bool* result) const = 0;
+
protected:
Policy() {}
diff --git a/policy_manager/updater_provider.h b/policy_manager/updater_provider.h
index ff76cce..293dcdc 100644
--- a/policy_manager/updater_provider.h
+++ b/policy_manager/updater_provider.h
@@ -69,10 +69,11 @@
// A variable returning the update target channel.
virtual Variable<std::string>* var_new_channel() = 0;
- // A variable indicating whether P2P updates are allowed.
+ // A variable indicating whether user settings allow P2P updates.
virtual Variable<bool>* var_p2p_enabled() = 0;
- // A variable indicating whether updates are allowed over a cellular network.
+ // A variable indicating whether user settings allow updates over a cellular
+ // network.
virtual Variable<bool>* var_cellular_enabled() = 0;
// A variable returning the number of consecutive failed update checks.