[bt][sm] Don't block legacy pairing on LTK encryption
We now consider locally initiated legacy pairing complete once all
expected keys have been exchanged and leave the link encrypted with the
STK. This improves interoperability with many peripherals and is
consistent with our responder implementation.
Bug: BT-801 #done
Bug: BT-549 #done
Test: 1. bt-host-unittests
2. Pairing with a Microsoft Designer Mouse succeeds with local IRK
distribution.
Change-Id: I4b0deef021629fa09d608541052c1de526fe1cb5
diff --git a/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.cc b/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.cc
index 2a1e6b9..a614ef68 100644
--- a/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.cc
+++ b/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.cc
@@ -34,7 +34,6 @@
PairingState::LegacyState::LegacyState(uint64_t id)
: id(id),
stk_encrypted(false),
- ltk_encrypted(false),
obtained_remote_keys(0u),
sent_local_keys(false),
has_tk(false),
@@ -58,8 +57,7 @@
}
bool PairingState::LegacyState::IsComplete() const {
- return features && stk_encrypted && KeyExchangeComplete() &&
- !WaitingForEncryptionWithLTK();
+ return features && stk_encrypted && KeyExchangeComplete();
}
bool PairingState::LegacyState::WaitingForTK() const {
@@ -101,15 +99,6 @@
return (features->local_key_distribution & KeyDistGen::kIdKey);
}
-bool PairingState::LegacyState::WaitingForEncryptionWithLTK() const {
- // When we are the responder we consider the pairing to be done after all keys
- // have been exchanged and do not wait for the master to encrypt with the LTK.
- // This is because some LE central implementations leave the link encrypted
- // with the STK until a re-connection.
- return features->initiator && (ShouldReceiveLTK() || ShouldSendLTK()) &&
- !ltk_encrypted;
-}
-
PairingState::PendingRequest::PendingRequest(SecurityLevel level,
PairingCallback callback)
: level(level), callback(std::move(callback)) {}
@@ -909,22 +898,11 @@
if (legacy_state_->InPhase2()) {
bt_log(TRACE, "sm", "link encrypted with STK");
EndLegacyPairingPhase2();
- return;
- }
-
- // If encryption was enabled after Phase 3 then this completes the pairing
- // procedure.
- if (legacy_state_->RequestedKeysObtained() &&
- legacy_state_->WaitingForEncryptionWithLTK()) {
- bt_log(TRACE, "sm", "link encrypted with LTK");
- legacy_state_->ltk_encrypted = true;
- CompleteLegacyPairing();
}
}
void PairingState::OnExpectedKeyReceived() {
ZX_DEBUG_ASSERT(legacy_state_);
- ZX_DEBUG_ASSERT(!legacy_state_->ltk_encrypted);
ZX_DEBUG_ASSERT(legacy_state_->stk_encrypted);
if (!legacy_state_->RequestedKeysObtained()) {
diff --git a/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.h b/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.h
index 60129ba..64afe28 100644
--- a/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.h
+++ b/src/connectivity/bluetooth/core/bt-host/sm/pairing_state.h
@@ -175,10 +175,6 @@
bool ShouldSendLTK() const; // True if we should send the LTK
bool ShouldSendIdentity() const; // True if we should send identity info
- // True if LTK will be exchanged and the link is yet to be encrypted with
- // the LTK.
- bool WaitingForEncryptionWithLTK() const;
-
// A unique token for this pairing state.
uint64_t id;
@@ -190,11 +186,6 @@
// in Phase 3. Otherwise we're in Phase 1 or 2.
bool stk_encrypted;
- // True if the link has been encrypted with the LTK. If the LTK should be
- // exchanged, then pairing is considered complete when the link is
- // encrypted with the LTK.
- bool ltk_encrypted;
-
// The remote keys that have been obtained so far.
KeyDistGenField obtained_remote_keys;
diff --git a/src/connectivity/bluetooth/core/bt-host/sm/pairing_state_unittest.cc b/src/connectivity/bluetooth/core/bt-host/sm/pairing_state_unittest.cc
index 547e82e..d0e36b1 100644
--- a/src/connectivity/bluetooth/core/bt-host/sm/pairing_state_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/sm/pairing_state_unittest.cc
@@ -1407,64 +1407,27 @@
EXPECT_EQ(0, pairing_callback_count());
EXPECT_EQ(0, pairing_data_callback_count());
+ constexpr uint16_t kEdiv = 1;
+ constexpr uint64_t kRand = 2;
+ constexpr uint16_t kDupEdiv = 3;
+ constexpr uint64_t kDupRand = 4;
+
+ // Send duplicate master identification. Pairing should complete with the
+ // first set of information. The second set should get ignored.
ReceiveEncryptionInformation(UInt128());
- ReceiveMasterIdentification(1, 2);
- ReceiveMasterIdentification(1, 2);
- RunLoopUntilIdle();
-
- EXPECT_EQ(1, pairing_failed_count());
- EXPECT_EQ(1, pairing_callback_count());
- EXPECT_EQ(1, pairing_complete_count());
- EXPECT_EQ(ErrorCode::kUnspecifiedReason, pairing_status().protocol_error());
- EXPECT_EQ(ErrorCode::kUnspecifiedReason, received_error_code());
- EXPECT_EQ(pairing_status(), pairing_complete_status());
-}
-
-// Initiator starts encryption with LTK after receiving LTK, ediv, and rand.
-// TODO(armansito): Test that the link isn't encrypted with the LTK until all
-// keys are received if the remote key distribution has more than just "enc key"
-// set.
-TEST_F(SMP_InitiatorPairingTest, Phase3EncryptionWithLTKFails) {
- UInt128 stk;
- FastForwardToSTKEncrypted(&stk, SecurityLevel::kEncrypted,
- KeyDistGen::kEncKey);
-
- UInt128 kLTK{{1, 2, 3, 4, 5, 6, 7, 8, 7, 6, 5, 4, 3, 2, 1, 0}};
- uint64_t kRand = 5;
- uint16_t kEDiv = 20;
-
- ReceiveEncryptionInformation(kLTK);
- ReceiveMasterIdentification(kRand, kEDiv);
+ ReceiveMasterIdentification(kRand, kEdiv);
+ ReceiveMasterIdentification(kDupRand, kDupEdiv);
RunLoopUntilIdle();
EXPECT_EQ(0, pairing_failed_count());
- EXPECT_EQ(0, pairing_callback_count());
-
- ASSERT_TRUE(fake_link()->ltk());
- EXPECT_EQ(kLTK, fake_link()->ltk()->value());
- EXPECT_EQ(kRand, fake_link()->ltk()->rand());
- EXPECT_EQ(kEDiv, fake_link()->ltk()->ediv());
- EXPECT_EQ(2, fake_link()->start_encryption_count());
-
- // Encryption fails.
- fake_link()->TriggerEncryptionChangeCallback(
- hci::Status(hci::StatusCode::kPinOrKeyMissing), false /* enabled */);
- RunLoopUntilIdle();
-
+ EXPECT_EQ(1, pairing_callback_count());
EXPECT_EQ(1, pairing_complete_count());
- EXPECT_EQ(1, auth_failure_callback_count());
- EXPECT_EQ(ErrorCode::kUnspecifiedReason, pairing_status().protocol_error());
- EXPECT_EQ(ErrorCode::kUnspecifiedReason, received_error_code());
+ EXPECT_EQ(1, pairing_data_callback_count());
+ EXPECT_TRUE(pairing_status().is_success());
EXPECT_EQ(pairing_status(), pairing_complete_status());
- EXPECT_EQ(hci::StatusCode::kPinOrKeyMissing,
- auth_failure_status().protocol_error());
-
- // Security properties should have been notified twice (once for the STK and
- // LTK each).
- EXPECT_EQ(2, new_sec_props_count());
-
- // The link is not secure since encryption failed.
- EXPECT_EQ(SecurityProperties(), pairing()->security());
+ ASSERT_TRUE(pairing_data().ltk);
+ EXPECT_EQ(kEdiv, pairing_data().ltk->key().ediv());
+ EXPECT_EQ(kRand, pairing_data().ltk->key().rand());
}
// Pairing completes after obtaining encryption information only.
@@ -1481,27 +1444,23 @@
ReceiveMasterIdentification(kRand, kEDiv);
RunLoopUntilIdle();
- EXPECT_EQ(0, pairing_failed_count());
- EXPECT_EQ(0, pairing_callback_count());
-
- ASSERT_TRUE(fake_link()->ltk());
- EXPECT_EQ(kLTK, fake_link()->ltk()->value());
- EXPECT_EQ(kRand, fake_link()->ltk()->rand());
- EXPECT_EQ(kEDiv, fake_link()->ltk()->ediv());
- EXPECT_EQ(2, fake_link()->start_encryption_count());
-
- // Encryption succeeds.
- fake_link()->TriggerEncryptionChangeCallback(hci::Status(),
- true /* enabled */);
- RunLoopUntilIdle();
-
- // Pairing should succeed without notifying any keys.
+ // Pairing should have succeeded.
EXPECT_EQ(0, pairing_failed_count());
EXPECT_EQ(1, pairing_callback_count());
EXPECT_EQ(1, pairing_complete_count());
EXPECT_TRUE(pairing_status());
EXPECT_EQ(pairing_status(), pairing_complete_status());
+ // LTK should have been assigned to the link.
+ ASSERT_TRUE(fake_link()->ltk());
+ EXPECT_EQ(kLTK, fake_link()->ltk()->value());
+ EXPECT_EQ(kRand, fake_link()->ltk()->rand());
+ EXPECT_EQ(kEDiv, fake_link()->ltk()->ediv());
+
+ // We don't re-encrypt with the LTK while the link is already authenticated
+ // with the STK.
+ EXPECT_EQ(1, fake_link()->start_encryption_count());
+
EXPECT_EQ(SecurityLevel::kEncrypted, sec_props().level());
EXPECT_EQ(16u, sec_props().enc_key_size());
EXPECT_FALSE(sec_props().secure_connections());
@@ -1748,28 +1707,23 @@
ReceiveIdentityAddress(kPeerAddr);
RunLoopUntilIdle();
- // Pairing still pending. Encryption request with LTK should have been made.
- EXPECT_EQ(0, pairing_failed_count());
- EXPECT_EQ(0, pairing_callback_count());
- EXPECT_EQ(0, pairing_complete_count());
- ASSERT_TRUE(fake_link()->ltk());
- EXPECT_EQ(kLTK, fake_link()->ltk()->value());
- EXPECT_EQ(kRand, fake_link()->ltk()->rand());
- EXPECT_EQ(kEDiv, fake_link()->ltk()->ediv());
- EXPECT_EQ(2, fake_link()->start_encryption_count());
-
- // Encryption succeeds.
- fake_link()->TriggerEncryptionChangeCallback(hci::Status(),
- true /* enabled */);
- RunLoopUntilIdle();
-
- // Pairing should succeed without notifying any keys.
+ // Pairing should have succeeded
EXPECT_EQ(0, pairing_failed_count());
EXPECT_EQ(1, pairing_callback_count());
EXPECT_EQ(1, pairing_complete_count());
EXPECT_TRUE(pairing_status());
EXPECT_EQ(pairing_status(), pairing_complete_status());
+ // LTK should have been assigned to the link.
+ ASSERT_TRUE(fake_link()->ltk());
+ EXPECT_EQ(kLTK, fake_link()->ltk()->value());
+ EXPECT_EQ(kRand, fake_link()->ltk()->rand());
+ EXPECT_EQ(kEDiv, fake_link()->ltk()->ediv());
+
+ // We don't re-encrypt with the LTK while the link is already authenticated
+ // with the STK.
+ EXPECT_EQ(1, fake_link()->start_encryption_count());
+
EXPECT_EQ(SecurityLevel::kEncrypted, sec_props().level());
EXPECT_EQ(16u, sec_props().enc_key_size());
EXPECT_FALSE(sec_props().secure_connections());
@@ -1853,14 +1807,16 @@
ASSERT_TRUE(pairing_status());
ASSERT_TRUE(ltk());
ASSERT_EQ(SecurityLevel::kEncrypted, sec_props().level());
- ASSERT_EQ(2, fake_link()->start_encryption_count()); // Twice for STK & LTK
+ ASSERT_EQ(1, fake_link()->start_encryption_count()); // Once for the STK
ASSERT_EQ(1, pairing_request_count());
// Receive a security request with the no MITM requirement. This should
// trigger an encryption key refresh and no pairing request.
ReceiveSecurityRequest();
EXPECT_EQ(1, pairing_request_count());
- ASSERT_EQ(3, fake_link()->start_encryption_count()); // Twice for STK & LTK
+
+ // Once for the STK and once again due to the locally initiated key refresh.
+ ASSERT_EQ(2, fake_link()->start_encryption_count());
// Receive a security request with a higher security requirement. This should
// trigger a pairing request.
