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}

.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.

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,

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,

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,

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

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

device/usb/BUILD.gn

@@ -47,7 +47,10 @@ source_set("usb") {
     deps += [ "//device/udev_linux" ]
   }
   if (is_chromeos) {
-    deps += [ "//chromeos" ]
+    deps += [
+      "//chromeos",
+      "//dbus",
+    ]
   }
 }
 

device/usb/DEPS

@@ -1,5 +1,6 @@
 include_rules = [
   "+chromeos",
+  "+dbus",
 
   "-net",
   "+net/base",

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));
   }
 }
 

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,