Block access to CCID-enabled YubiKeys

This patch adds all YubiKey devices with a CCID interface to the
UsbBlocklist. The blocklist functionality had to be updated to support
blocking all devices with a given vendor and product ID up to and
including a maximum device version as the set of particular device
firmware versions we would like to block is unknown.

This unfortunately means this change cannot also be rolled out as a
field trial.

Bug: 813280
Change-Id: Ic84e71bdc57201a6e43a29dcd6376e3d84bcd258
Reviewed-on: https://chromium-review.googlesource.com/927788
Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#538658}(cherry picked from commit f00604c002f54951628f5cdac821bb283178eba1)
Reviewed-on: https://chromium-review.googlesource.com/938001
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#594}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
diff --git a/chrome/browser/usb/usb_blocklist.cc b/chrome/browser/usb/usb_blocklist.cc
index a68f5636..818d7e9 100644
--- a/chrome/browser/usb/usb_blocklist.cc
+++ b/chrome/browser/usb/usb_blocklist.cc
@@ -4,7 +4,9 @@
 
 #include "chrome/browser/usb/usb_blocklist.h"
 
-#include "base/stl_util.h"
+#include <algorithm>
+#include <tuple>
+
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "components/variations/variations_associated_data.h"
@@ -15,27 +17,72 @@
 static base::LazyInstance<UsbBlocklist>::Leaky g_singleton =
     LAZY_INSTANCE_INITIALIZER;
 
-}  // namespace
+constexpr uint16_t kMaxVersion = 0xffff;
 
-// The == and < operators are necessary to use Entries in std::set.
-bool operator==(const UsbBlocklist::Entry& a, const UsbBlocklist::Entry& b) {
-  return a.vendor_id == b.vendor_id && a.product_id == b.product_id &&
-         a.version == b.version;
-}
+// Returns true if the passed string is exactly 4 digits long and only contains
+// valid hexadecimal characters (no leading 0x).
+bool IsHexComponent(base::StringPiece string) {
+  if (string.length() != 4)
+    return false;
 
-bool operator<(const UsbBlocklist::Entry& a, const UsbBlocklist::Entry& b) {
-  if (a.vendor_id == b.vendor_id) {
-    if (a.product_id == b.product_id)
-      return a.version < b.version;
-    return a.product_id < b.product_id;
+  // This is necessary because base::HexStringToUInt allows whitespace and the
+  // "0x" prefix in its input.
+  for (char c : string) {
+    if (c >= '0' && c <= '9')
+      continue;
+    if (c >= 'a' && c <= 'f')
+      continue;
+    if (c >= 'A' && c <= 'F')
+      continue;
+    return false;
   }
-  return a.vendor_id < b.vendor_id;
+  return true;
 }
 
+bool CompareEntry(const UsbBlocklist::Entry& a, const UsbBlocklist::Entry& b) {
+  return std::tie(a.vendor_id, a.product_id, a.max_version) <
+         std::tie(b.vendor_id, b.product_id, b.max_version);
+}
+
+// Returns true if an entry in (begin, end] matches the vendor and product IDs
+// of |entry| and has a device version greater than or equal to |entry|.
+template <class Iterator>
+bool EntryMatches(Iterator begin,
+                  Iterator end,
+                  const UsbBlocklist::Entry& entry) {
+  auto it = std::lower_bound(begin, end, entry, CompareEntry);
+  return it != end && it->vendor_id == entry.vendor_id &&
+         it->product_id == entry.product_id;
+}
+
+// This list must be sorted according to CompareEntry.
+const UsbBlocklist::Entry kStaticEntries[] = {
+    // Yubikey NEO - OTP and CCID
+    {0x1050, 0x0111, kMaxVersion},
+    // Yubikey NEO - CCID only
+    {0x1050, 0x0112, kMaxVersion},
+    // Yubikey NEO - U2F and CCID
+    {0x1050, 0x0115, kMaxVersion},
+    // Yubikey NEO - OTP, U2F and CCID
+    {0x1050, 0x0116, kMaxVersion},
+    // Google Gnubby (WinUSB firmware)
+    {0x1050, 0x0211, kMaxVersion},
+    // Yubikey 4 - CCID only
+    {0x1050, 0x0404, kMaxVersion},
+    // Yubikey 4 - OTP and CCID
+    {0x1050, 0x0405, kMaxVersion},
+    // Yubikey 4 - U2F and CCID
+    {0x1050, 0x0406, kMaxVersion},
+    // Yubikey 4 - OTP, U2F and CCID
+    {0x1050, 0x0407, kMaxVersion},
+};
+
+}  // namespace
+
 UsbBlocklist::Entry::Entry(uint16_t vendor_id,
                            uint16_t product_id,
-                           uint16_t version)
-    : vendor_id(vendor_id), product_id(product_id), version(version) {}
+                           uint16_t max_version)
+    : vendor_id(vendor_id), product_id(product_id), max_version(max_version) {}
 
 UsbBlocklist::~UsbBlocklist() {}
 
@@ -44,64 +91,54 @@
   return g_singleton.Get();
 }
 
-void UsbBlocklist::Exclude(const Entry& entry) {
-  blocklisted_devices_.insert(entry);
+bool UsbBlocklist::IsExcluded(const Entry& entry) const {
+  return EntryMatches(std::begin(kStaticEntries), std::end(kStaticEntries),
+                      entry) ||
+         EntryMatches(dynamic_entries_.begin(), dynamic_entries_.end(), entry);
 }
 
-void UsbBlocklist::Exclude(base::StringPiece blocklist_string) {
-  for (const auto& entry :
-       base::SplitStringPiece(blocklist_string, ",", base::TRIM_WHITESPACE,
-                              base::SPLIT_WANT_NONEMPTY)) {
-    std::vector<base::StringPiece> components = base::SplitStringPiece(
-        entry, ":", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-    if (components.size() != 3 || components[0].size() != 4 ||
-        components[1].size() != 4 || components[2].size() != 4) {
-      continue;
-    }
-
-    uint32_t vendor_id;
-    uint32_t product_id;
-    uint32_t version;
-    if (!base::HexStringToUInt(components[0], &vendor_id) ||
-        !base::HexStringToUInt(components[1], &product_id) ||
-        !base::HexStringToUInt(components[2], &version)) {
-      continue;
-    }
-
-    Exclude(Entry(vendor_id, product_id, version));
-  }
-}
-
-bool UsbBlocklist::IsExcluded(const Entry& entry) {
-  return base::ContainsKey(blocklisted_devices_, entry);
-}
-
-bool UsbBlocklist::IsExcluded(scoped_refptr<const device::UsbDevice> device) {
+bool UsbBlocklist::IsExcluded(
+    const scoped_refptr<const device::UsbDevice>& device) const {
   return IsExcluded(Entry(device->vendor_id(), device->product_id(),
                           device->device_version()));
 }
 
 void UsbBlocklist::ResetToDefaultValuesForTest() {
-  blocklisted_devices_.clear();
-  PopulateWithDefaultValues();
+  dynamic_entries_.clear();
   PopulateWithServerProvidedValues();
 }
 
 UsbBlocklist::UsbBlocklist() {
-  PopulateWithDefaultValues();
+  DCHECK(std::is_sorted(std::begin(kStaticEntries), std::end(kStaticEntries),
+                        CompareEntry));
   PopulateWithServerProvidedValues();
 }
 
-void UsbBlocklist::PopulateWithDefaultValues() {
-  // To add a device to the blocklist add an entry here as well as configuring
-  // a Finch trial so that the blocklist update is pushed out to existing users
-  // as quickly as possible, e.g.:
-  //
-  // Exclude({ 0x18D0, 0x58F0, 0x1BAD });
-}
-
 void UsbBlocklist::PopulateWithServerProvidedValues() {
   std::string blocklist_string = variations::GetVariationParamValue(
       "WebUSBBlocklist", "blocklist_additions");
-  Exclude(blocklist_string);
+
+  for (const auto& entry :
+       base::SplitStringPiece(blocklist_string, ",", base::TRIM_WHITESPACE,
+                              base::SPLIT_WANT_NONEMPTY)) {
+    std::vector<base::StringPiece> components = base::SplitStringPiece(
+        entry, ":", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+    if (components.size() != 3 || !IsHexComponent(components[0]) ||
+        !IsHexComponent(components[1]) || !IsHexComponent(components[2])) {
+      continue;
+    }
+
+    uint32_t vendor_id;
+    uint32_t product_id;
+    uint32_t max_version;
+    if (!base::HexStringToUInt(components[0], &vendor_id) ||
+        !base::HexStringToUInt(components[1], &product_id) ||
+        !base::HexStringToUInt(components[2], &max_version)) {
+      continue;
+    }
+
+    dynamic_entries_.emplace_back(vendor_id, product_id, max_version);
+  }
+
+  std::sort(dynamic_entries_.begin(), dynamic_entries_.end(), CompareEntry);
 }
diff --git a/chrome/browser/usb/usb_blocklist.h b/chrome/browser/usb/usb_blocklist.h
index bc5a915..643f285 100644
--- a/chrome/browser/usb/usb_blocklist.h
+++ b/chrome/browser/usb/usb_blocklist.h
@@ -7,12 +7,11 @@
 
 #include <stdint.h>
 
-#include <set>
+#include <vector>
 
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "base/strings/string_piece.h"
 
 namespace device {
 class UsbDevice;
@@ -20,15 +19,20 @@
 
 class UsbBlocklist final {
  public:
-  // An entry in the blocklist. Represents a specific version of a device that
-  // should not be accessible. These fields correspond to the idVendor,
-  // idProduct and bcdDevice fields of the device's USB device descriptor.
+  // An entry in the blocklist. Represents a device that should not be
+  // accessible using WebUSB.
   struct Entry {
     Entry(uint16_t vendor_id, uint16_t product_id, uint16_t version);
 
+    // Matched against the idVendor field of the USB Device Descriptor.
     uint16_t vendor_id;
+
+    // Matched against the idProduct field of the USB Device Descriptor.
     uint16_t product_id;
-    uint16_t version;
+
+    // Compared against the bcdDevice field of the USB Device Descriptor. Any
+    // value less than or equal to this will be considered a match.
+    uint16_t max_version;
   };
 
   ~UsbBlocklist();
@@ -36,30 +40,12 @@
   // Returns a singleton instance of the blocklist.
   static UsbBlocklist& Get();
 
-  // Adds a device to the blocklist to be excluded from access.
-  void Exclude(const Entry&);
-
-  // Adds a device to the blocklist by parsing a blocklist string and calling
-  // Exclude(entry).
-  //
-  // The blocklist string must be a comma-separated list of
-  // idVendor:idProduct:bcdDevice triples, where each member of the triple is a
-  // 16-bit integer written as exactly 4 hexadecimal digits. The triples may
-  // be separated by whitespace. Triple components are colon-separated and must
-  // not have whitespace around the colon.
-  //
-  // Invalid entries in the comma-separated list will be ignored.
-  //
-  // Example:
-  //   "1000:001C:0100, 1000:001D:0101, 123:ignored:0"
-  void Exclude(base::StringPiece blocklist_string);
-
   // Returns if a device is excluded from access.
-  bool IsExcluded(const Entry&);
-  bool IsExcluded(scoped_refptr<const device::UsbDevice>);
+  bool IsExcluded(const Entry& entry) const;
+  bool IsExcluded(const scoped_refptr<const device::UsbDevice>& device) const;
 
   // Size of the blocklist.
-  size_t size() { return blocklisted_devices_.size(); }
+  size_t GetDynamicEntryCountForTest() const { return dynamic_entries_.size(); }
 
   // Reload the blocklist for testing purposes.
   void ResetToDefaultValuesForTest();
@@ -70,14 +56,24 @@
 
   UsbBlocklist();
 
-  void PopulateWithDefaultValues();
-
-  // Populates the blocklist with values obtained dynamically from a server,
-  // able to be updated without shipping new executable versions.
+  // Populates the blocklist with values set via a Finch experiment which allows
+  // the set of blocked devices to be updated without shipping new executable
+  // versions.
+  //
+  // The variation string must be a comma-separated list of
+  // vendor_id:product_id:max_version triples, where each member of the triple
+  // is a 16-bit integer written as exactly 4 hexadecimal digits. The triples
+  // may be separated by whitespace. Triple components are colon-separated and
+  // must not have whitespace around the colon.
+  //
+  // Invalid entries in the comma-separated list will be ignored.
+  //
+  // Example:
+  //   "1000:001C:0100, 1000:001D:0101, 123:ignored:0"
   void PopulateWithServerProvidedValues();
 
   // Set of blocklist entries.
-  std::set<Entry> blocklisted_devices_;
+  std::vector<Entry> dynamic_entries_;
 
   DISALLOW_COPY_AND_ASSIGN(UsbBlocklist);
 };
diff --git a/chrome/browser/usb/usb_blocklist_unittest.cc b/chrome/browser/usb/usb_blocklist_unittest.cc
index 7855048d..552a4b6 100644
--- a/chrome/browser/usb/usb_blocklist_unittest.cc
+++ b/chrome/browser/usb/usb_blocklist_unittest.cc
@@ -2,95 +2,136 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/metrics/field_trial.h"
 #include "chrome/browser/usb/usb_blocklist.h"
-#include "components/variations/variations_associated_data.h"
+#include "components/variations/variations_params_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class UsbBlocklistTest : public testing::Test {
  public:
-  UsbBlocklistTest()
-      : field_trial_list_(new base::FieldTrialList(nullptr)),
-        list_(UsbBlocklist::Get()) {}
+  UsbBlocklistTest() : blocklist_(UsbBlocklist::Get()) {}
 
+  const UsbBlocklist& list() { return blocklist_; }
+
+  void SetDynamicBlocklist(base::StringPiece list) {
+    params_manager_.ClearAllVariationParams();
+
+    std::map<std::string, std::string> params;
+    params["blocklist_additions"] = list.as_string();
+    params_manager_.SetVariationParams("WebUSBBlocklist", params);
+
+    blocklist_.ResetToDefaultValuesForTest();
+  }
+
+ private:
   void TearDown() override {
     // Because UsbBlocklist is a singleton it must be cleared after tests run
     // to prevent leakage between tests.
-    field_trial_list_.reset();
-    list_.ResetToDefaultValuesForTest();
+    params_manager_.ClearAllVariationParams();
+    blocklist_.ResetToDefaultValuesForTest();
   }
 
-  std::unique_ptr<base::FieldTrialList> field_trial_list_;
-  UsbBlocklist& list_;
+  variations::testing::VariationParamsManager params_manager_;
+  UsbBlocklist& blocklist_;
 };
 
 TEST_F(UsbBlocklistTest, BasicExclusions) {
-  list_.Exclude({0x18D1, 0x58F0, 0x0100});
-  EXPECT_TRUE(list_.IsExcluded({0x18D1, 0x58F0, 0x0100}));
-  EXPECT_FALSE(list_.IsExcluded({0x18D1, 0x58F1, 0x0100}));
-  EXPECT_FALSE(list_.IsExcluded({0x18D0, 0x58F0, 0x0100}));
-  EXPECT_FALSE(list_.IsExcluded({0x18D1, 0x58F0, 0x0200}));
+  SetDynamicBlocklist("18D1:58F0:0100");
+  EXPECT_TRUE(list().IsExcluded({0x18D1, 0x58F0, 0x0100}));
+  // An older device version is also blocked.
+  EXPECT_TRUE(list().IsExcluded({0x18D1, 0x58F0, 0x0090}));
+  // A newer device version is not blocked.
+  EXPECT_FALSE(list().IsExcluded({0x18D1, 0x58F0, 0x0200}));
+  // Other devices with nearby vendor and product IDs are not blocked.
+  EXPECT_FALSE(list().IsExcluded({0x18D1, 0x58F1, 0x0100}));
+  EXPECT_FALSE(list().IsExcluded({0x18D1, 0x58EF, 0x0100}));
+  EXPECT_FALSE(list().IsExcluded({0x18D0, 0x58F0, 0x0100}));
+  EXPECT_FALSE(list().IsExcluded({0x18D2, 0x58F0, 0x0100}));
 }
 
 TEST_F(UsbBlocklistTest, StringsWithNoValidEntries) {
-  size_t previous_list_size = list_.size();
-  list_.Exclude("");
-  list_.Exclude("~!@#$%^&*()-_=+[]{}/*-");
-  list_.Exclude(":");
-  list_.Exclude("::");
-  list_.Exclude(",");
-  list_.Exclude(",,");
-  list_.Exclude(",::,");
-  list_.Exclude("1:2:3");
-  list_.Exclude("18D1:2:3000");
-  list_.Exclude("☯");
-  EXPECT_EQ(previous_list_size, list_.size());
+  SetDynamicBlocklist("");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("~!@#$%^&*()-_=+[]{}/*-");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist(":");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("::");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist(",");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist(",,");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist(",::,");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("1:2:3");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("18D1:2:3000");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("0000:0x00:0000");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("0000:   0:0000");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("000g:0000:0000");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
+
+  SetDynamicBlocklist("☯");
+  EXPECT_EQ(0u, list().GetDynamicEntryCountForTest());
 }
 
 TEST_F(UsbBlocklistTest, StringsWithOneValidEntry) {
-  size_t previous_list_size = list_.size();
-  list_.Exclude("18D1:58F0:0101");
-  EXPECT_EQ(++previous_list_size, list_.size());
-  EXPECT_TRUE(list_.IsExcluded({0x18D1, 0x58F0, 0x0101}));
+  SetDynamicBlocklist("18D1:58F0:0101");
+  EXPECT_EQ(1u, list().GetDynamicEntryCountForTest());
+  EXPECT_TRUE(list().IsExcluded({0x18D1, 0x58F0, 0x0101}));
 
-  list_.Exclude(" 18D1:58F0:0200  ");
-  EXPECT_EQ(++previous_list_size, list_.size());
-  EXPECT_TRUE(list_.IsExcluded({0x18D1, 0x58F0, 0x0200}));
+  SetDynamicBlocklist(" 18D1:58F0:0200  ");
+  EXPECT_EQ(1u, list().GetDynamicEntryCountForTest());
+  EXPECT_TRUE(list().IsExcluded({0x18D1, 0x58F0, 0x0200}));
 
-  list_.Exclude(", 18D1:58F0:0201,  ");
-  EXPECT_EQ(++previous_list_size, list_.size());
-  EXPECT_TRUE(list_.IsExcluded({0x18D1, 0x58F0, 0x0201}));
+  SetDynamicBlocklist(", 18D1:58F0:0201,  ");
+  EXPECT_EQ(1u, list().GetDynamicEntryCountForTest());
+  EXPECT_TRUE(list().IsExcluded({0x18D1, 0x58F0, 0x0201}));
 
-  list_.Exclude("18D1:58F0:0202, 0000:1:0000");
-  EXPECT_EQ(++previous_list_size, list_.size());
-  EXPECT_TRUE(list_.IsExcluded({0x18D1, 0x58F0, 0x0202}));
+  SetDynamicBlocklist("18D1:58F0:0202, 0000:1:0000");
+  EXPECT_EQ(1u, list().GetDynamicEntryCountForTest());
+  EXPECT_TRUE(list().IsExcluded({0x18D1, 0x58F0, 0x0202}));
 }
 
-TEST_F(UsbBlocklistTest, ServerProvidedBlocklist) {
-  if (base::FieldTrialList::TrialExists("WebUSBBlocklist")) {
-    // This code checks to make sure that when a field trial is launched it
-    // still contains our test data.
-    LOG(INFO) << "WebUSBBlocklist field trial already configured.";
-    ASSERT_NE(variations::GetVariationParamValue("WebUSBBlocklist",
-                                                 "blocklist_additions")
-                  .find("18D1:58F0:1BAD"),
-              std::string::npos)
-        << "ERROR: A WebUSBBlocklist field trial has been configured in\n"
-           "testing/variations/fieldtrial_testing_config.json and must\n"
-           "include this test's excluded device ID '18D1:58F0:1BAD' in\n"
-           "blocklist_additions.\n";
-  } else {
-    LOG(INFO) << "Creating WebUSBBlocklist field trial for test.";
-    // Create a field trial with test parameter.
-    std::map<std::string, std::string> params;
-    params["blocklist_additions"] = "18D1:58F0:1BAD";
-    variations::AssociateVariationParams("WebUSBBlocklist", "TestGroup",
-                                         params);
-    base::FieldTrialList::CreateFieldTrial("WebUSBBlocklist", "TestGroup");
-    // Refresh the blocklist based on the new field trial.
-    list_.ResetToDefaultValuesForTest();
-  }
+TEST_F(UsbBlocklistTest, StaticEntries) {
+  // The specific versions of these devices that we want to block are unknown.
+  // The device versions listed here are abitrary chosen to test that any device
+  // will be matched.
 
-  EXPECT_TRUE(list_.IsExcluded({0x18D1, 0x58F0, 0x1BAD}));
-  EXPECT_FALSE(list_.IsExcluded({0x18D1, 0x58F0, 0x0100}));
+  // Yubikey NEO - OTP and CCID
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0111, 0x0100}));
+  // Yubikey NEO - CCID only
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0112, 0x0100}));
+  // Yubikey NEO - U2F and CCID
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0115, 0x0100}));
+  // Yubikey NEO - OTP, U2F and CCID
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0116, 0x0100}));
+  // Google Gnubby (WinUSB firmware)
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0211, 0x0100}));
+  // Yubikey 4 - CCID only
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0404, 0x0100}));
+  // Yubikey 4 - OTP and CCID
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0405, 0x0100}));
+  // Yubikey 4 - U2F and CCID
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0406, 0x0100}));
+  // Yubikey 4 - OTP, U2F and CCID
+  EXPECT_TRUE(list().IsExcluded({0x1050, 0x0407, 0x0100}));
+
+  // The non-WinUSB version of the Google Gnubby firmware is not in the static
+  // list. Check that it is not matched despite a similar product ID.
+  EXPECT_FALSE(list().IsExcluded({0x1050, 0x0200, 0x0100}));
 }