Crostini port forwarding activate and add port

Port forwarding in Cros will allow users to add new ports and activate,
deactivate and remove existing ports.

When any of the utilities are called, the port forwarding library will
invoke the permission broker client to change the ChromeOS port
forwarding settings.

In future work we will also save port forwarding preferences such that a
user's preferences will persist over container shutdown.

Bug: chromium:848127
Test: crostini_port_forwarder_unittest.cc and everything builds
Change-Id: I9a0478f1486252fb09fab18ab3261e03d00eb480
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999972Reviewed-by: David Munro <davidmunro@google.com>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Nic Hollingum <hollingum@google.com>
Commit-Queue: Matthew Chen <matterchen@google.com>
Cr-Commit-Position: refs/heads/master@{#734303}

chrome/browser/chromeos/BUILD.gn

@@ -875,6 +875,8 @@ source_set("chromeos") {
     "crostini/crostini_package_operation_status.h",
     "crostini/crostini_package_service.cc",
     "crostini/crostini_package_service.h",
+    "crostini/crostini_port_forwarder.cc",
+    "crostini/crostini_port_forwarder.h",
     "crostini/crostini_pref_names.cc",
     "crostini/crostini_pref_names.h",
     "crostini/crostini_registry_service.cc",
@@ -2715,6 +2717,7 @@ source_set("unit_tests") {
     "crostini/crostini_mime_types_service_unittest.cc",
     "crostini/crostini_package_notification_unittest.cc",
     "crostini/crostini_package_service_unittest.cc",
+    "crostini/crostini_port_forwarder_unittest.cc",
     "crostini/crostini_registry_service_unittest.cc",
     "crostini/crostini_reporting_util_unittest.cc",
     "crostini/crostini_unsupported_action_notifier_unittest.cc",

chrome/browser/chromeos/crostini/crostini_port_forwarder.cc

+// Copyright 2020 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 <fcntl.h>
+
+#include "chrome/browser/chromeos/crostini/crostini_port_forwarder.h"
+
+#include "base/bind.h"
+#include "base/no_destructor.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+namespace crostini {
+
+// Currently, we are not supporting ethernet/mlan/usb port forwarding.
+constexpr char kDefaultInterfaceToForward[] = "wlan0";
+
+class CrostiniPortForwarderFactory : public BrowserContextKeyedServiceFactory {
+ public:
+  static CrostiniPortForwarder* GetForProfile(Profile* profile) {
+    return static_cast<CrostiniPortForwarder*>(
+        GetInstance()->GetServiceForBrowserContext(profile, true));
+  }
+
+  static CrostiniPortForwarderFactory* GetInstance() {
+    static base::NoDestructor<CrostiniPortForwarderFactory> factory;
+    return factory.get();
+  }
+
+ private:
+  friend class base::NoDestructor<CrostiniPortForwarderFactory>;
+
+  CrostiniPortForwarderFactory()
+      : BrowserContextKeyedServiceFactory(
+            "CrostiniPortForwarderService",
+            BrowserContextDependencyManager::GetInstance()) {}
+
+  ~CrostiniPortForwarderFactory() override = default;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override {
+    Profile* profile = Profile::FromBrowserContext(context);
+    return new CrostiniPortForwarder(profile);
+  }
+};
+
+CrostiniPortForwarder* CrostiniPortForwarder::GetForProfile(Profile* profile) {
+  return CrostiniPortForwarderFactory::GetForProfile(profile);
+}
+
+CrostiniPortForwarder::CrostiniPortForwarder(Profile* profile)
+    : profile_(profile) {
+  // TODO(matterchen): Profile will be used later, this is to remove warnings.
+  (void)profile_;
+}
+
+CrostiniPortForwarder::~CrostiniPortForwarder() = default;
+
+void CrostiniPortForwarder::OnActivatePortCompleted(
+    ResultCallback result_callback,
+    const PortRuleKey& key,
+    bool success) {
+  if (!success) {
+    forwarded_ports_.erase(key);
+    LOG(ERROR) << "Failed to activate port, port preference not added: "
+               << key.port_number;
+    std::move(result_callback).Run(success);
+    return;
+  }
+  // TODO(matterchen): Update current port forwarding preference.
+  std::move(result_callback).Run(success);
+}
+
+void CrostiniPortForwarder::OnAddPortCompleted(ResultCallback result_callback,
+                                               const std::string& label,
+                                               const PortRuleKey& key,
+                                               bool success) {
+  if (!success) {
+    forwarded_ports_.erase(key);
+    LOG(ERROR) << "Failed to activate port, port preference not added: "
+               << key.port_number;
+    std::move(result_callback).Run(success);
+    return;
+  }
+  // TODO(matterchen): Add new port forwarding preference.
+  std::move(result_callback).Run(success);
+}
+
+void CrostiniPortForwarder::TryActivatePort(
+    uint16_t port_number,
+    const Protocol& protocol_type,
+    const std::string& ipv4_addr,
+    chromeos::PermissionBrokerClient::ResultCallback result_callback) {
+  chromeos::PermissionBrokerClient* client =
+      chromeos::PermissionBrokerClient::Get();
+  if (!client) {
+    LOG(ERROR) << "Could not get permission broker client.";
+    std::move(result_callback).Run(false);
+    return;
+  }
+
+  int lifeline[2] = {-1, -1};
+  if (pipe(lifeline) < 0) {
+    LOG(ERROR) << "Failed to create a lifeline pipe";
+    std::move(result_callback).Run(false);
+    return;
+  }
+
+  PortRuleKey port_key = {
+      .port_number = port_number,
+      .protocol_type = protocol_type,
+      .input_ifname = kDefaultInterfaceToForward,
+  };
+
+  base::ScopedFD lifeline_local(lifeline[0]);
+  base::ScopedFD lifeline_remote(lifeline[1]);
+
+  forwarded_ports_[port_key] = std::move(lifeline_local);
+
+  if (Protocol::TCP == protocol_type) {
+    client->RequestTcpPortForward(
+        port_number, kDefaultInterfaceToForward, ipv4_addr, port_number,
+        std::move(lifeline_remote.get()), std::move(result_callback));
+  } else if (Protocol::UDP == protocol_type) {
+    client->RequestUdpPortForward(
+        port_number, kDefaultInterfaceToForward, ipv4_addr, port_number,
+        std::move(lifeline_remote.get()), std::move(result_callback));
+  }
+}
+
+void CrostiniPortForwarder::AddPort(uint16_t port_number,
+                                    const Protocol& protocol_type,
+                                    const std::string& label,
+                                    ResultCallback result_callback) {
+  PortRuleKey new_port_key = {
+      .port_number = port_number,
+      .protocol_type = protocol_type,
+      .input_ifname = kDefaultInterfaceToForward,
+  };
+
+  if (forwarded_ports_.find(new_port_key) != forwarded_ports_.end()) {
+    LOG(ERROR) << "Trying to add an already forwarded port.";
+    std::move(result_callback).Run(false);
+    return;
+  }
+
+  base::OnceCallback<void(bool)> on_add_port_completed =
+      base::BindOnce(&CrostiniPortForwarder::OnAddPortCompleted,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(result_callback),
+                     label, new_port_key);
+
+  // TODO(matterchen): Extract container IPv4 address.
+  TryActivatePort(port_number, protocol_type, "PLACEHOLDER_IP_ADDRESS",
+                  std::move(on_add_port_completed));
+}
+
+void CrostiniPortForwarder::ActivatePort(uint16_t port_number,
+                                         ResultCallback result_callback) {
+  // TODO(matterchen): Find the current port setting from profile preferences.
+  PortRuleKey existing_port_key = {
+      .port_number = port_number,
+      .protocol_type = Protocol::TCP,
+      .input_ifname = kDefaultInterfaceToForward,
+  };
+
+  base::OnceCallback<void(bool)> on_activate_port_completed =
+      base::BindOnce(&CrostiniPortForwarder::OnActivatePortCompleted,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(result_callback),
+                     existing_port_key);
+
+  // TODO(matterchen): Extract container IPv4 address.
+  CrostiniPortForwarder::TryActivatePort(port_number, Protocol::TCP,
+                                         "PLACEHOLDER_IP_ADDRESS",
+                                         std::move(on_activate_port_completed));
+}
+
+}  // namespace crostini

chrome/browser/chromeos/crostini/crostini_port_forwarder.h

+// Copyright 2020 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 CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_PORT_FORWARDER_H_
+#define CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_PORT_FORWARDER_H_
+
+#include <string>
+
+#include "base/files/scoped_file.h"
+#include "base/memory/weak_ptr.h"
+#include "base/values.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+class Profile;
+
+namespace crostini {
+
+class CrostiniPortForwarder : public KeyedService {
+ public:
+  enum class Protocol {
+    TCP,
+    UDP,
+  };
+
+  struct PortRuleKey {
+    uint16_t port_number;
+    const Protocol& protocol_type;
+    std::string input_ifname;
+
+    bool operator==(const PortRuleKey& other) const {
+      return port_number == other.port_number &&
+             protocol_type == other.protocol_type &&
+             input_ifname == other.input_ifname;
+    }
+  };
+
+  // Helper for using PortRuleKey as key entries in std::unordered_maps.
+  struct PortRuleKeyHasher {
+    std::size_t operator()(const PortRuleKey& k) const {
+      return ((std::hash<uint16_t>()(k.port_number) ^
+               (std::hash<Protocol>()(k.protocol_type) << 1)) >>
+              1) ^
+             (std::hash<std::string>()(k.input_ifname) << 1);
+    }
+  };
+
+  using ResultCallback = base::OnceCallback<void(bool)>;
+  void ActivatePort(uint16_t port_number, ResultCallback result_callback);
+  void AddPort(uint16_t port_number,
+               const Protocol& protocol_type,
+               const std::string& label,
+               ResultCallback result_callback);
+
+  static CrostiniPortForwarder* GetForProfile(Profile* profile);
+
+  explicit CrostiniPortForwarder(Profile* profile);
+  ~CrostiniPortForwarder() override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(CrostiniPortForwarderTest, AddPortDuplicateFail);
+  FRIEND_TEST_ALL_PREFIXES(CrostiniPortForwarderTest, AddPortMultipleSuccess);
+  FRIEND_TEST_ALL_PREFIXES(CrostiniPortForwarderTest, AddPortUdpAndTcpSuccess);
+  FRIEND_TEST_ALL_PREFIXES(CrostiniPortForwarderTest,
+                           TryActivatePortPermissionBrokerClientFail);
+  void OnActivatePortCompleted(ResultCallback result_callback,
+                               const PortRuleKey& key,
+                               bool success);
+  void OnAddPortCompleted(ResultCallback result_callback,
+                          const std::string& label,
+                          const PortRuleKey& key,
+                          bool success);
+  void TryActivatePort(uint16_t port_number,
+                       const Protocol& protocol_type,
+                       const std::string& ipv4_addr,
+                       base::OnceCallback<void(bool)> result_callback);
+
+  // For each port rule (protocol, port, interface), keep track of the fd which
+  // requested it so we can release it on removal / deactivate.
+  std::unordered_map<PortRuleKey, base::ScopedFD, PortRuleKeyHasher>
+      forwarded_ports_;
+
+  Profile* profile_;
+
+  base::WeakPtrFactory<CrostiniPortForwarder> weak_ptr_factory_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(CrostiniPortForwarder);
+
+};  // class
+
+}  // namespace crostini
+
+#endif  // CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_PORT_FORWARDER_H_

chrome/browser/chromeos/crostini/crostini_port_forwarder_unittest.cc

+// Copyright 2020 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 "chrome/browser/chromeos/crostini/crostini_port_forwarder.h"
+
+#include "chrome/browser/chromeos/crostini/crostini_test_helper.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chromeos/dbus/permission_broker/fake_permission_broker_client.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::Mock;
+using testing::Return;
+
+void TestingCallback(bool* out, bool in) {
+  *out = in;
+}
+
+namespace crostini {
+
+class CrostiniPortForwarderTest : public testing::Test {
+ public:
+  CrostiniPortForwarderTest() {}
+
+  ~CrostiniPortForwarderTest() override {}
+
+  void SetUp() override {
+    chromeos::PermissionBrokerClient::InitializeFake();
+    profile_ = std::make_unique<TestingProfile>();
+    test_helper_ = std::make_unique<CrostiniTestHelper>(profile_.get());
+    crostini_port_forwarder_ =
+        std::make_unique<CrostiniPortForwarder>(profile());
+  }
+
+  void TearDown() override {
+    chromeos::PermissionBrokerClient::Shutdown();
+    crostini_port_forwarder_.reset();
+    test_helper_.reset();
+    profile_.reset();
+  }
+
+ protected:
+  Profile* profile() { return profile_.get(); }
+
+  std::unique_ptr<CrostiniTestHelper> test_helper_;
+  std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<CrostiniPortForwarder> crostini_port_forwarder_;
+  content::BrowserTaskEnvironment task_environment_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CrostiniPortForwarderTest);
+};
+
+TEST_F(CrostiniPortForwarderTest, AddPortTcpSuccess) {
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+
+  bool success = false;
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::TCP,
+                                    "label0",
+                                    base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+}
+
+TEST_F(CrostiniPortForwarderTest, AddPortUdpSuccess) {
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+
+  bool success = false;
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::UDP,
+                                    "label0",
+                                    base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+}
+
+TEST_F(CrostiniPortForwarderTest, AddPortDuplicateFail) {
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+
+  bool success = false;
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::UDP,
+                                    "label0",
+                                    base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 1U);
+
+  // Leave success as == true.
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::UDP,
+                                    "label1",
+                                    base::BindOnce(&TestingCallback, &success));
+  EXPECT_FALSE(success);
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_TRUE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 1U);
+}
+
+TEST_F(CrostiniPortForwarderTest, AddPortUdpAndTcpSuccess) {
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+
+  bool success = false;
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::UDP,
+                                    "label0",
+                                    base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+
+  success = false;
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::TCP,
+                                    "label0",
+                                    base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 2U);
+}
+
+TEST_F(CrostiniPortForwarderTest, AddPortMultipleSuccess) {
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 0U);
+  crostini_port_forwarder_->AddPort(5000, CrostiniPortForwarder::Protocol::UDP,
+                                    "label0", base::DoNothing());
+  crostini_port_forwarder_->AddPort(5001, CrostiniPortForwarder::Protocol::TCP,
+                                    "label1", base::DoNothing());
+  crostini_port_forwarder_->AddPort(5002, CrostiniPortForwarder::Protocol::UDP,
+                                    "label2", base::DoNothing());
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 3U);
+}
+
+TEST_F(CrostiniPortForwarderTest, ActivatePortTcpSuccess) {
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+
+  bool success = false;
+  crostini_port_forwarder_->ActivatePort(
+      5000, base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(chromeos::FakePermissionBrokerClient::Get()->HasTcpPortForward(
+      5000, "wlan0"));
+  EXPECT_FALSE(chromeos::FakePermissionBrokerClient::Get()->HasUdpPortForward(
+      5000, "wlan0"));
+}
+
+TEST_F(CrostiniPortForwarderTest, TryActivatePortPermissionBrokerClientFail) {
+  bool success = false;
+  crostini_port_forwarder_->TryActivatePort(
+      5000, CrostiniPortForwarder::Protocol::TCP, "label0",
+      base::BindOnce(&TestingCallback, &success));
+  EXPECT_TRUE(success);
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 1U);
+  chromeos::PermissionBrokerClient::Shutdown();
+
+  // Leave success as == true.
+  crostini_port_forwarder_->TryActivatePort(
+      5001, CrostiniPortForwarder::Protocol::TCP, "label1",
+      base::BindOnce(&TestingCallback, &success));
+  EXPECT_FALSE(success);
+  EXPECT_EQ(crostini_port_forwarder_->forwarded_ports_.size(), 1U);
+  // Re-initialize otherwise Shutdown in TearDown phase will break.
+  chromeos::PermissionBrokerClient::InitializeFake();
+}
+
+}  // namespace crostini

chromeos/dbus/permission_broker/fake_permission_broker_client.cc

@@ -135,6 +135,20 @@ bool FakePermissionBrokerClient::HasUdpHole(uint16_t port,
   return udp_hole_set_.find(rule) != udp_hole_set_.end();
 }
 
+bool FakePermissionBrokerClient::HasTcpPortForward(
+    uint16_t port,
+    const std::string& interface) {
+  auto rule = std::make_pair(port, interface);
+  return tcp_forwarding_set_.find(rule) != tcp_forwarding_set_.end();
+}
+
+bool FakePermissionBrokerClient::HasUdpPortForward(
+    uint16_t port,
+    const std::string& interface) {
+  auto rule = std::make_pair(port, interface);
+  return udp_forwarding_set_.find(rule) != udp_forwarding_set_.end();
+}
+
 void FakePermissionBrokerClient::RequestTcpPortForward(
     uint16_t in_port,
     const std::string& in_interface,
@@ -142,7 +156,10 @@ void FakePermissionBrokerClient::RequestTcpPortForward(
     uint16_t dst_port,
     int lifeline_fd,
     ResultCallback callback) {
-  std::move(callback).Run(false);
+  // TODO(matterchen): Increase logic for adding duplicate ports.
+  auto rule = std::make_pair(in_port, in_interface);
+  tcp_forwarding_set_.insert(rule);
+  std::move(callback).Run(true);
 }
 
 void FakePermissionBrokerClient::RequestUdpPortForward(
@@ -152,7 +169,9 @@ void FakePermissionBrokerClient::RequestUdpPortForward(
     uint16_t dst_port,
     int lifeline_fd,
     ResultCallback callback) {
-  std::move(callback).Run(false);
+  auto rule = std::make_pair(in_port, in_interface);
+  udp_forwarding_set_.insert(rule);
+  std::move(callback).Run(true);
 }
 
 void FakePermissionBrokerClient::ReleaseTcpPortForward(

chromeos/dbus/permission_broker/fake_permission_broker_client.h

@@ -76,6 +76,12 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) FakePermissionBrokerClient
   // Returns true if UDP port has a hole.
   bool HasUdpHole(uint16_t port, const std::string& interface);
 
+  // Returns true if TCP port is being forwarded.
+  bool HasTcpPortForward(uint16_t port, const std::string& interface);
+
+  // Returns true if UDP port is being forwarded.
+  bool HasUdpPortForward(uint16_t port, const std::string& interface);
+
  private:
   using RuleSet =
       std::set<std::pair<uint16_t /* port */, std::string /* interface */>>;
@@ -88,6 +94,9 @@ class COMPONENT_EXPORT(PERMISSION_BROKER) FakePermissionBrokerClient
   RuleSet tcp_hole_set_;
   RuleSet udp_hole_set_;
 
+  RuleSet tcp_forwarding_set_;
+  RuleSet udp_forwarding_set_;
+
   RuleSet tcp_deny_rule_set_;
   RuleSet udp_deny_rule_set_;