Propagate Always on VPN settings to shill

The new class AlwaysOnVpnManager listens to changes in the
Always-on VPN preferences, and forwards any changes to shill
to configure the network correctly.

BUG=b:72861260
BUG=873123
TEST=unit tests

Change-Id: I287f52c7326645085e98d9bd59fd94fd9a692f5e
Reviewed-on: https://chromium-review.googlesource.com/1052508
Commit-Queue: Philipp Weiß <phweiss@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582149}

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -1426,6 +1426,10 @@ void UserSessionManager::FinalizePrepareProfile(Profile* profile) {
       ScreenTimeControllerFactory::GetForBrowserContext(profile);
       ConsumerStatusReportingServiceFactory::GetForBrowserContext(profile);
     }
+
+    // PrefService is ready, check whether we need to force a VPN connection.
+    always_on_vpn_manager_ =
+        std::make_unique<arc::AlwaysOnVpnManager>(profile->GetPrefs());
   }
 
   UpdateEasyUnlockKeys(user_context_);
@@ -2131,6 +2135,7 @@ void UserSessionManager::Shutdown() {
   token_handle_fetcher_.reset();
   token_handle_util_.reset();
   first_run::GoodiesDisplayer::Delete();
+  always_on_vpn_manager_.reset();
 }
 
 void UserSessionManager::SetSwitchesForUser(

chrome/browser/chromeos/login/session/user_session_manager.h

@@ -31,6 +31,7 @@
 #include "chromeos/dbus/session_manager_client.h"
 #include "chromeos/login/auth/authenticator.h"
 #include "chromeos/login/auth/user_context.h"
+#include "components/arc/net/always_on_vpn_manager.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
 #include "net/base/network_change_notifier.h"
@@ -613,6 +614,8 @@ class UserSessionManager
   // Mapped to |chrome::AttemptRestart|, except in tests.
   base::RepeatingClosure attempt_restart_closure_;
 
+  std::unique_ptr<arc::AlwaysOnVpnManager> always_on_vpn_manager_;
+
   base::WeakPtrFactory<UserSessionManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UserSessionManager);

chromeos/network/managed_network_configuration_handler.h

@@ -20,6 +20,7 @@
 namespace base {
 class DictionaryValue;
 class ListValue;
+class Value;
 }
 
 namespace chromeos {
@@ -90,6 +91,12 @@ class CHROMEOS_EXPORT ManagedNetworkConfigurationHandler {
       const base::Closure& callback,
       const network_handler::ErrorCallback& error_callback) = 0;
 
+  virtual void SetManagerProperty(
+      const std::string& property_name,
+      const base::Value& value,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback) = 0;
+
   // Initially configures an unconfigured network with the given user settings
   // and returns the new identifier to |callback| if successful. Fails if the
   // network was already configured by a call to this function or because of a

chromeos/network/managed_network_configuration_handler_impl.cc

@@ -383,6 +383,15 @@ void ManagedNetworkConfigurationHandlerImpl::SetProperties(
                      error_callback);
 }
 
+void ManagedNetworkConfigurationHandlerImpl::SetManagerProperty(
+    const std::string& property_name,
+    const base::Value& value,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  network_configuration_handler_->SetManagerProperty(property_name, value,
+                                                     callback, error_callback);
+}
+
 void ManagedNetworkConfigurationHandlerImpl::SetShillProperties(
     const std::string& service_path,
     std::unique_ptr<base::DictionaryValue> shill_dictionary,

chromeos/network/managed_network_configuration_handler_impl.h

@@ -21,6 +21,7 @@
 
 namespace base {
 class DictionaryValue;
+class Value;
 }
 
 namespace chromeos {
@@ -59,6 +60,12 @@ class CHROMEOS_EXPORT ManagedNetworkConfigurationHandlerImpl
       const base::Closure& callback,
       const network_handler::ErrorCallback& error_callback) override;
 
+  void SetManagerProperty(
+      const std::string& property_name,
+      const base::Value& value,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback) override;
+
   void CreateConfiguration(
       const std::string& userhash,
       const base::DictionaryValue& properties,

chromeos/network/mock_managed_network_configuration_handler.h

@@ -39,6 +39,11 @@ class CHROMEOS_EXPORT MockManagedNetworkConfigurationHandler
                     const base::DictionaryValue& user_settings,
                     const base::Closure& callback,
                     const network_handler::ErrorCallback& error_callback));
+  MOCK_METHOD4(SetManagerProperty,
+               void(const std::string& property_name,
+                    const base::Value& value,
+                    const base::Closure& callback,
+                    const network_handler::ErrorCallback& error_callback));
   MOCK_CONST_METHOD4(
       CreateConfiguration,
       void(const std::string& userhash,

chromeos/network/network_configuration_handler.cc

@@ -62,6 +62,15 @@ void SetNetworkProfileErrorCallback(
       error_callback, dbus_error_name, dbus_error_message);
 }
 
+void ManagerSetPropertiesErrorCallback(
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& dbus_error_name,
+    const std::string& dbus_error_message) {
+  network_handler::ShillErrorCallbackFunction(
+      "ShillManagerClient.SetProperties Failed", std::string(), error_callback,
+      dbus_error_name, dbus_error_message);
+}
+
 void LogConfigProperties(const std::string& desc,
                          const std::string& path,
                          const base::DictionaryValue& properties) {
@@ -444,6 +453,17 @@ void NetworkConfigurationHandler::SetNetworkProfile(
                  error_callback));
 }
 
+void NetworkConfigurationHandler::SetManagerProperty(
+    const std::string& property_name,
+    const base::Value& value,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  NET_LOG(USER) << "SetManagerProperty: " << property_name << ": " << value;
+  DBusThreadManager::Get()->GetShillManagerClient()->SetProperty(
+      property_name, value, callback,
+      base::Bind(&ManagerSetPropertiesErrorCallback, error_callback));
+}
+
 // NetworkStateHandlerObserver methods
 void NetworkConfigurationHandler::NetworkListChanged() {
   for (auto iter = configure_callbacks_.begin();

chromeos/network/network_configuration_handler.h

@@ -25,6 +25,7 @@
 namespace base {
 class DictionaryValue;
 class ListValue;
+class Value;
 }
 
 namespace dbus {
@@ -119,6 +120,12 @@ class CHROMEOS_EXPORT NetworkConfigurationHandler
                          const base::Closure& callback,
                          const network_handler::ErrorCallback& error_callback);
 
+  // Changes the value of a shill manager property.
+  void SetManagerProperty(const std::string& property_name,
+                          const base::Value& value,
+                          const base::Closure& callback,
+                          const network_handler::ErrorCallback& error_callback);
+
   // NetworkStateHandlerObserver
   void NetworkListChanged() override;
   void OnShuttingDown() override;

chromeos/network/network_configuration_handler_unittest.cc

@@ -182,6 +182,14 @@ class NetworkConfigurationHandlerTest : public testing::Test {
     get_properties_ = dictionary.CreateDeepCopy();
   }
 
+  void ManagerGetPropertiesCallback(const std::string& success_callback_name,
+                                    DBusMethodCallStatus call_status,
+                                    const base::DictionaryValue& result) {
+    if (call_status == chromeos::DBUS_METHOD_CALL_SUCCESS)
+      success_callback_name_ = success_callback_name;
+    manager_get_properties_ = result.CreateDeepCopy();
+  }
+
   void CreateConfigurationCallback(const std::string& service_path,
                                    const std::string& guid) {
     create_service_path_ = service_path;
@@ -276,6 +284,18 @@ class NetworkConfigurationHandlerTest : public testing::Test {
     return true;
   }
 
+  bool GetReceivedStringManagerProperty(const std::string& key,
+                                        std::string* result) {
+    if (!manager_get_properties_)
+      return false;
+    const base::Value* value =
+        manager_get_properties_->FindKeyOfType(key, base::Value::Type::STRING);
+    if (!value)
+      return false;
+    *result = value->GetString();
+    return true;
+  }
+
   FakeShillServiceClient* GetShillServiceClient() {
     return static_cast<FakeShillServiceClient*>(
         DBusThreadManager::Get()->GetShillServiceClient());
@@ -294,6 +314,7 @@ class NetworkConfigurationHandlerTest : public testing::Test {
   std::string success_callback_name_;
   std::string get_properties_path_;
   std::unique_ptr<base::DictionaryValue> get_properties_;
+  std::unique_ptr<base::DictionaryValue> manager_get_properties_;
   std::string create_service_path_;
 };
 
@@ -648,4 +669,22 @@ TEST_F(NetworkConfigurationHandlerTest, NetworkConfigurationObserver) {
       network_configuration_observer.get());
 }
 
+TEST_F(NetworkConfigurationHandlerTest, AlwaysOnVpn) {
+  const std::string vpn_package = "com.android.vpn";
+
+  network_configuration_handler_->SetManagerProperty(
+      shill::kAlwaysOnVpnPackageProperty, base::Value(vpn_package),
+      base::DoNothing(), base::Bind(&ErrorCallback));
+
+  DBusThreadManager::Get()->GetShillManagerClient()->GetProperties(
+      Bind(&NetworkConfigurationHandlerTest::ManagerGetPropertiesCallback,
+           base::Unretained(this), "ManagerGetProperties"));
+  base::RunLoop().RunUntilIdle();
+  std::string package_result;
+  EXPECT_EQ("ManagerGetProperties", success_callback_name_);
+  EXPECT_TRUE(GetReceivedStringManagerProperty(
+      shill::kAlwaysOnVpnPackageProperty, &package_result));
+  EXPECT_EQ(vpn_package, package_result);
+}
+
 }  // namespace chromeos

components/arc/BUILD.gn

@@ -44,6 +44,8 @@ static_library("arc") {
     "metrics/arc_metrics_service.h",
     "midis/arc_midis_bridge.cc",
     "midis/arc_midis_bridge.h",
+    "net/always_on_vpn_manager.cc",
+    "net/always_on_vpn_manager.h",
     "net/arc_net_host_impl.cc",
     "net/arc_net_host_impl.h",
     "obb_mounter/arc_obb_mounter_bridge.cc",
@@ -284,6 +286,7 @@ source_set("unit_tests") {
     "intent_helper/intent_filter_unittest.cc",
     "intent_helper/link_handler_model_unittest.cc",
     "metrics/arc_metrics_service_unittest.cc",
+    "net/always_on_vpn_manager_unittest.cc",
     "net/arc_net_host_impl_unittest.cc",
     "power/arc_power_bridge_unittest.cc",
     "timer/arc_timer_bridge_unittest.cc",

components/arc/net/DEPS

@@ -3,5 +3,6 @@ include_rules = [
   "+chromeos/network",
   "+components/onc",
   "+components/user_manager",
+  "+third_party/cros_system_api/dbus/service_constants.h",
   "+third_party/cros_system_api/dbus/shill",
 ]

components/arc/net/always_on_vpn_manager.cc

+// Copyright (c) 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/arc/net/always_on_vpn_manager.h"
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/logging.h"
+#include "base/values.h"
+#include "chromeos/network/managed_network_configuration_handler.h"
+#include "chromeos/network/network_handler.h"
+#include "components/arc/arc_prefs.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace {
+
+void SetPackageErrorCallback(
+    const std::string& error_name,
+    std::unique_ptr<base::DictionaryValue> error_data) {
+  DVLOG(1) << "Error while setting Always-On VPN package in shill: "
+           << error_name << ", " << *error_data;
+}
+
+}  // namespace
+
+namespace arc {
+
+AlwaysOnVpnManager::AlwaysOnVpnManager(PrefService* pref_service) {
+  registrar_.Init(pref_service);
+  registrar_.Add(
+      prefs::kAlwaysOnVpnPackage,
+      base::Bind(&AlwaysOnVpnManager::OnPrefChanged, base::Unretained(this)));
+  registrar_.Add(
+      prefs::kAlwaysOnVpnLockdown,
+      base::Bind(&AlwaysOnVpnManager::OnPrefChanged, base::Unretained(this)));
+  // update once with values before we started listening
+  OnPrefChanged();
+}
+
+AlwaysOnVpnManager::~AlwaysOnVpnManager() {
+  std::string package =
+      registrar_.prefs()->GetString(prefs::kAlwaysOnVpnPackage);
+  bool lockdown = registrar_.prefs()->GetBoolean(prefs::kAlwaysOnVpnLockdown);
+  if (lockdown && !package.empty()) {
+    chromeos::NetworkHandler::Get()
+        ->managed_network_configuration_handler()
+        ->SetManagerProperty(shill::kAlwaysOnVpnPackageProperty,
+                             base::Value(std::string()), base::DoNothing(),
+                             base::Bind(&SetPackageErrorCallback));
+  }
+  registrar_.RemoveAll();
+}
+
+void AlwaysOnVpnManager::OnPrefChanged() {
+  std::string always_on_vpn_package;
+  bool lockdown = registrar_.prefs()->GetBoolean(prefs::kAlwaysOnVpnLockdown);
+  // Only enforce blackholing if lockdown mode is enabled
+  if (lockdown) {
+    always_on_vpn_package =
+        registrar_.prefs()->GetString(prefs::kAlwaysOnVpnPackage);
+  }
+  chromeos::NetworkHandler::Get()
+      ->managed_network_configuration_handler()
+      ->SetManagerProperty(shill::kAlwaysOnVpnPackageProperty,
+                           base::Value(always_on_vpn_package),
+                           base::DoNothing(),
+                           base::Bind(&SetPackageErrorCallback));
+}
+
+}  // namespace arc

components/arc/net/always_on_vpn_manager.h

+// Copyright (c) 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_ARC_NET_ALWAYS_ON_VPN_MANAGER_H_
+#define COMPONENTS_ARC_NET_ALWAYS_ON_VPN_MANAGER_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_service.h"
+
+namespace arc {
+
+class AlwaysOnVpnManager {
+ public:
+  explicit AlwaysOnVpnManager(PrefService* pref_service);
+  ~AlwaysOnVpnManager();
+
+ private:
+  // Callback for the registrar
+  void OnPrefChanged();
+
+  PrefChangeRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(AlwaysOnVpnManager);
+};
+
+}  // namespace arc
+
+#endif  // COMPONENTS_ARC_NET_ALWAYS_ON_VPN_MANAGER_H_

components/arc/net/always_on_vpn_manager_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/arc/net/always_on_vpn_manager.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/run_loop.h"
+#include "base/values.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_manager_client.h"
+#include "chromeos/network/network_handler.h"
+#include "components/arc/arc_prefs.h"
+#include "components/prefs/testing_pref_service.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace {
+
+constexpr const char kVpnPackage[] = "com.android.vpn";
+const base::Value kVpnPackageValue(kVpnPackage);
+
+void OnGetProperties(chromeos::DBusMethodCallStatus* call_status_out,
+                     std::string* package_name_out,
+                     const base::Closure& callback,
+                     chromeos::DBusMethodCallStatus call_status,
+                     const base::DictionaryValue& result) {
+  *call_status_out = call_status;
+  const base::Value* value = result.FindKeyOfType(
+      shill::kAlwaysOnVpnPackageProperty, base::Value::Type::STRING);
+  if (value != nullptr)
+    *package_name_out = value->GetString();
+  callback.Run();
+}
+
+void CheckStatus(chromeos::DBusMethodCallStatus call_status) {
+  ASSERT_EQ(chromeos::DBUS_METHOD_CALL_SUCCESS, call_status);
+}
+
+std::string GetAlwaysOnPackageName() {
+  chromeos::DBusMethodCallStatus call_status =
+      chromeos::DBUS_METHOD_CALL_FAILURE;
+  std::string package_name;
+  chromeos::ShillManagerClient* shill_manager =
+      chromeos::DBusThreadManager::Get()->GetShillManagerClient();
+  base::RunLoop run_loop;
+  shill_manager->GetProperties(
+      base::Bind(&OnGetProperties, base::Unretained(&call_status),
+                 base::Unretained(&package_name), run_loop.QuitClosure()));
+  run_loop.Run();
+  CheckStatus(call_status);
+  return package_name;
+}
+
+}  // namespace
+
+namespace arc {
+namespace {
+
+class AlwaysOnVpnManagerTest : public testing::Test {
+ public:
+  AlwaysOnVpnManagerTest() = default;
+
+  void SetUp() override {
+    chromeos::DBusThreadManager::Initialize();
+    EXPECT_TRUE(chromeos::DBusThreadManager::Get()->IsUsingFakes());
+    chromeos::NetworkHandler::Initialize();
+    EXPECT_TRUE(chromeos::NetworkHandler::IsInitialized());
+    arc::prefs::RegisterProfilePrefs(pref_service()->registry());
+  }
+
+  void TearDown() override {
+    chromeos::NetworkHandler::Shutdown();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+
+  TestingPrefServiceSimple* pref_service() { return &pref_service_; }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingPrefServiceSimple pref_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(AlwaysOnVpnManagerTest);
+};
+
+TEST_F(AlwaysOnVpnManagerTest, SetPackageWhileLockdownUnset) {
+  auto always_on_manager = std::make_unique<AlwaysOnVpnManager>(pref_service());
+
+  EXPECT_EQ(std::string(), GetAlwaysOnPackageName());
+
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnPackage, kVpnPackageValue);
+
+  EXPECT_EQ(std::string(), GetAlwaysOnPackageName());
+}
+
+TEST_F(AlwaysOnVpnManagerTest, SetPackageWhileLockdownTrue) {
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnLockdown, base::Value(true));
+
+  auto always_on_manager = std::make_unique<AlwaysOnVpnManager>(pref_service());
+
+  EXPECT_EQ(std::string(), GetAlwaysOnPackageName());
+
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnPackage, kVpnPackageValue);
+
+  EXPECT_EQ(kVpnPackage, GetAlwaysOnPackageName());
+
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnPackage,
+                      base::Value(base::StringPiece()));
+
+  EXPECT_EQ(std::string(), GetAlwaysOnPackageName());
+}
+
+TEST_F(AlwaysOnVpnManagerTest, SetPackageThatsAlreadySetAtBoot) {
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnLockdown, base::Value(true));
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnPackage, kVpnPackageValue);
+
+  auto always_on_manager = std::make_unique<AlwaysOnVpnManager>(pref_service());
+
+  EXPECT_EQ(kVpnPackage, GetAlwaysOnPackageName());
+}
+
+TEST_F(AlwaysOnVpnManagerTest, SetLockdown) {
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnPackage, kVpnPackageValue);
+
+  auto always_on_manager = std::make_unique<AlwaysOnVpnManager>(pref_service());
+
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnLockdown, base::Value(true));
+
+  EXPECT_EQ(kVpnPackage, GetAlwaysOnPackageName());
+
+  pref_service()->Set(arc::prefs::kAlwaysOnVpnLockdown, base::Value(false));
+
+  EXPECT_EQ(std::string(), GetAlwaysOnPackageName());
+}
+
+}  // namespace
+}  // namespace arc

components/sync_wifi/wifi_config_delegate_chromeos_unittest.cc

@@ -59,6 +59,12 @@ class FakeManagedNetworkConfigurationHandler
                      const ErrorCallback& error_callback) override {
     NOTIMPLEMENTED();
   }
+  void SetManagerProperty(const std::string& property_name,
+                          const base::Value& value,
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) override {
+    NOTIMPLEMENTED();
+  }
   void CreateConfiguration(const std::string& userhash,
                            const base::DictionaryValue& properties,
                            const ServiceResultCallback& callback,