[CrOS PhoneHub] Add Nearby medium within SecureChannel

This CL adds a new ConnectionMedium::kNearbyConnections enum value and
adds a few potential error cases with this value. Actual handling of
Nearby Connections requests is left as a TODO in
PendingConnectionManagerImpl.

Note that this does not actually add a ConnectionMedium value to the
top-level API, and all incoming requests are assumed to be BLE at this
point. The parameter is added to the Mojo API as part of the follow-up
CL at [1], and some tests verifying the functionality in this CL are
added in that CL as well.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2390965

Bug: 1106937
Change-Id: I8e29613d6f2a11403d6815abebabeb21b64babe5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391587
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#804276}

chromeos/services/secure_channel/pending_connection_manager_impl.cc

@@ -58,9 +58,6 @@ void PendingConnectionManagerImpl::HandleConnectionRequest(
     const ConnectionAttemptDetails& connection_attempt_details,
     std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
     ConnectionPriority connection_priority) {
-  DCHECK_EQ(ConnectionMedium::kBluetoothLowEnergy,
-            connection_attempt_details.connection_medium());
-
   // If the client has canceled the request, it does not need to be processed.
   if (!client_connection_parameters->IsClientWaitingForResponse()) {
     PA_LOG(VERBOSE)
@@ -81,14 +78,14 @@ void PendingConnectionManagerImpl::HandleConnectionRequest(
       connection_attempt_details);
 
   // Process the role-specific details.
-  switch (connection_attempt_details.connection_role()) {
-    case ConnectionRole::kInitiatorRole:
-      HandleBleInitiatorRequest(connection_attempt_details,
+  switch (connection_attempt_details.connection_medium()) {
+    case ConnectionMedium::kBluetoothLowEnergy:
+      HandleBleRequest(connection_attempt_details,
                        std::move(client_connection_parameters),
                        connection_priority);
       break;
-    case ConnectionRole::kListenerRole:
-      HandleBleListenerRequest(connection_attempt_details,
+    case ConnectionMedium::kNearbyConnections:
+      HandleNearbyRequest(connection_attempt_details,
                           std::move(client_connection_parameters),
                           connection_priority);
       break;
@@ -151,6 +148,24 @@ void PendingConnectionManagerImpl::OnConnectionAttemptFinishedWithoutConnection(
   RemoveMapEntriesForFinishedConnectionAttempt(connection_attempt_details);
 }
 
+void PendingConnectionManagerImpl::HandleBleRequest(
+    const ConnectionAttemptDetails& connection_attempt_details,
+    std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
+    ConnectionPriority connection_priority) {
+  switch (connection_attempt_details.connection_role()) {
+    case ConnectionRole::kInitiatorRole:
+      HandleBleInitiatorRequest(connection_attempt_details,
+                                std::move(client_connection_parameters),
+                                connection_priority);
+      break;
+    case ConnectionRole::kListenerRole:
+      HandleBleListenerRequest(connection_attempt_details,
+                               std::move(client_connection_parameters),
+                               connection_priority);
+      break;
+  }
+}
+
 void PendingConnectionManagerImpl::HandleBleInitiatorRequest(
     const ConnectionAttemptDetails& connection_attempt_details,
     std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
@@ -217,6 +232,22 @@ void PendingConnectionManagerImpl::HandleBleListenerRequest(
   }
 }
 
+void PendingConnectionManagerImpl::HandleNearbyRequest(
+    const ConnectionAttemptDetails& connection_attempt_details,
+    std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
+    ConnectionPriority connection_priority) {
+  switch (connection_attempt_details.connection_role()) {
+    case ConnectionRole::kInitiatorRole:
+      // TODO(khorimoto): Attempt Nearby Connection.
+      break;
+    case ConnectionRole::kListenerRole:
+      NOTREACHED()
+          << "PendingConnectionManagerImpl::HandleConnectionRequest(): "
+          << "Nearby Connections is not supported in the listener role.";
+      break;
+  }
+}
+
 void PendingConnectionManagerImpl::RemoveMapEntriesForFinishedConnectionAttempt(
     const ConnectionAttemptDetails& connection_attempt_details) {
   // Make a copy of |connection_attempt_details|, since it belongs to the

chromeos/services/secure_channel/pending_connection_manager_impl.h

@@ -80,6 +80,10 @@ class PendingConnectionManagerImpl : public PendingConnectionManager,
   void OnConnectionAttemptFinishedWithoutConnection(
       const ConnectionAttemptDetails& connection_attempt_details) override;
 
+  void HandleBleRequest(
+      const ConnectionAttemptDetails& connection_attempt_details,
+      std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
+      ConnectionPriority connection_priority);
   void HandleBleInitiatorRequest(
       const ConnectionAttemptDetails& connection_attempt_details,
       std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
@@ -89,6 +93,11 @@ class PendingConnectionManagerImpl : public PendingConnectionManager,
       std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
       ConnectionPriority connection_priority);
 
+  void HandleNearbyRequest(
+      const ConnectionAttemptDetails& connection_attempt_details,
+      std::unique_ptr<ClientConnectionParameters> client_connection_parameters,
+      ConnectionPriority connection_priority);
+
   void RemoveMapEntriesForFinishedConnectionAttempt(
       const ConnectionAttemptDetails& connection_attempt_details);
 

chromeos/services/secure_channel/public/cpp/shared/connection_medium.cc

@@ -11,8 +11,14 @@ namespace chromeos {
 namespace secure_channel {
 
 std::ostream& operator<<(std::ostream& stream, const ConnectionMedium& medium) {
-  DCHECK(medium == ConnectionMedium::kBluetoothLowEnergy);
+  switch (medium) {
+    case ConnectionMedium::kBluetoothLowEnergy:
       stream << "[BLE]";
+      break;
+    case ConnectionMedium::kNearbyConnections:
+      stream << "[Nearby Connections]";
+      break;
+  }
   return stream;
 }
 

chromeos/services/secure_channel/public/cpp/shared/connection_medium.h

@@ -12,8 +12,14 @@ namespace chromeos {
 namespace secure_channel {
 
 // Enumeration of all mediums through which connections can be created.
-// Currently, only BLE connections are available.
-enum class ConnectionMedium { kBluetoothLowEnergy };
+enum class ConnectionMedium {
+  // Uses BLE to create a GATT connection
+  kBluetoothLowEnergy,
+
+  // Uses the Nearby Connections library to connect over Bluetooth Classic, then
+  // upgrade the connection to WebRTC for higher bandwidth.
+  kNearbyConnections
+};
 
 std::ostream& operator<<(std::ostream& stream, const ConnectionMedium& medium);
 

chromeos/services/secure_channel/public/mojom/secure_channel.mojom

@@ -12,7 +12,7 @@ enum ConnectionAttemptFailureReason {
   AUTHENTICATION_ERROR,
 
   // An advertisement could not be generated for this connection. This likely
-  // means that one (or both) devices have not synced ntheir BeaconSeeds
+  // means that one (or both) devices have not synced their BeaconSeeds
   // recently.
   COULD_NOT_GENERATE_ADVERTISEMENT,
 
@@ -27,12 +27,18 @@ enum ConnectionAttemptFailureReason {
   // The provided local device does not have a persistent symmetric key set.
   LOCAL_DEVICE_INVALID_PSK,
 
+  // The provided local device does not have a valid Bluetooth address.
+  LOCAL_DEVICE_INVALID_BLUETOOTH_ADDRESS,
+
   // The provided remote device does not have a public key set.
   REMOTE_DEVICE_INVALID_PUBLIC_KEY,
 
   // The provided remote device does not have a persistent symmetric key set.
   REMOTE_DEVICE_INVALID_PSK,
 
+  // The provided remote device does not have a valid Bluetooth address.
+  REMOTE_DEVICE_INVALID_BLUETOOTH_ADDRESS,
+
   // Timeouts occurred trying to contact the remote device.
   TIMEOUT_FINDING_DEVICE,
 
@@ -40,7 +46,12 @@ enum ConnectionAttemptFailureReason {
   ADAPTER_DISABLED,
 
   // The local Bluetooth adapter is not present.
-  ADAPTER_NOT_PRESENT
+  ADAPTER_NOT_PRESENT,
+
+  // Tried to connect using a role which is unsupported for the connection
+  // medium (e.g., Nearby Connections only supports the listener role, not the
+  // initiator role).
+  UNSUPPORTED_ROLE_FOR_MEDIUM
 };
 
 enum ConnectionCreationDetail {

chromeos/services/secure_channel/secure_channel_impl.cc

@@ -183,22 +183,33 @@ void SecureChannelImpl::ProcessConnectionRequest(
 
   // Check 2: Is the provided device to connect valid? If not, notify client and
   // return early.
-  if (CheckForInvalidInputDevice(api_fn_name, device_to_connect,
-                                 client_connection_parameters.get(),
-                                 false /* is_local_device */)) {
+  if (CheckForInvalidInputDevice(
+          api_fn_name, device_to_connect, client_connection_parameters.get(),
+          connection_medium, false /* is_local_device */)) {
     return;
   }
 
   // Check 3: Is the provided local device valid? If not, notify client and
   // return early.
-  if (CheckForInvalidInputDevice(api_fn_name, local_device,
-                                 client_connection_parameters.get(),
-                                 true /* is_local_device */)) {
+  if (CheckForInvalidInputDevice(
+          api_fn_name, local_device, client_connection_parameters.get(),
+          connection_medium, true /* is_local_device */)) {
     return;
   }
 
   // Check 4: Medium-specific verification.
   switch (connection_medium) {
+    case ConnectionMedium::kNearbyConnections:
+      // Nearby Connections only supports certain roles.
+      if (CheckForInvalidNearbyRole(api_fn_name,
+                                    client_connection_parameters.get(),
+                                    connection_role)) {
+        return;
+      }
+
+      // Nearby Connections requires Bluetooth functionality, so fall through to
+      // the Bluetooth case below.
+      FALLTHROUGH;
     case ConnectionMedium::kBluetoothLowEnergy:
       // Is the local Bluetooth adapter disabled or not present? If either,
       // notify client and return early.
@@ -294,9 +305,10 @@ bool SecureChannelImpl::CheckForInvalidInputDevice(
     ApiFunctionName api_fn_name,
     const multidevice::RemoteDevice& device,
     ClientConnectionParameters* client_connection_parameters,
+    ConnectionMedium connection_medium,
     bool is_local_device) {
   base::Optional<InvalidRemoteDeviceReason> potential_invalid_reason =
-      AddDeviceToCacheIfPossible(api_fn_name, device);
+      AddDeviceToCacheIfPossible(api_fn_name, device, connection_medium);
   if (!potential_invalid_reason)
     return false;
 
@@ -319,11 +331,35 @@ bool SecureChannelImpl::CheckForInvalidInputDevice(
                     REMOTE_DEVICE_INVALID_PSK,
           client_connection_parameters);
       break;
+    case InvalidRemoteDeviceReason::kInvalidBluetoothAddress:
+      RejectRequestForReason(api_fn_name,
+                             is_local_device
+                                 ? mojom::ConnectionAttemptFailureReason::
+                                       LOCAL_DEVICE_INVALID_BLUETOOTH_ADDRESS
+                                 : mojom::ConnectionAttemptFailureReason::
+                                       REMOTE_DEVICE_INVALID_BLUETOOTH_ADDRESS,
+                             client_connection_parameters);
+      break;
   }
 
   return true;
 }
 
+bool SecureChannelImpl::CheckForInvalidNearbyRole(
+    ApiFunctionName api_fn_name,
+    ClientConnectionParameters* client_connection_parameters,
+    ConnectionRole connection_role) {
+  if (connection_role == ConnectionRole::kListenerRole) {
+    RejectRequestForReason(
+        api_fn_name,
+        mojom::ConnectionAttemptFailureReason::UNSUPPORTED_ROLE_FOR_MEDIUM,
+        client_connection_parameters);
+    return true;
+  }
+
+  return false;
+}
+
 bool SecureChannelImpl::CheckIfBluetoothAdapterDisabledOrNotPresent(
     ApiFunctionName api_fn_name,
     ClientConnectionParameters* client_connection_parameters) {
@@ -347,7 +383,8 @@ bool SecureChannelImpl::CheckIfBluetoothAdapterDisabledOrNotPresent(
 base::Optional<SecureChannelImpl::InvalidRemoteDeviceReason>
 SecureChannelImpl::AddDeviceToCacheIfPossible(
     ApiFunctionName api_fn_name,
-    const multidevice::RemoteDevice& device) {
+    const multidevice::RemoteDevice& device,
+    ConnectionMedium connection_medium) {
   if (device.public_key.empty()) {
     PA_LOG(WARNING) << "SecureChannelImpl::" << api_fn_name << "(): "
                     << "Provided device has an invalid public key. Cannot "
@@ -362,6 +399,15 @@ SecureChannelImpl::AddDeviceToCacheIfPossible(
     return InvalidRemoteDeviceReason::kInvalidPsk;
   }
 
+  if (connection_medium == ConnectionMedium::kNearbyConnections &&
+      device.bluetooth_public_address.empty()) {
+    PA_LOG(WARNING) << "SecureChannelImpl::" << api_fn_name << "(): "
+                    << "Provided device does not have a valid Bluetooth "
+                    << "address, which is required to use Nearby Connections. "
+                    << "Cannot process request.";
+    return InvalidRemoteDeviceReason::kInvalidBluetoothAddress;
+  }
+
   remote_device_cache_->SetRemoteDevices({device});
   return base::nullopt;
 }

chromeos/services/secure_channel/secure_channel_impl.h

@@ -63,7 +63,11 @@ class SecureChannelImpl : public mojom::SecureChannel,
   explicit SecureChannelImpl(
       scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);
 
-  enum class InvalidRemoteDeviceReason { kInvalidPublicKey, kInvalidPsk };
+  enum class InvalidRemoteDeviceReason {
+    kInvalidPublicKey,
+    kInvalidPsk,
+    kInvalidBluetoothAddress
+  };
 
   enum class ApiFunctionName { kListenForConnection, kInitiateConnection };
   friend std::ostream& operator<<(std::ostream& stream,
@@ -139,8 +143,16 @@ class SecureChannelImpl : public mojom::SecureChannel,
       ApiFunctionName api_fn_name,
       const multidevice::RemoteDevice& device,
       ClientConnectionParameters* client_connection_parameters,
+      ConnectionMedium connection_medium,
       bool is_local_device);
 
+  // Checks for whether |connection_role| is valid for a connection via the
+  // Nearby Connections library.
+  bool CheckForInvalidNearbyRole(
+      ApiFunctionName api_fn_name,
+      ClientConnectionParameters* client_connection_parameters,
+      ConnectionRole connection_role);
+
   // Checks if |bluetooth_adapter_| is disabled or not present and rejects the
   // connection request if so. Returns whether the request was rejected.
   bool CheckIfBluetoothAdapterDisabledOrNotPresent(
@@ -152,7 +164,8 @@ class SecureChannelImpl : public mojom::SecureChannel,
   // device is not added to the cache.
   base::Optional<InvalidRemoteDeviceReason> AddDeviceToCacheIfPossible(
       ApiFunctionName api_fn_name,
-      const multidevice::RemoteDevice& device);
+      const multidevice::RemoteDevice& device,
+      ConnectionMedium connection_medium);
 
   scoped_refptr<device::BluetoothAdapter> bluetooth_adapter_;
   std::unique_ptr<TimerFactory> timer_factory_;