Open a firewall hole when a TCP server or UDP socket is bound.

chromeos/network/firewall_hole.cc

+// Copyright 2015 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 "chromeos/network/firewall_hole.h"
+
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/sys_info.h"
+#include "base/threading/worker_pool.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/permission_broker_client.h"
+#include "dbus/file_descriptor.h"
+
+namespace chromeos {
+
+namespace {
+
+// Creates a pair of file descriptors that form a "lifeline" between Chrome and
+// firewalld. If this pipe is closed unexpectedly (i.e. Chrome crashes) then
+// firewalld will notice and close the hole in the firewall.
+void CreateValidLifeline(dbus::FileDescriptor* lifeline_local,
+                         dbus::FileDescriptor* lifeline_remote) {
+  int lifeline[2] = {-1, -1};
+  if (pipe2(lifeline, O_CLOEXEC) < 0) {
+    PLOG(ERROR) << "Failed to create a lifeline pipe";
+    return;
+  }
+
+  lifeline_local->PutValue(lifeline[0]);
+  lifeline_local->CheckValidity();
+
+  lifeline_remote->PutValue(lifeline[1]);
+  lifeline_remote->CheckValidity();
+}
+
+const char* PortTypeToString(FirewallHole::PortType type) {
+  switch (type) {
+    case FirewallHole::PortType::TCP:
+      return "TCP";
+    case FirewallHole::PortType::UDP:
+      return "UDP";
+    default:

[pneubeck (no reviews), 2015/03/08 03:35:01]

could you just add a 'return' after the switch instead of using 'default'?
Without default you get a nice compile time error if a case is missing. With
default+NOTREACHED you only get a runtime error if you ever execute the missing
case.

[Reilly Grant (use Gerrit), 2015/03/09 19:57:06]

Done.

+      NOTREACHED();
+      return nullptr;
+  }
+}
+
+void PortReleased(FirewallHole::PortType type,
+                  uint16_t port,
+                  const std::string& interface,
+                  FirewallHole::ScopedFileDescriptor lifeline_fd,
+                  bool success) {
+  if (!success) {
+    LOG(WARNING) << "Failed to release firewall hole for "
+                 << PortTypeToString(type) << " port " << port << " on "
+                 << interface << ".";
+  }
+}
+
+}  // namespace
+
+void CHROMEOS_EXPORT FirewallHole::FileDescriptorDeleter::operator()(
+    dbus::FileDescriptor* fd) {
+  base::WorkerPool::PostTask(
+      FROM_HERE, base::Bind(&base::DeletePointer<dbus::FileDescriptor>, fd),
+      false);
+}
+
+// static
+void FirewallHole::Open(PortType type,
+                        uint16_t port,
+                        const std::string& interface,
+                        const OpenCallback& callback) {
+  ScopedFileDescriptor lifeline_local(new dbus::FileDescriptor());
+  ScopedFileDescriptor lifeline_remote(new dbus::FileDescriptor());
+
+  // This closure shares pointers with the one below. PostTaskAndReply
+  // guarantees that it will always be deleted first.
+  base::Closure create_lifeline_closure = base::Bind(
+      &CreateValidLifeline, lifeline_local.get(), lifeline_remote.get());
+
+  base::WorkerPool::PostTaskAndReply(
+      FROM_HERE, create_lifeline_closure,
+      base::Bind(&RequestPortAccess, type, port, interface,
+                 base::Passed(&lifeline_local), base::Passed(&lifeline_remote),
+                 callback),
+      false);
+}
+
+FirewallHole::~FirewallHole() {
+  base::Callback<void(bool)> port_released_closure = base::Bind(
+      &PortReleased, type_, port_, interface_, base::Passed(&lifeline_fd_));
+
+  if (base::SysInfo::IsRunningOnChromeOS()) {
+    PermissionBrokerClient* client =

[pneubeck (no reviews), 2015/03/08 03:35:01]

will you be even able to test this code path in browser/unit_tests executed on
linux?
Looking at the implementation of SysInfo::IsRunningOnChromeOS, it is uncommon to
fake the OS version in tests:
https://code.google.com/p/chromium/codesearch#search/&q=SysInfo::SetChromeOSV...

Is there a reason why you have to make this conditional depending on whether it
runs on Linux or ChromeOS ?
I'd leave that conditional IsRunningOnChromeOS() out.

[Reilly Grant (use Gerrit), 2015/03/09 19:57:06]

The goal is to make it so that Chromium built for Chromium OS, running on Linux
(which is my development environment), will execute as much of this code as
possible, without actually requiring the permission broker to be available over
DBus.

[pneubeck (no reviews), 2015/03/09 20:03:46]

But you're also disabling this code paths from test where you have to test it.

See here, how other clients are replaced by stub implementations if you run
Chrome build for ChromeOS on Linux:

https://code.google.com/p/chromium/codesearch#chromium/src/chromeos/dbus/dbus...

+        DBusThreadManager::Get()->GetPermissionBrokerClient();
+    DCHECK(client) << "Could not get permission broker client.";
+    if (client) {
+      switch (type_) {
+        case PortType::TCP:
+          client->ReleaseTcpPort(port_, interface_, port_released_closure);
+          break;
+        case PortType::UDP:
+          client->ReleaseUdpPort(port_, interface_, port_released_closure);
+          break;
+      }
+
+      // Return now. Otherwise, the closure is run immediately to clean up.
+      return;
+    }
+  }
+
+  port_released_closure.Run(true);
+}
+
+void FirewallHole::RequestPortAccess(PortType type,
+                                     uint16_t port,
+                                     const std::string& interface,
+                                     ScopedFileDescriptor lifeline_local,
+                                     ScopedFileDescriptor lifeline_remote,
+                                     const OpenCallback& callback) {
+  if (!lifeline_local->is_valid() || !lifeline_remote->is_valid()) {
+    callback.Run(nullptr);
+    return;
+  }
+
+  base::Callback<void(bool)> access_granted_closure =
+      base::Bind(&PortAccessGranted, type, port, interface,
+                 base::Passed(&lifeline_local), callback);
+
+  if (base::SysInfo::IsRunningOnChromeOS()) {

[pneubeck (no reviews), 2015/03/08 03:35:01]

same comment as above.

+    PermissionBrokerClient* client =
+        DBusThreadManager::Get()->GetPermissionBrokerClient();
+    DCHECK(client) << "Could not get permission broker client.";
+    if (!client) {
+      callback.Run(nullptr);
+      return;
+    }
+
+    switch (type) {
+      case PortType::TCP:
+        client->RequestTcpPortAccess(port, interface, *lifeline_remote,
+                                     access_granted_closure);
+        break;
+      case PortType::UDP:
+        client->RequestUdpPortAccess(port, interface, *lifeline_remote,
+                                     access_granted_closure);
+        break;
+      default:
+        NOTREACHED();
+        callback.Run(nullptr);
+    }
+  } else {
+    access_granted_closure.Run(true);
+  }
+}
+
+void FirewallHole::PortAccessGranted(PortType type,
+                                     uint16_t port,
+                                     const std::string& interface,
+                                     ScopedFileDescriptor lifeline_fd,
+                                     const FirewallHole::OpenCallback& callback,
+                                     bool success) {
+  if (success) {
+    callback.Run(make_scoped_ptr(
+        new FirewallHole(type, port, interface, lifeline_fd.Pass())));
+  } else {
+    callback.Run(nullptr);
+  }
+}
+
+FirewallHole::FirewallHole(PortType type,
+                           uint16_t port,
+                           const std::string& interface,
+                           ScopedFileDescriptor lifeline_fd)
+    : type_(type),
+      port_(port),
+      interface_(interface),
+      lifeline_fd_(lifeline_fd.Pass()) {
+}
+
+}  // namespace chromeos