From e30b44e5d05722b43747fd04dcb8c6b52c5b56bc Mon Sep 17 00:00:00 2001
From: Rob Pitkin <[email protected]>
Date: Tue, 24 Mar 2026 12:20:55 -0700
Subject: [PATCH] [WebUSB] Add UMA for control transfer permission telemetry

This CL adds comprehensive UMA telemetry to track the impact of the
WebUSB control transfer security fix (b:489711638).

Two distinct metrics are introduced:

1. WebUsb.ControlTransferPermissionOutcome: An enumerated histogram
tracking the high-level result of every permission check (Allowed,
Blocked, Interface Not Found, or No Configuration). This provides the
denominator needed to calculate real-world breakage ratios.

2. WebUsb.ControlTransferBlocked.{Direction}.{Type}: A tokenized sparse
histogram that records the specific USB interface class code being
blocked. This allows for granular root-cause analysis, helping
distinguish between malicious tunneling attempts and legitimate,
non-standard device routing that may have been caught by the new
restrictions.

The HasControlTransferPermission method signature was updated to include
the transfer direction to support the tokenized variants.

Bug: 489711638
Test: Verified with chrome://histograms using local test page.
Change-Id: Ibf7d49eb6b06b77e0c105b722fa03e55c6833a37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7685741
Reviewed-by: Reilly Grant <[email protected]>
Reviewed-by: Alvin Ji <[email protected]>
Commit-Queue: Rob Pitkin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1604293}
---

diff --git a/services/device/usb/mojo/device_impl.cc b/services/device/usb/mojo/device_impl.cc
index 2c6967bb..9368b8c 100644
--- a/services/device/usb/mojo/device_impl.cc
+++ b/services/device/usb/mojo/device_impl.cc
@@ -10,6 +10,7 @@
 #include <memory>
 #include <numeric>
 #include <optional>
+#include <string_view>
 #include <utility>
 #include <vector>
 
@@ -18,6 +19,8 @@
 #include "base/functional/callback.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/metrics/histogram_functions.h"
+#include "base/strings/strcat.h"
 #include "base/strings/stringprintf.h"
 #include "services/device/public/cpp/device_features.h"
 #include "services/device/public/cpp/usb/usb_utils.h"
@@ -28,6 +31,7 @@
 
 using mojom::UsbControlTransferParamsPtr;
 using mojom::UsbControlTransferRecipient;
+using mojom::UsbControlTransferType;
 using mojom::UsbIsochronousPacketPtr;
 using mojom::UsbTransferDirection;
 using mojom::UsbTransferStatus;
@@ -109,6 +113,32 @@
   return total_bytes;
 }
 
+// Helper to log blocked transfers to the correct variant.
+void LogBlockedControlTransfer(uint8_t class_code,
+                               UsbTransferDirection direction,
+                               UsbControlTransferType type) {
+  std::string_view direction_str =
+      (direction == UsbTransferDirection::INBOUND) ? "Inbound" : "Outbound";
+  std::string_view type_str;
+  switch (type) {
+    case UsbControlTransferType::STANDARD:
+      type_str = "Standard";
+      break;
+    case UsbControlTransferType::CLASS:
+      type_str = "Class";
+      break;
+    case UsbControlTransferType::VENDOR:
+      type_str = "Vendor";
+      break;
+    default:
+      return;  // Skip RESERVED type
+  }
+
+  base::UmaHistogramSparse(base::StrCat({"WebUsb.ControlTransferBlocked.",
+                                         direction_str, ".", type_str}),
+                           class_code);
+}
+
 }  // namespace
 
 // static
@@ -156,7 +186,8 @@
 }
 
 bool DeviceImpl::HasControlTransferPermission(
-    mojom::UsbControlTransferType type,
+    UsbTransferDirection direction,
+    UsbControlTransferType type,
     UsbControlTransferRecipient recipient,
     uint16_t index) {
   DCHECK(device_handle_);
@@ -167,14 +198,20 @@
   // of the `index` field (wIndex in the spec) for these types as either 0 or a
   // Language ID. Since they are not used for interface-based routing, they
   // are always allowed.
-  if (type == mojom::UsbControlTransferType::STANDARD &&
+  if (type == UsbControlTransferType::STANDARD &&
       (recipient == UsbControlTransferRecipient::DEVICE ||
        recipient == UsbControlTransferRecipient::OTHER)) {
+    base::UmaHistogramEnumeration(
+        "WebUsb.ControlTransferPermissionOutcome",
+        WebUsbControlTransferPermissionOutcome::kAllowed);
     return true;
   }
 
   const mojom::UsbConfigurationInfo* config = device_->GetActiveConfiguration();
   if (!config) {
+    base::UmaHistogramEnumeration(
+        "WebUsb.ControlTransferPermissionOutcome",
+        WebUsbControlTransferPermissionOutcome::kError_NoConfiguration);
     return false;
   }
 
@@ -209,6 +246,10 @@
                        features::kWebUsbProtectedClassControlTransferBlock)) {
     for (const auto& alternate : interface->alternates) {
       if (blocked_interface_classes_.contains(alternate->class_code)) {
+        LogBlockedControlTransfer(alternate->class_code, direction, type);
+        base::UmaHistogramEnumeration(
+            "WebUsb.ControlTransferPermissionOutcome",
+            WebUsbControlTransferPermissionOutcome::kBlocked);
         return false;
       }
     }
@@ -218,12 +259,25 @@
   // must actually exist in the current configuration.
   if (recipient == UsbControlTransferRecipient::INTERFACE ||
       recipient == UsbControlTransferRecipient::ENDPOINT) {
-    return interface != nullptr;
+    bool has_permission = interface != nullptr;
+    if (has_permission) {
+      base::UmaHistogramEnumeration(
+          "WebUsb.ControlTransferPermissionOutcome",
+          WebUsbControlTransferPermissionOutcome::kAllowed);
+    } else {
+      base::UmaHistogramEnumeration(
+          "WebUsb.ControlTransferPermissionOutcome",
+          WebUsbControlTransferPermissionOutcome::kError_InterfaceNotFound);
+    }
+    return has_permission;
   }
 
   // For DEVICE and OTHER recipients, if we reached here, it means either no
   // interface was identified by wIndex, or the interface it identified is
   // not protected. These requests are allowed for device-level management.
+  base::UmaHistogramEnumeration(
+      "WebUsb.ControlTransferPermissionOutcome",
+      WebUsbControlTransferPermissionOutcome::kAllowed);
   return true;
 }
 
@@ -387,8 +441,8 @@
     return;
   }
 
-  if (HasControlTransferPermission(params->type, params->recipient,
-                                   params->index)) {
+  if (HasControlTransferPermission(UsbTransferDirection::INBOUND, params->type,
+                                   params->recipient, params->index)) {
     auto buffer = base::MakeRefCounted<base::RefCountedBytes>(length);
     device_handle_->ControlTransfer(
         UsbTransferDirection::INBOUND, params->type, params->recipient,
@@ -411,8 +465,8 @@
     return;
   }
 
-  if (HasControlTransferPermission(params->type, params->recipient,
-                                   params->index) &&
+  if (HasControlTransferPermission(UsbTransferDirection::OUTBOUND, params->type,
+                                   params->recipient, params->index) &&
       (allow_security_key_requests_ ||
        !IsAndroidSecurityKeyRequest(params, data))) {
     auto buffer = base::MakeRefCounted<base::RefCountedBytes>(data);
diff --git a/services/device/usb/mojo/device_impl.h b/services/device/usb/mojo/device_impl.h
index 8651ece..e888a27 100644
--- a/services/device/usb/mojo/device_impl.h
+++ b/services/device/usb/mojo/device_impl.h
@@ -23,6 +23,19 @@
 
 namespace device::usb {
 
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+// LINT.IfChange(WebUsbControlTransferPermissionOutcome)