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 7758a3e2c31d819b30e159fd32921471854a9046..6c7e6e22989cb97f384e957283d6f503eb509c72 100644
--- a/content/browser/bluetooth/bluetooth_dispatcher_host.cc
+++ b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
@@ -314,10 +314,12 @@ struct BluetoothDispatcherHost::RequestDeviceSession {
  public:
   RequestDeviceSession(int thread_id,
                        int request_id,
+                       GURL 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) {}
 
@@ -343,6 +345,7 @@ struct BluetoothDispatcherHost::RequestDeviceSession {
 
   const int thread_id;
   const int request_id;
+  const GURL origin;
   const std::vector<BluetoothScanFilter> filters;
   const std::vector<BluetoothUUID> optional_services;
   scoped_ptr<BluetoothChooser> chooser;
@@ -495,12 +498,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;
   }
@@ -530,7 +533,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.";
 }
 
@@ -635,7 +638,9 @@ 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->GetLastCommittedURL().GetOrigin(), filters,
+      optional_services);
   int chooser_id = request_device_sessions_.Add(session);
 
   BluetoothChooser::EventHandler chooser_event_handler =
@@ -645,8 +650,7 @@ void BluetoothDispatcherHost::OnRequestDevice(
           WebContents::FromRenderFrameHost(render_frame_host)) {
     if (WebContentsDelegate* delegate = web_contents->GetDelegate()) {
       session->chooser = delegate->RunBluetoothChooser(
-          web_contents, chooser_event_handler,
-          render_frame_host->GetLastCommittedURL().GetOrigin());
+          web_contents, chooser_event_handler, session->origin);
     }
   }
   if (!session->chooser) {
@@ -687,17 +691,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);
@@ -718,18 +719,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);
@@ -762,7 +763,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(
@@ -773,7 +774,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));
 }
@@ -781,6 +782,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);
@@ -788,7 +790,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;
@@ -831,13 +833,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;
@@ -860,6 +863,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);
@@ -877,8 +881,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;
@@ -901,6 +905,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(
@@ -918,8 +923,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;
@@ -942,11 +947,19 @@ 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);
 
+  // Make sure the origin is allowed to access the device.

[Jeffrey Yasskin, 2016/01/06 00:47:57]

Instead of "Make sure", we should probably say why returning is the right thing
to do. e.g. "If the frame navigates to a different origin while the
StopNotifications() request is in flight, we won't find the right device
address, so we can't stop its notifications. Return instead." (Although that's
probably not the right reason, since it would imply that we leak the
notification sessions on navigation, and we shouldn't let that happen.)

[ortuno, 2016/01/13 01:41:44]

These checks are in place in case of a hostile renderer. There isn't much a
hostile renderer could do with stopNotifications(), so I could remove it.

[Jeffrey Yasskin, 2016/01/13 02:31:36]

The effect would be that a renderer could guess another origin's characteristic
ID, navigate there, and then stop their session. Because the IDs are 128 bits
and random, that guess isn't going to happen, so I think it's safe to remove the
early-return. But mention that analysis in a comment.

[ortuno, 2016/01/13 22:12:26]

As discussed, I added a comment explaining why we need this check.

+  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()) {
@@ -961,13 +974,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 =
@@ -1093,6 +1114,7 @@ void BluetoothDispatcherHost::FinishClosingChooser(
   DCHECK_EQ(static_cast<int>(event),
             static_cast<int>(BluetoothChooser::Event::SELECTED));
 
+  // device_id is device_address since we passed it to the chooser.

[Jeffrey Yasskin, 2016/01/06 00:47:57]

Maybe "is the device address that RequestDeviceSession passes to
chooser->AddDevice()."

[ortuno, 2016/01/13 01:41:44]

Done.

   const device::BluetoothDevice* const device = adapter_->GetDevice(device_id);
   if (device == nullptr) {
     VLOG(1) << "Device " << device_id << " no longer in adapter";
@@ -1109,8 +1131,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.spec(), 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
@@ -1164,13 +1190,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,
@@ -1251,9 +1277,19 @@ void BluetoothDispatcherHost::OnStopNotifySession(
 }
 
 BluetoothDispatcherHost::CacheQueryResult
-BluetoothDispatcherHost::QueryCacheForDevice(const std::string& device_id) {
+BluetoothDispatcherHost::QueryCacheForDevice(const std::string& origin,
+                                             const std::string& device_id) {
+  if (!allowed_devices_map_.HasDevicePermissionFromDeviceId(origin,
+                                                            device_id)) {
+    bad_message::ReceivedBadMessage(
+        this, bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);

[Jeffrey Yasskin, 2016/01/06 00:47:57]

I'm not sure we can kill the renderer for using a bad device: if the user
revokes access while a message is in flight, I think we'll hit this case, and we
don't want to kill that renderer.

[ortuno, 2016/01/13 01:41:44]

True. It would happen anytime you revoked access even if there are no messages
in flight. Since it's not spec'ed yet and we don't support revoking permissions
the only case in which the renderer would crash would be a hostile renderer. So
I propose we leave this in until it's spec'ed and we support revoking
permissions.

[Jeffrey Yasskin, 2016/01/13 02:31:36]

'k.

+    return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
+  }
+
   CacheQueryResult result;
-  result.device = adapter_->GetDevice(device_id);
+  result.device = adapter_->GetDevice(
+      allowed_devices_map_.GetDeviceAddress(origin, device_id));
+
   // 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.
@@ -1266,6 +1302,7 @@ BluetoothDispatcherHost::QueryCacheForDevice(const std::string& device_id) {
 
 BluetoothDispatcherHost::CacheQueryResult
 BluetoothDispatcherHost::QueryCacheForService(
+    const std::string& origin,
     const std::string& service_instance_id) {
   auto device_iter = service_to_device_.find(service_instance_id);
 
@@ -1275,10 +1312,17 @@ BluetoothDispatcherHost::QueryCacheForService(
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }
 
-  // TODO(ortuno): Check if domain has access to device.
-  // https://crbug.com/493459
+  const std::string& device_address = device_iter->second;
+  // Kill the renderer if the origin is not allowed to access the device.
+  if (!allowed_devices_map_.HasDevicePermissionFromDeviceAddress(
+          origin, device_address)) {
+    bad_message::ReceivedBadMessage(

[Jeffrey Yasskin, 2016/01/06 00:47:57]

Ditto here; not sure we can kill this renderer.

[ortuno, 2016/01/13 01:41:44]

Same here. We should leave in case of hostile renderer until we support revoking
permissions.

+        this, bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);
+    return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
+  }
 
-  CacheQueryResult result = QueryCacheForDevice(device_iter->second);
+  CacheQueryResult result = QueryCacheForDevice(
+      origin, allowed_devices_map_.GetDeviceId(origin, device_address));
 
   if (result.outcome != CacheQueryOutcome::SUCCESS) {
     return result;
@@ -1293,6 +1337,7 @@ BluetoothDispatcherHost::QueryCacheForService(
 
 BluetoothDispatcherHost::CacheQueryResult
 BluetoothDispatcherHost::QueryCacheForCharacteristic(
+    const std::string& origin,
     const std::string& characteristic_instance_id) {
   auto characteristic_iter =
       characteristic_to_service_.find(characteristic_instance_id);
@@ -1304,7 +1349,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;
   }
@@ -1320,14 +1366,21 @@ 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);
+}
+
+std::string BluetoothDispatcherHost::GetOrigin(int frame_routing_id) {
+  return RenderFrameHostImpl::FromID(render_process_id_, frame_routing_id)
+      ->GetLastCommittedURL()
+      .GetOrigin()
+      .spec();
 }
 
 void BluetoothDispatcherHost::ShowBluetoothOverviewLink() {