From a1f13fc4c4f4aabb3da9ad9c851ee4f4d238085b Mon Sep 17 00:00:00 2001 From: Jakub Pawlowski Date: Thu, 24 Oct 2019 20:12:35 +0200 Subject: [PATCH] Use AddressWithType in PairingHandlerLe Bug: 142341141 Test: updated unittests Change-Id: Ic543148ca2d36f44e64ffb00744adf6d557b84c3 --- gd/security/initial_informations.h | 9 +++---- gd/security/pairing_handler_le_legacy.cc | 26 +++++++++++-------- .../pairing_handler_le_secure_connections.cc | 12 ++++++--- gd/security/pairing_handler_le_unittest.cc | 16 +++++++----- gd/security/test/mocks.h | 4 +-- gd/security/test/pairing_handler_le_pair_test.cc | 30 +++++++++------------- gd/security/ui.h | 6 ++--- 7 files changed, 54 insertions(+), 49 deletions(-) diff --git a/gd/security/initial_informations.h b/gd/security/initial_informations.h index 3ada65cdc..db560bd9a 100644 --- a/gd/security/initial_informations.h +++ b/gd/security/initial_informations.h @@ -23,6 +23,7 @@ #include "common/bidi_queue.h" #include "common/callback.h" #include "crypto_toolbox/crypto_toolbox.h" +#include "hci/address_with_type.h" #include "hci/le_security_interface.h" #include "os/handler.h" #include "packet/base_packet_builder.h" @@ -42,7 +43,7 @@ using DistributedKeys = /* This class represents the result of pairing, as returned from Pairing Handler */ struct PairingResult { - Address connection_address; + hci::AddressWithType connection_address; DistributedKeys distributed_keys; }; @@ -62,8 +63,7 @@ struct MyOobData { /* This structure is filled and send to PairingHandlerLe to initiate the Pairing process with remote device */ struct InitialInformations { hci::Role my_role; - Address my_connection_address; - uint8_t my_connection_address_type; + hci::AddressWithType my_connection_address; /* My capabilities, as in pairing request/response */ struct { @@ -78,8 +78,7 @@ struct InitialInformations { /* was it remote device that initiated the Pairing ? */ bool remotely_initiated; uint16_t connection_handle; - Address remote_connection_address; - uint8_t remote_connection_address_type; + hci::AddressWithType remote_connection_address; std::string remote_name; /* contains pairing request, if the pairing was remotely initiated */ diff --git a/gd/security/pairing_handler_le_legacy.cc b/gd/security/pairing_handler_le_legacy.cc index bebaef4a8..0c3a73b27 100644 --- a/gd/security/pairing_handler_le_legacy.cc +++ b/gd/security/pairing_handler_le_legacy.cc @@ -130,9 +130,10 @@ StkOrFailure PairingHandlerLe::DoLegacyStage2(const InitialInformations& i, cons // LOG(INFO) << +(IAmMaster(i)) << " i.i.remote_connection_address.address = " << i.remote_connection_address; // LOG(INFO) << +(IAmMaster(i)) << " i.my_connection_address_type = " << +i.my_connection_address_type; // LOG(INFO) << +(IAmMaster(i)) << " i.i.my_connection_address.address = " << i.my_connection_address; - Octet16 mconfirm = crypto_toolbox::c1(tk, mrand, preq.data(), pres.data(), i.my_connection_address_type, - i.my_connection_address.address, i.remote_connection_address_type, - i.remote_connection_address.address); + Octet16 mconfirm = crypto_toolbox::c1( + tk, mrand, preq.data(), pres.data(), (uint8_t)i.my_connection_address.GetAddressType(), + i.my_connection_address.GetAddress().address, (uint8_t)i.remote_connection_address.GetAddressType(), + i.remote_connection_address.GetAddress().address); LOG_INFO("Master sends Mconfirm"); SendL2capPacket(i, PairingConfirmBuilder::Create(mconfirm)); @@ -162,9 +163,10 @@ StkOrFailure PairingHandlerLe::DoLegacyStage2(const InitialInformations& i, cons // LOG(INFO) << +(IAmMaster(i)) << " i.i.my_connection_address.address = " << i.my_connection_address; // LOG(INFO) << +(IAmMaster(i)) << " i.remote_connection_address_type = " << +i.remote_connection_address_type; // LOG(INFO) << +(IAmMaster(i)) << " i.i.remote_connection_address.address = " << i.remote_connection_address; - Octet16 sconfirm_generated = crypto_toolbox::c1(tk, srand, preq.data(), pres.data(), i.my_connection_address_type, - i.my_connection_address.address, i.remote_connection_address_type, - i.remote_connection_address.address); + Octet16 sconfirm_generated = crypto_toolbox::c1( + tk, srand, preq.data(), pres.data(), (uint8_t)i.my_connection_address.GetAddressType(), + i.my_connection_address.GetAddress().address, (uint8_t)i.remote_connection_address.GetAddressType(), + i.remote_connection_address.GetAddress().address); if (sconfirm != sconfirm_generated) { LOG_INFO("sconfirm does not match generated value"); @@ -178,9 +180,10 @@ StkOrFailure PairingHandlerLe::DoLegacyStage2(const InitialInformations& i, cons std::vector preq(pairing_request.begin(), pairing_request.end()); std::vector pres(pairing_response.begin(), pairing_response.end()); - Octet16 sconfirm = crypto_toolbox::c1(tk, srand, preq.data(), pres.data(), i.remote_connection_address_type, - i.remote_connection_address.address, i.my_connection_address_type, - i.my_connection_address.address); + Octet16 sconfirm = crypto_toolbox::c1( + tk, srand, preq.data(), pres.data(), (uint8_t)i.remote_connection_address.GetAddressType(), + i.remote_connection_address.GetAddress().address, (uint8_t)i.my_connection_address.GetAddressType(), + i.my_connection_address.GetAddress().address); LOG_INFO("Slave waits for the Mconfirm"); auto mconfirm_pkt = WaitPairingConfirm(); @@ -208,8 +211,9 @@ StkOrFailure PairingHandlerLe::DoLegacyStage2(const InitialInformations& i, cons // LOG(INFO) << +(IAmMaster(i)) << " i.remote_connection_address_type = " << +i.remote_connection_address_type; // LOG(INFO) << +(IAmMaster(i)) << " i.i.remote_connection_address.address = " << i.remote_connection_address; Octet16 mconfirm_generated = crypto_toolbox::c1( - tk, mrand, preq.data(), pres.data(), i.remote_connection_address_type, i.remote_connection_address.address, - i.my_connection_address_type, i.my_connection_address.address); + tk, mrand, preq.data(), pres.data(), (uint8_t)i.remote_connection_address.GetAddressType(), + i.remote_connection_address.GetAddress().address, (uint8_t)i.my_connection_address.GetAddressType(), + i.my_connection_address.GetAddress().address); if (mconfirm != mconfirm_generated) { LOG_INFO("mconfirm does not match generated value"); diff --git a/gd/security/pairing_handler_le_secure_connections.cc b/gd/security/pairing_handler_le_secure_connections.cc index 91af82a07..f5b0a0fff 100644 --- a/gd/security/pairing_handler_le_secure_connections.cc +++ b/gd/security/pairing_handler_le_secure_connections.cc @@ -134,11 +134,15 @@ Stage2ResultOrFailure PairingHandlerLe::DoSecureConnectionsStage2(const InitialI uint8_t b[7]; if (IAmMaster(i)) { - memcpy(a, i.my_connection_address.address, 7); - memcpy(b, i.remote_connection_address.address, 7); + memcpy(a, i.my_connection_address.GetAddress().address, 6); + a[6] = (uint8_t)i.my_connection_address.GetAddressType(); + memcpy(b, i.remote_connection_address.GetAddress().address, 6); + b[6] = (uint8_t)i.remote_connection_address.GetAddressType(); } else { - memcpy(a, i.remote_connection_address.address, 7); - memcpy(b, i.my_connection_address.address, 7); + memcpy(a, i.remote_connection_address.GetAddress().address, 6); + a[6] = (uint8_t)i.remote_connection_address.GetAddressType(); + memcpy(b, i.my_connection_address.GetAddress().address, 6); + b[6] = (uint8_t)i.my_connection_address.GetAddressType(); } Octet16 ltk, mac_key; diff --git a/gd/security/pairing_handler_le_unittest.cc b/gd/security/pairing_handler_le_unittest.cc index e5239b471..3610700ff 100644 --- a/gd/security/pairing_handler_le_unittest.cc +++ b/gd/security/pairing_handler_le_unittest.cc @@ -120,8 +120,7 @@ class PairingHandlerUnitTest : public testing::Test { InitialInformations initial_informations{ .my_role = hci::Role::MASTER, - .my_connection_address = {}, - .my_connection_address_type = 0x00, + .my_connection_address = {{}, hci::AddressType::PUBLIC_DEVICE_ADDRESS}, .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, @@ -131,8 +130,7 @@ InitialInformations initial_informations{ .responder_key_distribution = 0x03}, .remotely_initiated = false, - .remote_connection_address = {}, - .remote_connection_address_type = 0x01, + .remote_connection_address = {{}, hci::AddressType::RANDOM_DEVICE_ADDRESS}, .ui_handler = &uiMock, .le_security_interface = &leSecurityMock, .OnPairingFinished = OnPairingFinished, @@ -221,8 +219,10 @@ TEST_F(PairingHandlerUnitTest, test_secure_connections_just_works) { // Start of authentication stage 2 uint8_t a[7]; uint8_t b[7]; - memcpy(b, initial_informations.remote_connection_address.address, 7); - memcpy(a, initial_informations.my_connection_address.address, 7); + memcpy(b, initial_informations.remote_connection_address.GetAddress().address, 6); + b[6] = (uint8_t)initial_informations.remote_connection_address.GetAddressType(); + memcpy(a, initial_informations.my_connection_address.GetAddress().address, 6); + a[6] = (uint8_t)initial_informations.my_connection_address.GetAddressType(); Octet16 ltk, mac_key; crypto_toolbox::f5(dhkey.data(), Na, Nb, a, b, &mac_key, <k); @@ -254,6 +254,7 @@ TEST_F(PairingHandlerUnitTest, test_secure_connections_just_works) { InitialInformations initial_informations_trsi{ .my_role = hci::Role::MASTER, + .my_connection_address = hci::AddressWithType(), .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, @@ -263,6 +264,7 @@ InitialInformations initial_informations_trsi{ .responder_key_distribution = 0x03}, .remotely_initiated = true, + .remote_connection_address = hci::AddressWithType(), .ui_handler = &uiMock, .le_security_interface = &leSecurityMock, .OnPairingFinished = OnPairingFinished, @@ -289,6 +291,7 @@ TEST_F(PairingHandlerUnitTest, test_remote_slave_initiating) { InitialInformations initial_informations_trmi{ .my_role = hci::Role::SLAVE, + .my_connection_address = hci::AddressWithType(), .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, @@ -298,6 +301,7 @@ InitialInformations initial_informations_trmi{ .responder_key_distribution = 0x03}, .remotely_initiated = true, + .remote_connection_address = hci::AddressWithType(), .pairing_request = PairingRequestView::Create(BuilderToView( PairingRequestBuilder::Create(IoCapability::NO_INPUT_NO_OUTPUT, OobDataFlag::NOT_PRESENT, AuthReqMaskBondingFlag | AuthReqMaskMitm | AuthReqMaskSc, 16, 0x03, 0x03))), diff --git a/gd/security/test/mocks.h b/gd/security/test/mocks.h index c347d3679..812c9a143 100644 --- a/gd/security/test/mocks.h +++ b/gd/security/test/mocks.h @@ -32,8 +32,8 @@ class UIMock : public UI { UIMock() {} ~UIMock() override = default; - MOCK_METHOD2(DisplayPairingPrompt, void(const bluetooth::hci::Address&, std::string&)); - MOCK_METHOD1(CancelPairingPrompt, void(const bluetooth::hci::Address&)); + MOCK_METHOD2(DisplayPairingPrompt, void(const bluetooth::hci::AddressWithType&, std::string&)); + MOCK_METHOD1(CancelPairingPrompt, void(const bluetooth::hci::AddressWithType&)); MOCK_METHOD1(DisplayConfirmValue, void(uint32_t)); MOCK_METHOD0(DisplayEnterPasskeyDialog, void()); MOCK_METHOD1(DisplayPasskey, void(uint32_t)); diff --git a/gd/security/test/pairing_handler_le_pair_test.cc b/gd/security/test/pairing_handler_le_pair_test.cc index 81a6b6423..1bed89c3c 100644 --- a/gd/security/test/pairing_handler_le_pair_test.cc +++ b/gd/security/test/pairing_handler_le_pair_test.cc @@ -34,6 +34,8 @@ using testing::InvokeWithoutArgs; using testing::Matcher; using testing::SaveArg; +using bluetooth::hci::Address; +using bluetooth::hci::AddressType; using bluetooth::hci::CommandCompleteView; using bluetooth::hci::CommandStatusView; using bluetooth::hci::EncryptionChangeBuilder; @@ -82,10 +84,10 @@ namespace security { namespace { Address ADDRESS_MASTER{{0x26, 0x64, 0x76, 0x86, 0xab, 0xba}}; -uint8_t ADDRESS_TYPE_MASTER = 0x01; +AddressType ADDRESS_TYPE_MASTER = AddressType::RANDOM_DEVICE_ADDRESS; Address ADDRESS_SLAVE{{0x33, 0x58, 0x24, 0x76, 0x11, 0x89}}; -uint8_t ADDRESS_TYPE_SLAVE = 0x01; +AddressType ADDRESS_TYPE_SLAVE = AddressType::RANDOM_DEVICE_ADDRESS; std::optional pairing_result_master; std::optional pairing_result_slave; @@ -164,8 +166,7 @@ class PairingHandlerPairTest : public testing::Test { master_setup = { .my_role = hci::Role::MASTER, - .my_connection_address = ADDRESS_MASTER, - .my_connection_address_type = ADDRESS_TYPE_MASTER, + .my_connection_address = {ADDRESS_MASTER, ADDRESS_TYPE_MASTER}, .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, @@ -176,8 +177,7 @@ class PairingHandlerPairTest : public testing::Test { .remotely_initiated = false, .connection_handle = CONN_HANDLE_MASTER, - .remote_connection_address = ADDRESS_SLAVE, - .remote_connection_address_type = ADDRESS_TYPE_SLAVE, + .remote_connection_address = {ADDRESS_SLAVE, ADDRESS_TYPE_SLAVE}, .ui_handler = &master_ui_handler, .le_security_interface = &master_le_security_mock, .proper_l2cap_interface = up_buffer_a_.get(), @@ -188,8 +188,7 @@ class PairingHandlerPairTest : public testing::Test { slave_setup = { .my_role = hci::Role::SLAVE, - .my_connection_address = ADDRESS_SLAVE, - .my_connection_address_type = ADDRESS_TYPE_SLAVE, + .my_connection_address = {ADDRESS_SLAVE, ADDRESS_TYPE_SLAVE}, .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, .auth_req = AuthReqMaskBondingFlag | AuthReqMaskMitm | AuthReqMaskSc, @@ -198,8 +197,7 @@ class PairingHandlerPairTest : public testing::Test { .responder_key_distribution = KeyMaskId | KeyMaskSign}, .remotely_initiated = true, .connection_handle = CONN_HANDLE_SLAVE, - .remote_connection_address = ADDRESS_MASTER, - .remote_connection_address_type = ADDRESS_TYPE_MASTER, + .remote_connection_address = {ADDRESS_MASTER, ADDRESS_TYPE_MASTER}, .ui_handler = &slave_ui_handler, .le_security_interface = &slave_le_security_mock, .proper_l2cap_interface = up_buffer_b_.get(), @@ -337,8 +335,7 @@ TEST_F(PairingHandlerPairTest, test_secure_connections_just_works) { TEST_F(PairingHandlerPairTest, test_secure_connections_just_works_slave_initiated) { master_setup = { .my_role = hci::Role::MASTER, - .my_connection_address = ADDRESS_MASTER, - .my_connection_address_type = ADDRESS_TYPE_MASTER, + .my_connection_address = {ADDRESS_MASTER, ADDRESS_TYPE_MASTER}, .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, .auth_req = AuthReqMaskBondingFlag | AuthReqMaskMitm | AuthReqMaskSc, @@ -347,8 +344,7 @@ TEST_F(PairingHandlerPairTest, test_secure_connections_just_works_slave_initiate .responder_key_distribution = KeyMaskId | KeyMaskSign}, .remotely_initiated = true, .connection_handle = CONN_HANDLE_MASTER, - .remote_connection_address = ADDRESS_SLAVE, - .remote_connection_address_type = ADDRESS_TYPE_SLAVE, + .remote_connection_address = {ADDRESS_SLAVE, ADDRESS_TYPE_SLAVE}, .ui_handler = &master_ui_handler, .le_security_interface = &master_le_security_mock, .proper_l2cap_interface = up_buffer_a_.get(), @@ -358,8 +354,7 @@ TEST_F(PairingHandlerPairTest, test_secure_connections_just_works_slave_initiate slave_setup = { .my_role = hci::Role::SLAVE, - .my_connection_address = ADDRESS_SLAVE, - .my_connection_address_type = ADDRESS_TYPE_SLAVE, + .my_connection_address = {ADDRESS_SLAVE, ADDRESS_TYPE_SLAVE}, .myPairingCapabilities = {.io_capability = IoCapability::NO_INPUT_NO_OUTPUT, .oob_data_flag = OobDataFlag::NOT_PRESENT, .auth_req = AuthReqMaskBondingFlag | AuthReqMaskMitm | AuthReqMaskSc, @@ -368,8 +363,7 @@ TEST_F(PairingHandlerPairTest, test_secure_connections_just_works_slave_initiate .responder_key_distribution = KeyMaskId | KeyMaskSign}, .remotely_initiated = false, .connection_handle = CONN_HANDLE_SLAVE, - .remote_connection_address = ADDRESS_MASTER, - .remote_connection_address_type = ADDRESS_TYPE_MASTER, + .remote_connection_address = {ADDRESS_MASTER, ADDRESS_TYPE_MASTER}, .ui_handler = &slave_ui_handler, .le_security_interface = &slave_le_security_mock, .proper_l2cap_interface = up_buffer_b_.get(), diff --git a/gd/security/ui.h b/gd/security/ui.h index f41137d6b..83041def1 100644 --- a/gd/security/ui.h +++ b/gd/security/ui.h @@ -18,7 +18,7 @@ #pragma once -#include "hci/address.h" +#include "hci/address_with_type.h" // Through this interface we talk to the user, asking for confirmations/acceptance. class UI { @@ -26,11 +26,11 @@ class UI { virtual ~UI(){}; /* Remote device tries to initiate pairing, ask user to confirm */ - virtual void DisplayPairingPrompt(const bluetooth::hci::Address& address, std::string& name) = 0; + virtual void DisplayPairingPrompt(const bluetooth::hci::AddressWithType& address, std::string& name) = 0; /* Remove the pairing prompt from DisplayPairingPrompt, i.e. remote device disconnected, or some application requested * bond with this device */ - virtual void CancelPairingPrompt(const bluetooth::hci::Address& address) = 0; + virtual void CancelPairingPrompt(const bluetooth::hci::AddressWithType& address) = 0; /* Display value for Comprision */ virtual void DisplayConfirmValue(uint32_t numeric_value) = 0; -- 2.11.0