From 8f848255dea47710275f1eac7c4e635b1f5faa0b Mon Sep 17 00:00:00 2001 From: Rahul Sabnis <rahulsabnis@google.com> Date: Tue, 9 Aug 2022 19:03:11 +0000 Subject: [PATCH] Reconfigure Address policy on last bond removed Bug: 237572866 Bug: 195410559 Test: system/gd/cert/run --clean --sl4a OobPairingSl4aTest Test: system/gd/cert/run --clean --sl4a_sl4a OobPairingTest Tag: #refactor Ignore-AOSP-First: Security fix Merged-In: Id1c22953c3921810d708e4aee7a8cfc0c4f97718 Change-Id: Id1c22953c3921810d708e4aee7a8cfc0c4f97718 (cherry picked from commit 66bfae6d32c9863431b12a5dbb5817e0efe6f0a8) (cherry picked from commit f4aa35adf8ed2e06a3d1273c18d3a3561644e0a4) Merged-In: Id1c22953c3921810d708e4aee7a8cfc0c4f97718 --- system/bta/dm/bta_dm_act.cc | 21 ++++++++++++++------- system/bta/dm/bta_dm_api.cc | 14 ++++++++++++++ system/bta/dm/bta_dm_int.h | 2 ++ system/bta/include/bta_api.h | 11 +++++++++++ system/btif/src/btif_storage.cc | 12 +++++++++++- system/gd/hci/le_address_manager.cc | 9 +++++++++ system/main/shim/btm_api.cc | 6 ++++++ system/main/shim/btm_api.h | 9 +++++++++ system/stack/btm/btm_ble.cc | 13 ++++++++++++- system/stack/btm/btm_ble_gap.cc | 2 +- system/test/mock/mock_bta_dm_act.h | 9 +++++++++ system/test/mock/mock_main_shim_btm_api.cc | 5 +++++ 12 files changed, 103 insertions(+), 10 deletions(-) diff --git a/system/bta/dm/bta_dm_act.cc b/system/bta/dm/bta_dm_act.cc index 33837430f54..55f7871c9e3 100644 --- a/system/bta/dm/bta_dm_act.cc +++ b/system/bta/dm/bta_dm_act.cc @@ -652,13 +652,6 @@ void bta_dm_remove_device(const RawAddress& bd_addr) { if (!other_address_connected && !other_address.IsEmpty()) { bta_dm_process_remove_device(other_address); } - - /* Check the length of the paired devices, and if 0 then reset IRK */ - auto paired_devices = btif_config_get_paired_devices(); - if (paired_devices.empty()) { - LOG_INFO("Last paired device removed, resetting IRK"); - btm_ble_reset_id(); - } } /******************************************************************************* @@ -4021,6 +4014,20 @@ void bta_dm_clear_event_filter(void) { bluetooth::shim::BTM_ClearEventFilter(); } +/******************************************************************************* + * + * Function bta_dm_ble_reset_id + * + * Description Reset the local adapter BLE keys. + * + * Parameters: + * + ******************************************************************************/ +void bta_dm_ble_reset_id(void) { + VLOG(1) << "bta_dm_ble_reset_id in bta_dm_act"; + bluetooth::shim::BTM_BleResetId(); +} + /******************************************************************************* * * Function bta_dm_gattc_callback diff --git a/system/bta/dm/bta_dm_api.cc b/system/bta/dm/bta_dm_api.cc index fa15a7e2217..eded208dc74 100644 --- a/system/bta/dm/bta_dm_api.cc +++ b/system/bta/dm/bta_dm_api.cc @@ -675,3 +675,17 @@ void BTA_DmClearEventFilter(void) { APPL_TRACE_API("BTA_DmClearEventFilter"); do_in_main_thread(FROM_HERE, base::Bind(bta_dm_clear_event_filter)); } + +/******************************************************************************* + * + * Function BTA_DmBleResetId + * + * Description This function resets the ble keys such as IRK + * + * Returns void + * + ******************************************************************************/ +void BTA_DmBleResetId(void) { + APPL_TRACE_API("BTA_DmBleResetId"); + do_in_main_thread(FROM_HERE, base::Bind(bta_dm_ble_reset_id)); +} diff --git a/system/bta/dm/bta_dm_int.h b/system/bta/dm/bta_dm_int.h index eaf2fa3882a..d952dccea26 100644 --- a/system/bta/dm/bta_dm_int.h +++ b/system/bta/dm/bta_dm_int.h @@ -543,6 +543,8 @@ extern tBTA_DM_PEER_DEVICE* bta_dm_find_peer_device( extern void bta_dm_clear_event_filter(void); +extern void bta_dm_ble_reset_id(void); + uint8_t bta_dm_search_get_state(); void bta_dm_search_set_state(uint8_t state); diff --git a/system/bta/include/bta_api.h b/system/bta/include/bta_api.h index 447823a6a85..59d6d9805eb 100644 --- a/system/bta/include/bta_api.h +++ b/system/bta/include/bta_api.h @@ -1203,4 +1203,15 @@ extern void BTA_VendorInit(void); ******************************************************************************/ extern void BTA_DmClearEventFilter(void); +/******************************************************************************* + * + * Function BTA_DmBleResetId + * + * Description This function resets the ble keys such as IRK + * + * Returns void + * + ******************************************************************************/ +extern void BTA_DmBleResetId(void); + #endif /* BTA_API_H */ diff --git a/system/btif/src/btif_storage.cc b/system/btif/src/btif_storage.cc index 865bcf9508e..bd7e89c8b5e 100644 --- a/system/btif/src/btif_storage.cc +++ b/system/btif/src/btif_storage.cc @@ -896,6 +896,13 @@ bt_status_t btif_storage_remove_bonded_device( /* write bonded info immediately */ btif_config_flush(); + + /* Check the length of the paired devices, and if 0 then reset IRK */ + auto paired_devices = btif_config_get_paired_devices(); + if (paired_devices.empty()) { + LOG_INFO("Last paired device removed, resetting IRK"); + BTA_DmBleResetId(); + } return ret ? BT_STATUS_SUCCESS : BT_STATUS_FAIL; } @@ -1279,7 +1286,10 @@ bt_status_t btif_storage_add_ble_local_key(const Octet16& key, return BT_STATUS_FAIL; } int ret = btif_config_set_bin("Adapter", name, key.data(), key.size()); - btif_config_save(); + // Had to change this to flush to get it to work on test. + // Seems to work in the real world on a phone... but not sure why there's a + // race in test. Investigate b/239828132 + btif_config_flush(); return ret ? BT_STATUS_SUCCESS : BT_STATUS_FAIL; } diff --git a/system/gd/hci/le_address_manager.cc b/system/gd/hci/le_address_manager.cc index 28f51004a78..3271d16a396 100644 --- a/system/gd/hci/le_address_manager.cc +++ b/system/gd/hci/le_address_manager.cc @@ -51,6 +51,15 @@ void LeAddressManager::SetPrivacyPolicyForInitiatorAddress( bool supports_ble_privacy, std::chrono::milliseconds minimum_rotation_time, std::chrono::milliseconds maximum_rotation_time) { + // Need to update some parameteres like IRK + if (supports_ble_privacy && address_policy_ != AddressPolicy::POLICY_NOT_SET) { + LOG_INFO("Updating rotation parameters."); + rotation_irk_ = rotation_irk; + minimum_rotation_time_ = minimum_rotation_time; + maximum_rotation_time_ = maximum_rotation_time; + set_random_address(); + return; + } ASSERT(address_policy_ == AddressPolicy::POLICY_NOT_SET); ASSERT(address_policy != AddressPolicy::POLICY_NOT_SET); ASSERT_LOG(registered_clients_.empty(), "Policy must be set before clients are registered."); diff --git a/system/main/shim/btm_api.cc b/system/main/shim/btm_api.cc index 6e799313f3c..8b761c0138e 100644 --- a/system/main/shim/btm_api.cc +++ b/system/main/shim/btm_api.cc @@ -37,6 +37,7 @@ #include "main/shim/shim.h" #include "main/shim/stack.h" #include "osi/include/allocator.h" +#include "stack/btm/btm_ble_int.h" #include "stack/btm/btm_int_types.h" #include "stack/include/bt_hdr.h" #include "stack/include/bt_octets.h" @@ -1334,3 +1335,8 @@ tBTM_STATUS bluetooth::shim::BTM_ClearEventFilter() { controller_get_interface()->clear_event_filter(); return BTM_SUCCESS; } + +tBTM_STATUS bluetooth::shim::BTM_BleResetId() { + btm_ble_reset_id(); + return BTM_SUCCESS; +} diff --git a/system/main/shim/btm_api.h b/system/main/shim/btm_api.h index c1dd4be659a..1e10849d1f9 100644 --- a/system/main/shim/btm_api.h +++ b/system/main/shim/btm_api.h @@ -1821,6 +1821,15 @@ tBTM_STATUS BTM_BleGetEnergyInfo(tBTM_BLE_ENERGY_INFO_CBACK* p_ener_cback); ******************************************************************************/ tBTM_STATUS BTM_ClearEventFilter(void); +/******************************************************************************* + * + * Function BTM_BleResetId + * + * Description Resets the local BLE keys + * + *******************************************************************************/ +tBTM_STATUS BTM_BleResetId(void); + /** * Send remote name request to GD shim Name module */ diff --git a/system/stack/btm/btm_ble.cc b/system/stack/btm/btm_ble.cc index 9d313b3ea84..3a18f258da9 100644 --- a/system/stack/btm/btm_ble.cc +++ b/system/stack/btm/btm_ble.cc @@ -32,6 +32,7 @@ #include "main/shim/l2c_api.h" #include "main/shim/shim.h" #include "osi/include/allocator.h" +#include "osi/include/properties.h" #include "stack/btm/btm_dev.h" #include "stack/btm/btm_int_types.h" #include "stack/btm/security_device_record.h" @@ -56,6 +57,11 @@ extern bool btm_ble_init_pseudo_addr(tBTM_SEC_DEV_REC* p_dev_rec, extern void gatt_notify_phy_updated(tGATT_STATUS status, uint16_t handle, uint8_t tx_phy, uint8_t rx_phy); + +#ifndef PROPERTY_BLE_PRIVACY_ENABLED +#define PROPERTY_BLE_PRIVACY_ENABLED "bluetooth.core.gap.le.privacy.enabled" +#endif + /******************************************************************************/ /* External Function to be called by other modules */ /******************************************************************************/ @@ -82,7 +88,7 @@ void BTM_SecAddBleDevice(const RawAddress& bd_addr, tBT_DEVICE_TYPE dev_type, p_dev_rec->conn_params.peripheral_latency = BTM_BLE_CONN_PARAM_UNDEF; LOG_DEBUG("Device added, handle=0x%x, p_dev_rec=%p, bd_addr=%s", - p_dev_rec->ble_hci_handle, p_dev_rec, bd_addr.ToString().c_str()); + p_dev_rec->ble_hci_handle, p_dev_rec, PRIVATE_ADDRESS(bd_addr)); } memset(p_dev_rec->sec_bd_name, 0, sizeof(tBTM_BD_NAME)); @@ -2049,6 +2055,11 @@ static void btm_ble_reset_id_impl(const Octet16& rand1, const Octet16& rand2) { /* proceed generate ER */ btm_cb.devcb.ble_encryption_key_value = rand2; btm_notify_new_key(BTM_BLE_KEY_TYPE_ER); + + /* if privacy is enabled, update the irk and RPA in the LE address manager */ + if (btm_cb.ble_ctr_cb.privacy_mode != BTM_PRIVACY_NONE) { + BTM_BleConfigPrivacy(true); + } } struct reset_id_data { diff --git a/system/stack/btm/btm_ble_gap.cc b/system/stack/btm/btm_ble_gap.cc index 76f6c6f62ca..c2fda917657 100644 --- a/system/stack/btm/btm_ble_gap.cc +++ b/system/stack/btm/btm_ble_gap.cc @@ -805,7 +805,7 @@ bool BTM_BleConfigPrivacy(bool privacy_mode) { GAP_BleAttrDBUpdate(GATT_UUID_GAP_CENTRAL_ADDR_RESOL, &gap_ble_attr_value); - bluetooth::shim::ACL_ConfigureLePrivacy(privacy_mode); + bluetooth::shim::ACL_ConfigureLePrivacy(privacy_mode); return true; } diff --git a/system/test/mock/mock_bta_dm_act.h b/system/test/mock/mock_bta_dm_act.h index 5a566dcb7de..af586006e61 100644 --- a/system/test/mock/mock_bta_dm_act.h +++ b/system/test/mock/mock_bta_dm_act.h @@ -264,6 +264,15 @@ struct bta_dm_clear_event_filter { }; extern struct bta_dm_clear_event_filter bta_dm_clear_event_filter; +// Name: bta_dm_ble_reset_id +// Params: None +// Return: void +struct bta_dm_ble_reset_id { + std::function<void()> body{[]() {}}; + void operator()() { body(); }; +}; +extern struct bta_dm_ble_reset_id bta_dm_ble_reset_id; + // Name: bta_dm_ble_passkey_reply // Params: const RawAddress& bd_addr, bool accept, uint32_t passkey // Return: void diff --git a/system/test/mock/mock_main_shim_btm_api.cc b/system/test/mock/mock_main_shim_btm_api.cc index d799c2027ba..5b21a7618cc 100644 --- a/system/test/mock/mock_main_shim_btm_api.cc +++ b/system/test/mock/mock_main_shim_btm_api.cc @@ -429,3 +429,8 @@ tBTM_STATUS bluetooth::shim::BTM_ClearEventFilter() { mock_function_count_map[__func__]++; return BTM_SUCCESS; } + +tBTM_STATUS bluetooth::shim::BTM_BleResetId() { + mock_function_count_map[__func__]++; + return BTM_SUCCESS; +} -- GitLab