Allow top-level frames to request any USB device.

This patch changes the rules for WebUSB so that a top level frame can
request access to any USB device, while embedded frames must have an
origin matching the allowed origins structure read from the device.

Integration tests for WebUSB are added which exercise this.

BUG=492204

Review-Url: https://codereview.chromium.org/2611773004
Cr-Commit-Position: refs/heads/master@{#442105}

chrome/browser/ui/startup/bad_flags_prompt.cc

@@ -87,12 +87,6 @@ void ShowBadFlagsPrompt(Browser* browser) {
     // if they are not.
     switches::kUnsafelyTreatInsecureOriginAsSecure,
 
-    // This flag disables WebUSB's CORS-like checks for origin to device
-    // communication, allowing any origin to ask the user for permission to
-    // connect to a device. It is intended for manufacturers testing their
-    // existing devices until https://crbug.com/598766 is implemented.
-    switches::kDisableWebUsbSecurity,
-
     NULL
   };
 

chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc

@@ -566,6 +566,8 @@ TEST_F(ExtensionPrinterHandlerTest, GetUsbPrinters) {
   extension_printer_handler_->StartGetPrinters(
       base::Bind(&RecordPrinterList, &call_count, &printers, &is_done));
 
+  base::RunLoop().RunUntilIdle();
+
   FakePrinterProviderAPI* fake_api = GetPrinterProviderAPI();
   ASSERT_TRUE(fake_api);
   ASSERT_EQ(1u, fake_api->pending_get_printers_count());

chrome/browser/usb/usb_browsertest.cc

+// Copyright 2016 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 <memory>
+#include <string>
+
+#include "base/command_line.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/usb/usb_chooser_controller.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
+#include "device/base/mock_device_client.h"
+#include "device/usb/mock_usb_device.h"
+#include "device/usb/mock_usb_service.h"
+#include "device/usb/public/interfaces/chooser_service.mojom.h"
+#include "device/usb/webusb_descriptors.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "services/service_manager/public/cpp/interface_registry.h"
+
+using content::RenderFrameHost;
+using device::MockDeviceClient;
+using device::MockUsbDevice;
+
+namespace {
+
+class FakeChooserView : public ChooserController::View {
+ public:
+  explicit FakeChooserView(std::unique_ptr<ChooserController> controller)
+      : controller_(std::move(controller)) {
+    controller_->set_view(this);
+  }
+
+  ~FakeChooserView() override { controller_->set_view(nullptr); }
+
+  void OnOptionsInitialized() override {
+    if (controller_->NumOptions())
+      controller_->Select({0});
+    else
+      controller_->Cancel();
+    delete this;
+  }
+
+  void OnOptionAdded(size_t index) override { NOTREACHED(); }
+  void OnOptionRemoved(size_t index) override { NOTREACHED(); }
+  void OnOptionUpdated(size_t index) override { NOTREACHED(); }
+  void OnAdapterEnabledChanged(bool enabled) override { NOTREACHED(); }
+  void OnRefreshStateChanged(bool refreshing) override { NOTREACHED(); }
+
+ private:
+  std::unique_ptr<ChooserController> controller_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeChooserView);
+};
+
+class FakeChooserService : public device::usb::ChooserService {
+ public:
+  static void Create(RenderFrameHost* render_frame_host,
+                     device::usb::ChooserServiceRequest request) {
+    mojo::MakeStrongBinding(
+        base::MakeUnique<FakeChooserService>(render_frame_host),
+        std::move(request));
+  }
+
+  explicit FakeChooserService(RenderFrameHost* render_frame_host)
+      : render_frame_host_(render_frame_host) {}
+
+  ~FakeChooserService() override {}
+
+  // device::usb::ChooserService:
+  void GetPermission(std::vector<device::usb::DeviceFilterPtr> device_filters,
+                     const GetPermissionCallback& callback) override {
+    auto chooser_controller = base::MakeUnique<UsbChooserController>(
+        render_frame_host_, std::move(device_filters), callback);
+    new FakeChooserView(std::move(chooser_controller));
+  }
+
+ private:
+  RenderFrameHost* const render_frame_host_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeChooserService);
+};
+
+class WebUsbTest : public InProcessBrowserTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(
+        switches::kEnableExperimentalWebPlatformFeatures);
+    InProcessBrowserTest::SetUpCommandLine(command_line);
+  }
+
+  void SetUpOnMainThread() override {
+    embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    device_client_.reset(new MockDeviceClient());
+    scoped_refptr<MockUsbDevice> mock_device(
+        new MockUsbDevice(0, 0, "Test Manufacturer", "Test Device", "123456"));
+    device_client_->usb_service()->AddDevice(mock_device);
+
+    mock_device =
+        new MockUsbDevice(1, 0, "Test Manufacturer", "Test Device", "ABCDEF");
+    auto allowed_origins = base::MakeUnique<device::WebUsbAllowedOrigins>();
+    allowed_origins->origins.push_back(
+        embedded_test_server()->GetURL("localhost", "/").GetOrigin());
+    mock_device->set_webusb_allowed_origins(std::move(allowed_origins));
+    device_client_->usb_service()->AddDevice(mock_device);
+  }
+
+ private:
+  std::unique_ptr<MockDeviceClient> device_client_;
+};
+
+IN_PROC_BROWSER_TEST_F(WebUsbTest, RequestAndGetDevices) {
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL("localhost", "/simple_page.html"));
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  RenderFrameHost* render_frame_host = web_contents->GetMainFrame();
+  EXPECT_THAT(render_frame_host->GetLastCommittedOrigin().Serialize(),
+              testing::StartsWith("http://localhost:"));
+
+  render_frame_host->GetInterfaceRegistry()->AddInterface(
+      base::Bind(&FakeChooserService::Create, render_frame_host));
+
+  // The mock device with vendorId == 0 has no WebUSB allowed origin descriptor
+  // but because this is a top level frame it will be allowed.
+  std::string result;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      web_contents,
+      "navigator.usb.requestDevice({ filters: [ { vendorId: 0 } ] })"
+      "    .then(device => {"
+      "        domAutomationController.send(device.serialNumber);"
+      "     });",
+      &result));
+  EXPECT_EQ("123456", result);
+
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      web_contents,
+      "navigator.usb.getDevices()"
+      "    .then(devices => {"
+      "        domAutomationController.send(devices.length.toString());"
+      "     });",
+      &result));
+  EXPECT_EQ("1", result);
+}
+
+IN_PROC_BROWSER_TEST_F(WebUsbTest, RequestAndGetDevicesInIframe) {
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL("localhost", "/page_with_iframe.html"));
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  RenderFrameHost* main_frame = web_contents->GetMainFrame();
+  EXPECT_THAT(main_frame->GetLastCommittedOrigin().Serialize(),
+              testing::StartsWith("http://localhost:"));
+  RenderFrameHost* embedded_frame = ChildFrameAt(main_frame, 0);
+  EXPECT_THAT(embedded_frame->GetLastCommittedOrigin().Serialize(),
+              testing::StartsWith("http://localhost:"));
+
+  embedded_frame->GetInterfaceRegistry()->AddInterface(
+      base::Bind(&FakeChooserService::Create, embedded_frame));
+
+  // The mock device with vendorId == 0 has no allowed origin descriptor so an
+  // embedded frame will not be able to select it.
+  std::string result;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      embedded_frame,
+      "navigator.usb.requestDevice({ filters: [ { vendorId: 0 } ] })"
+      "    .catch(e => { domAutomationController.send(e.toString()); });",
+      &result));
+  EXPECT_EQ("NotFoundError: No device selected.", result);
+
+  // The mock device with vendorId == 1 does however have the embedded test
+  // server listed as an allowed origin.
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      embedded_frame,
+      "navigator.usb.requestDevice({ filters: [ { vendorId: 1 } ] })"
+      "    .then(device => {"
+      "        domAutomationController.send(device.serialNumber);"
+      "     });",
+      &result));
+  EXPECT_EQ("ABCDEF", result);
+
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      embedded_frame,
+      "navigator.usb.getDevices()"
+      "    .then(devices => {"
+      "        domAutomationController.send(devices.length.toString());"
+      "     });",
+      &result));
+  EXPECT_EQ("1", result);
+}
+
+}  // namespace

chrome/browser/usb/usb_chooser_controller.cc

@@ -8,7 +8,6 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/net/referrer.h"
@@ -21,7 +20,6 @@
 #include "chrome/browser/usb/usb_chooser_context_factory.h"
 #include "chrome/browser/usb/web_usb_histograms.h"
 #include "chrome/browser/usb/web_usb_permission_provider.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/render_frame_host.h"
@@ -35,6 +33,9 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
+using content::RenderFrameHost;
+using content::WebContents;
+
 namespace {
 
 Browser* GetBrowser() {
@@ -71,11 +72,10 @@ base::string16 GetDeviceName(scoped_refptr<device::UsbDevice> device) {
 }  // namespace
 
 UsbChooserController::UsbChooserController(
-    content::RenderFrameHost* owner,
+    RenderFrameHost* render_frame_host,
     mojo::Array<device::usb::DeviceFilterPtr> device_filters,
-    content::RenderFrameHost* render_frame_host,
     const device::usb::ChooserService::GetPermissionCallback& callback)
-    : ChooserController(owner,
+    : ChooserController(render_frame_host,
                         IDS_USB_DEVICE_CHOOSER_PROMPT_ORIGIN,
                         IDS_USB_DEVICE_CHOOSER_PROMPT_EXTENSION_NAME),
       render_frame_host_(render_frame_host),
@@ -141,8 +141,8 @@ void UsbChooserController::Select(const std::vector<size_t>& indices) {
   DCHECK_EQ(1u, indices.size());
   size_t index = indices[0];
   DCHECK_LT(index, devices_.size());
-  content::WebContents* web_contents =
-      content::WebContents::FromRenderFrameHost(render_frame_host_);
+  WebContents* web_contents =
+      WebContents::FromRenderFrameHost(render_frame_host_);
   GURL embedding_origin =
       web_contents->GetMainFrame()->GetLastCommittedURL().GetOrigin();
   Profile* profile =
@@ -223,11 +223,20 @@ void UsbChooserController::GotUsbDeviceList(
 
 bool UsbChooserController::DisplayDevice(
     scoped_refptr<device::UsbDevice> device) const {
-  return device::UsbDeviceFilter::MatchesAny(device, filters_) &&
-         !UsbBlocklist::Get().IsExcluded(device) &&
-         (base::CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kDisableWebUsbSecurity) ||
-          device::FindInWebUsbAllowedOrigins(
-              device->webusb_allowed_origins(),
-              render_frame_host_->GetLastCommittedURL().GetOrigin()));
+  if (!device::UsbDeviceFilter::MatchesAny(device, filters_))
+    return false;
+
+  if (UsbBlocklist::Get().IsExcluded(device))
+    return false;
+
+  // Embedded frames must have their origin in the list provided by the device.
+  RenderFrameHost* main_frame =
+      WebContents::FromRenderFrameHost(render_frame_host_)->GetMainFrame();
+  if (render_frame_host_ != main_frame) {
+    return device::FindInWebUsbAllowedOrigins(
+        device->webusb_allowed_origins(),
+        render_frame_host_->GetLastCommittedURL().GetOrigin());
+  }
+
+  return true;
 }

chrome/browser/usb/usb_chooser_controller.h

@@ -33,9 +33,8 @@ class UsbChooserController : public ChooserController,
                              public device::UsbService::Observer {
  public:
   UsbChooserController(
-      content::RenderFrameHost* owner,
-      mojo::Array<device::usb::DeviceFilterPtr> device_filters,
       content::RenderFrameHost* render_frame_host,
+      mojo::Array<device::usb::DeviceFilterPtr> device_filters,
       const device::usb::ChooserService::GetPermissionCallback& callback);
   ~UsbChooserController() override;
 

chrome/browser/usb/usb_chooser_controller_unittest.cc

@@ -54,7 +54,7 @@ class UsbChooserControllerTest : public ChromeRenderViewHostTestHarness {
         content::WebContentsTester::For(web_contents());
     web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl));
     usb_chooser_controller_.reset(new UsbChooserController(
-        main_rfh(), std::move(device_filters), main_rfh(), callback));
+        main_rfh(), std::move(device_filters), callback));
     mock_usb_chooser_view_.reset(new MockUsbChooserView());
     usb_chooser_controller_->set_view(mock_usb_chooser_view_.get());
   }

chrome/browser/usb/web_usb_chooser_service.cc

@@ -36,7 +36,7 @@ void WebUsbChooserService::GetPermission(
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
   std::unique_ptr<UsbChooserController> usb_chooser_controller(
       new UsbChooserController(render_frame_host_, std::move(device_filters),
-                               render_frame_host_, callback));
+                               callback));
   std::unique_ptr<ChooserBubbleDelegate> chooser_bubble_delegate(
       new ChooserBubbleDelegate(render_frame_host_,
                                 std::move(usb_chooser_controller)));

chrome/browser/usb/web_usb_permission_provider.cc

@@ -7,20 +7,19 @@
 #include <stddef.h>
 #include <utility>
 
-#include "base/command_line.h"
 #include "base/stl_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/usb/usb_blocklist.h"
 #include "chrome/browser/usb/usb_chooser_context.h"
 #include "chrome/browser/usb/usb_chooser_context_factory.h"
 #include "chrome/browser/usb/usb_tab_helper.h"
-#include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "device/usb/usb_device.h"
 #include "device/usb/webusb_descriptors.h"
 
+using content::RenderFrameHost;
 using content::WebContents;
 
 namespace {
@@ -29,10 +28,6 @@ bool FindOriginInDescriptorSet(const device::WebUsbAllowedOrigins* set,
                                const GURL& origin,
                                const uint8_t* configuration_value,
                                const uint8_t* first_interface) {
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableWebUsbSecurity))
-    return true;
-
   if (!set)
     return false;
   if (base::ContainsValue(set->origins, origin))
@@ -57,7 +52,7 @@ bool FindOriginInDescriptorSet(const device::WebUsbAllowedOrigins* set,
 
 // static
 bool WebUSBPermissionProvider::HasDevicePermission(
-    content::RenderFrameHost* render_frame_host,
+    RenderFrameHost* render_frame_host,
     scoped_refptr<const device::UsbDevice> device) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -66,8 +61,8 @@ bool WebUSBPermissionProvider::HasDevicePermission(
 
   WebContents* web_contents =
       WebContents::FromRenderFrameHost(render_frame_host);
-  GURL embedding_origin =
-      web_contents->GetMainFrame()->GetLastCommittedURL().GetOrigin();
+  RenderFrameHost* main_frame = web_contents->GetMainFrame();
+  GURL embedding_origin = main_frame->GetLastCommittedURL().GetOrigin();
   GURL requesting_origin = render_frame_host->GetLastCommittedURL().GetOrigin();
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
@@ -86,12 +81,17 @@ bool WebUSBPermissionProvider::HasDevicePermission(
   if (!device->permission_granted())
     return true;
 
-  return FindOriginInDescriptorSet(device->webusb_allowed_origins(),
-                                   requesting_origin, nullptr, nullptr);
+  // Embedded frames must have their origin in the list provided by the device.
+  if (render_frame_host != main_frame) {
+    return FindOriginInDescriptorSet(device->webusb_allowed_origins(),
+                                     requesting_origin, nullptr, nullptr);
+  }
+
+  return true;
 }
 
 WebUSBPermissionProvider::WebUSBPermissionProvider(
-    content::RenderFrameHost* render_frame_host)
+    RenderFrameHost* render_frame_host)
     : render_frame_host_(render_frame_host), weak_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(render_frame_host_);
@@ -113,10 +113,19 @@ bool WebUSBPermissionProvider::HasConfigurationPermission(
     uint8_t requested_configuration_value,
     scoped_refptr<const device::UsbDevice> device) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  return FindOriginInDescriptorSet(
-      device->webusb_allowed_origins(),
-      render_frame_host_->GetLastCommittedURL().GetOrigin(),
-      &requested_configuration_value, nullptr);
+
+  // Embedded frames may only access configurations if their origin in the list
+  // provided by the device.
+  RenderFrameHost* main_frame =
+      WebContents::FromRenderFrameHost(render_frame_host_)->GetMainFrame();
+  if (render_frame_host_ != main_frame) {
+    return FindOriginInDescriptorSet(
+        device->webusb_allowed_origins(),
+        render_frame_host_->GetLastCommittedURL().GetOrigin(),
+        &requested_configuration_value, nullptr);
+  }
+
+  return true;
 }
 
 bool WebUSBPermissionProvider::HasFunctionPermission(
@@ -124,10 +133,19 @@ bool WebUSBPermissionProvider::HasFunctionPermission(
     uint8_t configuration_value,
     scoped_refptr<const device::UsbDevice> device) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  return FindOriginInDescriptorSet(
-      device->webusb_allowed_origins(),
-      render_frame_host_->GetLastCommittedURL().GetOrigin(),
-      &configuration_value, &requested_function);
+
+  // Embedded frames may only access configurations if their origin in the list
+  // provided by the device.
+  RenderFrameHost* main_frame =
+      WebContents::FromRenderFrameHost(render_frame_host_)->GetMainFrame();
+  if (render_frame_host_ != main_frame) {
+    return FindOriginInDescriptorSet(
+        device->webusb_allowed_origins(),
+        render_frame_host_->GetLastCommittedURL().GetOrigin(),
+        &configuration_value, &requested_function);
+  }
+
+  return true;
 }
 
 void WebUSBPermissionProvider::IncrementConnectionCount() {

chrome/common/chrome_switches.cc

@@ -283,9 +283,6 @@ const char kDisableSettingsWindow[]          = "disable-settings-window";
 const char kDisableWebNotificationCustomLayouts[] =
     "disable-web-notification-custom-layouts";
 
-// Disables WebUSB's CORS-like checks for origin->device communication.
-const char kDisableWebUsbSecurity[] = "disable-webusb-security";
-
 // Some tests seem to require the application to close when the last
 // browser window is closed. Thus, we need a switch to force this behavior
 // for ChromeOS Aura, disable "zero window mode".

chrome/common/chrome_switches.h

@@ -96,7 +96,6 @@ extern const char kDisablePushApiBackgroundMode[];
 extern const char kDisableQuic[];
 extern const char kDisableSettingsWindow[];
 extern const char kDisableWebNotificationCustomLayouts[];
-extern const char kDisableWebUsbSecurity[];
 extern const char kDisableZeroBrowsersOpenForTests[];
 extern const char kDiskCacheDir[];
 extern const char kDiskCacheSize[];

chrome/test/BUILD.gn

@@ -1839,6 +1839,7 @@ test("browser_tests") {
       "../browser/ui/webui/webui_webview_browsertest.cc",
       "../browser/ui/zoom/zoom_controller_browsertest.cc",
       "../browser/unload_browsertest.cc",
+      "../browser/usb/usb_browsertest.cc",
       "../browser/web_bluetooth_browsertest.cc",
       "../common/mac/app_mode_chrome_locator_browsertest.mm",
       "../common/mac/mock_launchd.cc",
@@ -1921,6 +1922,7 @@ test("browser_tests") {
       "//content/test:test_support",
       "//crypto:platform",
       "//crypto:test_support",
+      "//device/base:mocks",
       "//device/bluetooth:mocks",
       "//device/serial:test_support",
       "//device/usb:test_support",

device/usb/mock_usb_service.cc

@@ -7,11 +7,16 @@
 #include <string>
 #include <vector>
 
+#include "base/threading/thread_task_runner_handle.h"
 #include "device/usb/usb_device.h"
 
 namespace device {
 
-MockUsbService::MockUsbService() : UsbService(nullptr, nullptr) {}
+MockUsbService::MockUsbService()
+    : UsbService(base::ThreadTaskRunnerHandle::IsSet()
+                     ? base::ThreadTaskRunnerHandle::Get()
+                     : nullptr,
+                 nullptr) {}
 
 MockUsbService::~MockUsbService() {
   // Shutdown() must be called before the base class destructor.