[bt][gap-bredr] Store link keys from pairing
In PairingState, receiving a Link Key Notification updates the
connection link key. Check that the link key provides the expected level
of security (is not a legacy pairing key and has expected level of MITM
protection).
Bug: 967
Test: in bt-host-unittests, GAP_PairingStateTest.*LinkKey*
Change-Id: I1eb6c40b2074ef9603f5f93255c7276339fce3a2
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.cc b/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.cc
index 3103bd5..29ef771 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.cc
@@ -181,7 +181,45 @@
}
ZX_ASSERT(is_pairing());
- // TODO(xow): Store link key.
+ // The association model and resulting link security properties are computed by both the Link
+ // Manager (controller) and the host subsystem, so check that they agree.
+ sm::SecurityProperties sec_props;
+ if (key_type == hci::LinkKeyType::kChangedCombination) {
+ if (!link_->ltk()) {
+ bt_log(WARN, "gap-bredr",
+ "Got Changed Combination key but link %#.4x (id: %s) has no current key", handle(),
+ bt_str(peer_id()));
+ state_ = State::kFailed;
+ SignalStatus(hci::Status(HostError::kInsufficientSecurity));
+ return;
+ }
+ } else {
+ sec_props = sm::SecurityProperties(key_type);
+ current_pairing_->security_properties = sec_props;
+ }
+
+ // Link keys resulting from legacy pairing are assigned lowest security level and we reject them.
+ if (sec_props.level() == sm::SecurityLevel::kNoSecurity) {
+ bt_log(WARN, "gap-bredr", "Link key (type %hhu) for %#.4x (id: %s) has insufficient security",
+ key_type, handle(), bt_str(peer_id()));
+ state_ = State::kFailed;
+ SignalStatus(hci::Status(HostError::kInsufficientSecurity));
+ return;
+ }
+
+ // If we performed an association procedure for MITM protection then expect the controller to
+ // produce a corresponding "authenticated" link key. Inversely, do not accept a link key reported
+ // as authenticated if we haven't performed the corresponding association procedure because it
+ // may provide a false high expectation of security to the user or application.
+ if (sec_props.authenticated() != current_pairing_->authenticated) {
+ bt_log(WARN, "gap-bredr", "Expected %sauthenticated link key for %#.4x (id: %s), got %hhu",
+ current_pairing_->authenticated ? "" : "un", handle(), bt_str(peer_id()), key_type);
+ state_ = State::kFailed;
+ SignalStatus(hci::Status(HostError::kInsufficientSecurity));
+ return;
+ }
+
+ link_->set_bredr_link_key(hci::LinkKey(link_key, 0, 0), key_type);
if (initiator()) {
state_ = State::kInitiatorWaitAuthComplete;
} else {
@@ -234,8 +272,6 @@
state_ = State::kFailed;
}
- // TODO(xow): Write link key to Connection::ltk to register new security
- // properties.
SignalStatus(status);
}
@@ -340,6 +376,8 @@
current_pairing_->expected_event =
GetExpectedEvent(current_pairing_->local_iocap, current_pairing_->peer_iocap);
ZX_DEBUG_ASSERT(GetStateForPairingEvent(current_pairing_->expected_event) != State::kFailed);
+ current_pairing_->authenticated =
+ IsPairingAuthenticated(current_pairing_->local_iocap, current_pairing_->peer_iocap);
}
PairingAction GetInitiatorPairingAction(IOCapability initiator_cap, IOCapability responder_cap) {
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.h b/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.h
index 9df7d4d..1202da4 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.h
+++ b/src/connectivity/bluetooth/core/bt-host/gap/pairing_state.h
@@ -15,6 +15,7 @@
#include "src/connectivity/bluetooth/core/bt-host/hci/connection.h"
#include "src/connectivity/bluetooth/core/bt-host/hci/hci.h"
#include "src/connectivity/bluetooth/core/bt-host/sm/smp.h"
+#include "src/connectivity/bluetooth/core/bt-host/sm/types.h"
namespace bt {
namespace gap {
@@ -283,6 +284,12 @@
// HCI event to respond to in order to complete or reject pairing.
hci::EventCode expected_event;
+
+ // True if this pairing is expected to be resistant to MITM attacks.
+ bool authenticated;
+
+ // Security properties of the link key received from the controller.
+ std::optional<sm::SecurityProperties> security_properties;
};
static const char* ToString(State state);
diff --git a/src/connectivity/bluetooth/core/bt-host/gap/pairing_state_unittest.cc b/src/connectivity/bluetooth/core/bt-host/gap/pairing_state_unittest.cc
index 8b1a15a..5c76f4e 100644
--- a/src/connectivity/bluetooth/core/bt-host/gap/pairing_state_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/gap/pairing_state_unittest.cc
@@ -30,7 +30,9 @@
const auto kTestPeerIoCap = IOCapability::kDisplayOnly;
const uint32_t kTestPasskey = 123456;
const auto kTestLinkKeyValue = Random<UInt128>();
+const auto kTestUnauthenticatedLinkKeyType = hci::LinkKeyType::kUnauthenticatedCombination192;
const auto kTestAuthenticatedLinkKeyType = hci::LinkKeyType::kAuthenticatedCombination192;
+const auto kTestLegacyLinkKeyType = hci::LinkKeyType::kCombination;
void NoOpStatusCallback(hci::ConnectionHandle, hci::Status){};
void NoOpUserConfirmationCallback(bool){};
@@ -245,7 +247,7 @@
pairing_state->OnIoCapabilityResponse(kTestPeerIoCap);
pairing_state->OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
pairing_state->OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state->OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state->OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
pairing_state->OnAuthenticationComplete(hci::StatusCode::kSuccess);
}
@@ -372,7 +374,7 @@
// Keep advancing state machine.
pairing_state.OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state.OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state.OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
EXPECT_FALSE(pairing_state.initiator());
ASSERT_EQ(0, status_handler.call_count());
@@ -425,6 +427,77 @@
EXPECT_EQ(hci::Status(HostError::kNotReady), *status_handler.status());
}
+TEST(GAP_PairingStateTest, UnexpectedLinkKeyAuthenticationRaisesError) {
+ TestStatusHandler status_handler;
+ auto connection = MakeFakeConnection();
+ PairingState pairing_state(kTestPeerId, &connection, status_handler.MakeStatusCallback());
+ NoOpPairingDelegate pairing_delegate(sm::IOCapability::kDisplayOnly);
+ pairing_state.SetPairingDelegate(pairing_delegate.GetWeakPtr());
+
+ // Advance state machine.
+ pairing_state.OnIoCapabilityResponse(IOCapability::kDisplayYesNo);
+ ASSERT_FALSE(pairing_state.initiator());
+ static_cast<void>(pairing_state.OnIoCapabilityRequest());
+ pairing_state.OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
+ pairing_state.OnSimplePairingComplete(hci::StatusCode::kSuccess);
+
+ // Provide an authenticated link key when this should have resulted in an
+ // unauthenticated link key.
+ pairing_state.OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+
+ EXPECT_EQ(1, status_handler.call_count());
+ ASSERT_TRUE(status_handler.handle());
+ EXPECT_EQ(kTestHandle, *status_handler.handle());
+ ASSERT_TRUE(status_handler.status());
+ EXPECT_EQ(hci::Status(HostError::kInsufficientSecurity), *status_handler.status());
+}
+
+TEST(GAP_PairingStateTest, LegacyPairingLinkKeyRaisesError) {
+ TestStatusHandler status_handler;
+ auto connection = MakeFakeConnection();
+ PairingState pairing_state(kTestPeerId, &connection, status_handler.MakeStatusCallback());
+ NoOpPairingDelegate pairing_delegate(sm::IOCapability::kNoInputNoOutput);
+ pairing_state.SetPairingDelegate(pairing_delegate.GetWeakPtr());
+
+ // Advance state machine.
+ pairing_state.OnIoCapabilityResponse(IOCapability::kDisplayYesNo);
+ ASSERT_FALSE(pairing_state.initiator());
+ static_cast<void>(pairing_state.OnIoCapabilityRequest());
+ pairing_state.OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
+ pairing_state.OnSimplePairingComplete(hci::StatusCode::kSuccess);
+
+ // Provide a legacy pairing link key type.
+ pairing_state.OnLinkKeyNotification(kTestLinkKeyValue, kTestLegacyLinkKeyType);
+
+ EXPECT_EQ(1, status_handler.call_count());
+ ASSERT_TRUE(status_handler.handle());
+ EXPECT_EQ(kTestHandle, *status_handler.handle());
+ ASSERT_TRUE(status_handler.status());
+ EXPECT_EQ(hci::Status(HostError::kInsufficientSecurity), *status_handler.status());
+}
+
+TEST(GAP_PairingStateTest, PairingSetsConnectionLinkKey) {
+ TestStatusHandler status_handler;
+ auto connection = MakeFakeConnection();
+ PairingState pairing_state(kTestPeerId, &connection, status_handler.MakeStatusCallback());
+ NoOpPairingDelegate pairing_delegate(sm::IOCapability::kNoInputNoOutput);
+ pairing_state.SetPairingDelegate(pairing_delegate.GetWeakPtr());
+
+ // Advance state machine.
+ pairing_state.OnIoCapabilityResponse(IOCapability::kDisplayYesNo);
+ ASSERT_FALSE(pairing_state.initiator());
+ static_cast<void>(pairing_state.OnIoCapabilityRequest());
+ pairing_state.OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
+ pairing_state.OnSimplePairingComplete(hci::StatusCode::kSuccess);
+
+ ASSERT_FALSE(connection.ltk());
+ pairing_state.OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
+ ASSERT_TRUE(connection.ltk());
+ EXPECT_EQ(kTestLinkKeyValue, connection.ltk()->value());
+
+ EXPECT_EQ(0, status_handler.call_count());
+}
+
// Event injectors. Return values are necessarily ignored in order to make types
// match, so don't use these functions to test return values. Likewise,
// arguments have been filled with test defaults for a successful pairing flow.
@@ -447,7 +520,7 @@
pairing_state->OnSimplePairingComplete(hci::StatusCode::kSuccess);
}
void LinkKeyNotification(PairingState* pairing_state) {
- pairing_state->OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state->OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
}
void AuthenticationComplete(PairingState* pairing_state) {
pairing_state->OnAuthenticationComplete(hci::StatusCode::kSuccess);
@@ -731,7 +804,7 @@
pairing_state().OnIoCapabilityResponse(kTestPeerIoCap);
pairing_state().OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
pairing_state().OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
ASSERT_TRUE(pairing_state().initiator());
EXPECT_EQ(0, connection().start_encryption_count());
@@ -753,7 +826,7 @@
pairing_state().OnIoCapabilityResponse(kTestPeerIoCap);
pairing_state().OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
pairing_state().OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
pairing_state().OnAuthenticationComplete(hci::StatusCode::kSuccess);
ASSERT_TRUE(pairing_state().initiator());
@@ -771,7 +844,7 @@
static_cast<void>(pairing_state().OnIoCapabilityRequest());
pairing_state().OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
pairing_state().OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
ASSERT_FALSE(pairing_state().initiator());
RETURN_IF_FATAL(InjectEvent());
@@ -789,7 +862,7 @@
pairing_state().OnIoCapabilityResponse(kTestPeerIoCap);
pairing_state().OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
pairing_state().OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
pairing_state().OnAuthenticationComplete(hci::StatusCode::kSuccess);
ASSERT_TRUE(pairing_state().initiator());
@@ -835,7 +908,7 @@
pairing_state().OnIoCapabilityResponse(kTestPeerIoCap);
pairing_state().OnUserConfirmationRequest(kTestPasskey, NoOpUserConfirmationCallback);
pairing_state().OnSimplePairingComplete(hci::StatusCode::kSuccess);
- pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestAuthenticatedLinkKeyType);
+ pairing_state().OnLinkKeyNotification(kTestLinkKeyValue, kTestUnauthenticatedLinkKeyType);
// Inject failure status.
pairing_state().OnAuthenticationComplete(hci::StatusCode::kAuthenticationFailure);