Skip to content

Commit

Permalink
pw_bluetooth_sapphire: Add flag to enable/disable legacy pairing
Browse files Browse the repository at this point in the history
Add a flag in bt-host that only allows legacy pairing features to be
used when flag is enabled. Follow up CL sets flag value based on product
assembly (fxr/1103486).

Create new BrEdrConnectionManagerLegacyPairingTest fixture to test when
legacy pairing is enabled.

Bug: 42173830
Test: fx test //src/connectivity/bluetooth
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1102453
GitOrigin-RevId: c6524b78afbf0b7e5573d77b0707e1bb986c0316
Change-Id: I9ef21dfbef3eb17ad54aa0be8cb818c5dd77c3d5
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/232752
Pigweed-Auto-Submit: Jason Graffius <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Reviewed-by: Ben Lawson <[email protected]>
Lint: Lint 🤖 <[email protected]>
  • Loading branch information
Lulu Wang authored and CQ Bot Account committed Aug 31, 2024
1 parent 8bb8089 commit 107eecb
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 8 deletions.
10 changes: 9 additions & 1 deletion pw_bluetooth_sapphire/host/gap/adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class AdapterImpl final : public Adapter {
explicit AdapterImpl(pw::async::Dispatcher& pw_dispatcher,
hci::Transport::WeakPtr hci,
gatt::GATT::WeakPtr gatt,
Config config,
std::unique_ptr<l2cap::ChannelManager> l2cap);
~AdapterImpl() override;

Expand Down Expand Up @@ -559,6 +560,9 @@ class AdapterImpl final : public Adapter {
// for service discovery.
gatt::GATT::WeakPtr gatt_;

// Contains feature flags based on the product's configuration
Config config_;

// Objects that abstract the controller for connection and advertising
// procedures.
std::unique_ptr<hci::LowEnergyAdvertiser> hci_le_advertiser_;
Expand Down Expand Up @@ -594,13 +598,15 @@ class AdapterImpl final : public Adapter {
AdapterImpl::AdapterImpl(pw::async::Dispatcher& pw_dispatcher,
hci::Transport::WeakPtr hci,
gatt::GATT::WeakPtr gatt,
Config config,
std::unique_ptr<l2cap::ChannelManager> l2cap)
: identifier_(Random<AdapterId>()),
hci_(std::move(hci)),
init_state_(State::kNotInitialized),
peer_cache_(pw_dispatcher),
l2cap_(std::move(l2cap)),
gatt_(std::move(gatt)),
config_(config),
dispatcher_(pw_dispatcher),
weak_self_(this),
weak_self_adapter_(this) {
Expand Down Expand Up @@ -1523,6 +1529,7 @@ void AdapterImpl::InitializeStep4() {
state_.features.HasBit(/*page=*/0,
hci_spec::LMPFeature::kInterlacedPageScan),
state_.IsLocalSecureConnectionsSupported(),
config_.legacy_pairing_enabled,
dispatcher_);
bredr_connection_manager_->AttachInspect(
adapter_node_, kInspectBrEdrConnectionManagerNodeName);
Expand Down Expand Up @@ -1792,9 +1799,10 @@ std::unique_ptr<Adapter> Adapter::Create(
pw::async::Dispatcher& pw_dispatcher,
hci::Transport::WeakPtr hci,
gatt::GATT::WeakPtr gatt,
Config config,
std::unique_ptr<l2cap::ChannelManager> l2cap) {
return std::make_unique<AdapterImpl>(
pw_dispatcher, hci, gatt, std::move(l2cap));
pw_dispatcher, hci, gatt, config, std::move(l2cap));
}

} // namespace bt::gap
8 changes: 8 additions & 0 deletions pw_bluetooth_sapphire/host/gap/adapter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,13 @@ class AdapterTest : public TestingBase {

auto l2cap = std::make_unique<l2cap::testing::FakeL2cap>(dispatcher());
gatt_ = std::make_unique<gatt::testing::FakeLayer>(dispatcher());
Adapter::Config config = {
.legacy_pairing_enabled = false,
};
adapter_ = Adapter::Create(dispatcher(),
transport()->GetWeakPtr(),
gatt_->GetWeakPtr(),
config,
std::move(l2cap));
}

Expand Down Expand Up @@ -1344,9 +1348,13 @@ TEST_F(AdapterConstructorTest, GattCallbacks) {
EXPECT_EQ(set_persist_cb_count, 0);
EXPECT_EQ(set_retrieve_cb_count, 0);

Adapter::Config config = {
.legacy_pairing_enabled = false,
};
auto adapter = Adapter::Create(dispatcher(),
transport()->GetWeakPtr(),
gatt_->GetWeakPtr(),
config,
std::move(l2cap_));

EXPECT_EQ(set_persist_cb_count, 1);
Expand Down
32 changes: 27 additions & 5 deletions pw_bluetooth_sapphire/host/gap/bredr_connection_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ BrEdrConnectionManager::BrEdrConnectionManager(
l2cap::ChannelManager* l2cap,
bool use_interlaced_scan,
bool local_secure_connections_supported,
bool legacy_pairing_enabled,
pw::async::Dispatcher& dispatcher)
: hci_(std::move(hci)),
cache_(peer_cache),
Expand All @@ -189,6 +190,7 @@ BrEdrConnectionManager::BrEdrConnectionManager(
page_scan_window_(0),
use_interlaced_scan_(use_interlaced_scan),
local_secure_connections_supported_(local_secure_connections_supported),
legacy_pairing_enabled_(legacy_pairing_enabled),
dispatcher_(dispatcher),
weak_self_(this) {
BT_DEBUG_ASSERT(hci_.is_alive());
Expand Down Expand Up @@ -891,12 +893,20 @@ void BrEdrConnectionManager::CompleteConnectionSetup(
}

// Now that interrogation has successfully completed, check if the peer's
// feature bits indicate SSP support. If yes, use SecurePairingState to
// perform pairing otherwise, use LegacyPairingState.
// feature bits indicate SSP support. If not, use LegacyPairingState to
// perform pairing if legacy pairing is enabled.
PairingStateManager::PairingStateType pairing_type =
PairingStateManager::PairingStateType::kLegacyPairing;
if (peer->IsSecureSimplePairingSupported()) {
pairing_type = PairingStateManager::PairingStateType::kSecureSimplePairing;
PairingStateManager::PairingStateType::kSecureSimplePairing;
if (!peer->IsSecureSimplePairingSupported()) {
if (!legacy_pairing_enabled_) {
bt_log(WARN,
"gap-bredr",
"Peer %s does not support SSP but legacy pairing is not enabled "
"so pairing cannot occur",
bt_str(peer_id));
return;
}
pairing_type = PairingStateManager::PairingStateType::kLegacyPairing;
}
conn_state.CreateOrUpdatePairingState(
pairing_type, pairing_delegate_, security_mode());
Expand Down Expand Up @@ -1471,6 +1481,18 @@ BrEdrConnectionManager::OnLinkKeyNotification(
return hci::CommandChannel::EventCallbackResult::kContinue;
}

if (!legacy_pairing_enabled_ &&
key_type == pw::bluetooth::emboss::KeyType::COMBINATION) {
bt_log(WARN,
"gap-bredr",
"Got %u key type in link key notification for peer %s but legacy "
"pairing is not enabled",
static_cast<uint8_t>(key_type),
bt_str(peer->identifier()));
cache_->LogBrEdrBondingEvent(false);
return hci::CommandChannel::EventCallbackResult::kContinue;
}

bt_log(INFO,
"gap-bredr",
"got link key notification (key type: %u, peer: %s)",
Expand Down
76 changes: 74 additions & 2 deletions pw_bluetooth_sapphire/host/gap/bredr_connection_manager_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ class BrEdrConnectionManagerTest : public TestingBase {
l2cap_.get(),
/*use_interlaced_scan=*/true,
/*local_secure_connections_supported=*/true,
/*legacy_pairing_enabled=*/false,
dispatcher());

RunUntilIdle();
Expand Down Expand Up @@ -916,7 +917,78 @@ class BrEdrConnectionManagerTest : public TestingBase {
BT_DISALLOW_COPY_AND_ASSIGN_ALLOW_MOVE(BrEdrConnectionManagerTest);
};

using BrEdrConnectionManagerLegacyPairingTest = BrEdrConnectionManagerTest;
class BrEdrConnectionManagerLegacyPairingTest
: public BrEdrConnectionManagerTest {
public:
void SetUp() override {
TestingBase::SetUp();
InitializeACLDataChannel(kBrEdrBufferInfo, kLeBufferInfo);

peer_cache_ = std::make_unique<PeerCache>(dispatcher());
l2cap_ = std::make_unique<l2cap::testing::FakeL2cap>(dispatcher());

// Respond to BrEdrConnectionManager controller setup with success.
EXPECT_CMD_PACKET_OUT(test_device(),
testing::WritePageTimeoutPacket(static_cast<uint16_t>(
pw::bluetooth::emboss::PageTimeout::DEFAULT)),
&kWritePageTimeoutRsp);
EXPECT_CMD_PACKET_OUT(test_device(),
testing::WritePinTypePacket(static_cast<uint8_t>(
pw::bluetooth::emboss::PinType::VARIABLE)),
&kWritePinTypeRsp);

connection_manager_ = std::make_unique<BrEdrConnectionManager>(
transport()->GetWeakPtr(),
peer_cache_.get(),
kLocalDevAddr,
l2cap_.get(),
/*use_interlaced_scan=*/true,
/*local_secure_connections_supported=*/true,
/*legacy_pairing_enabled=*/true,
dispatcher());

RunUntilIdle();

test_device()->SetTransactionCallback([this] { transaction_count_++; });
}

void TearDown() override {
int expected_transaction_count = transaction_count();
if (connection_manager_ != nullptr) {
expected_transaction_count += 2;
// deallocating the connection manager disables connectivity.
EXPECT_CMD_PACKET_OUT(
test_device(), kReadScanEnable, &kReadScanEnableRspBoth);
EXPECT_CMD_PACKET_OUT(
test_device(), kWriteScanEnableInq, &kWriteScanEnableRsp);
connection_manager_ = nullptr;
}
RunUntilIdle();
// A disconnection may also occur for a queued disconnection, allow up to 1
// extra transaction.
EXPECT_LE(expected_transaction_count, transaction_count());
EXPECT_GE(expected_transaction_count + 1, transaction_count());
// Don't trigger the transaction callback for the rest.
test_device()->ClearTransactionCallback();
test_device()->Stop();
l2cap_ = nullptr;
peer_cache_ = nullptr;
TestingBase::TearDown();
}

protected:
BrEdrConnectionManager* connmgr() const { return connection_manager_.get(); }
PeerCache* peer_cache() const { return peer_cache_.get(); }
l2cap::testing::FakeL2cap* l2cap() const { return l2cap_.get(); }
int transaction_count() const { return transaction_count_; }

private:
std::unique_ptr<BrEdrConnectionManager> connection_manager_;
std::unique_ptr<PeerCache> peer_cache_;
std::unique_ptr<l2cap::testing::FakeL2cap> l2cap_;
int transaction_count_ = 0;
};

// Legacy pairing requires a PIN code to be displayed for the peer to enter, so
// this cannot happen when we do not have any display output capabilities.
TEST_F(BrEdrConnectionManagerLegacyPairingTest,
Expand Down Expand Up @@ -2408,7 +2480,7 @@ TEST_F(BrEdrConnectionManagerTest, SearchAfterConnected) {
QueueDisconnection(kConnectionHandle);
}

TEST_F(BrEdrConnectionManagerTest, SearchOnReconnect) {
TEST_F(BrEdrConnectionManagerLegacyPairingTest, SearchOnReconnect) {
size_t search_cb_count = 0;
auto search_cb = [&](auto id, const auto& attributes) {
auto* peer = peer_cache()->FindByAddress(kTestDevAddr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ class AdapterId : public Identifier<uint64_t> {
// supported.
class Adapter {
public:
struct Config {
// When True, BR/EDR pairing may attempt to use legacy pairing if the peer
// does not support SSP.
bool legacy_pairing_enabled = false;
};

static constexpr const char* kMetricsInspectNodeName = "metrics";

// Optionally, a FakeL2cap may be passed for testing purposes as |l2cap|. If
Expand All @@ -83,6 +89,7 @@ class Adapter {
pw::async::Dispatcher& pw_dispatcher,
hci::Transport::WeakPtr hci,
gatt::GATT::WeakPtr gatt,
Config config,
std::unique_ptr<l2cap::ChannelManager> l2cap = nullptr);
virtual ~Adapter() = default;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class BrEdrConnectionManager final {
l2cap::ChannelManager* l2cap,
bool use_interlaced_scan,
bool local_secure_connections_supported,
bool legacy_pairing_enabled,
pw::async::Dispatcher& dispatcher);
~BrEdrConnectionManager();

Expand Down Expand Up @@ -409,6 +410,10 @@ class BrEdrConnectionManager final {
// True when local Host and Controller support BR/EDR Secure Connections
bool local_secure_connections_supported_;

// When True, BR/EDR pairing may attempt to use legacy pairing if the peer
// does not support SSP.
bool legacy_pairing_enabled_;

// Outstanding incoming and outgoing connection requests from remote peer with
// |PeerId|.
std::unordered_map<PeerId, BrEdrConnectionRequest> connection_requests_;
Expand Down

0 comments on commit 107eecb

Please sign in to comment.