Add configuration and interface permission checks to DeviceImpl.

New methods to check whether a client has permission to access a device
configuration or interface have been added to the PermissionProvider
interface. DeviceManagerImpl then uses the new Bind() method to give
DeviceImpl a new pointer to the object it has been using for permission
checks. Permission checks are not required for bulk, interrupt and
isochronous transfers because they require the interface to be claimed
first.

BUG=492204

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

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

chrome/browser/devtools/device/usb/android_usb_browsertest.cc

@@ -223,6 +223,12 @@ class MockUsbDeviceHandle : public UsbDeviceHandle {
     }
   }
 
+  bool FindInterfaceByEndpoint(uint8_t endpoint_address,
+                               uint8_t* interface_number) {
+    NOTIMPLEMENTED();
+    return false;
+  }
+
   template <class D>
   void append(D data) {
     std::copy(reinterpret_cast<char*>(&data),

chrome/browser/usb/web_usb_permission_provider.cc

@@ -11,10 +11,16 @@
 #include "content/public/browser/render_frame_host.h"
 #include "device/core/device_client.h"
 
+using device::usb::WebUsbDescriptorSet;
+using device::usb::WebUsbConfigurationSubsetPtr;
+using device::usb::WebUsbFunctionSubsetPtr;
+
 namespace {
 
-bool FindOriginInDescriptorSet(const device::usb::WebUsbDescriptorSet* set,
-                               const GURL& origin) {
+bool FindOriginInDescriptorSet(const WebUsbDescriptorSet* set,
+                               const GURL& origin,
+                               const uint8_t* configuration_value,
+                               const uint8_t* interface_number) {
   if (!set)
     return false;
   for (size_t i = 0; i < set->origins.size(); ++i) {
@@ -24,13 +30,20 @@ bool FindOriginInDescriptorSet(const device::usb::WebUsbDescriptorSet* set,
   for (size_t i = 0; i < set->configurations.size(); ++i) {
     const device::usb::WebUsbConfigurationSubsetPtr& config =
         set->configurations[i];
+    if (configuration_value &&
+        *configuration_value != config->configuration_value)
+      continue;
     for (size_t j = 0; i < config->origins.size(); ++j) {
       if (origin.spec() == config->origins[j])
         return true;
     }
     for (size_t j = 0; j < config->functions.size(); ++j) {
       const device::usb::WebUsbFunctionSubsetPtr& function =
           config->functions[j];
+      // TODO(reillyg): Implement support for Interface Association Descriptors
+      // so that this check will match associated interfaces.
+      if (interface_number && *interface_number != function->first_interface)
+        continue;
       for (size_t k = 0; k < function->origins.size(); ++k) {
         if (origin.spec() == function->origins[k])
           return true;
@@ -54,7 +67,7 @@ void WebUSBPermissionProvider::Create(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(render_frame_host);
 
-  // The created object is strongly bound to (and owned by) the pipe.
+  // The created object is owned by its bindings.
   new WebUSBPermissionProvider(render_frame_host, request.Pass());
 }
 
@@ -63,8 +76,11 @@ WebUSBPermissionProvider::~WebUSBPermissionProvider() {}
 WebUSBPermissionProvider::WebUSBPermissionProvider(
     content::RenderFrameHost* render_frame_host,
     mojo::InterfaceRequest<PermissionProvider> request)
-    : binding_(this, request.Pass()),
-      render_frame_host_(render_frame_host) {}
+    : render_frame_host_(render_frame_host) {
+  bindings_.set_connection_error_handler(base::Bind(
+      &WebUSBPermissionProvider::OnConnectionError, base::Unretained(this)));
+  bindings_.AddBinding(this, request.Pass());
+}
 
 void WebUSBPermissionProvider::HasDevicePermission(
     mojo::Array<device::usb::DeviceInfoPtr> requested_devices,
@@ -75,10 +91,43 @@ void WebUSBPermissionProvider::HasDevicePermission(
   mojo::Array<mojo::String> allowed_guids(0);
   for (size_t i = 0; i < requested_devices.size(); ++i) {
     const device::usb::DeviceInfoPtr& device = requested_devices[i];
-    if (FindOriginInDescriptorSet(device->webusb_allowed_origins.get(),
-                                  origin) &&
+    if (FindOriginInDescriptorSet(device->webusb_allowed_origins.get(), origin,
+                                  nullptr, nullptr) &&
         EnableWebUsbOnAnyOrigin())
       allowed_guids.push_back(device->guid);
   }
   callback.Run(allowed_guids.Pass());
 }
+
+void WebUSBPermissionProvider::HasConfigurationPermission(
+    uint8_t requested_configuration_value,
+    device::usb::DeviceInfoPtr device,
+    const HasInterfacePermissionCallback& callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  callback.Run(FindOriginInDescriptorSet(
+      device->webusb_allowed_origins.get(),
+      render_frame_host_->GetLastCommittedURL().GetOrigin(),
+      &requested_configuration_value, nullptr));
+}
+
+void WebUSBPermissionProvider::HasInterfacePermission(
+    uint8_t requested_interface,
+    uint8_t configuration_value,
+    device::usb::DeviceInfoPtr device,
+    const HasInterfacePermissionCallback& callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  callback.Run(FindOriginInDescriptorSet(
+      device->webusb_allowed_origins.get(),
+      render_frame_host_->GetLastCommittedURL().GetOrigin(),
+      &configuration_value, &requested_interface));
+}
+
+void WebUSBPermissionProvider::Bind(
+    mojo::InterfaceRequest<device::usb::PermissionProvider> request) {
+  bindings_.AddBinding(this, request.Pass());
+}
+
+void WebUSBPermissionProvider::OnConnectionError() {
+  if (bindings_.empty())
+    delete this;
+}

chrome/browser/usb/web_usb_permission_provider.h

@@ -6,9 +6,9 @@
 #define CHROME_BROWSER_USB_WEB_USB_PERMISSION_PROVIDER_H_
 
 #include "device/devices_app/usb/public/interfaces/permission_provider.mojom.h"
+#include "mojo/common/weak_binding_set.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/array.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/interface_request.h"
-#include "third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h"
 
 namespace content {
 class RenderFrameHost;
@@ -30,8 +30,21 @@ class WebUSBPermissionProvider : public device::usb::PermissionProvider {
   void HasDevicePermission(
       mojo::Array<device::usb::DeviceInfoPtr> requested_devices,
       const HasDevicePermissionCallback& callback) override;
-
-  mojo::StrongBinding<PermissionProvider> binding_;
+  void HasConfigurationPermission(
+      uint8_t requested_configuration,
+      device::usb::DeviceInfoPtr device,
+      const HasInterfacePermissionCallback& callback) override;
+  void HasInterfacePermission(
+      uint8_t requested_interface,
+      uint8_t configuration_value,
+      device::usb::DeviceInfoPtr device,
+      const HasInterfacePermissionCallback& callback) override;
+  void Bind(
+      mojo::InterfaceRequest<device::usb::PermissionProvider> request) override;
+
+  void OnConnectionError();
+
+  mojo::WeakBindingSet<PermissionProvider> bindings_;
   content::RenderFrameHost* const render_frame_host_;
 };
 

device/BUILD.gn

@@ -112,6 +112,8 @@ test("device_unittests") {
     sources += [
       "devices_app/usb/device_impl_unittest.cc",
       "devices_app/usb/device_manager_impl_unittest.cc",
+      "devices_app/usb/fake_permission_provider.cc",
+      "devices_app/usb/fake_permission_provider.h",
       "test/test_device_client.cc",
       "test/test_device_client.h",
       "test/usb_test_gadget_impl.cc",

device/device_tests.gyp

@@ -62,6 +62,8 @@
         'bluetooth/test/test_bluetooth_adapter_observer.h',
         'devices_app/usb/device_impl_unittest.cc',
         'devices_app/usb/device_manager_impl_unittest.cc',
+        'devices_app/usb/fake_permission_provider.cc',
+        'devices_app/usb/fake_permission_provider.h',
         'hid/hid_connection_unittest.cc',
         'hid/hid_device_filter_unittest.cc',
         'hid/hid_report_descriptor_unittest.cc',