Fix retroactive pairing test

PiperOrigin-RevId: 532853830

fastpair/handshake/fast_pair_gatt_service_client_impl_test.cc

@@ -173,32 +173,6 @@ class FastPairGattServiceClientTest : public testing::Test {
         ByteArray(std::string(kPasskeyharacteristicAdvertisementByte)));
   }
 
-  void InsertKeyBasedGattCharacteristicsWithNoValue() {
-    key_based_characteristic_ = gatt_server_->CreateCharacteristic(
-        kFastPairServiceUuid, kKeyBasedCharacteristicUuidV2, permissions_,
-        properties_);
-
-    passkey_characteristic_ = gatt_server_->CreateCharacteristic(
-        kFastPairServiceUuid, kPasskeyCharacteristicUuidV2, permissions_,
-        properties_);
-    gatt_server_->UpdateCharacteristic(
-        passkey_characteristic_.value(),
-        ByteArray(std::string(kPasskeyharacteristicAdvertisementByte)));
-  }
-
-  void InsertPasskeyGattCharacteristicsWithNoValue() {
-    key_based_characteristic_ = gatt_server_->CreateCharacteristic(
-        kFastPairServiceUuid, kKeyBasedCharacteristicUuidV2, permissions_,
-        properties_);
-    gatt_server_->UpdateCharacteristic(
-        key_based_characteristic_.value(),
-        ByteArray(std::string(kKeyBasedCharacteristicAdvertisementByte)));
-
-    passkey_characteristic_ = gatt_server_->CreateCharacteristic(
-        kFastPairServiceUuid, kPasskeyCharacteristicUuidV2, permissions_,
-        properties_);
-  }
-
   void InitializeFastPairGattServiceClient() {
     FastPairDevice device(kMetadataId, provider_address_,
                           Protocol::kFastPairInitialPairing);
@@ -283,20 +257,6 @@ TEST_F(FastPairGattServiceClientTest, FailedDiscoverServiceAndCharacteristics) {
   EXPECT_EQ(GetInitializedCallbackResult(), PairFailure::kCreateGattConnection);
 }
 
-TEST_F(FastPairGattServiceClientTest, FailedGetKeyBasedCharacteristics) {
-  InsertKeyBasedGattCharacteristicsWithNoValue();
-  InitializeFastPairGattServiceClient();
-  EXPECT_EQ(GetInitializedCallbackResult(),
-            PairFailure::kKeyBasedPairingCharacteristicDiscovery);
-}
-
-TEST_F(FastPairGattServiceClientTest, FailedToGetPasskeyCharacteristics) {
-  InsertPasskeyGattCharacteristicsWithNoValue();
-  InitializeFastPairGattServiceClient();
-  EXPECT_EQ(GetInitializedCallbackResult(),
-            PairFailure::kPasskeyCharacteristicDiscovery);
-}
-
 TEST_F(FastPairGattServiceClientTest, SuccessfulWriteKeyBaseCharacteristics) {
   InsertCorrectGattCharacteristics();
   InitializeFastPairGattServiceClient();

fastpair/message_stream/BUILD

@@ -57,6 +57,7 @@ cc_library(
     deps = [
         ":message_stream",
         "//fastpair/common",
+        "//internal/platform:base",
         "//internal/platform:comm",
         "//internal/platform:logging",
         "//internal/platform:test_util",
@@ -70,6 +71,25 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "fake_gatt_callbacks",
+    testonly = True,
+    hdrs = [
+        "fake_gatt_callbacks.h",
+    ],
+    visibility = [
+        "//:__subpackages__",
+        "//fastpair:__subpackages__",
+    ],
+    deps = [
+        "//internal/platform:comm",
+        "//internal/platform:types",
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+    ],
+)
+
 cc_library(
     name = "fake_medium_observer",
     testonly = True,

fastpair/message_stream/fake_gatt_callbacks.h

@@ -0,0 +1,91 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef THIRD_PARTY_NEARBY_FASTPAIR_MESSAGE_STREAM_FAKE_GATT_CALLBACKS_H_
+#define THIRD_PARTY_NEARBY_FASTPAIR_MESSAGE_STREAM_FAKE_GATT_CALLBACKS_H_
+
+#include <optional>
+#include <string>
+
+#include "absl/container/flat_hash_map.h"
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
+#include "internal/platform/ble_v2.h"
+#include "internal/platform/future.h"
+
+namespace nearby {
+namespace fastpair {
+
+// Fake GATT characteristic callbacks for tests.
+// Register the callback returned by `GetGattCallback()` with the GattServer and
+// then communicate with the GATT client via `characteristics_`.
+class FakeGattCallbacks {
+  using GattCharacteristic = ::nearby::api::ble_v2::GattCharacteristic;
+
+ public:
+  struct CharacteristicData {
+    // Value written to the characteristic by the gatt client
+    Future<std::string> write_value;
+    // Write result returned to the gatt client.
+    absl::Status write_result = absl::OkStatus();
+    // Value returned to the gatt client when they try to read the
+    // characteristic
+    absl::StatusOr<std::string> read_value =
+        absl::FailedPreconditionError("characteristic not set");
+    absl::AnyInvocable<absl::Status(absl::string_view)> write_callback =
+        [&](absl::string_view data) {
+          write_value.Set(std::string(data));
+          return write_result;
+        };
+    absl::AnyInvocable<absl::StatusOr<std::string>()> read_callback = [&]() {
+      return read_value;
+    };
+  };
+
+  BleV2Medium::ServerGattConnectionCallback GetGattCallback() {
+    return BleV2Medium::ServerGattConnectionCallback{
+        .on_characteristic_read_cb =
+            [&](const api::ble_v2::BlePeripheral& remote_device,
+                const GattCharacteristic& characteristic, int offset,
+                BleV2Medium::ServerGattConnectionCallback::ReadValueCallback
+                    callback) {
+              auto it = characteristics_.find(characteristic);
+              if (it == characteristics_.end()) {
+                callback(absl::NotFoundError("characteristic not found"));
+                return;
+              }
+              callback(it->second.read_callback());
+            },
+        .on_characteristic_write_cb =
+            [&](const api::ble_v2::BlePeripheral& remote_device,
+                const GattCharacteristic& characteristic, int offset,
+                absl::string_view data,
+                BleV2Medium::ServerGattConnectionCallback::WriteValueCallback
+                    callback) {
+              auto it = characteristics_.find(characteristic);
+              if (it == characteristics_.end()) {
+                callback(absl::NotFoundError("characteristic not found"));
+                return;
+              }
+              callback(it->second.write_callback(data));
+            }};
+  }
+
+  absl::flat_hash_map<GattCharacteristic, CharacteristicData> characteristics_;
+};
+
+}  // namespace fastpair
+}  // namespace nearby
+
+#endif  // THIRD_PARTY_NEARBY_FASTPAIR_MESSAGE_STREAM_FAKE_GATT_CALLBACKS_H_

fastpair/message_stream/fake_provider.cc

@@ -27,6 +27,7 @@ namespace nearby {
 namespace fastpair {
 
 namespace {
+
 static EC_POINT *load_public_key(absl::string_view public_key) {
   CHECK_EQ(public_key.size(), kPublicKeyByteSize);
   BN_CTX *bn_ctx;
@@ -198,31 +199,16 @@ std::string FakeProvider::CreateSharedSecret(
       Crypto::Sha256(secret).AsStringView().substr(0, kAccountKeySize));
 }
 
-void FakeProvider::StartGattServer(KeyBasedPairingCallback kbp_callback) {
-  kbp_callback_ = std::move(kbp_callback);
-  gatt_server_ = ble_.StartGattServer(/*ServerGattConnectionCallback=*/{
-      .on_characteristic_write_cb =
-          [this](const api::ble_v2::BlePeripheral &remote_device,
-                 const api::ble_v2::GattCharacteristic &characteristic,
-                 int offset, absl::string_view data,
-                 BleV2Medium::ServerGattConnectionCallback::WriteValueCallback
-                     callback) {
-            if (characteristic == key_based_characteristic_) {
-              std::string response = kbp_callback_(data);
-              if (response.empty()) {
-                callback(absl::InvalidArgumentError("KBP write failed"));
-              } else {
-                callback(absl::OkStatus());
-                absl::Status status = gatt_server_->NotifyCharacteristicChanged(
-                    characteristic, false, ByteArray(response));
-                if (!status.ok()) {
-                  NEARBY_LOGS(INFO) << "Notify KBP failed " << status;
-                }
-              }
-            } else {
-              callback(absl::OkStatus());
-            }
-          }});
+void FakeProvider::StartGattServer(
+    BleV2Medium::ServerGattConnectionCallback callback) {
+  gatt_server_ = ble_.StartGattServer(std::move(callback));
+}
+
+absl::Status FakeProvider::NotifyKeyBasedPairing(ByteArray response) {
+  CHECK_NE(gatt_server_, nullptr);
+  CHECK(key_based_characteristic_.has_value());
+  return gatt_server_->NotifyCharacteristicChanged(*key_based_characteristic_,
+                                                   false, response);
 }
 
 }  // namespace fastpair

fastpair/message_stream/fake_provider.h

@@ -17,6 +17,7 @@
 
 #include <deque>
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -32,6 +33,7 @@
 #include "fastpair/message_stream/message.h"
 #include "internal/platform/ble_v2.h"
 #include "internal/platform/bluetooth_classic.h"
+#include "internal/platform/bluetooth_utils.h"
 #include "internal/platform/count_down_latch.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/medium_environment.h"
@@ -136,22 +138,24 @@ class FakeProvider {
     return provider_adapter_.GetMacAddress();
   }
 
-  void StartGattServer(KeyBasedPairingCallback kbp_callback);
+  std::string GetMacAddressAsBytes() const {
+    return std::string(
+        BluetoothUtils::FromString(provider_adapter_.GetMacAddress()));
+  }
+
+  void StartGattServer(BleV2Medium::ServerGattConnectionCallback callback);
 
   void InsertCorrectGattCharacteristics() {
     key_based_characteristic_ = gatt_server_->CreateCharacteristic(
         kFastPairServiceUuid, kKeyBasedCharacteristicUuidV2, permissions_,
         properties_);
-    gatt_server_->UpdateCharacteristic(
-        key_based_characteristic_.value(),
-        ByteArray(std::string(kKeyBasedCharacteristicAdvertisementByte)));
+    CHECK(key_based_characteristic_.has_value());
 
     passkey_characteristic_ = gatt_server_->CreateCharacteristic(
         kFastPairServiceUuid, kPasskeyCharacteristicUuidV2, permissions_,
         properties_);
-    gatt_server_->UpdateCharacteristic(
-        passkey_characteristic_.value(),
-        ByteArray(std::string(kPasskeyharacteristicAdvertisementByte)));
+
+    CHECK(passkey_characteristic_.has_value());
   }
 
   void LoadAntiSpoofingKey(absl::string_view private_key,
@@ -160,6 +164,7 @@ class FakeProvider {
   std::string DecryptKbpRequest(absl::string_view request);
   std::string Encrypt(absl::string_view data);
 
+  absl::Status NotifyKeyBasedPairing(ByteArray response);
   std::optional<GattCharacteristic> key_based_characteristic_;
   std::optional<GattCharacteristic> passkey_characteristic_;
 
@@ -177,7 +182,6 @@ class FakeProvider {
   std::unique_ptr<EVP_PKEY, void (*)(EVP_PKEY*)> anti_spoofing_key_{
       nullptr, EVP_PKEY_free};
   std::string account_key_;
-  KeyBasedPairingCallback kbp_callback_;
   SingleThreadExecutor provider_thread_;
 };
 

fastpair/retroactive/BUILD

@@ -47,6 +47,7 @@ cc_test(
     ],
     deps = [
         ":retroactive",
+        "//fastpair/message_stream:fake_gatt_callbacks",
         "//fastpair/message_stream:fake_provider",
         "//fastpair/proto:fastpair_cc_proto",
         "//fastpair/server_access:test_support",

fastpair/retroactive/retroactive.cc

@@ -138,8 +138,7 @@ void Retroactive::SetPairingStep(PairingStep step) {
               return;
             }
             NEARBY_LOGS(INFO) << "Sending Key Based Pairing request to "
-                              << absl::BytesToHexString(
-                                     controller_->GetDevice().GetBleAddress());
+                              << controller_->GetDevice().GetBleAddress();
             auto decryptor = data_encryptor_.Get().result().get();
             // TODO(jsobczak): Use real seeker address
             (gatt.result())

fastpair/retroactive/retroactive_test.cc

@@ -21,6 +21,7 @@
 #include "gtest/gtest.h"
 #include "absl/status/status.h"
 #include "absl/strings/escaping.h"
+#include "fastpair/message_stream/fake_gatt_callbacks.h"
 #include "fastpair/message_stream/fake_provider.h"
 #include "fastpair/proto/fastpair_rpcs.proto.h"
 #include "fastpair/server_access/fake_fast_pair_repository.h"
@@ -54,8 +55,7 @@ class RetroactiveTest : public testing::Test {
 
     provider_.DiscoverProvider(seeker_medium);
     remote_device_ = seeker_medium.GetRemoteDevice(provider_.GetMacAddress());
-    provider_.StartGattServer(
-        [this](absl::string_view data) { return KbpCallback(data); });
+    provider_.StartGattServer(gatt_callbacks_.GetGattCallback());
     provider_.InsertCorrectGattCharacteristics();
     ASSERT_TRUE(remote_device_.IsValid());
   }
@@ -73,15 +73,40 @@ class RetroactiveTest : public testing::Test {
     repository_.SetFakeMetadata(model_id, metadata);
   }
 
-  std::string KbpCallback(absl::string_view request) {
+  // The medium environment must be initialized (started) before adding
+  // adapters.
+  MediumEnvironmentStarter env_;
+  Mediums mediums_;
+  FakeProvider provider_;
+  BluetoothDevice remote_device_;
+  FakeFastPairRepository repository_;
+  FakeGattCallbacks gatt_callbacks_;
+};
+
+TEST_F(RetroactiveTest, Constructor) {
+  FastPairController controller(&mediums_, remote_device_);
+  Retroactive retro(&controller);
+}
+
+TEST_F(RetroactiveTest, Pair) {
+  SetUpFastPairRepository(kModelId, absl::HexStringToBytes(kBobPublicKey));
+  FastPairController controller(&mediums_, remote_device_);
+  provider_.EnableProviderRfcomm();
+  provider_.LoadAntiSpoofingKey(absl::HexStringToBytes(kBobPrivateKey),
+                                absl::HexStringToBytes(kBobPublicKey));
+  std::string provider_ble_address = provider_.GetMacAddressAsBytes();
+  gatt_callbacks_.characteristics_[*provider_.key_based_characteristic_]
+      .write_callback = [&](absl::string_view request) {
     // https://developers.google.com/nearby/fast-pair/specifications/characteristics#table1.1
+    // Example valid KBP request: "0010aabbccddeeff111213141516"
     // Byte 0, 0x00 = Key-based Pairing Request
     // Byte 1, 0x10 (Bit 3 set) = retroactive pairing
     // Bytes 2 - 7, aabbccddeeff, provider's address
     // Bytes 8 - 13, 111213141516, seeker's address
     // Bytes 14 - 15, (not included), random salt
-    std::string expected_kbp_request =
-        absl::HexStringToBytes("0010aabbccddeeff111213141516");
+    std::string expected_kbp_request = absl::HexStringToBytes("0010") +
+                                       provider_ble_address +
+                                       absl::HexStringToBytes("111213141516");
 
     // https://developers.google.com/nearby/fast-pair/specifications/characteristics#table1.2.2
     // Byte 0, 0x01 = Key-based Pairing Response
@@ -93,35 +118,17 @@ class RetroactiveTest : public testing::Test {
     NEARBY_LOGS(INFO) << "KBP request " << absl::BytesToHexString(request);
     std::string decrypted_request = provider_.DecryptKbpRequest(request);
     EXPECT_EQ(decrypted_request.size(), kEncryptedDataByteSize);
+    NEARBY_LOGS(INFO) << "KBP decrypted request "
+                      << absl::BytesToHexString(decrypted_request);
     // The last bytes in decrypted request are random, so we ignore them.
     EXPECT_EQ(decrypted_request.substr(0, expected_kbp_request.size()),
               expected_kbp_request);
 
     ByteArray response(provider_.Encrypt(kbp_response));
+    EXPECT_OK(provider_.NotifyKeyBasedPairing(response));
+    return absl::OkStatus();
+  };
 
-    return response.string_data();
-  }
-
-  // The medium environment must be initialized (started) before adding
-  // adapters.
-  MediumEnvironmentStarter env_;
-  Mediums mediums_;
-  FakeProvider provider_;
-  BluetoothDevice remote_device_;
-  FakeFastPairRepository repository_;
-};
-
-TEST_F(RetroactiveTest, Constructor) {
-  FastPairController controller(&mediums_, remote_device_);
-  Retroactive retro(&controller);
-}
-
-TEST_F(RetroactiveTest, DISABLED_Pair) {
-  SetUpFastPairRepository(kModelId, absl::HexStringToBytes(kBobPublicKey));
-  FastPairController controller(&mediums_, remote_device_);
-  provider_.EnableProviderRfcomm();
-  provider_.LoadAntiSpoofingKey(absl::HexStringToBytes(kBobPrivateKey),
-                                absl::HexStringToBytes(kBobPublicKey));
   Retroactive retro(&controller);
 
   Future<absl::Status> result = retro.Pair();
@@ -129,10 +136,11 @@ TEST_F(RetroactiveTest, DISABLED_Pair) {
   // Provider sends their ModelId.
   provider_.WriteProviderBytes(absl::HexStringToBytes("03010003ABCDEF"));
   // Provider sends their BLE address.
-  provider_.WriteProviderBytes(absl::HexStringToBytes("03020006AABBCCDDEEFF"));
+  EXPECT_EQ(provider_ble_address.size(), 6);
+  provider_.WriteProviderBytes(absl::HexStringToBytes("03020006") +
+                               provider_ble_address);
 
-  // EXPECT_TRUE(result.Get(absl::Minutes(5)).ok());
-  EXPECT_TRUE(result.Get(absl::Seconds(5)).ok());
+  EXPECT_TRUE(result.Get(absl::Minutes(5)).ok());
 }
 
 }  // namespace

internal/platform/implementation/g3/ble_v2.cc

@@ -565,6 +565,11 @@ bool BleV2Medium::GattServer::RemoveCharacteristicSubscription(
   return false;
 }
 
+bool BleV2Medium::GattServer::HasCharacteristic(
+    const api::ble_v2::GattCharacteristic& characteristic) {
+  return characteristics_.find(characteristic) != characteristics_.end();
+}
+
 void BleV2Medium::GattServer::Stop() {
   NEARBY_LOGS(INFO) << "G3 Ble GattServer Stop";
   characteristics_.clear();
@@ -611,13 +616,12 @@ BleV2Medium::GattClient::GetCharacteristic(const Uuid& service_uuid,
   }
   BleV2Medium::GattServer* gatt_server =
       static_cast<BleV2Medium::GattServer*>(*borrowed);
-  absl::StatusOr<ByteArray> value = gatt_server->ReadCharacteristic(
-      peripheral_, characteristic, /*offset=*/0);
-  if (!value.ok()) {
+  if (!gatt_server->HasCharacteristic(characteristic)) {
     NEARBY_LOGS(WARNING)
-        << "G3 Ble GattClient GetCharacteristic, can't read characteristic=("
+        << "G3 Ble GattClient GetCharacteristic, characteristic=("
         << characteristic.service_uuid.Get16BitAsString() << ","
-        << std::string(characteristic.uuid) << ")" << value.status();
+        << std::string(characteristic.uuid)
+        << ") not registered on GATT server";
     return std::nullopt;
   }
   NEARBY_LOGS(INFO)

internal/platform/implementation/g3/ble_v2.h

@@ -274,6 +274,9 @@ class BleV2Medium : public api::ble_v2::BleMedium {
         const BleV2Peripheral& remote_device,
         const api::ble_v2::GattCharacteristic& characteristic);
 
+    bool HasCharacteristic(
+        const api::ble_v2::GattCharacteristic& characteristic);
+
    private:
     using SubscriberKey =
         std::pair<const BleV2Peripheral*, api::ble_v2::GattCharacteristic>;