bluetooth: Implement allowed devices map

content/browser/bluetooth/bluetooth_dispatcher_host.cc

Index: content/browser/bluetooth/bluetooth_dispatcher_host.cc
diff --git a/content/browser/bluetooth/bluetooth_dispatcher_host.cc b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
index fcb4ada4d4029ff35814bbae1f0fa55a4e70dbc5..c8a4732f51e43a3e90c5c940cfa1e0c5c20e40ef 100644
--- a/content/browser/bluetooth/bluetooth_dispatcher_host.cc
+++ b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
@@ -340,10 +340,12 @@ struct BluetoothDispatcherHost::RequestDeviceSession {
  public:
   RequestDeviceSession(int thread_id,
                        int request_id,
+                       url::Origin origin,
                        const std::vector<BluetoothScanFilter>& filters,
                        const std::vector<BluetoothUUID>& optional_services)
       : thread_id(thread_id),
         request_id(request_id),
+        origin(origin),
         filters(filters),
         optional_services(optional_services) {}
 
@@ -369,6 +371,7 @@ struct BluetoothDispatcherHost::RequestDeviceSession {
 
   const int thread_id;
   const int request_id;
+  const url::Origin origin;
   const std::vector<BluetoothScanFilter> filters;
   const std::vector<BluetoothUUID> optional_services;
   scoped_ptr<BluetoothChooser> chooser;
@@ -521,12 +524,12 @@ void BluetoothDispatcherHost::GattServicesDiscovered(
     device::BluetoothAdapter* adapter,
     device::BluetoothDevice* device) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  const std::string& device_id = device->GetAddress();
-  VLOG(1) << "Services discovered for device: " << device_id;
+  const std::string& device_address = device->GetAddress();
+  VLOG(1) << "Services discovered for device: " << device_address;
 
-  devices_with_discovered_services_.insert(device_id);
+  devices_with_discovered_services_.insert(device_address);
 
-  auto iter = pending_primary_services_requests_.find(device_id);
+  auto iter = pending_primary_services_requests_.find(device_address);
   if (iter == pending_primary_services_requests_.end()) {
     return;
   }
@@ -556,7 +559,7 @@ void BluetoothDispatcherHost::GattServicesDiscovered(
         break;
     }
   }
-  DCHECK(!ContainsKey(pending_primary_services_requests_, device_id))
+  DCHECK(!ContainsKey(pending_primary_services_requests_, device_address))
       << "Sending get-service responses unexpectedly queued another request.";
 }
 
@@ -661,7 +664,8 @@ void BluetoothDispatcherHost::OnRequestDevice(
   // Create storage for the information that backs the chooser, and show the
   // chooser.
   RequestDeviceSession* const session = new RequestDeviceSession(
-      thread_id, request_id, filters, optional_services);
+      thread_id, request_id, render_frame_host->GetLastCommittedOrigin(),
+      filters, optional_services);
   int chooser_id = request_device_sessions_.Add(session);
 
   BluetoothChooser::EventHandler chooser_event_handler =
@@ -672,6 +676,8 @@ void BluetoothDispatcherHost::OnRequestDevice(
     if (WebContentsDelegate* delegate = web_contents->GetDelegate()) {
       session->chooser = delegate->RunBluetoothChooser(
           web_contents, chooser_event_handler,
+          // TODO(ortuno): Replace with GetLastCommittedOrigin.
+          // http://crbug.com/577451
           render_frame_host->GetLastCommittedURL().GetOrigin());
     }
   }
@@ -713,17 +719,14 @@ void BluetoothDispatcherHost::OnRequestDevice(
 
 void BluetoothDispatcherHost::OnConnectGATT(int thread_id,
                                             int request_id,
+                                            int frame_routing_id,
                                             const std::string& device_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::CONNECT_GATT);
   const base::TimeTicks start_time = base::TimeTicks::Now();
 
-  // TODO(ortuno): Right now it's pointless to check if the domain has access to
-  // the device, because any domain can connect to any device. But once
-  // permissions are implemented we should check that the domain has access to
-  // the device. https://crbug.com/484745
-
-  const CacheQueryResult query_result = QueryCacheForDevice(device_id);
+  const CacheQueryResult query_result =
+      QueryCacheForDevice(GetOrigin(frame_routing_id), device_id);
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordConnectGATTOutcome(query_result.outcome);
@@ -744,18 +747,18 @@ void BluetoothDispatcherHost::OnConnectGATT(int thread_id,
 void BluetoothDispatcherHost::OnGetPrimaryService(
     int thread_id,
     int request_id,
+    int frame_routing_id,
     const std::string& device_id,
     const std::string& service_uuid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::GET_PRIMARY_SERVICE);
   RecordGetPrimaryServiceService(BluetoothUUID(service_uuid));
 
-  // TODO(ortuno): Check if device_id is in "allowed devices"
-  // https://crbug.com/493459
   // TODO(ortuno): Check if service_uuid is in "allowed services"
   // https://crbug.com/493460
 
-  const CacheQueryResult query_result = QueryCacheForDevice(device_id);
+  const CacheQueryResult query_result =
+      QueryCacheForDevice(GetOrigin(frame_routing_id), device_id);
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordGetPrimaryServiceOutcome(query_result.outcome);
@@ -788,7 +791,7 @@ void BluetoothDispatcherHost::OnGetPrimaryService(
   }
 
   // 3.
-  if (IsServicesDiscoveryCompleteForDevice(device_id)) {
+  if (IsServicesDiscoveryCompleteForDevice(query_result.device->GetAddress())) {
     VLOG(1) << "Service not found in device.";
     RecordGetPrimaryServiceOutcome(UMAGetPrimaryServiceOutcome::NOT_FOUND);
     Send(new BluetoothMsg_GetPrimaryServiceError(
@@ -799,7 +802,7 @@ void BluetoothDispatcherHost::OnGetPrimaryService(
   VLOG(1) << "Adding service request to pending requests.";
   // 4.
   AddToPendingPrimaryServicesRequest(
-      device_id,
+      query_result.device->GetAddress(),
       PrimaryServicesRequest(thread_id, request_id, service_uuid,
                              PrimaryServicesRequest::GET_PRIMARY_SERVICE));
 }
@@ -807,6 +810,7 @@ void BluetoothDispatcherHost::OnGetPrimaryService(
 void BluetoothDispatcherHost::OnGetCharacteristic(
     int thread_id,
     int request_id,
+    int frame_routing_id,
     const std::string& service_instance_id,
     const std::string& characteristic_uuid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -814,7 +818,7 @@ void BluetoothDispatcherHost::OnGetCharacteristic(
   RecordGetCharacteristicCharacteristic(characteristic_uuid);
 
   const CacheQueryResult query_result =
-      QueryCacheForService(service_instance_id);
+      QueryCacheForService(GetOrigin(frame_routing_id), service_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     return;
@@ -857,13 +861,14 @@ void BluetoothDispatcherHost::OnGetCharacteristic(
 void BluetoothDispatcherHost::OnReadValue(
     int thread_id,
     int request_id,
+    int frame_routing_id,
     const std::string& characteristic_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_READ_VALUE);
 
-  const CacheQueryResult query_result =
-      QueryCacheForCharacteristic(characteristic_instance_id);
+  const CacheQueryResult query_result = QueryCacheForCharacteristic(
+      GetOrigin(frame_routing_id), characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     return;
@@ -886,6 +891,7 @@ void BluetoothDispatcherHost::OnReadValue(
 void BluetoothDispatcherHost::OnWriteValue(
     int thread_id,
     int request_id,
+    int frame_routing_id,
     const std::string& characteristic_instance_id,
     const std::vector<uint8_t>& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -903,8 +909,8 @@ void BluetoothDispatcherHost::OnWriteValue(
     return;
   }
 
-  const CacheQueryResult query_result =
-      QueryCacheForCharacteristic(characteristic_instance_id);
+  const CacheQueryResult query_result = QueryCacheForCharacteristic(
+      GetOrigin(frame_routing_id), characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     return;
@@ -927,6 +933,7 @@ void BluetoothDispatcherHost::OnWriteValue(
 void BluetoothDispatcherHost::OnStartNotifications(
     int thread_id,
     int request_id,
+    int frame_routing_id,
     const std::string& characteristic_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
@@ -944,8 +951,8 @@ void BluetoothDispatcherHost::OnStartNotifications(
   // TODO(ortuno): Check if notify/indicate bit is set.
   // http://crbug.com/538869
 
-  const CacheQueryResult query_result =
-      QueryCacheForCharacteristic(characteristic_instance_id);
+  const CacheQueryResult query_result = QueryCacheForCharacteristic(
+      GetOrigin(frame_routing_id), characteristic_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     return;
@@ -968,11 +975,21 @@ void BluetoothDispatcherHost::OnStartNotifications(
 void BluetoothDispatcherHost::OnStopNotifications(
     int thread_id,
     int request_id,
+    int frame_routing_id,
     const std::string& characteristic_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_STOP_NOTIFICATIONS);
 
+  // Check the origin is allowed to access the device. We perform this check in

[palmer, 2016/01/15 01:13:59]

Since this code is the same as that on line 1010, I wonder if you could extract
it out into a helper function. A meaningful name and interface might even
obviate the comments, e.g.:

    bool CanFrameAccessInstance(int frame_id, const std::string&
characteristic_instance_id) {
      return QueryCacheForCharacteristic(GetOrigin(frame_id),
                                         characteristic_instance_id)
          .outcome != CacheQueryOutcome::BAD_RENDERER;
    }

[ortuno, 2016/01/15 02:52:22]

Done.

+  // case a hostile renderer is trying to stop notifications for a device
+  // that the renderer is not allowed to access.
+  if (QueryCacheForCharacteristic(GetOrigin(frame_routing_id),
+                                  characteristic_instance_id)
+          .outcome == CacheQueryOutcome::BAD_RENDERER) {
+    return;
+  }
+
   auto notify_session_iter =
       characteristic_id_to_notify_session_.find(characteristic_instance_id);
   if (notify_session_iter == characteristic_id_to_notify_session_.end()) {
@@ -987,13 +1004,21 @@ void BluetoothDispatcherHost::OnStopNotifications(
 
 void BluetoothDispatcherHost::OnRegisterCharacteristicObject(
     int thread_id,
+    int frame_routing_id,
     const std::string& characteristic_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // Make sure the origin is allowed to access the device.
+  if (QueryCacheForCharacteristic(GetOrigin(frame_routing_id),
+                                  characteristic_instance_id)
+          .outcome == CacheQueryOutcome::BAD_RENDERER) {
+    return;
+  }
   active_characteristic_threads_[characteristic_instance_id].insert(thread_id);
 }
 
 void BluetoothDispatcherHost::OnUnregisterCharacteristicObject(
     int thread_id,
+    int frame_routing_id,
     const std::string& characteristic_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   auto active_iter =
@@ -1119,6 +1144,8 @@ void BluetoothDispatcherHost::FinishClosingChooser(
   DCHECK_EQ(static_cast<int>(event),
             static_cast<int>(BluetoothChooser::Event::SELECTED));
 
+  // device_id is device_address that RequestDeviceSession passes to

[palmer, 2016/01/15 01:13:59]

Nit: Delineate identifiers with |...|.

Also, conflating device IDs with device addresses seems a bit confusing? I may
have mentioned this before, but it may be that we need more typeful interfaces
here, e.g. a |DeviceId| class that has-a string and whose constructor validates
that the string adheres to the lexical rules for a device ID. Then |AddDevice|
can explain itself more clearly by returning a |DeviceId|, functions can declare
that they take |DeviceId|s as input, et c.

[ortuno, 2016/01/15 02:52:22]

I know, I spent quite a bit of time trying to decide the best way to tackle this
issue, which becomes even more confusing since BluetoothDevice's also have a
device_id. I like your idea and opened an issue for it: http://crbug.com/577962

+  // chooser->AddDevice().
   const device::BluetoothDevice* const device = adapter_->GetDevice(device_id);
   if (device == nullptr) {
     VLOG(1) << "Device " << device_id << " no longer in adapter";
@@ -1135,8 +1162,12 @@ void BluetoothDispatcherHost::FinishClosingChooser(
   for (BluetoothUUID uuid : device->GetUUIDs())
     VLOG(1) << "\t" << uuid.canonical_value();
 
+  const std::string& device_id_for_origin = allowed_devices_map_.AddDevice(
+      session->origin, device->GetAddress(), session->filters,
+      session->optional_services);
+
   content::BluetoothDevice device_ipc(
-      device->GetAddress(),  // id
+      device_id_for_origin,  // id
       device->GetName(),     // name
       content::BluetoothDevice::ValidatePower(
           device->GetInquiryTxPower()),  // tx_power
@@ -1189,13 +1220,13 @@ void BluetoothDispatcherHost::AddToServicesMapAndSendGetPrimaryServiceSuccess(
     int thread_id,
     int request_id) {
   const std::string& service_identifier = service.GetIdentifier();
-  const std::string& device_id = service.GetDevice()->GetAddress();
+  const std::string& device_address = service.GetDevice()->GetAddress();
   auto insert_result =
-      service_to_device_.insert(make_pair(service_identifier, device_id));
+      service_to_device_.insert(make_pair(service_identifier, device_address));
 
   // If a value is already in map, DCHECK it's valid.
   if (!insert_result.second)
-    DCHECK_EQ(insert_result.first->second, device_id);
+    DCHECK_EQ(insert_result.first->second, device_address);
 
   RecordGetPrimaryServiceOutcome(UMAGetPrimaryServiceOutcome::SUCCESS);
   Send(new BluetoothMsg_GetPrimaryServiceSuccess(thread_id, request_id,
@@ -1276,9 +1307,19 @@ void BluetoothDispatcherHost::OnStopNotifySession(
 }
 
 BluetoothDispatcherHost::CacheQueryResult
-BluetoothDispatcherHost::QueryCacheForDevice(const std::string& device_id) {
+BluetoothDispatcherHost::QueryCacheForDevice(const url::Origin& origin,
+                                             const std::string& device_id) {
+  const std::string& device_address =
+      allowed_devices_map_.GetDeviceAddress(origin, device_id);
+  if (device_address.empty()) {
+    bad_message::ReceivedBadMessage(
+        this, bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);
+    return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
+  }
+
   CacheQueryResult result;
-  result.device = adapter_->GetDevice(device_id);
+  result.device = adapter_->GetDevice(device_address);
+
   // When a device can't be found in the BluetoothAdapter, that generally
   // indicates that it's gone out of range. We reject with a NetworkError in
   // that case.
@@ -1291,6 +1332,7 @@ BluetoothDispatcherHost::QueryCacheForDevice(const std::string& device_id) {
 
 BluetoothDispatcherHost::CacheQueryResult
 BluetoothDispatcherHost::QueryCacheForService(
+    const url::Origin& origin,
     const std::string& service_instance_id) {
   auto device_iter = service_to_device_.find(service_instance_id);
 
@@ -1300,10 +1342,16 @@ BluetoothDispatcherHost::QueryCacheForService(
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }
 
-  // TODO(ortuno): Check if domain has access to device.
-  // https://crbug.com/493459
+  const std::string& device_id =
+      allowed_devices_map_.GetDeviceId(origin, device_iter->second);
+  // Kill the renderer if the origin is not allowed to access the device.
+  if (device_id.empty()) {
+    bad_message::ReceivedBadMessage(
+        this, bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);
+    return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
+  }
 
-  CacheQueryResult result = QueryCacheForDevice(device_iter->second);
+  CacheQueryResult result = QueryCacheForDevice(origin, device_id);
 
   if (result.outcome != CacheQueryOutcome::SUCCESS) {
     return result;
@@ -1318,6 +1366,7 @@ BluetoothDispatcherHost::QueryCacheForService(
 
 BluetoothDispatcherHost::CacheQueryResult
 BluetoothDispatcherHost::QueryCacheForCharacteristic(
+    const url::Origin& origin,
     const std::string& characteristic_instance_id) {
   auto characteristic_iter =
       characteristic_to_service_.find(characteristic_instance_id);
@@ -1329,7 +1378,8 @@ BluetoothDispatcherHost::QueryCacheForCharacteristic(
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }
 
-  CacheQueryResult result = QueryCacheForService(characteristic_iter->second);
+  CacheQueryResult result =
+      QueryCacheForService(origin, characteristic_iter->second);
   if (result.outcome != CacheQueryOutcome::SUCCESS) {
     return result;
   }
@@ -1345,14 +1395,19 @@ BluetoothDispatcherHost::QueryCacheForCharacteristic(
 }
 
 bool BluetoothDispatcherHost::IsServicesDiscoveryCompleteForDevice(
-    const std::string& device_id) {
-  return ContainsKey(devices_with_discovered_services_, device_id);
+    const std::string& device_address) {
+  return ContainsKey(devices_with_discovered_services_, device_address);
 }
 
 void BluetoothDispatcherHost::AddToPendingPrimaryServicesRequest(
-    const std::string& device_id,
+    const std::string& device_address,
     const PrimaryServicesRequest& request) {
-  pending_primary_services_requests_[device_id].push_back(request);
+  pending_primary_services_requests_[device_address].push_back(request);
+}
+
+url::Origin BluetoothDispatcherHost::GetOrigin(int frame_routing_id) {
+  return RenderFrameHostImpl::FromID(render_process_id_, frame_routing_id)
+      ->GetLastCommittedOrigin();
 }
 
 void BluetoothDispatcherHost::ShowBluetoothOverviewLink() {