Implement secure_channel::BleServiceDataHelperImpl.

This implementation is a port of tether::BleServiceDataHelperImpl, but
correctly uses its provided DevicePair arguments, and uses a
RemoteDeviceCache for device storage.

This CL also fixes a small bug in tether::BleServiceDataHelperImpl that
caused it to attempt to identify *all* known Tether hosts, instead of the
requested hosts.

Bug: 824568, 752273
Change-Id: Ib8ab47dfcb2889392b6ec5f519aed706ffc513d9
Reviewed-on: https://chromium-review.googlesource.com/1098146
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567121}

chromeos/components/tether/ble_service_data_helper_impl.cc

@@ -18,7 +18,6 @@
 #include "components/cryptauth/ble/ble_advertisement_generator.h"
 #include "components/cryptauth/foreground_eid_generator.h"
 #include "components/cryptauth/local_device_data_provider.h"
-#include "device/bluetooth/bluetooth_device.h"
 
 namespace chromeos {
 
@@ -145,6 +144,13 @@ base::Optional<secure_channel::BleServiceDataHelper::DeviceWithBackgroundBool>
 BleServiceDataHelperImpl::PerformIdentifyRemoteDevice(
     const std::string& service_data,
     const secure_channel::DeviceIdPairSet& device_id_pair_set) {
+  std::vector<std::string> remote_device_ids;
+  for (const auto& device_id_pair : device_id_pair_set) {
+    // It's fine to ignore device_id_pair.local_device_id(); it's the same for
+    // each entry.
+    remote_device_ids.push_back(device_id_pair.remote_device_id());
+  }
+
   std::string device_id;
   bool is_background_advertisement = false;
 
@@ -161,7 +167,7 @@ BleServiceDataHelperImpl::PerformIdentifyRemoteDevice(
     }
 
     device_id = foreground_eid_generator_->IdentifyRemoteDeviceByAdvertisement(
-        service_data, registered_remote_device_ids_, beacon_seeds);
+        service_data, remote_device_ids, beacon_seeds);
   }
 
   // If the device has not yet been identified, try identifying |service_data|
@@ -169,8 +175,14 @@ BleServiceDataHelperImpl::PerformIdentifyRemoteDevice(
   if (chromeos::switches::IsInstantTetheringBackgroundAdvertisingSupported() &&
       device_id.empty() && service_data.size() >= kMinNumBytesInServiceData &&
       service_data.size() <= kMaxNumBytesInBackgroundServiceData) {
+    cryptauth::RemoteDeviceRefList remote_devices;
+    for (auto remote_device : tether_hosts_from_last_fetch_) {
+      if (base::ContainsValue(remote_device_ids, remote_device.GetDeviceId()))
+        remote_devices.push_back(remote_device);
+    }
+
     device_id = background_eid_generator_->IdentifyRemoteDeviceByAdvertisement(
-        service_data, tether_hosts_from_last_fetch_);
+        service_data, remote_devices);
     is_background_advertisement = true;
   }
 
@@ -202,12 +214,6 @@ void BleServiceDataHelperImpl::OnTetherHostsUpdated() {
 void BleServiceDataHelperImpl::OnTetherHostsFetched(
     const cryptauth::RemoteDeviceRefList& tether_hosts) {
   tether_hosts_from_last_fetch_ = tether_hosts;
-
-  registered_remote_device_ids_.clear();
-  std::transform(tether_hosts_from_last_fetch_.begin(),
-                 tether_hosts_from_last_fetch_.end(),
-                 std::back_inserter(registered_remote_device_ids_),
-                 [](const auto& host) { return host.GetDeviceId(); });
 }
 
 base::Optional<std::string>

chromeos/components/tether/ble_service_data_helper_impl.h

@@ -91,7 +91,6 @@ class BleServiceDataHelperImpl : public secure_channel::BleServiceDataHelper,
   std::unique_ptr<cryptauth::ForegroundEidGenerator> foreground_eid_generator_;
 
   cryptauth::RemoteDeviceRefList tether_hosts_from_last_fetch_;
-  std::vector<std::string> registered_remote_device_ids_;
 
   base::WeakPtrFactory<BleServiceDataHelperImpl> weak_ptr_factory_;
 

chromeos/components/tether/ble_service_data_helper_impl_unittest.cc

@@ -19,7 +19,6 @@
 #include "components/cryptauth/mock_local_device_data_provider.h"
 #include "components/cryptauth/remote_device_cache.h"
 #include "components/cryptauth/remote_device_test_util.h"
-#include "device/bluetooth/test/mock_bluetooth_device.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -49,28 +48,6 @@ const char fake_beacon_seed2_data[] = "fakeBeaconSeed2Data";
 const int64_t fake_beacon_seed2_start_ms = adjacent_eid_start_ms;
 const int64_t fake_beacon_seed2_end_ms = adjacent_eid_end_ms;
 
-class MockBluetoothDeviceWithServiceData : public device::MockBluetoothDevice {
- public:
-  MockBluetoothDeviceWithServiceData(device::MockBluetoothAdapter* adapter,
-                                     const std::string& device_address,
-                                     const std::string& service_data)
-      : device::MockBluetoothDevice(adapter,
-                                    /* bluetooth_class */ 0,
-                                    "name",
-                                    device_address,
-                                    false,
-                                    false) {
-    for (size_t i = 0; i < service_data.size(); i++) {
-      service_data_.push_back(static_cast<uint8_t>(service_data[i]));
-    }
-  }
-
-  const std::vector<uint8_t>* service_data() { return &service_data_; }
-
- private:
-  std::vector<uint8_t> service_data_;
-};
-
 std::unique_ptr<cryptauth::ForegroundEidGenerator::EidData>
 CreateFakeBackgroundScanFilter() {
   cryptauth::DataWithTimestamp current(current_eid_data, current_eid_start_ms,

chromeos/services/secure_channel/BUILD.gn

@@ -101,11 +101,13 @@ static_library("secure_channel") {
 
   deps = [
     "//base",
+    "//chromeos",
     "//chromeos/components/proximity_auth/logging",
     "//chromeos/services/secure_channel/public/cpp/shared",
     "//chromeos/services/secure_channel/public/cpp/shared:connection_priority",
     "//chromeos/services/secure_channel/public/mojom",
     "//components/cryptauth",
+    "//components/cryptauth/ble",
     "//device/bluetooth",
     "//services/service_manager/public/cpp",
   ]
@@ -211,6 +213,8 @@ source_set("unit_tests") {
     "//chromeos/services/secure_channel/public/mojom",
     "//chromeos/services/secure_channel/public/mojom:unit_tests",
     "//components/cryptauth:test_support",
+    "//components/cryptauth/ble",
+    "//components/cryptauth/ble:test_support",
     "//device/bluetooth:mocks",
     "//services/service_manager/public/cpp/test:test_support",
     "//testing/gmock",

chromeos/services/secure_channel/ble_service_data_helper_impl.cc

@@ -4,14 +4,36 @@
 
 #include "chromeos/services/secure_channel/ble_service_data_helper_impl.h"
 
+#include "base/containers/flat_map.h"
 #include "base/memory/ptr_util.h"
 #include "base/no_destructor.h"
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/components/proximity_auth/logging/logging.h"
+#include "chromeos/services/secure_channel/ble_constants.h"
+#include "components/cryptauth/background_eid_generator.h"
+#include "components/cryptauth/ble/ble_advertisement_generator.h"
+#include "components/cryptauth/foreground_eid_generator.h"
 #include "components/cryptauth/remote_device_cache.h"
+#include "components/cryptauth/remote_device_ref.h"
 
 namespace chromeos {
 
 namespace secure_channel {
 
+namespace {
+
+// Valid advertisement service data must be at least 2 bytes.
+// As of June 2018, valid background advertisement service data is exactly 2
+// bytes, which identify the advertising device to the scanning device.
+// Valid foreground advertisement service data must include at least 4 bytes:
+// 2 bytes associated with the scanning device (used as a scan filter) and 2
+// bytes which identify the advertising device to the scanning device.
+const size_t kMinNumBytesInServiceData = 2;
+const size_t kMaxNumBytesInBackgroundServiceData = 3;
+const size_t kMinNumBytesInForegroundServiceData = 4;
+
+}  // namespace
+
 // static
 BleServiceDataHelperImpl::Factory*
     BleServiceDataHelperImpl::Factory::test_factory_ = nullptr;
@@ -41,31 +63,130 @@ BleServiceDataHelperImpl::Factory::BuildInstance(
 
 BleServiceDataHelperImpl::BleServiceDataHelperImpl(
     cryptauth::RemoteDeviceCache* remote_device_cache)
-    : remote_device_cache_(remote_device_cache) {}
+    : remote_device_cache_(remote_device_cache),
+      background_eid_generator_(
+          std::make_unique<cryptauth::BackgroundEidGenerator>()),
+      foreground_eid_generator_(
+          std::make_unique<cryptauth::ForegroundEidGenerator>()) {}
 
 BleServiceDataHelperImpl::~BleServiceDataHelperImpl() = default;
 
 std::unique_ptr<cryptauth::DataWithTimestamp>
 BleServiceDataHelperImpl::GenerateForegroundAdvertisement(
     const DeviceIdPair& device_id_pair) {
-  // TODO(hansberry): Implement.
-  NOTIMPLEMENTED();
-
-  // Use remote_device_cache_ to prevent compiler warning.
-  // TODO(hansberry): Remove.
-  remote_device_cache_->GetRemoteDevices();
-  return nullptr;
+  base::Optional<cryptauth::RemoteDeviceRef> local_device =
+      remote_device_cache_->GetRemoteDevice(device_id_pair.local_device_id());
+  if (!local_device) {
+    PA_LOG(ERROR) << "Requested local device does not exist: "
+                  << cryptauth::RemoteDeviceRef::TruncateDeviceIdForLogs(
+                         device_id_pair.local_device_id());
+    return nullptr;
+  }
+
+  base::Optional<cryptauth::RemoteDeviceRef> remote_device =
+      remote_device_cache_->GetRemoteDevice(device_id_pair.remote_device_id());
+  if (!remote_device) {
+    PA_LOG(ERROR) << "Requested remote device does not exist: "
+                  << cryptauth::RemoteDeviceRef::TruncateDeviceIdForLogs(
+                         device_id_pair.remote_device_id());
+    return nullptr;
+  }
+
+  return cryptauth::BleAdvertisementGenerator::GenerateBleAdvertisement(
+      *remote_device, local_device->public_key());
 }
 
 base::Optional<BleServiceDataHelper::DeviceWithBackgroundBool>
 BleServiceDataHelperImpl::PerformIdentifyRemoteDevice(
     const std::string& service_data,
     const DeviceIdPairSet& device_id_pair_set) {
-  // TODO(hansberry): Implement.
-  NOTIMPLEMENTED();
+  base::flat_map<std::string, std::vector<std::string>>
+      local_device_id_to_remote_device_ids_map;
+  for (const auto& device_id_pair : device_id_pair_set) {
+    if (!remote_device_cache_->GetRemoteDevice(
+            device_id_pair.local_device_id())) {
+      PA_LOG(ERROR) << "Requested local device does not exist"
+                    << cryptauth::RemoteDeviceRef::TruncateDeviceIdForLogs(
+                           device_id_pair.local_device_id());
+      continue;
+    }
+
+    if (!remote_device_cache_->GetRemoteDevice(
+            device_id_pair.remote_device_id())) {
+      PA_LOG(ERROR) << "Requested remote device does not exist"
+                    << cryptauth::RemoteDeviceRef::TruncateDeviceIdForLogs(
+                           device_id_pair.remote_device_id());
+      continue;
+    }
+
+    local_device_id_to_remote_device_ids_map[device_id_pair.local_device_id()]
+        .push_back(device_id_pair.remote_device_id());
+  }
+
+  for (const auto& map_entry : local_device_id_to_remote_device_ids_map) {
+    auto device_with_background_bool = PerformIdentifyRemoteDevice(
+        service_data, map_entry.first, map_entry.second);
+    if (device_with_background_bool)
+      return device_with_background_bool;
+  }
+
   return base::nullopt;
 }
 
+base::Optional<BleServiceDataHelper::DeviceWithBackgroundBool>
+BleServiceDataHelperImpl::PerformIdentifyRemoteDevice(
+    const std::string& service_data,
+    const std::string& local_device_id,
+    const std::vector<std::string>& remote_device_ids) {
+  std::string identified_device_id;
+  bool is_background_advertisement = false;
+
+  // First try, identifying |service_data| as a foreground advertisement.
+  if (service_data.size() >= kMinNumBytesInForegroundServiceData) {
+    std::vector<cryptauth::BeaconSeed> beacon_seeds =
+        remote_device_cache_->GetRemoteDevice(local_device_id)->beacon_seeds();
+
+    identified_device_id =
+        foreground_eid_generator_->IdentifyRemoteDeviceByAdvertisement(
+            service_data, remote_device_ids, beacon_seeds);
+  }
+
+  // If the device has not yet been identified, try identifying |service_data|
+  // as a background advertisement.
+  if (chromeos::switches::IsInstantTetheringBackgroundAdvertisingSupported() &&
+      identified_device_id.empty() &&
+      service_data.size() >= kMinNumBytesInServiceData &&
+      service_data.size() <= kMaxNumBytesInBackgroundServiceData) {
+    cryptauth::RemoteDeviceRefList remote_devices;
+    std::transform(remote_device_ids.begin(), remote_device_ids.end(),
+                   std::back_inserter(remote_devices), [this](auto device_id) {
+                     return *remote_device_cache_->GetRemoteDevice(device_id);
+                   });
+
+    identified_device_id =
+        background_eid_generator_->IdentifyRemoteDeviceByAdvertisement(
+            service_data, remote_devices);
+    is_background_advertisement = true;
+  }
+
+  // If the service data does not correspond to an advertisement from a device
+  // on this account, ignore it.
+  if (identified_device_id.empty())
+    return base::nullopt;
+
+  return secure_channel::BleServiceDataHelper::DeviceWithBackgroundBool(
+      *remote_device_cache_->GetRemoteDevice(identified_device_id),
+      is_background_advertisement);
+}
+
+void BleServiceDataHelperImpl::SetTestDoubles(
+    std::unique_ptr<cryptauth::BackgroundEidGenerator> background_eid_generator,
+    std::unique_ptr<cryptauth::ForegroundEidGenerator>
+        foreground_eid_generator) {
+  background_eid_generator_ = std::move(background_eid_generator);
+  foreground_eid_generator_ = std::move(foreground_eid_generator);
+}
+
 }  // namespace secure_channel
 
 }  // namespace chromeos

chromeos/services/secure_channel/ble_service_data_helper_impl.h

@@ -15,6 +15,8 @@
 #include "components/cryptauth/remote_device_ref.h"
 
 namespace cryptauth {
+class BackgroundEidGenerator;
+class ForegroundEidGenerator;
 class RemoteDeviceCache;
 }  // namespace cryptauth
 
@@ -40,6 +42,8 @@ class BleServiceDataHelperImpl : public BleServiceDataHelper {
   ~BleServiceDataHelperImpl() override;
 
  private:
+  friend class SecureChannelBleServiceDataHelperImplTest;
+
   BleServiceDataHelperImpl(cryptauth::RemoteDeviceCache* remote_device_cache);
 
   // BleServiceDataHelper:
@@ -49,7 +53,20 @@ class BleServiceDataHelperImpl : public BleServiceDataHelper {
       const std::string& service_data,
       const DeviceIdPairSet& device_id_pair_set) override;
 
+  base::Optional<BleServiceDataHelper::DeviceWithBackgroundBool>
+  PerformIdentifyRemoteDevice(
+      const std::string& service_data,
+      const std::string& local_device_id,
+      const std::vector<std::string>& remote_device_ids);
+
+  void SetTestDoubles(std::unique_ptr<cryptauth::BackgroundEidGenerator>
+                          background_eid_generator,
+                      std::unique_ptr<cryptauth::ForegroundEidGenerator>
+                          foreground_eid_generator);
+
   cryptauth::RemoteDeviceCache* remote_device_cache_;
+  std::unique_ptr<cryptauth::BackgroundEidGenerator> background_eid_generator_;
+  std::unique_ptr<cryptauth::ForegroundEidGenerator> foreground_eid_generator_;
 
   DISALLOW_COPY_AND_ASSIGN(BleServiceDataHelperImpl);
 };

components/cryptauth/ble/ble_advertisement_generator.h

@@ -11,6 +11,9 @@
 #include "components/cryptauth/foreground_eid_generator.h"
 
 namespace chromeos {
+namespace secure_channel {
+class SecureChannelBleServiceDataHelperImplTest;
+}  // namespace secure_channel
 namespace tether {
 class BleAdvertiserImplTest;
 class BleServiceDataHelperImplTest;
@@ -43,6 +46,8 @@ class BleAdvertisementGenerator {
 
  private:
   friend class CryptAuthBleAdvertisementGeneratorTest;
+  friend class chromeos::secure_channel::
+      SecureChannelBleServiceDataHelperImplTest;
   friend class chromeos::tether::BleAdvertiserImplTest;
   friend class chromeos::tether::BleServiceDataHelperImplTest;
   friend class chromeos::tether::AdHocBleAdvertiserImplTest;