[Bluetooth] Only permit service connections for allowlisted UUIDs.

Nearby Connections clients (e.g., Nearby Share) consume the Adapter
methods to connect to or listen on RFCOMM services from the
untrusted Nearby utility process. Only allow known UUIDs of these
clients to be used with these methods.

Bug: b:162975217, 1155668
Change-Id: I8b94dda97df0f694b37c22fa59adfacc17b5093f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2573117Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833952}

chrome/browser/chromeos/nearby/BUILD.gn

@@ -13,6 +13,7 @@ source_set("bluetooth_adapter_util") {
 
   deps = [
     "//base",
+    "//chromeos/services/nearby/public/cpp",
     "//device/bluetooth",
     "//device/bluetooth:deprecated_experimental_mojo",
     "//mojo/public/cpp/bindings",

chrome/browser/chromeos/nearby/bluetooth_adapter_util.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "chromeos/services/nearby/public/cpp/nearby_client_uuids.h"
 #include "device/bluetooth/adapter.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
@@ -15,10 +16,18 @@ namespace nearby {
 
 void MakeSelfOwnedBluetoothAdapter(
     mojo::PendingReceiver<bluetooth::mojom::Adapter> pending_receiver,
-    scoped_refptr<device::BluetoothAdapter> adapter) {
-  mojo::MakeSelfOwnedReceiver(
-      std::make_unique<bluetooth::Adapter>(std::move(adapter)),
-      std::move(pending_receiver));
+    scoped_refptr<device::BluetoothAdapter> bluetooth_adapter) {
+  auto adapter =
+      std::make_unique<bluetooth::Adapter>(std::move(bluetooth_adapter));
+
+  // Nearby Connections clients will be initiating and listening for connections
+  // from the untrusted Nearby utility process. Pre-emptively allowlist their
+  // UUIDs here in the browser process.
+  for (const auto& uuid : GetNearbyClientUuids()) {
+    adapter->AllowConnectionsForUuid(uuid);
+  }
+
+  mojo::MakeSelfOwnedReceiver(std::move(adapter), std::move(pending_receiver));
 }
 
 }  // namespace nearby

chromeos/services/nearby/DEPS

 include_rules = [
   "+components/keyed_service/core/keyed_service.h",
+  "+device/bluetooth/public/cpp/bluetooth_uuid.h",
 ]

chromeos/services/nearby/public/cpp/BUILD.gn

@@ -3,7 +3,11 @@
 # found in the LICENSE file.
 
 static_library("cpp") {
-  sources = [ "nearby_process_manager.h" ]
+  sources = [
+    "nearby_client_uuids.cc",
+    "nearby_client_uuids.h",
+    "nearby_process_manager.h",
+  ]
 
   deps = [
     "//base",

chromeos/services/nearby/public/cpp/nearby_client_uuids.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/services/nearby/public/cpp/nearby_client_uuids.h"
+
+#include <set>
+#include <string>
+
+#include "base/no_destructor.h"
+#include "base/stl_util.h"
+
+namespace chromeos {
+namespace nearby {
+
+namespace {
+const char kNearbySharingUuid[] = "a82efa21-ae5c-3dde-9bbc-f16da7b16c5a";
+const char kSecureChannelUuid[] = "a384bd4f-41ea-3b02-8901-8c2ed9a79970";
+}  // namespace
+
+const std::vector<device::BluetoothUUID>& GetNearbyClientUuids() {
+  static const base::NoDestructor<std::vector<device::BluetoothUUID>>
+      kAllowedUuids([] {
+        // This literal initialization unfortunately does not work with
+        // base::NoDestructor.
+        std::vector<device::BluetoothUUID> allowed_uuids{
+            device::BluetoothUUID(kNearbySharingUuid),
+            device::BluetoothUUID(kSecureChannelUuid)};
+        return allowed_uuids;
+      }());
+
+  return *kAllowedUuids;
+}
+
+bool IsNearbyClientUuid(const device::BluetoothUUID& uuid) {
+  static const base::NoDestructor<std::set<device::BluetoothUUID>>
+      kAllowedUuidSet(std::begin(GetNearbyClientUuids()),
+                      std::end(GetNearbyClientUuids()));
+  return base::Contains(*kAllowedUuidSet, uuid);
+}
+
+}  // namespace nearby
+}  // namespace chromeos

chromeos/services/nearby/public/cpp/nearby_client_uuids.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_SERVICES_NEARBY_PUBLIC_CPP_NEARBY_CLIENT_UUIDS_H_
+#define CHROMEOS_SERVICES_NEARBY_PUBLIC_CPP_NEARBY_CLIENT_UUIDS_H_
+
+#include <vector>
+
+#include "device/bluetooth/public/cpp/bluetooth_uuid.h"
+
+namespace chromeos {
+namespace nearby {
+
+// Returns a list of Bluetooth Service UUIDs corresponding to current clients
+// of Nearby Connections (e.g., Nearby Share). Callers can use this function or
+// IsNearbyClientUuid() to verify that outgoing or incoming Bluetooth
+// connections are initiated by said recognized Nearby Connections clients.
+const std::vector<device::BluetoothUUID>& GetNearbyClientUuids();
+
+// Helper function to check if |uuid| is present in list returned by
+// GetNearbyClientUuids().
+bool IsNearbyClientUuid(const device::BluetoothUUID& uuid);
+
+}  // namespace nearby
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_NEARBY_PUBLIC_CPP_NEARBY_CLIENT_UUIDS_H_

device/bluetooth/adapter.cc

@@ -175,6 +175,11 @@ void Adapter::ConnectToServiceInsecurely(
     const std::string& address,
     const device::BluetoothUUID& service_uuid,
     ConnectToServiceInsecurelyCallback callback) {
+  if (!base::Contains(allowed_uuids_, service_uuid)) {
+    std::move(callback).Run(/*result=*/nullptr);
+    return;
+  }
+
   auto* device = adapter_->GetDevice(address);
   if (device) {
     OnDeviceFetchedForInsecureServiceConnection(service_uuid,
@@ -203,6 +208,11 @@ void Adapter::CreateRfcommServiceInsecurely(
     const std::string& service_name,
     const device::BluetoothUUID& service_uuid,
     CreateRfcommServiceInsecurelyCallback callback) {
+  if (!base::Contains(allowed_uuids_, service_uuid)) {
+    std::move(callback).Run(/*server_socket=*/mojo::NullRemote());
+    return;
+  }
+
   device::BluetoothAdapter::ServiceOptions service_options;
   service_options.name = service_name;
   service_options.require_authentication = false;
@@ -286,6 +296,11 @@ void Adapter::GattServicesDiscovered(device::BluetoothAdapter* adapter,
   }
 }
 
+void Adapter::AllowConnectionsForUuid(
+    const device::BluetoothUUID& service_uuid) {
+  allowed_uuids_.emplace(service_uuid);
+}
+
 void Adapter::OnDeviceFetchedForInsecureServiceConnection(
     const device::BluetoothUUID& service_uuid,
     ConnectToServiceInsecurelyCallback callback,

device/bluetooth/adapter.h

@@ -48,8 +48,6 @@ class Adapter : public mojom::Adapter,
                        SetDiscoverableCallback callback) override;
   void SetName(const std::string& name, SetNameCallback callback) override;
   void StartDiscoverySession(StartDiscoverySessionCallback callback) override;
-  // TODO(b/162975217): Add a mechanism to allowlist which address and UUID
-  // pairs clients are allowed to create a connection to.
   void ConnectToServiceInsecurely(
       const std::string& address,
       const device::BluetoothUUID& service_uuid,
@@ -77,6 +75,10 @@ class Adapter : public mojom::Adapter,
   void GattServicesDiscovered(device::BluetoothAdapter* adapter,
                               device::BluetoothDevice* device) override;
 
+  // Permit untrusted clients to initiate outgoing connections, or listen on
+  // incoming connections, with |service_uuid|.
+  void AllowConnectionsForUuid(const device::BluetoothUUID& service_uuid);
+
  private:
   void OnDeviceFetchedForInsecureServiceConnection(
       const device::BluetoothUUID& service_uuid,
@@ -132,6 +134,10 @@ class Adapter : public mojom::Adapter,
                          ConnectToServiceInsecurelyCallback>>
       pending_connect_to_service_args_;
 
+  // Allowed UUIDs for untrusted clients to initiate outgoing connections, or
+  // listen on incoming connections.
+  std::set<device::BluetoothUUID> allowed_uuids_;
+
   base::WeakPtrFactory<Adapter> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(Adapter);

device/bluetooth/adapter_unittest.cc

@@ -32,6 +32,7 @@ namespace {
 
 const char kKnownDeviceAddress[] = "00:00:00:00:01";
 const char kUnknownDeviceAddress[] = "00:00:00:00:02";
+const char kServiceName[] = "ServiceName";
 const char kServiceId[] = "0000abcd-0000-0000-0000-000000000001";
 const char kDeviceServiceDataStr[] = "ServiceData";
 
@@ -192,12 +193,28 @@ TEST_F(AdapterTest, TestRegisterAdvertisement_ScanResponseData) {
   VerifyAdvertisementWithScanData();
 }
 
+TEST_F(AdapterTest, TestConnectToServiceInsecurely_DisallowedUuid) {
+  // Do not call Adapter::AllowConnectionsForUuid();
+
+  base::RunLoop run_loop;
+  adapter_->ConnectToServiceInsecurely(
+      kKnownDeviceAddress, device::BluetoothUUID(kServiceId),
+      base::BindLambdaForTesting(
+          [&](mojom::ConnectToServiceResultPtr connect_to_service_result) {
+            EXPECT_FALSE(connect_to_service_result);
+            run_loop.Quit();
+          }));
+  run_loop.Run();
+}
+
 TEST_F(AdapterTest, TestConnectToServiceInsecurely_KnownDevice_Success) {
   EXPECT_CALL(
       *mock_known_bluetooth_device_,
       ConnectToServiceInsecurely(device::BluetoothUUID(kServiceId), _, _))
       .WillOnce(RunOnceCallback<1>(mock_bluetooth_socket_));
 
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
   base::RunLoop run_loop;
   adapter_->ConnectToServiceInsecurely(
       kKnownDeviceAddress, device::BluetoothUUID(kServiceId),
@@ -215,6 +232,8 @@ TEST_F(AdapterTest, TestConnectToServiceInsecurely_KnownDevice_Error) {
       ConnectToServiceInsecurely(device::BluetoothUUID(kServiceId), _, _))
       .WillOnce(RunOnceCallback<2>("Error"));
 
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
   base::RunLoop run_loop;
   adapter_->ConnectToServiceInsecurely(
       kKnownDeviceAddress, device::BluetoothUUID(kServiceId),
@@ -241,6 +260,8 @@ TEST_F(
               IsGattServicesDiscoveryComplete())
       .WillOnce(Return(true));
 
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
   base::RunLoop run_loop;
   adapter_->ConnectToServiceInsecurely(
       kUnknownDeviceAddress, device::BluetoothUUID(kServiceId),
@@ -279,6 +300,8 @@ TEST_F(
                       Return(false)))
       .WillRepeatedly(Return(true));
 
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
   base::RunLoop run_loop;
   adapter_->ConnectToServiceInsecurely(
       kUnknownDeviceAddress, device::BluetoothUUID(kServiceId),
@@ -295,6 +318,8 @@ TEST_F(AdapterTest, TestConnectToServiceInsecurely_UnknownDevice_Error) {
               ConnectDevice(kUnknownDeviceAddress, _, _, _))
       .WillOnce(RunOnceCallback<3>());
 
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
   base::RunLoop run_loop;
   adapter_->ConnectToServiceInsecurely(
       kUnknownDeviceAddress, device::BluetoothUUID(kServiceId),
@@ -307,6 +332,8 @@ TEST_F(AdapterTest, TestConnectToServiceInsecurely_UnknownDevice_Error) {
 }
 #else
 TEST_F(AdapterTest, TestConnectToServiceInsecurely_UnknownDevice) {
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
   base::RunLoop run_loop;
   adapter_->ConnectToServiceInsecurely(
       kUnknownDeviceAddress, device::BluetoothUUID(kServiceId),
@@ -319,4 +346,54 @@ TEST_F(AdapterTest, TestConnectToServiceInsecurely_UnknownDevice) {
 }
 #endif
 
+TEST_F(AdapterTest, TestCreateRfcommServiceInsecurely_DisallowedUuid) {
+  // Do not call Adapter::AllowConnectionsForUuid();
+
+  base::RunLoop run_loop;
+  adapter_->CreateRfcommServiceInsecurely(
+      kServiceName, device::BluetoothUUID(kServiceId),
+      base::BindLambdaForTesting(
+          [&](mojo::PendingRemote<mojom::ServerSocket> pending_server_socket) {
+            EXPECT_FALSE(pending_server_socket);
+            run_loop.Quit();
+          }));
+  run_loop.Run();
+}
+
+TEST_F(AdapterTest, TestCreateRfcommServiceInsecurely_Error) {
+  EXPECT_CALL(*mock_bluetooth_adapter_,
+              CreateRfcommService(device::BluetoothUUID(kServiceId), _, _, _))
+      .WillOnce(RunOnceCallback<3>("Error"));
+
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
+  base::RunLoop run_loop;
+  adapter_->CreateRfcommServiceInsecurely(
+      kServiceName, device::BluetoothUUID(kServiceId),
+      base::BindLambdaForTesting(
+          [&](mojo::PendingRemote<mojom::ServerSocket> pending_server_socket) {
+            EXPECT_FALSE(pending_server_socket);
+            run_loop.Quit();
+          }));
+  run_loop.Run();
+}
+
+TEST_F(AdapterTest, TestCreateRfcommServiceInsecurely_Success) {
+  EXPECT_CALL(*mock_bluetooth_adapter_,
+              CreateRfcommService(device::BluetoothUUID(kServiceId), _, _, _))
+      .WillOnce(RunOnceCallback<2>(mock_bluetooth_socket_));
+
+  adapter_->AllowConnectionsForUuid(device::BluetoothUUID(kServiceId));
+
+  base::RunLoop run_loop;
+  adapter_->CreateRfcommServiceInsecurely(
+      kServiceName, device::BluetoothUUID(kServiceId),
+      base::BindLambdaForTesting(
+          [&](mojo::PendingRemote<mojom::ServerSocket> pending_server_socket) {
+            EXPECT_TRUE(pending_server_socket);
+            run_loop.Quit();
+          }));
+  run_loop.Run();
+}
+
 }  // namespace bluetooth