Open USB devices through the permission broker's OpenPath method.

The permission broker now supports opening devices on behalf of Chrome.
This change starts doing so for USB devices. This will allow the
permission broker to apply restrictions to the file descriptor before it
is passed to Chrome.

FakePermissionBrokerClient is given a simple implementation of OpenPath
for testing purposes.

BUG=496469

Review URL: https://codereview.chromium.org/1172533002

Cr-Commit-Position: refs/heads/master@{#335876}

10 files changed, 121 insertions, 17 deletions

diff --git a/.gn b/.gn
index c186410..9af1760 100644
--- a/.gn
+++ b/.gn
@@ -44,7 +44,8 @@ check_targets = [
   "//crypto/*",
   "//data/*",
   "//dbus/*",
-  "//device/*",
+
+  #"//device/*",  # Ran into http://crbug.com/500761 adding dbus dependency
 
   #"//extensions/*",  # Lots of errors.
   #"//gin/*",  # Easy.
diff --git a/chromeos/dbus/fake_permission_broker_client.cc b/chromeos/dbus/fake_permission_broker_client.cc
index 067dca7..85ac1b8 100644
--- a/chromeos/dbus/fake_permission_broker_client.cc
+++ b/chromeos/dbus/fake_permission_broker_client.cc
@@ -4,12 +4,43 @@
 
 #include "chromeos/dbus/fake_permission_broker_client.h"
 
+#include <fcntl.h>
+
+#include "base/bind.h"
 #include "base/callback.h"
+#include "base/location.h"
 #include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/thread_task_runner_handle.h"
+#include "base/threading/worker_pool.h"
 #include "dbus/file_descriptor.h"
 
 namespace chromeos {
 
+namespace {
+
+// So that real devices can be accessed by tests and "Chromium OS on Linux" this
+// function implements a simplified version of the method implemented by the
+// permission broker by opening the path specified and returning the resulting
+// file descriptor.
+void OpenPathAndValidate(
+    const std::string& path,
+    const PermissionBrokerClient::OpenPathCallback& callback,
+    scoped_refptr<base::TaskRunner> task_runner) {
+  int fd = HANDLE_EINTR(open(path.c_str(), O_RDWR));
+  if (fd < 0) {
+    PLOG(ERROR) << "Failed to open '" << path << "'";
+  } else {
+    dbus::FileDescriptor dbus_fd;
+    dbus_fd.PutValue(fd);
+    dbus_fd.CheckValidity();
+    task_runner->PostTask(FROM_HERE,
+                          base::Bind(callback, base::Passed(&dbus_fd)));
+  }
+}
+
+}  // namespace
+
 FakePermissionBrokerClient::FakePermissionBrokerClient() {}
 
 FakePermissionBrokerClient::~FakePermissionBrokerClient() {}
@@ -29,6 +60,14 @@ void FakePermissionBrokerClient::RequestPathAccess(
   callback.Run(true);
 }
 
+void FakePermissionBrokerClient::OpenPath(const std::string& path,
+                                          const OpenPathCallback& callback) {
+  base::WorkerPool::PostTask(FROM_HERE,
+                             base::Bind(&OpenPathAndValidate, path, callback,
+                                        base::ThreadTaskRunnerHandle::Get()),
+                             false);
+}
+
 void FakePermissionBrokerClient::RequestTcpPortAccess(
     uint16 port,
     const std::string& interface,
diff --git a/chromeos/dbus/fake_permission_broker_client.h b/chromeos/dbus/fake_permission_broker_client.h
index 501fef0..6a1ddee 100644
--- a/chromeos/dbus/fake_permission_broker_client.h
+++ b/chromeos/dbus/fake_permission_broker_client.h
@@ -23,6 +23,8 @@ class CHROMEOS_EXPORT FakePermissionBrokerClient
   void RequestPathAccess(const std::string& path,
                          int interface_id,
                          const ResultCallback& callback) override;
+  void OpenPath(const std::string& path,
+                const OpenPathCallback& callback) override;
   void RequestTcpPortAccess(uint16 port,
                             const std::string& interface,
                             const dbus::FileDescriptor& lifeline_fd,
diff --git a/chromeos/dbus/mock_permission_broker_client.h b/chromeos/dbus/mock_permission_broker_client.h
index 1c764b6..d5c9c02 100644
--- a/chromeos/dbus/mock_permission_broker_client.h
+++ b/chromeos/dbus/mock_permission_broker_client.h
@@ -26,6 +26,8 @@ class MockPermissionBrokerClient : public PermissionBrokerClient {
                void(const std::string& path,
                     int interface_id,
                     const ResultCallback& callback));
+  MOCK_METHOD2(OpenPath,
+               void(const std::string& path, const OpenPathCallback& callback));
   MOCK_METHOD4(RequestTcpPortAccess,
                void(uint16 port,
                     const std::string& interface,
diff --git a/chromeos/dbus/permission_broker_client.cc b/chromeos/dbus/permission_broker_client.cc
index 51d8337..4ba4d31 100644
--- a/chromeos/dbus/permission_broker_client.cc
+++ b/chromeos/dbus/permission_broker_client.cc
@@ -12,6 +12,7 @@
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 using permission_broker::kCheckPathAccess;
+using permission_broker::kOpenPath;
 using permission_broker::kPermissionBrokerInterface;
 using permission_broker::kPermissionBrokerServiceName;
 using permission_broker::kPermissionBrokerServicePath;
@@ -51,6 +52,17 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
                                   weak_ptr_factory_.GetWeakPtr(), callback));
   }
 
+  void OpenPath(const std::string& path,
+                const OpenPathCallback& callback) override {
+    dbus::MethodCall method_call(kPermissionBrokerInterface, kOpenPath);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(path);
+    proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&PermissionBrokerClientImpl::OnOpenPathResponse,
+                   weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+
   void RequestTcpPortAccess(uint16 port,
                             const std::string& interface,
                             const dbus::FileDescriptor& lifeline_fd,
@@ -129,6 +141,20 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     callback.Run(result);
   }
 
+  void OnOpenPathResponse(const OpenPathCallback& callback,
+                          dbus::Response* response) {
+    dbus::FileDescriptor fd;
+    if (response) {
+      dbus::MessageReader reader(response);
+      if (!reader.PopFileDescriptor(&fd))
+        LOG(WARNING) << "Could not parse response: " << response->ToString();
+    } else {
+      LOG(WARNING) << "Access request method call failed.";
+    }
+
+    callback.Run(fd.Pass());
+  }
+
   dbus::ObjectProxy* proxy_;
 
   // Note: This should remain the last member so that it will be destroyed
diff --git a/chromeos/dbus/permission_broker_client.h b/chromeos/dbus/permission_broker_client.h
index 4dfe4fa..dd5ecca 100644
--- a/chromeos/dbus/permission_broker_client.h
+++ b/chromeos/dbus/permission_broker_client.h
@@ -11,10 +11,7 @@
 #include "base/callback.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client.h"
-
-namespace dbus {
-class FileDescriptor;
-}
+#include "dbus/file_descriptor.h"
 
 namespace chromeos {
 
@@ -32,6 +29,9 @@ class CHROMEOS_EXPORT PermissionBrokerClient : public DBusClient {
   // the operation that it was submitted alongside.
   typedef base::Callback<void(bool)> ResultCallback;
 
+  // An OpenPathCallback callback is run when an OpenPath request is completed.
+  typedef base::Callback<void(dbus::FileDescriptor)> OpenPathCallback;
+
   ~PermissionBrokerClient() override;
 
   static PermissionBrokerClient* Create();
@@ -53,6 +53,11 @@ class CHROMEOS_EXPORT PermissionBrokerClient : public DBusClient {
                                  int interface_id,
                                  const ResultCallback& callback) = 0;
 
+  // OpenPath requests that the permission broker open the device node
+  // identified by |path| and return the resulting file descriptor.
+  virtual void OpenPath(const std::string& path,
+                        const OpenPathCallback& callback) = 0;
+
   // Requests the |port| be opened on the firewall for incoming TCP/IP
   // connections received on |interface| (an empty string indicates all
   // interfaces). An open pipe must be passed as |lifeline_fd| so that the
diff --git a/device/usb/BUILD.gn b/device/usb/BUILD.gn
index aa74a106..4a5caff 100644
--- a/device/usb/BUILD.gn
+++ b/device/usb/BUILD.gn
@@ -47,7 +47,10 @@ source_set("usb") {
     deps += [ "//device/udev_linux" ]
   }
   if (is_chromeos) {
-    deps += [ "//chromeos" ]
+    deps += [
+      "//chromeos",
+      "//dbus",
+    ]
   }
 }
 
diff --git a/device/usb/DEPS b/device/usb/DEPS
index c114bca..25ff287 100644
--- a/device/usb/DEPS
+++ b/device/usb/DEPS
@@ -1,5 +1,6 @@
 include_rules = [
   "+chromeos",
+  "+dbus",
 
   "-net",
   "+net/base",
diff --git a/device/usb/usb_device_impl.cc b/device/usb/usb_device_impl.cc
index 4f0f56a..114e6ac 100644
--- a/device/usb/usb_device_impl.cc
+++ b/device/usb/usb_device_impl.cc
@@ -22,6 +22,7 @@
 #if defined(OS_CHROMEOS)
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/permission_broker_client.h"
+#include "dbus/file_descriptor.h"
 #endif  // defined(OS_CHROMEOS)
 
 namespace device {
@@ -134,9 +135,9 @@ void UsbDeviceImpl::Open(const OpenCallback& callback) {
   chromeos::PermissionBrokerClient* client =
       chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
   DCHECK(client) << "Could not get permission broker client.";
-  client->RequestPathAccess(
-      device_path_, -1,
-      base::Bind(&UsbDeviceImpl::OnPathRequestComplete, this, callback));
+  client->OpenPath(
+      device_path_,
+      base::Bind(&UsbDeviceImpl::OnOpenRequestComplete, this, callback));
 #else
   blocking_task_runner_->PostTask(
       FROM_HERE,
@@ -243,14 +244,31 @@ void UsbDeviceImpl::RefreshConfiguration() {
 
 #if defined(OS_CHROMEOS)
 
-void UsbDeviceImpl::OnPathRequestComplete(const OpenCallback& callback,
-                                          bool success) {
-  if (success) {
-    blocking_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&UsbDeviceImpl::OpenOnBlockingThread, this, callback));
+void UsbDeviceImpl::OnOpenRequestComplete(const OpenCallback& callback,
+                                          dbus::FileDescriptor fd) {
+  blocking_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&UsbDeviceImpl::OpenOnBlockingThreadWithFd, this,
+                            base::Passed(&fd), callback));
+}
+
+void UsbDeviceImpl::OpenOnBlockingThreadWithFd(dbus::FileDescriptor fd,
+                                               const OpenCallback& callback) {
+  fd.CheckValidity();
+  if (!fd.is_valid()) {
+    USB_LOG(EVENT) << "Did not get valid device handle from permission broker.";
+    task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr));
+    return;
+  }
+
+  PlatformUsbDeviceHandle handle;
+  const int rv = libusb_open_fd(platform_device_, fd.TakeValue(), &handle);
+  if (LIBUSB_SUCCESS == rv) {
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&UsbDeviceImpl::Opened, this, handle, callback));
   } else {
-    callback.Run(nullptr);
+    USB_LOG(EVENT) << "Failed to open device: "
+                   << ConvertPlatformUsbErrorToString(rv);
+    task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr));
   }
 }
 
diff --git a/device/usb/usb_device_impl.h b/device/usb/usb_device_impl.h
index 877c1c5..3e9c8d0 100644
--- a/device/usb/usb_device_impl.h
+++ b/device/usb/usb_device_impl.h
@@ -21,6 +21,10 @@ namespace base {
 class SequencedTaskRunner;
 }
 
+namespace dbus {
+class FileDescriptor;
+}
+
 namespace device {
 
 class UsbDeviceHandleImpl;
@@ -78,7 +82,10 @@ class UsbDeviceImpl : public UsbDevice {
 
  private:
 #if defined(OS_CHROMEOS)
-  void OnPathRequestComplete(const OpenCallback& callback, bool success);
+  void OnOpenRequestComplete(const OpenCallback& callback,
+                             dbus::FileDescriptor fd);
+  void OpenOnBlockingThreadWithFd(dbus::FileDescriptor fd,
+                                  const OpenCallback& callback);
 #endif
   void OpenOnBlockingThread(const OpenCallback& callback);
   void Opened(PlatformUsbDeviceHandle platform_handle,