pw_bluetooth_sapphire: Handle BR/EDR CTKD in SM after connection

Handle the case where BR/EDR cross-transport key derivation sets the LE LTK after the LE SecurityManager was constructed. Bug: b/388607971 Change-Id: Id60eb77f8f6a462915959258722855e01f7010bf Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/266040 Commit-Queue: Ben Lawson <[email protected]> Reviewed-by: Faraaz Sareshwala <[email protected]> Docs-Not-Needed: Ben Lawson <[email protected]> Lint: Lint 🤖 <[email protected]>
google · Feb 13, 2025 · 4fc31df · 4fc31df
1 parent 2e41749
commit 4fc31df
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 0 deletions.
diff --git a/pw_bluetooth_sapphire/host/hci/low_energy_connection.cc b/pw_bluetooth_sapphire/host/hci/low_energy_connection.cc
@@ -153,6 +153,9 @@ LowEnergyConnection::OnLELongTermKeyRequestEvent(const EventPacket& event) {
         status_event, TRACE, "hci-le", "failed to reply to LTK request");
   };
 
+  // TODO(fxbug.dev/388607971): The LTK may be stale if BR/EDR cross-transport
+  // key derivation was performed. Maybe move this method to
+  // sm::SecurityManager.
   if (ltk() && ltk()->rand() == rand && ltk()->ediv() == ediv) {
     auto cmd = CommandPacket::New<
         pw::bluetooth::emboss::LELongTermKeyRequestReplyCommandWriter>(

diff --git a/pw_bluetooth_sapphire/host/sm/security_manager.cc b/pw_bluetooth_sapphire/host/sm/security_manager.cc
@@ -604,6 +604,31 @@ void SecurityManagerImpl::UpgradeSecurityInternal() {
   }
 
   const PendingRequest& next_req = request_queue_.front();
+
+  // BR/EDR cross-transport key derivation could have created a new LE LTK that
+  // meets the requirements of the next request. Only central can start
+  // encryption, so we skip this and request a security upgrade as peripheral.
+  if (low_energy_link_->role() ==
+      pw::bluetooth::emboss::ConnectionRole::CENTRAL) {
+    std::optional<sm::LTK> ltk = GetExistingLtkFromPeerCache();
+    // If the new LTK isn't going to satisfy the request anyway, we can ignore
+    // it and start pairing.
+    if (ltk && ltk != ltk_ && ltk->security().level() >= next_req.level) {
+      bt_log(INFO,
+             "sm",
+             "starting encryption with LTK from PeerCache (peer: %s, handle: "
+             "%#.4x)",
+             bt_str(peer_->identifier()),
+             low_energy_link_->handle());
+
+      OnNewLongTermKey(*ltk);
+
+      current_phase_ = StartingEncryption();
+      PW_CHECK(low_energy_link_->StartEncryption());
+      return;
+    }
+  }
+
   if (fit::result result = RequestSecurityUpgrade(next_req.level);
       result.is_error()) {
     next_req.callback(ToResult(result.error_value()), security());

diff --git a/pw_bluetooth_sapphire/host/sm/security_manager_test.cc b/pw_bluetooth_sapphire/host/sm/security_manager_test.cc
@@ -4776,6 +4776,28 @@ TEST_F(InitiatorPairingTest, SecurityRequestReceivedWhileStartingEncryption) {
   EXPECT_EQ(1, new_sec_props_count());
 }
 
+TEST_F(InitiatorPairingTest, BrEdrCtkdHappenedAfterSecurityManagerCreation) {
+  InitializePeer(kPeerPublicAddr);
+  NewSecurityManager(
+      Role::kInitiator, IOCapability::kDisplayOnly, BondableMode::Bondable);
+
+  sm::PairingData pairing_data;
+  pairing_data.local_ltk = kAuthenticatedSecureKey;
+  pairing_data.peer_ltk = kAuthenticatedSecureKey;
+  peer().MutLe().SetBondData(pairing_data);
+
+  // The security upgrade should simply start encryption with the LTK we just
+  // set.
+  UpgradeSecurity(SecurityLevel::kEncrypted);
+  RunUntilIdle();
+  EXPECT_EQ(0, pairing_request_count());
+  EXPECT_EQ(1, fake_link()->start_encryption_count());
+  fake_link()->TriggerEncryptionChangeCallback(fit::ok(/*enabled=*/true));
+  RunUntilIdle();
+  EXPECT_EQ(1, new_sec_props_count());
+  EXPECT_EQ(security_callback_count(), 1);
+}
+
 }  // namespace
 }  // namespace bt::sm
 // inclusive-language: enable