Skip to content

Commit

Permalink
pw_bluetooth_sapphire: Wait for LE Pairing in SSP
Browse files Browse the repository at this point in the history 
Wait for LE Pairing to complete before starting Secure Simple Pairing in
case LE cross-transport key derivation produces a BR/EDR link key.

Bug: b/388607971
Change-Id: I938e6412b25dfd9c90d666de60edff8258f8319a
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/267572
Docs-Not-Needed: Ben Lawson <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Pigweed-Auto-Submit: Ben Lawson <[email protected]>
Lint: Lint 🤖 <[email protected]>
Reviewed-by: Faraaz Sareshwala <[email protected]>
  • Loading branch information
@BenjaminLawson
BenjaminLawson authored and CQ Bot Account committed Feb 14, 2025
1 parent 3fc29bc commit 4b8b525
Show file tree
Hide file tree
Showing 3 changed files with 172 additions and 20 deletions.
6 changes: 6 additions & 0 deletions ...ire/host/gap/public/pw_bluetooth_sapphire/internal/host/gap/secure_simple_pairing_state.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,10 @@ class SecureSimplePairingState final {
// pairing.
kIdle,

// As initiator, wait for the Low Energy pairing procedure to complete
// (before doing SSP).
kInitiatorWaitLEPairingComplete,

// As initiator, wait for Link Key Request.
kInitiatorWaitLinkKeyRequest,

Expand Down Expand Up @@ -554,6 +558,8 @@ class SecureSimplePairingState final {
InspectProperties inspect_properties_;
inspect::Node inspect_node_;

WeakSelf<SecureSimplePairingState> weak_self_{this};

BT_DISALLOW_COPY_ASSIGN_AND_MOVE(SecureSimplePairingState);
};

Expand Down
60 changes: 40 additions & 20 deletions pw_bluetooth_sapphire/host/gap/secure_simple_pairing_state.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ void SecureSimplePairingState::InitiatePairing(
BrEdrSecurityRequirements security_requirements, StatusCallback status_cb) {
// TODO(fxbug.dev/42082728): Reject pairing if peer/local device don't support
// Secure Connections and SC is required
if (state() == State::kIdle) {
if (state() == State::kIdle ||
state() == State::kInitiatorWaitLEPairingComplete) {
PW_CHECK(!is_pairing());

// If the current link key already meets the security requirements, skip
Expand All @@ -112,20 +113,15 @@ void SecureSimplePairingState::InitiatePairing(
// security requirements impossible, skip pairing and report failure
// immediately.

current_pairing_ =
Pairing::MakeInitiator(security_requirements,
outgoing_connection_,
peer_->MutBrEdr().RegisterPairing());
PairingRequest request{.security_requirements = security_requirements,
.status_callback = std::move(status_cb)};
request_queue_.push_back(std::move(request));
bt_log(DEBUG,
"gap-bredr",
"Initiating pairing on %#.4x (id %s)",
handle(),
bt_str(peer_id()));
state_ = State::kInitiatorWaitLinkKeyRequest;
send_auth_request_callback_();

if (state() == State::kInitiatorWaitLEPairingComplete) {
return;
}

InitiateNextPairingRequest();
return;
}

Expand Down Expand Up @@ -159,12 +155,35 @@ void SecureSimplePairingState::InitiateNextPairingRequest() {
return;
}

// "If a BR/EDR/LE device supports LE Secure Connections, then it shall
// initiate pairing on only one transport at a time to the same remote
// device." (v6.0, Vol 3, Part C, Sec. 14.2)
if (peer_->le() && peer_->le()->is_pairing()) {
bt_log(INFO,
"gap-bredr",
"Waiting for LE pairing to complete on %#.4x (id %s)",
handle(),
bt_str(peer_id()));
state_ = State::kInitiatorWaitLEPairingComplete;
peer_->MutLe().add_pairing_completion_callback(
[self = weak_self_.GetWeakPtr()]() {
if (!self.is_alive() ||
self->state_ != State::kInitiatorWaitLEPairingComplete) {
return;
}
self->state_ = State::kIdle;
self->InitiateNextPairingRequest();
});
return;
}

PairingRequest& request = request_queue_.front();

current_pairing_ =
Pairing::MakeInitiator(request.security_requirements,
outgoing_connection_,
peer_->MutBrEdr().RegisterPairing());

bt_log(DEBUG,
"gap-bredr",
"Initiating queued pairing on %#.4x (id %s)",
Expand Down Expand Up @@ -215,15 +234,16 @@ std::optional<IoCapability> SecureSimplePairingState::OnIoCapabilityRequest() {
}

void SecureSimplePairingState::OnIoCapabilityResponse(IoCapability peer_iocap) {
// If we preivously provided a key for peer to pair, but that didn't work,
// If we previously provided a key for peer to pair, but that didn't work,
// they may try to re-pair. Cancel the previous pairing if they try to
// restart.
if (state() == State::kWaitEncryption) {
PW_CHECK(is_pairing());
current_pairing_ = nullptr;
state_ = State::kIdle;
}
if (state() == State::kIdle) {
if (state() == State::kIdle ||
state() == State::kInitiatorWaitLEPairingComplete) {
PW_CHECK(!is_pairing());
current_pairing_ = Pairing::MakeResponder(
peer_iocap, outgoing_connection_, peer_->MutBrEdr().RegisterPairing());
Expand Down Expand Up @@ -413,7 +433,8 @@ void SecureSimplePairingState::OnSimplePairingComplete(

std::optional<hci_spec::LinkKey> SecureSimplePairingState::OnLinkKeyRequest() {
if (state() != State::kIdle &&
state() != State::kInitiatorWaitLinkKeyRequest) {
state() != State::kInitiatorWaitLinkKeyRequest &&
state() != State::kInitiatorWaitLEPairingComplete) {
FailWithUnexpectedEvent(__func__);
return std::nullopt;
}
Expand Down Expand Up @@ -447,17 +468,15 @@ std::optional<hci_spec::LinkKey> SecureSimplePairingState::OnLinkKeyRequest() {

// The link key request may be received outside of Simple Pairing (e.g. when
// the peer initiates the authentication procedure).
if (state() == State::kIdle) {
if (!is_pairing()) {
if (link_key.has_value()) {
PW_CHECK(!is_pairing());
current_pairing_ =
Pairing::MakeResponderForBonded(peer_->MutBrEdr().RegisterPairing());
state_ = State::kWaitEncryption;
return link_key->key();
}
return std::optional<hci_spec::LinkKey>();
}

PW_CHECK(is_pairing());

if (link_key.has_value() &&
Expand Down Expand Up @@ -487,8 +506,7 @@ void SecureSimplePairingState::OnLinkKeyNotification(
bt_str(peer_id()));

// When not pairing, only connection link key changes are allowed.
if (state() == State::kIdle &&
key_type == hci_spec::LinkKeyType::kChangedCombination) {
if (!is_pairing() && key_type == hci_spec::LinkKeyType::kChangedCombination) {
if (!link_->ltk()) {
bt_log(WARN,
"gap-bredr",
Expand Down Expand Up @@ -791,6 +809,8 @@ const char* SecureSimplePairingState::ToString(
switch (state) {
case State::kIdle:
return "Idle";
case State::kInitiatorWaitLEPairingComplete:
return "InitiatorWaitLEPairingComplete";
case State::kInitiatorWaitLinkKeyRequest:
return "InitiatorWaitLinkKeyRequest";
case State::kInitiatorWaitIoCapRequest:
Expand Down
126 changes: 126 additions & 0 deletions pw_bluetooth_sapphire/host/gap/secure_simple_pairing_state_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3493,5 +3493,131 @@ TEST_F(PairingStateTest, SetNullSecurityManagerChannelIgnored) {
pairing_state.SetSecurityManagerChannel(l2cap::Channel::WeakPtr());
}

TEST_F(PairingStateTest, InitiatorWaitForLEPairingToComplete) {
NoOpPairingDelegate pairing_delegate(kTestLocalIoCap);
SecureSimplePairingState pairing_state(
peer()->GetWeakPtr(),
pairing_delegate.GetWeakPtr(),
connection()->GetWeakPtr(),
/*outgoing_connection=*/false,
MakeAuthRequestCallback(),
NoOpStatusCallback,
/*low_energy_address_delegate=*/this,
/*controller_remote_public_key_validation_supported=*/true,
sm_factory_func(),
dispatcher());

std::optional<Peer::PairingToken> pairing_token =
peer()->MutLe().RegisterPairing();

// Queue 2 requests to ensure that edge case is handled.
TestStatusHandler status_handler_0;
pairing_state.InitiatePairing(kNoSecurityRequirements,
status_handler_0.MakeStatusCallback());
TestStatusHandler status_handler_1;
pairing_state.InitiatePairing(kNoSecurityRequirements,
status_handler_1.MakeStatusCallback());
RunUntilIdle();
EXPECT_EQ(auth_request_count(), 0u);

pairing_token.reset();
EXPECT_EQ(auth_request_count(), 1u);

AdvanceToEncryptionAsInitiator(&pairing_state);
EXPECT_TRUE(pairing_state.initiator());
ASSERT_EQ(0, status_handler_0.call_count());
ASSERT_EQ(0, status_handler_1.call_count());

connection()->TriggerEncryptionChangeCallback(fit::ok(true));
ASSERT_TRUE(status_handler_0.status());
EXPECT_EQ(fit::ok(), *status_handler_0.status());
ASSERT_TRUE(status_handler_1.status());
EXPECT_EQ(fit::ok(), *status_handler_1.status());
}

TEST_F(PairingStateTest,
LinkKeyRequestWhileInitiatorWaitsForLEPairingToComplete) {
NoOpPairingDelegate pairing_delegate(kTestLocalIoCap);
SecureSimplePairingState pairing_state(
peer()->GetWeakPtr(),
pairing_delegate.GetWeakPtr(),
connection()->GetWeakPtr(),
/*outgoing_connection=*/false,
MakeAuthRequestCallback(),
NoOpStatusCallback,
/*low_energy_address_delegate=*/this,
/*controller_remote_public_key_validation_supported=*/true,
sm_factory_func(),
dispatcher());

std::optional<Peer::PairingToken> pairing_token =
peer()->MutLe().RegisterPairing();

TestStatusHandler status_handler_0;
pairing_state.InitiatePairing(kNoSecurityRequirements,
status_handler_0.MakeStatusCallback());
RunUntilIdle();
EXPECT_EQ(auth_request_count(), 0u);

EXPECT_TRUE(peer()->MutBrEdr().SetBondData(
sm::LTK(sm::SecurityProperties(kTestUnauthenticatedLinkKeyType192),
kTestLinkKey)));

static_cast<void>(pairing_state.OnLinkKeyRequest());
ASSERT_EQ(0, status_handler_0.call_count());

connection()->TriggerEncryptionChangeCallback(fit::ok(true));
ASSERT_TRUE(status_handler_0.status());
EXPECT_EQ(fit::ok(), *status_handler_0.status());

// The end of LE pairing should be ignored now.
pairing_token.reset();
EXPECT_EQ(auth_request_count(), 0u);
}

TEST_F(PairingStateTest,
IoCapabilityResponseWhileInitiatorWaitsForLEPairingToComplete) {
NoOpPairingDelegate pairing_delegate(kTestLocalIoCap);
SecureSimplePairingState pairing_state(
peer()->GetWeakPtr(),
pairing_delegate.GetWeakPtr(),
connection()->GetWeakPtr(),
/*outgoing_connection=*/false,
MakeAuthRequestCallback(),
NoOpStatusCallback,
/*low_energy_address_delegate=*/this,
/*controller_remote_public_key_validation_supported=*/true,
sm_factory_func(),
dispatcher());

std::optional<Peer::PairingToken> pairing_token =
peer()->MutLe().RegisterPairing();

TestStatusHandler status_handler_0;
pairing_state.InitiatePairing(kNoSecurityRequirements,
status_handler_0.MakeStatusCallback());
RunUntilIdle();
EXPECT_EQ(auth_request_count(), 0u);

pairing_state.OnIoCapabilityResponse(kTestPeerIoCap);
ASSERT_FALSE(pairing_state.initiator());
static_cast<void>(pairing_state.OnIoCapabilityRequest());
pairing_state.OnUserConfirmationRequest(kTestPasskey,
NoOpUserConfirmationCallback);

pairing_state.OnSimplePairingComplete(
pw::bluetooth::emboss::StatusCode::SUCCESS);
pairing_state.OnLinkKeyNotification(kTestLinkKeyValue,
kTestUnauthenticatedLinkKeyType192);

connection()->TriggerEncryptionChangeCallback(fit::ok(true));
ASSERT_TRUE(status_handler_0.status());
EXPECT_EQ(fit::ok(), *status_handler_0.status());

// The end of LE pairing should be ignored now.
pairing_token.reset();
EXPECT_EQ(auth_request_count(), 0u);
}

} // namespace
} // namespace bt::gap

0 comments on commit 4b8b525

Please sign in to comment.