Abstract firewall proxy interface

Provide an abstraction for firewall proxy, to allow different proxy
implementations. Chrome OS will continue to use the implementation
based on permission_broker.

Bug: 23757625
TEST=Build an image with "wifi_bootstrapping" USE flag, boot it up
TEST=on a device. Verify a provisioning AP is up "Developer xxx"
TEST=and a client can connect to it.

Change-Id: I7d902d8741bb12f79b3b1458e114d6305cfb6339

6 files changed, 281 insertions, 145 deletions

diff --git a/apmanager.gyp b/apmanager.gyp
index 0489656..0d3433a 100644
--- a/apmanager.gyp
+++ b/apmanager.gyp
@@ -83,6 +83,7 @@
         'firewall_manager.cc',
         'hostapd_monitor.cc',
         'manager.cc',
+        'permission_broker_dbus_proxy.cc',
         'process_factory.cc',
         'service.cc',
         'shill_proxy.cc',
diff --git a/firewall_manager.cc b/firewall_manager.cc
index 64e7e75..765b2b5 100644
--- a/firewall_manager.cc
+++ b/firewall_manager.cc
@@ -19,167 +19,71 @@
 #include <base/bind.h>
 #include <chromeos/errors/error.h>
 
+// TODO(zqiu): should put this under a compiler flag.
+#include "apmanager/permission_broker_dbus_proxy.h"
+
 using std::string;
 
 namespace apmanager {
 
 namespace {
-
 const uint16_t kDhcpServerPort = 67;
-const int kInvalidFd = -1;
-
 }  // namespace
 
-FirewallManager::FirewallManager()
-    : lifeline_read_fd_(kInvalidFd),
-      lifeline_write_fd_(kInvalidFd) {}
+FirewallManager::FirewallManager() {}
 
-FirewallManager::~FirewallManager() {
-  if (lifeline_read_fd_ != kInvalidFd) {
-    close(lifeline_read_fd_);
-    close(lifeline_write_fd_);
-  }
-}
+FirewallManager::~FirewallManager() {}
 
 void FirewallManager::Init(const scoped_refptr<dbus::Bus>& bus) {
-  CHECK(!permission_broker_proxy_) << "Already started";
-
-  if (!SetupLifelinePipe()) {
-    return;
-  }
-
-  permission_broker_proxy_.reset(new org::chromium::PermissionBrokerProxy(bus));
-
-  // This will connect the name owner changed signal in DBus object proxy,
-  // The callback will be invoked as soon as service is avalilable. and will
-  // be cleared after it is invoked. So this will be an one time callback.
-  permission_broker_proxy_->GetObjectProxy()->WaitForServiceToBeAvailable(
-      base::Bind(&FirewallManager::OnServiceAvailable, base::Unretained(this)));
-
-  // This will continuously monitor the name owner of the service. However,
-  // it does not connect the name owner changed signal in DBus object proxy
-  // for some reason. In order to connect the name owner changed signal,
-  // either WaitForServiceToBeAvaiable or ConnectToSignal need to be invoked.
-  // Since we're not interested in any signals from the proxy,
-  // WaitForServiceToBeAvailable is used.
-  permission_broker_proxy_->GetObjectProxy()->SetNameOwnerChangedCallback(
-      base::Bind(&FirewallManager::OnServiceNameChanged,
-                 base::Unretained(this)));
+  CHECK(!firewall_proxy_) << "Already started";
+  // TODO(zqiu): should put this under a compiler flag.
+  firewall_proxy_.reset(
+      new PermissionBrokerDBusProxy(
+          bus,
+          base::Bind(&FirewallManager::OnFirewallServiceAppeared,
+                     weak_factory_.GetWeakPtr()),
+          base::Bind(&FirewallManager::OnFirewallServiceVanished,
+                     weak_factory_.GetWeakPtr())));
 }
 
 void FirewallManager::RequestDHCPPortAccess(const std::string& interface) {
-  CHECK(permission_broker_proxy_) << "Proxy not initialized yet";
+  CHECK(firewall_proxy_) << "Proxy not initialized yet";
   if (dhcp_access_interfaces_.find(interface) !=
       dhcp_access_interfaces_.end()) {
     LOG(ERROR) << "DHCP access already requested for interface: " << interface;
     return;
   }
-  RequestUdpPortAccess(interface, kDhcpServerPort);
+  firewall_proxy_->RequestUdpPortAccess(interface, kDhcpServerPort);
   dhcp_access_interfaces_.insert(interface);
 }
 
 void FirewallManager::ReleaseDHCPPortAccess(const std::string& interface) {
-  CHECK(permission_broker_proxy_) << "Proxy not initialized yet";
+  CHECK(firewall_proxy_) << "Proxy not initialized yet";
   if (dhcp_access_interfaces_.find(interface) ==
       dhcp_access_interfaces_.end()) {
     LOG(ERROR) << "DHCP access has not been requested for interface: "
                << interface;
     return;
   }
-  ReleaseUdpPortAccess(interface, kDhcpServerPort);
+  firewall_proxy_->ReleaseUdpPortAccess(interface, kDhcpServerPort);
   dhcp_access_interfaces_.erase(interface);
 }
 
-bool FirewallManager::SetupLifelinePipe() {
-  if (lifeline_read_fd_ != kInvalidFd) {
-    LOG(ERROR) << "Lifeline pipe already created";
-    return false;
-  }
-
-  // Setup lifeline pipe.
-  int fds[2];
-  if (pipe(fds) != 0) {
-    PLOG(ERROR) << "Failed to create lifeline pipe";
-    return false;
-  }
-  lifeline_read_fd_ = fds[0];
-  lifeline_write_fd_ = fds[1];
-
-  return true;
-}
-
-void FirewallManager::OnServiceAvailable(bool service_available) {
-  LOG(INFO) << "FirewallManager::OnServiceAvailabe " << service_available;
-  // Nothing to be done if proxy service is not available.
-  if (!service_available) {
-    return;
-  }
+void FirewallManager::OnFirewallServiceAppeared() {
+  LOG(INFO) << __func__;
   RequestAllPortsAccess();
 }
 
-void FirewallManager::OnServiceNameChanged(const string& old_owner,
-                                           const string& new_owner) {
-  LOG(INFO) << "FirewallManager::OnServiceNameChanged old " << old_owner
-            << " new " << new_owner;
-  // Nothing to be done if no owner is attached to the proxy service.
-  if (new_owner.empty()) {
-    return;
-  }
-  RequestAllPortsAccess();
+void FirewallManager::OnFirewallServiceVanished() {
+  // Nothing need to be done.
+  LOG(INFO) << __func__;
 }
 
 void FirewallManager::RequestAllPortsAccess() {
   // Request access to DHCP port for all specified interfaces.
   for (const auto& dhcp_interface : dhcp_access_interfaces_) {
-    RequestUdpPortAccess(dhcp_interface, kDhcpServerPort);
-  }
-}
-
-void FirewallManager::RequestUdpPortAccess(const string& interface,
-                                           uint16_t port) {
-  bool allowed = false;
-  // Pass the read end of the pipe to permission_broker, for it to monitor this
-  // process.
-  dbus::FileDescriptor fd(lifeline_read_fd_);
-  fd.CheckValidity();
-  chromeos::ErrorPtr error;
-  if (!permission_broker_proxy_->RequestUdpPortAccess(port,
-                                                      interface,
-                                                      fd,
-                                                      &allowed,
-                                                      &error)) {
-    LOG(ERROR) << "Failed to request UDP port access: "
-               << error->GetCode() << " " << error->GetMessage();
-    return;
-  }
-  if (!allowed) {
-    LOG(ERROR) << "Access request for UDP port " << port
-               << " on interface " << interface << " is denied";
-    return;
-  }
-  LOG(INFO) << "Access granted for UDP port " << port
-            << " on interface " << interface;;
-}
-
-void FirewallManager::ReleaseUdpPortAccess(const string& interface,
-                                           uint16_t port) {
-  chromeos::ErrorPtr error;
-  bool success;
-  if (!permission_broker_proxy_->ReleaseUdpPort(port,
-                                                interface,
-                                                &success,
-                                                &error)) {
-    LOG(ERROR) << "Failed to release UDP port access: "
-               << error->GetCode() << " " << error->GetMessage();
-    return;
-  }
-  if (!success) {
-    LOG(ERROR) << "Release request for UDP port " << port
-               << " on interface " << interface << " is denied";
-    return;
+    firewall_proxy_->RequestUdpPortAccess(dhcp_interface, kDhcpServerPort);
   }
-  LOG(INFO) << "Access released for UDP port " << port
-            << " on interface " << interface;
 }
 
 }  // namespace apmanager
diff --git a/firewall_manager.h b/firewall_manager.h
index 4dd4d04..401aca4 100644
--- a/firewall_manager.h
+++ b/firewall_manager.h
@@ -21,9 +21,11 @@
 #include <string>
 
 #include <base/macros.h>
-#include <base/memory/scoped_ptr.h>
+#include <base/memory/ref_counted.h>
+#include <base/memory/weak_ptr.h>
+#include <dbus/bus.h>
 
-#include "permission_broker/dbus-proxies.h"
+#include "apmanager/firewall_proxy_interface.h"
 
 // Class for managing required firewall rules for apmanager.
 namespace apmanager {
@@ -40,36 +42,21 @@ class FirewallManager final {
   void ReleaseDHCPPortAccess(const std::string& interface);
 
  private:
-  // Setup lifeline pipe to allow the remote firewall server
-  // (permission_broker) to monitor this process, so it can remove the firewall
-  // rules in case this process crashes.
-  bool SetupLifelinePipe();
+  // Invoked when remote firewall service appeared/vanished.
+  void OnFirewallServiceAppeared();
+  void OnFirewallServiceVanished();
 
-  void OnServiceAvailable(bool service_available);
-  void OnServiceNameChanged(const std::string& old_owner,
-                            const std::string& new_owner);
-
-  // This is called when a new instance of permission_broker is detected. Since
-  // the new instance doesn't have any knowledge of previously port access
-  // requests, re-issue those requests to permission_broker to get in sync.
+  // This is called when a new instance of firewall proxy is detected. Since
+  // the new instance doesn't have any knowledge of previous port access
+  // requests, re-issue those requests to the proxy to get in sync.
   void RequestAllPortsAccess();
 
-  // Request/release UDP port access for the specified interface and port.
-  void RequestUdpPortAccess(const std::string& interface, uint16_t port);
-  void ReleaseUdpPortAccess(const std::string& interface, uint16_t port);
-
-  // DBus proxy for permission_broker.
-  std::unique_ptr<org::chromium::PermissionBrokerProxy>
-      permission_broker_proxy_;
-  // File descriptors for the two end of the pipe use for communicating with
-  // remote firewall server (permission_broker), where the remote firewall
-  // server will use the read end of the pipe to detect when this process exits.
-  int lifeline_read_fd_;
-  int lifeline_write_fd_;
+  std::unique_ptr<FirewallProxyInterface> firewall_proxy_;
 
   // List of interfaces with DHCP port access.
   std::set<std::string> dhcp_access_interfaces_;
 
+  base::WeakPtrFactory<FirewallManager> weak_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(FirewallManager);
 };
 
diff --git a/firewall_proxy_interface.h b/firewall_proxy_interface.h
new file mode 100644
index 0000000..461ed53
--- /dev/null
+++ b/firewall_proxy_interface.h
@@ -0,0 +1,36 @@
+//
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef APMANAGER_FIREWALL_PROXY_INTERFACE_H_
+#define APMANAGER_FIREWALL_PROXY_INTERFACE_H_
+
+#include <string>
+
+namespace apmanager {
+
+class FirewallProxyInterface {
+ public:
+  virtual ~FirewallProxyInterface() {}
+
+  virtual bool RequestUdpPortAccess(const std::string& interface,
+                                    uint16_t port) = 0;
+  virtual bool ReleaseUdpPortAccess(const std::string& interface,
+                                    uint16_t port) = 0;
+};
+
+}  // namespace apmanager
+
+#endif  // APMANAGER_FIREWALL_PROXY_INTERFACE_H_
diff --git a/permission_broker_dbus_proxy.cc b/permission_broker_dbus_proxy.cc
new file mode 100644
index 0000000..62adcb0
--- /dev/null
+++ b/permission_broker_dbus_proxy.cc
@@ -0,0 +1,136 @@
+//
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "apmanager/permission_broker_dbus_proxy.h"
+
+#include <base/bind.h>
+#include <chromeos/errors/error.h>
+
+#include "apmanager/event_dispatcher.h"
+
+using std::string;
+
+namespace apmanager {
+
+PermissionBrokerDBusProxy::PermissionBrokerDBusProxy(
+    const scoped_refptr<dbus::Bus>& bus,
+    const base::Closure& service_appeared_callback,
+    const base::Closure& service_vanished_callback)
+    : proxy_(new org::chromium::PermissionBrokerProxy(bus)),
+      dispatcher_(EventDispatcher::GetInstance()),
+      service_appeared_callback_(service_appeared_callback),
+      service_vanished_callback_(service_vanished_callback),
+      service_available_(false) {
+  // Monitor service owner changes. This callback lives for the lifetime of
+  // the ObjectProxy.
+  proxy_->GetObjectProxy()->SetNameOwnerChangedCallback(
+      base::Bind(&PermissionBrokerDBusProxy::OnServiceOwnerChanged,
+                 weak_factory_.GetWeakPtr()));
+
+  // One time callback when service becomes available.
+  proxy_->GetObjectProxy()->WaitForServiceToBeAvailable(
+      base::Bind(&PermissionBrokerDBusProxy::OnServiceAvailable,
+                 weak_factory_.GetWeakPtr()));
+
+  // Setup lifeline pipe to allow permission_broker to monitor this process,
+  // so it can automatically remove the firewall rules when this process
+  // crashed.
+  int fds[2];
+  CHECK(pipe(fds) == 0) << "Failed to create lifeline pipe";
+  lifeline_read_fd_ = fds[0];
+  lifeline_write_fd_ = fds[1];
+}
+
+PermissionBrokerDBusProxy::~PermissionBrokerDBusProxy() {
+  close(lifeline_read_fd_);
+  close(lifeline_write_fd_);
+}
+
+bool PermissionBrokerDBusProxy::RequestUdpPortAccess(const string& interface,
+                                                     uint16_t port) {
+  if (!service_available_) {
+    LOG(ERROR) << "permission_broker service not available";
+    return false;
+  }
+
+  // Pass the read end of the pipe to permission_broker, to allow it to
+  // monitor on this process.
+  dbus::FileDescriptor fd(lifeline_read_fd_);
+  fd.CheckValidity();
+  bool allowed = false;
+  chromeos::ErrorPtr error;
+  if (!proxy_->RequestUdpPortAccess(port, interface, fd, &allowed, &error)) {
+    LOG(ERROR) << "Failed to request UDP port access: "
+               << error->GetCode() << " " << error->GetMessage();
+    return false;
+  }
+  if (!allowed) {
+    LOG(ERROR) << "Access request for UDP port " << port
+               << " on interface " << interface << " is denied";
+    return false;
+  }
+  LOG(INFO) << "Access granted for UDP port " << port
+            << " on interface " << interface;
+  return true;
+}
+
+bool PermissionBrokerDBusProxy::ReleaseUdpPortAccess(const string& interface,
+                                                     uint16_t port) {
+  if (!service_available_) {
+    LOG(ERROR) << "permission_broker service not available";
+    return false;
+  }
+
+  chromeos::ErrorPtr error;
+  bool success;
+  if (!proxy_->ReleaseUdpPort(port, interface, &success, &error)) {
+    LOG(ERROR) << "Failed to release UDP port access: "
+               << error->GetCode() << " " << error->GetMessage();
+    return false;
+  }
+  if (!success) {
+    LOG(ERROR) << "Release request for UDP port " << port
+               << " on interface " << interface << " is denied";
+    return false;
+  }
+  LOG(INFO) << "Access released for UDP port " << port
+            << " on interface " << interface;
+  return true;
+}
+
+void PermissionBrokerDBusProxy::OnServiceAvailable(bool available) {
+  LOG(INFO) << __func__ << ": " << available;
+  // The callback might invoke calls to the ObjectProxy, so defer the callback
+  // to event loop.
+  if (available && !service_appeared_callback_.is_null()) {
+    dispatcher_->PostTask(service_appeared_callback_);
+  } else if (!available && !service_vanished_callback_.is_null()) {
+    dispatcher_->PostTask(service_vanished_callback_);
+  }
+  service_available_ = available;
+}
+
+void PermissionBrokerDBusProxy::OnServiceOwnerChanged(const string& old_owner,
+                                                     const string& new_owner) {
+  LOG(INFO) << __func__ << " old: " << old_owner << " new: " << new_owner;
+  if (new_owner.empty()) {
+    OnServiceAvailable(false);
+  } else {
+    OnServiceAvailable(true);
+  }
+}
+
+}  // namespace apmanager
diff --git a/permission_broker_dbus_proxy.h b/permission_broker_dbus_proxy.h
new file mode 100644
index 0000000..6c4d050
--- /dev/null
+++ b/permission_broker_dbus_proxy.h
@@ -0,0 +1,72 @@
+//
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef APMANAGER_PERMISSION_BROKER_DBUS_PROXY_H_
+#define APMANAGER_PERMISSION_BROKER_DBUS_PROXY_H_
+
+#include <string>
+
+#include <base/macros.h>
+#include <base/memory/scoped_ptr.h>
+#include <permission_broker/dbus-proxies.h>
+
+#include "apmanager/firewall_proxy_interface.h"
+
+namespace apmanager {
+
+class EventDispatcher;
+
+class PermissionBrokerDBusProxy : public FirewallProxyInterface {
+ public:
+  PermissionBrokerDBusProxy(const scoped_refptr<dbus::Bus>& bus,
+                            const base::Closure& service_appeared_callback,
+                            const base::Closure& service_vanished_callback);
+  ~PermissionBrokerDBusProxy() override;
+
+  // Request/release access for an UDP port |port} on an interface |interface|.
+  bool RequestUdpPortAccess(const std::string& interface,
+                            uint16_t port) override;
+  bool ReleaseUdpPortAccess(const std::string& interface,
+                            uint16_t port) override;
+
+ private:
+  // Called when service appeared or vanished.
+  void OnServiceAvailable(bool service_available);
+
+  // Service name owner changed handler.
+  void OnServiceOwnerChanged(const std::string& old_owner,
+                            const std::string& new_owner);
+
+  std::unique_ptr<org::chromium::PermissionBrokerProxy> proxy_;
+  EventDispatcher* dispatcher_;
+
+  // File descriptors for the two end of the pipe use for communicating with
+  // permission_broker, where it will use the read end of the pipe to detect
+  // when this process exits.
+  int lifeline_read_fd_;
+  int lifeline_write_fd_;
+
+  base::Closure service_appeared_callback_;
+  base::Closure service_vanished_callback_;
+  bool service_available_;
+
+  base::WeakPtrFactory<PermissionBrokerDBusProxy> weak_factory_{this};
+  DISALLOW_COPY_AND_ASSIGN(PermissionBrokerDBusProxy);
+};
+
+}  // namespace apmanager
+
+#endif  // APMANAGER_PERMISSION_BROKER_DBUS_PROXY_H_