WebBluetooth - Scanning should use the same device id hashing as Gatt.

This CL adds a new method to WebBluetoothDeviceId which allows us to
create a device id from a device address without adding any services
which allows scanning to share the same device address -> device id
origin specific mapping.

Bug: 897312
Change-Id: Ib3dda1c2d47138486c1457eb25a8dc602b52e2f0
Reviewed-on: https://chromium-review.googlesource.com/c/1405368
Commit-Queue: Doug Turner <dougt@chromium.org>
Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623520}
diff --git a/content/browser/bluetooth/bluetooth_allowed_devices.cc b/content/browser/bluetooth/bluetooth_allowed_devices.cc
index a3c2607..dda74a0 100644
--- a/content/browser/bluetooth/bluetooth_allowed_devices.cc
+++ b/content/browser/bluetooth/bluetooth_allowed_devices.cc
@@ -26,15 +26,24 @@
 const WebBluetoothDeviceId& BluetoothAllowedDevices::AddDevice(
     const std::string& device_address,
     const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
+  auto& device_id = AddDevice(device_address);
+  AddUnionOfServicesTo(options, &device_id_to_services_map_[device_id]);
+
+  // Currently, devices that are added with WebBluetoothRequestDeviceOptionsPtr
+  // |options| come from RequestDevice() and therefore have the ablity to be
+  // connected to.
+  device_id_to_connectable_map_[device_id] = true;
+
+  return device_id;
+}
+
+const WebBluetoothDeviceId& BluetoothAllowedDevices::AddDevice(
+    const std::string& device_address) {
   DVLOG(1) << "Adding a device to Map of Allowed Devices.";
 
   auto id_iter = device_address_to_id_map_.find(device_address);
   if (id_iter != device_address_to_id_map_.end()) {
     DVLOG(1) << "Device already in map of allowed devices.";
-    const auto& device_id = id_iter->second;
-
-    AddUnionOfServicesTo(options, &device_id_to_services_map_[device_id]);
-
     return device_address_to_id_map_[device_address];
   }
   const WebBluetoothDeviceId device_id = GenerateUniqueDeviceId();
@@ -42,7 +51,6 @@
 
   device_address_to_id_map_[device_address] = device_id;
   device_id_to_address_map_[device_id] = device_address;
-  AddUnionOfServicesTo(options, &device_id_to_services_map_[device_id]);
 
   CHECK(device_id_set_.insert(device_id).second);
 
@@ -62,6 +70,9 @@
   CHECK(device_id_to_address_map_.erase(device_id));
   CHECK(device_id_to_services_map_.erase(device_id));
 
+  // Not all devices are connectable.
+  device_id_to_connectable_map_.erase(device_id);
+
   // 2. Remove from set of ids.
   CHECK(device_id_set_.erase(device_id));
 }
@@ -105,6 +116,14 @@
              : base::ContainsKey(id_iter->second, service_uuid);
 }
 
+bool BluetoothAllowedDevices::IsAllowedToGATTConnect(
+    const WebBluetoothDeviceId& device_id) const {
+  auto id_iter = device_id_to_connectable_map_.find(device_id);
+  if (id_iter == device_id_to_connectable_map_.end())
+    return false;
+  return id_iter->second;
+}
+
 WebBluetoothDeviceId BluetoothAllowedDevices::GenerateUniqueDeviceId() {
   WebBluetoothDeviceId device_id = WebBluetoothDeviceId::Create();
   while (base::ContainsKey(device_id_set_, device_id)) {
diff --git a/content/browser/bluetooth/bluetooth_allowed_devices.h b/content/browser/bluetooth/bluetooth_allowed_devices.h
index dc5223c2..ce9d66d 100644
--- a/content/browser/bluetooth/bluetooth_allowed_devices.h
+++ b/content/browser/bluetooth/bluetooth_allowed_devices.h
@@ -40,6 +40,10 @@
       const std::string& device_address,
       const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options);
 
+  // Same as the above version of |AddDevice| but does not add any services to
+  // the device id -> services map.
+  const WebBluetoothDeviceId& AddDevice(const std::string& device_address);
+
   // Removes the Bluetooth Device with |device_address| from the map of allowed
   // devices.
   void RemoveDevice(const std::string& device_address);
@@ -62,6 +66,8 @@
       const WebBluetoothDeviceId& device_id,
       const device::BluetoothUUID& service_uuid) const;
 
+  bool IsAllowedToGATTConnect(const WebBluetoothDeviceId& device_id) const;
+
  private:
   typedef std::unordered_map<std::string, WebBluetoothDeviceId>
       DeviceAddressToIdMap;
@@ -73,6 +79,9 @@
       std::unordered_set<device::BluetoothUUID, device::BluetoothUUIDHash>,
       WebBluetoothDeviceIdHash>
       DeviceIdToServicesMap;
+  typedef std::
+      unordered_map<WebBluetoothDeviceId, bool, WebBluetoothDeviceIdHash>
+          DeviceIdToConnectableMap;
 
   // Returns an id guaranteed to be unique for the map. The id is randomly
   // generated so that an origin can't guess the id used in another origin.
@@ -85,6 +94,7 @@
   DeviceAddressToIdMap device_address_to_id_map_;
   DeviceIdToAddressMap device_id_to_address_map_;
   DeviceIdToServicesMap device_id_to_services_map_;
+  DeviceIdToConnectableMap device_id_to_connectable_map_;
 
   // Keep track of all device_ids in the map.
   std::unordered_set<WebBluetoothDeviceId, WebBluetoothDeviceIdHash>
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index 0ec5c2e..4728c4f 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -365,7 +365,7 @@
 }
 
 void WebBluetoothServiceImpl::DeviceAdvertisementReceived(
-    const std::string& device_id,
+    const std::string& device_address,
     const base::Optional<std::string>& device_name,
     const base::Optional<std::string>& advertisement_name,
     base::Optional<int8_t> rssi,
@@ -382,15 +382,7 @@
   auto client = scanning_clients_.begin();
   while (client != scanning_clients_.end()) {
     auto device = blink::mojom::WebBluetoothDevice::New();
-    // TODO(dougt):
-    // Currently, Web Bluetooth (GATT Connections) obfuscate the device
-    // address, here called the |device->id|. However, for scanning, it
-    // may be desirable to expose the actual MAC.
-    //
-    // What needs to happen next is for us to figure out if we can expose
-    // the MAC address or if we need to obfuscate like we do with GATT
-    // Connections.
-    device->id = WebBluetoothDeviceId(device_id, /*is_mac_address=*/true);
+    device->id = allowed_devices().AddDevice(device_address);
     device->name = device_name;
 
     auto result = blink::mojom::WebBluetoothScanResult::New();
@@ -520,6 +512,12 @@
     RemoteServerConnectCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+  if (!allowed_devices().IsAllowedToGATTConnect(device_id)) {
+    std::move(callback).Run(
+        blink::mojom::WebBluetoothResult::GATT_NOT_AUTHORIZED);
+    return;
+  }
+
   const CacheQueryResult query_result = QueryCacheForDevice(device_id);
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
diff --git a/content/common/bluetooth/web_bluetooth_device_id.cc b/content/common/bluetooth/web_bluetooth_device_id.cc
index 8dbae42..4d6f095 100644
--- a/content/common/bluetooth/web_bluetooth_device_id.cc
+++ b/content/common/bluetooth/web_bluetooth_device_id.cc
@@ -16,11 +16,10 @@
 
 }  // namespace
 
-WebBluetoothDeviceId::WebBluetoothDeviceId() : is_mac_address_(false) {}
+WebBluetoothDeviceId::WebBluetoothDeviceId() {}
 
-WebBluetoothDeviceId::WebBluetoothDeviceId(std::string device_id,
-                                           bool is_mac_address)
-    : device_id_(std::move(device_id)), is_mac_address_(is_mac_address) {
+WebBluetoothDeviceId::WebBluetoothDeviceId(std::string device_id)
+    : device_id_(std::move(device_id)) {
   CHECK(IsValid());
 }
 
@@ -43,17 +42,11 @@
 
   base::Base64Encode(bytes, &bytes);
 
-  return WebBluetoothDeviceId(std::move(bytes), false);
+  return WebBluetoothDeviceId(std::move(bytes));
 }
 
 // static
-bool WebBluetoothDeviceId::IsValid(const std::string& device_id,
-                                   bool is_mac_address) {
-  if (is_mac_address) {
-    // TODO(dougt) We should validate this as a MAC address.
-    return true;
-  }
-
+bool WebBluetoothDeviceId::IsValid(const std::string& device_id) {
   std::string decoded;
   if (!base::Base64Decode(device_id, &decoded)) {
     return false;
@@ -77,7 +70,7 @@
 }
 
 bool WebBluetoothDeviceId::IsValid() const {
-  return WebBluetoothDeviceId::IsValid(device_id_, is_mac_address_);
+  return WebBluetoothDeviceId::IsValid(device_id_);
 }
 
 bool WebBluetoothDeviceId::operator==(
diff --git a/content/common/bluetooth/web_bluetooth_device_id.h b/content/common/bluetooth/web_bluetooth_device_id.h
index cba30bc..87445e8 100644
--- a/content/common/bluetooth/web_bluetooth_device_id.h
+++ b/content/common/bluetooth/web_bluetooth_device_id.h
@@ -22,9 +22,8 @@
   // resulting object will DCHECK-fail.
   WebBluetoothDeviceId();
 
-  // CHECKS that |device_id| is valid if |is_mac_address| is false.
-  explicit WebBluetoothDeviceId(std::string device_id,
-                                bool is_mac_address = false);
+  // CHECKS that |device_id| is valid.
+  explicit WebBluetoothDeviceId(std::string device_id);
   ~WebBluetoothDeviceId();
 
   // Returns the string that represents this WebBluetoothDeviceId.
@@ -34,12 +33,9 @@
   // string and base64-encoding it.
   static WebBluetoothDeviceId Create();
 
-  // If |is_mac_address| is true, the method will return true if |device_id| is
-  // a MAC address.  If |is_mac_address| is false, this method will return true
-  // if |device_id| results in a 128bit base64-encoding string. Otherwise
-  // returns false.
-  static bool IsValid(const std::string& device_id,
-                      bool is_mac_address = false);
+  // This method will return true. if |device_id| results in a 128bit
+  // base64-encoding string. Otherwise returns false.
+  static bool IsValid(const std::string& device_id);
 
   bool IsValid() const;
 
@@ -48,7 +44,6 @@
 
  private:
   std::string device_id_;
-  bool is_mac_address_;
 };
 
 // This is required by gtest to print a readable output on test failures.
diff --git a/third_party/blink/web_tests/bluetooth/requestLEScan/attempt-to-connect-after-scan.https.html b/third_party/blink/web_tests/bluetooth/requestLEScan/attempt-to-connect-after-scan.https.html
new file mode 100644
index 0000000..ced558f
--- /dev/null
+++ b/third_party/blink/web_tests/bluetooth/requestLEScan/attempt-to-connect-after-scan.https.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../resources/testdriver.js"></script>
+<script src="../../resources/testdriver-vendor.js"></script>
+<script src="../../external/wpt/bluetooth/resources/bluetooth-helpers.js"></script>
+<script src="../../external/wpt/bluetooth/resources/bluetooth-scanning-helpers.js"></script>
+<script>
+'use strict';
+const test_desc = 'Attempt to connect to scan result.';
+
+bluetooth_test(async (t) => {
+  const fake_central =
+      await navigator.bluetooth.test.simulateCentral({state: 'powered-on'});
+
+  const fake_device_address = '09:09:09:09:09:09';
+
+  const fake_device = await fake_central.simulatePreconnectedPeripheral({
+    address: fake_device_address,
+    name: 'Some Device',
+    knownServiceUUIDs: [],
+  });
+
+  await callWithTrustedClick(async () => {
+    await navigator.bluetooth.requestLEScan({acceptAllAdvertisements: true});
+  });
+
+  const eventWatcher =
+      new EventWatcher(t, navigator.bluetooth, ['advertisementreceived']);
+
+  let scan = {
+    deviceAddress: fake_device_address,
+    rssi: 100,
+    scanRecord: {
+      txPower: 20,
+    },
+  };
+
+  fake_central.simulateAdvertisementReceived(scan);
+
+  const expected =
+      new DOMException('GATT operation not authorized.', 'SecurityError');
+
+  await eventWatcher.wait_for(['advertisementreceived']).then(async (e) => {
+    assert_promise_rejects_with_message(e.device.gatt.connect(), expected);
+  })
+}, test_desc);
+</script>
diff --git a/third_party/blink/web_tests/bluetooth/requestLEScan/device-ids-match.https.html b/third_party/blink/web_tests/bluetooth/requestLEScan/device-ids-match.https.html
new file mode 100644
index 0000000..73bd091
--- /dev/null
+++ b/third_party/blink/web_tests/bluetooth/requestLEScan/device-ids-match.https.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../resources/testdriver.js"></script>
+<script src="../../resources/testdriver-vendor.js"></script>
+<script src="../../external/wpt/bluetooth/resources/bluetooth-helpers.js"></script>
+<script src="../../external/wpt/bluetooth/resources/bluetooth-scanning-helpers.js"></script>
+<script>
+'use strict';
+const test_desc = 'Device address have the same device id.';
+
+bluetooth_test(async (t) => {
+  const fake_central =
+      await navigator.bluetooth.test.simulateCentral({state: 'powered-on'});
+
+  await callWithTrustedClick(async () => {
+    await navigator.bluetooth.requestLEScan({acceptAllAdvertisements: true});
+  });
+
+  const eventWatcher =
+      new EventWatcher(t, navigator.bluetooth, ['advertisementreceived']);
+
+  let scan = {
+    deviceAddress: '09:09:09:09:09:09',
+    rssi: 100,
+    scanRecord: {
+      txPower: 20,
+    },
+  };
+
+  let actual_device_id;
+
+  fake_central.simulateAdvertisementReceived(scan);
+  await eventWatcher.wait_for(['advertisementreceived']).then(e => {
+    actual_device_id = e.device.id;
+  });
+
+  fake_central.simulateAdvertisementReceived(scan);
+  await eventWatcher.wait_for(['advertisementreceived']).then(e => {
+    assert_equals(e.device.id, actual_device_id);
+  });
+
+  scan.deviceAddress = 'FF:FF:FF:FF:FF:FF'
+  fake_central.simulateAdvertisementReceived(scan);
+  await eventWatcher.wait_for(['advertisementreceived']).then(e => {
+    assert_not_equals(e.device.id, actual_device_id);
+  });
+}, test_desc);
+</script>