Explicitly disallow USB mass storage devices in WebUSB

In crrev.com/c/2548705, we disabled access to USB mass storage devices
to all clients aside from VMs in the USB service. However, as these were
formerly accessible from chrome.usb when allowed in policy, we want to
push this logic up into the consumers of the USB service.

While Blink already disallows claiming mass storage interfaces, this CL
ensures we keep the intended behaviour by having the browser process
hide mass storage devices from the renderer process entirely.

Bug: 1169558
Change-Id: I36a41fda581870bb4131234ae82ca5bedea360d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2670379
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#850442}

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

@@ -141,7 +141,8 @@ class FakeAndroidUsbDeviceInfo : public FakeUsbDeviceInfo {
                           /*product_id=*/0,
                           kDeviceManufacturer,
                           kDeviceModel,
-                          kDeviceSerial),
+                          kDeviceSerial,
+                          std::vector<UsbConfigurationInfoPtr>()),
         broken_traits_(is_broken) {}
 
   bool broken_traits() const { return broken_traits_; }

chrome/browser/usb/usb_chooser_context.cc

@@ -34,6 +34,7 @@ constexpr char kProductIdKey[] = "product-id";
 constexpr char kSerialNumberKey[] = "serial-number";
 constexpr char kVendorIdKey[] = "vendor-id";
 constexpr int kDeviceIdWildcard = -1;
+constexpr int kUsbClassMassStorage = 0x08;
 
 // Reasons a permission may be closed. These are used in histograms so do not
 // remove/reorder entries. Only add at the end just before
@@ -115,6 +116,28 @@ base::Value DeviceIdsToValue(int vendor_id, int product_id) {
   return device_value;
 }
 
+bool IsMassStorageInterface(const device::mojom::UsbInterfaceInfo& interface) {
+  for (auto& alternate : interface.alternates) {
+    if (alternate->class_code == kUsbClassMassStorage)
+      return true;
+  }
+  return false;
+}
+
+bool ShouldExposeDevice(const device::mojom::UsbDeviceInfo& device_info) {
+  // blink::USBDevice::claimInterface() disallows claiming mass storage
+  // interfaces, but explicitly prevent access in the browser process as
+  // ChromeOS would allow these interfaces to be claimed.
+
+  for (auto& configuration : device_info.configurations) {
+    for (auto& interface : configuration->interfaces) {
+      if (!IsMassStorageInterface(*interface))
+        return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 void UsbChooserContext::DeviceObserver::OnDeviceAdded(
@@ -158,7 +181,10 @@ void UsbChooserContext::InitDeviceList(
     std::vector<device::mojom::UsbDeviceInfoPtr> devices) {
   for (auto& device_info : devices) {
     DCHECK(device_info);
-    devices_.insert(std::make_pair(device_info->guid, std::move(device_info)));
+    if (ShouldExposeDevice(*device_info)) {
+      devices_.insert(
+          std::make_pair(device_info->guid, std::move(device_info)));
+    }
   }
   is_initialized_ = true;
 
@@ -542,6 +568,8 @@ void UsbChooserContext::OnDeviceAdded(
   DCHECK(device_info);
   // Update the device list.
   DCHECK(!base::Contains(devices_, device_info->guid));
+  if (!ShouldExposeDevice(*device_info))
+    return;
   devices_.insert(std::make_pair(device_info->guid, device_info->Clone()));
 
   // Notify all observers.
@@ -552,6 +580,12 @@ void UsbChooserContext::OnDeviceAdded(
 void UsbChooserContext::OnDeviceRemoved(
     device::mojom::UsbDeviceInfoPtr device_info) {
   DCHECK(device_info);
+
+  if (!ShouldExposeDevice(*device_info)) {
+    DCHECK(!base::Contains(devices_, device_info->guid));
+    return;
+  }
+
   // Update the device list.
   DCHECK(base::Contains(devices_, device_info->guid));
   devices_.erase(device_info->guid);

chrome/browser/usb/usb_chooser_context_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/no_destructor.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind.h"
 #include "base/values.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
@@ -1272,3 +1273,36 @@ TEST_F(UsbChooserContextTest,
                           /*product_id=*/1357,
                           /*name=*/"Unknown product 0x054D from vendor 0x18D2");
 }
+
+TEST_F(UsbChooserContextTest, MassStorageHidden) {
+  GURL url("https://www.google.com");
+  const auto origin = url::Origin::Create(url);
+
+  // Mass storage devices should be hidden.
+  std::vector<device::mojom::UsbConfigurationInfoPtr> storage_configs;
+  storage_configs.push_back(
+      device::FakeUsbDeviceInfo::CreateConfiguration(0x08, 0x06, 0x50));
+  UsbDeviceInfoPtr storage_device_info = device_manager_.CreateAndAddDevice(
+      0, 0, "vendor1", "storage", "123ABC", std::move(storage_configs));
+
+  // Composite devices with both mass storage and allowed interfaces should be
+  // shown.
+  std::vector<device::mojom::UsbConfigurationInfoPtr> complex_configs;
+  complex_configs.push_back(
+      device::FakeUsbDeviceInfo::CreateConfiguration(0x08, 0x06, 0x50, 1));
+  complex_configs.push_back(
+      device::FakeUsbDeviceInfo::CreateConfiguration(0xff, 0x42, 0x1, 2));
+  UsbDeviceInfoPtr complex_device_info = device_manager_.CreateAndAddDevice(
+      0, 0, "vendor2", "complex", "456DEF", std::move(complex_configs));
+
+  UsbChooserContext* chooser_context = GetChooserContext(profile());
+
+  base::RunLoop loop;
+  chooser_context->GetDevices(
+      base::BindLambdaForTesting([&](std::vector<UsbDeviceInfoPtr> devices) {
+        EXPECT_EQ(1u, devices.size());
+        EXPECT_EQ(complex_device_info->product_name, devices[0]->product_name);
+        loop.Quit();
+      }));
+  loop.Run();
+}

services/device/public/cpp/test/fake_usb_device_info.cc

@@ -44,6 +44,7 @@ FakeUsbDeviceInfo::FakeUsbDeviceInfo(uint16_t usb_version,
   device_info_.manufacturer_name = base::UTF8ToUTF16(manufacturer_string);
   device_info_.product_name = base::UTF8ToUTF16(product_string);
   device_info_.serial_number = base::UTF8ToUTF16(serial_number);
+  AddConfig(CreateConfiguration(0xFF, 0x00, 0x00));
 }
 
 FakeUsbDeviceInfo::FakeUsbDeviceInfo(uint16_t vendor_id,
@@ -135,4 +136,25 @@ void FakeUsbDeviceInfo::NotifyDeviceRemoved() {
     observer.OnDeviceRemoved(this);
 }
 
+mojom::UsbConfigurationInfoPtr FakeUsbDeviceInfo::CreateConfiguration(
+    uint8_t device_class,
+    uint8_t subclass_code,
+    uint8_t protocol_code,
+    uint8_t configuration_value) {
+  auto alternate = device::mojom::UsbAlternateInterfaceInfo::New();
+  alternate->alternate_setting = 0;
+  alternate->class_code = device_class;
+  alternate->subclass_code = subclass_code;
+  alternate->protocol_code = protocol_code;
+
+  auto interface = device::mojom::UsbInterfaceInfo::New();
+  interface->interface_number = 0;
+  interface->alternates.push_back(std::move(alternate));
+
+  auto config = device::mojom::UsbConfigurationInfo::New();
+  config->configuration_value = configuration_value;
+  config->interfaces.push_back(std::move(interface));
+  return config;
+}
+
 }  // namespace device

services/device/public/cpp/test/fake_usb_device_info.h

@@ -22,6 +22,8 @@ namespace device {
 
 // This class acts like device::UsbDevice and provides mojom::UsbDeviceInfo.
 // It should be used together with FakeUsbDeviceManager just for testing.
+// For convenience, a default configuration will be set if the chosen
+// constructor does not explicitly set these.
 class FakeUsbDeviceInfo : public base::RefCounted<FakeUsbDeviceInfo> {
  public:
   class Observer : public base::CheckedObserver {
@@ -77,6 +79,12 @@ class FakeUsbDeviceInfo : public base::RefCounted<FakeUsbDeviceInfo> {
   void SetMockDevice(MockUsbMojoDevice* device) { mock_device_ = device; }
   MockUsbMojoDevice* mock_device() const { return mock_device_; }
 
+  static mojom::UsbConfigurationInfoPtr CreateConfiguration(
+      uint8_t device_class,
+      uint8_t subclass_code,
+      uint8_t protocol_code,
+      uint8_t configuration_value = 1);
+
  protected:
   friend class RefCounted<FakeUsbDeviceInfo>;
   virtual ~FakeUsbDeviceInfo();