firewalld: Add D-Bus ObjectManager to track the Firewalld service lifetime

Added ObjectManager to firewalld to allow permission_broker to track the
lifetime of the top D-Bus service object and restart permission_broker
if firewall crashes/restarts.

This will also allow to wait for Firewalld to come up and finish initialization
before permission_broker D-Bus appears on the bus, which would eliminate
weird race condiftions when web server asks permission_broker to open
TCP ports too early (before firewalld is up and running).

BUG=brillo:1240
TEST=`FEATURES=test emerge-link firewalld permission_broker apmanager webserver`

Change-Id: I1f575b74c6a1e8e75cd4d33b6b70dda5b95f5339
Reviewed-on: https://chromium-review.googlesource.com/284975
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>

8 files changed, 23 insertions, 11 deletions

diff --git a/dbus_bindings/dbus-service-config.json b/dbus_bindings/dbus-service-config.json
new file mode 100644
index 0000000..f62c2d2
--- /dev/null
+++ b/dbus_bindings/dbus-service-config.json
@@ -0,0 +1,6 @@
+{
+  "service_name": "org.chromium.Firewalld",
+  "object_manager": {
+    "object_path": "/org/chromium/Firewalld"
+  }
+}
diff --git a/dbus_bindings/org.chromium.Firewalld.xml b/dbus_bindings/org.chromium.Firewalld.xml
index 0328c98..97ea215 100644
--- a/dbus_bindings/org.chromium.Firewalld.xml
+++ b/dbus_bindings/org.chromium.Firewalld.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="utf-8" ?>
-<node name="/org/chromium/Firewalld">
+<node name="/org/chromium/Firewalld/Firewall">
   <interface name="org.chromium.Firewalld">
     <method name="PunchTcpHole">
       <arg type="q" name="port" direction="in" />
diff --git a/dbus_interface.h b/dbus_interface.h
index a1fe761..9ba2fb2 100644
--- a/dbus_interface.h
+++ b/dbus_interface.h
@@ -7,7 +7,6 @@
 
 namespace firewalld {
 
-const char kFirewallInterface[] = "org.chromium.Firewalld";
 const char kFirewallServicePath[] = "/org/chromium/Firewalld";
 const char kFirewallServiceName[] = "org.chromium.Firewalld";
 
diff --git a/firewall_daemon.cc b/firewall_daemon.cc
index bcc7404..a72c093 100644
--- a/firewall_daemon.cc
+++ b/firewall_daemon.cc
@@ -11,8 +11,14 @@
 
 namespace firewalld {
 
+FirewallDaemon::FirewallDaemon()
+    : chromeos::DBusServiceDaemon{kFirewallServiceName,
+                                  dbus::ObjectPath{kFirewallServicePath}} {
+}
+
 void FirewallDaemon::RegisterDBusObjectsAsync(AsyncEventSequencer* sequencer) {
-  firewall_service_.reset(new firewalld::FirewallService(bus_));
+  firewall_service_.reset(
+      new firewalld::FirewallService{object_manager_.get()});
   firewall_service_->RegisterAsync(
       sequencer->GetHandler("Service.RegisterAsync() failed.", true));
 }
diff --git a/firewall_daemon.h b/firewall_daemon.h
index b452247..19debf4 100644
--- a/firewall_daemon.h
+++ b/firewall_daemon.h
@@ -18,7 +18,7 @@ namespace firewalld {
 
 class FirewallDaemon : public chromeos::DBusServiceDaemon {
  public:
-  FirewallDaemon() : chromeos::DBusServiceDaemon(kFirewallServiceName) {}
+  FirewallDaemon();
 
  protected:
   void RegisterDBusObjectsAsync(AsyncEventSequencer* sequencer) override;
diff --git a/firewall_service.cc b/firewall_service.cc
index 2bc5645..6b2c4d4 100644
--- a/firewall_service.cc
+++ b/firewall_service.cc
@@ -9,10 +9,11 @@
 
 namespace firewalld {
 
-FirewallService::FirewallService(const scoped_refptr<dbus::Bus>& bus)
+FirewallService::FirewallService(
+    chromeos::dbus_utils::ExportedObjectManager* object_manager)
     : org::chromium::FirewalldAdaptor(&iptables_),
-      dbus_object_{nullptr, bus, dbus::ObjectPath{kFirewallServicePath}},
-      weak_ptr_factory_{this} {}
+      dbus_object_{object_manager, object_manager->GetBus(),
+                   org::chromium::FirewalldAdaptor::GetObjectPath()} {}
 
 void FirewallService::RegisterAsync(const CompletionAction& callback) {
   RegisterWithDBusObject(&dbus_object_);
diff --git a/firewall_service.h b/firewall_service.h
index 6ffc2c0..ba91f6d 100644
--- a/firewall_service.h
+++ b/firewall_service.h
@@ -23,7 +23,8 @@ namespace firewalld {
 
 class FirewallService : public org::chromium::FirewalldAdaptor {
  public:
-  explicit FirewallService(const scoped_refptr<dbus::Bus>& bus);
+  explicit FirewallService(
+      chromeos::dbus_utils::ExportedObjectManager* object_manager);
   virtual ~FirewallService() = default;
 
   // Connects to D-Bus system bus and exports methods.
@@ -37,7 +38,7 @@ class FirewallService : public org::chromium::FirewalldAdaptor {
       permission_broker_;
   IpTables iptables_;
 
-  base::WeakPtrFactory<FirewallService> weak_ptr_factory_;
+  base::WeakPtrFactory<FirewallService> weak_ptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(FirewallService);
 };
 
diff --git a/firewalld.gyp b/firewalld.gyp
index e243411..ffb7773 100644
--- a/firewalld.gyp
+++ b/firewalld.gyp
@@ -22,7 +22,6 @@
           'variables': {
             'dbus_service_config': '<(platform2_root)/permission_broker/dbus_bindings/dbus-service-config.json',
             'proxy_output_file': 'include/permission_broker/dbus-proxies.h',
-            'dbus_adaptors_out_dir': '',
           },
           'sources': [
             '<(platform2_root)/permission_broker/dbus_bindings/org.chromium.PermissionBroker.xml',
@@ -35,7 +34,7 @@
       'target_name': 'firewalld-dbus-adaptor',
       'type': 'none',
       'variables': {
-        'dbus_adaptors_type': 'adaptor',
+        'dbus_service_config': 'dbus_bindings/dbus-service-config.json',
         'dbus_adaptors_out_dir': 'include/firewalld/dbus_adaptor',
       },
       'sources': [