From 6f21c18f5b44c674058cd9dec18469f57ff6ea0d Mon Sep 17 00:00:00 2001 From: chenjianhua Date: Tue, 14 May 2024 14:50:53 +0800 Subject: [PATCH] fix(bt/bluedroid): Fixed BLE security vulnerability when using fixed IRK --- components/bt/host/bluedroid/Kconfig.in | 11 ++++++++ .../include/common/bluedroid_user_config.h | 8 +++++- .../common/include/common/bt_target.h | 10 +++++-- .../bluedroid/stack/btm/btm_ble_privacy.c | 3 +++ .../bt/host/bluedroid/stack/btm/btm_dev.c | 27 +++++++++++++------ 5 files changed, 48 insertions(+), 11 deletions(-) diff --git a/components/bt/host/bluedroid/Kconfig.in b/components/bt/host/bluedroid/Kconfig.in index a83f1395d8..2ed377bdc9 100644 --- a/components/bt/host/bluedroid/Kconfig.in +++ b/components/bt/host/bluedroid/Kconfig.in @@ -293,6 +293,17 @@ config BT_SMP_SLAVE_CON_PARAMS_UPD_ENABLE In order to reduce the pairing time, slave actively initiates connection parameters update during pairing. +config BT_BLE_SMP_ID_RESET_ENABLE + bool "Reset device identity when all bonding records are deleted" + depends on BT_BLE_SMP_ENABLE + default n + help + There are tracking risks associated with using a fixed or static IRK. + If enabled this option, Bluedroid will assign a new randomly-generated IRK + when all pairing and bonding records are deleted. This would decrease the ability + of a previously paired peer to be used to determine whether a device + with which it previously shared an IRK is within range. + config BT_STACK_NO_LOG bool "Disable BT debug logs (minimize bin size)" depends on BT_BLUEDROID_ENABLED diff --git a/components/bt/host/bluedroid/common/include/common/bluedroid_user_config.h b/components/bt/host/bluedroid/common/include/common/bluedroid_user_config.h index ce8e80a2d5..0e4d15c425 100644 --- a/components/bt/host/bluedroid/common/include/common/bluedroid_user_config.h +++ b/components/bt/host/bluedroid/common/include/common/bluedroid_user_config.h @@ -209,7 +209,13 @@ #define UC_BT_SMP_MAX_BONDS 8 #endif -//Device Nane Maximum Length +#ifdef CONFIG_BT_BLE_SMP_ID_RESET_ENABLE +#define UC_BT_BLE_SMP_ID_RESET_ENABLE CONFIG_BT_BLE_SMP_ID_RESET_ENABLE +#else +#define UC_BT_BLE_SMP_ID_RESET_ENABLE FALSE +#endif + +//Device Name Maximum Length #ifdef CONFIG_BT_MAX_DEVICE_NAME_LEN #define UC_MAX_LOC_BD_NAME_LEN CONFIG_BT_MAX_DEVICE_NAME_LEN #else diff --git a/components/bt/host/bluedroid/common/include/common/bt_target.h b/components/bt/host/bluedroid/common/include/common/bt_target.h index b193805dfb..d8b91db21b 100644 --- a/components/bt/host/bluedroid/common/include/common/bt_target.h +++ b/components/bt/host/bluedroid/common/include/common/bt_target.h @@ -285,6 +285,12 @@ #define SMP_SLAVE_CON_PARAMS_UPD_ENABLE FALSE #endif /* UC_BT_SMP_SLAVE_CON_PARAMS_UPD_ENABLE */ +#if (UC_BT_BLE_SMP_ID_RESET_ENABLE) +#define BLE_SMP_ID_RESET_ENABLE TRUE +#else +#define BLE_SMP_ID_RESET_ENABLE FALSE +#endif + #ifdef UC_BTDM_BLE_ADV_REPORT_FLOW_CTRL_SUPP #define BLE_ADV_REPORT_FLOW_CONTROL (UC_BTDM_BLE_ADV_REPORT_FLOW_CTRL_SUPP && BLE_INCLUDED) #endif /* UC_BTDM_BLE_ADV_REPORT_FLOW_CTRL_SUPP */ @@ -564,7 +570,7 @@ #define BT_CLASSIC_BQB_INCLUDED FALSE #endif -/* This feature is used to eanble interleaved scan*/ +/* This feature is used to enable interleaved scan*/ #ifndef BTA_HOST_INTERLEAVE_SEARCH #define BTA_HOST_INTERLEAVE_SEARCH FALSE #endif @@ -1380,7 +1386,7 @@ #define GATT_CONFORMANCE_TESTING FALSE #endif -/* number of background connection device allowence, ideally to be the same as WL size +/* number of background connection device allowance, ideally to be the same as WL size */ #ifndef GATT_MAX_BG_CONN_DEV #define GATT_MAX_BG_CONN_DEV 8 /*MAX is 32*/ diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c b/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c index be4b9d1541..9d937b718e 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c @@ -1147,6 +1147,9 @@ void btm_ble_add_default_entry_to_resolving_list(void) BD_ADDR peer_addr = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0}; BT_OCTET16 peer_irk = {0x0}; + // Remove the existing entry in resolving list When resetting the device identity + btsnd_hcic_ble_rm_device_resolving_list(BLE_ADDR_PUBLIC, peer_addr); + btsnd_hcic_ble_add_device_resolving_list (BLE_ADDR_PUBLIC, peer_addr, peer_irk, btm_cb.devcb.id_keys.irk); } #endif diff --git a/components/bt/host/bluedroid/stack/btm/btm_dev.c b/components/bt/host/bluedroid/stack/btm/btm_dev.c index f9e3ed2bd4..169dcc1449 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_dev.c +++ b/components/bt/host/bluedroid/stack/btm/btm_dev.c @@ -179,20 +179,31 @@ BOOLEAN BTM_SecAddDevice (BD_ADDR bd_addr, DEV_CLASS dev_class, BD_NAME bd_name, *******************************************************************************/ BOOLEAN BTM_SecDeleteDevice (BD_ADDR bd_addr, tBT_TRANSPORT transport) { - tBTM_SEC_DEV_REC *p_dev_rec; if (BTM_IsAclConnectionUp(bd_addr, transport)) { BTM_TRACE_WARNING("%s FAILED: Cannot Delete when connection is active\n", __func__); return FALSE; } + if ((p_dev_rec = btm_find_dev(bd_addr)) != NULL) { /* Tell controller to get rid of the link key, if it has one stored */ BTM_DeleteStoredLinkKey (p_dev_rec->bd_addr, NULL); - btm_sec_free_dev(p_dev_rec, transport); + btm_sec_free_dev(p_dev_rec, transport); } +#if (BLE_SMP_ID_RESET_ENABLE == TRUE) + /* + * There are tracking risks associated with using a fixed or static IRK. + * A best-practices approach, when all pairing and bonding records are deleted, + * assign a new randomly-generated IRK. + */ + if (list_is_empty(btm_cb.p_sec_dev_rec_list)) { + btm_ble_reset_id(); + } +#endif + return TRUE; } @@ -640,7 +651,7 @@ tBTM_SEC_DEV_REC *btm_find_oldest_dev (void) tBTM_SEC_DEV_REC *p_dev_rec = NULL; tBTM_SEC_DEV_REC *p_oldest = NULL; list_node_t *p_node = NULL; - UINT32 ot = 0xFFFFFFFF; + UINT32 old_ts = 0xFFFFFFFF; /* First look for the non-paired devices for the oldest entry */ for (p_node = list_begin(btm_cb.p_sec_dev_rec_list); p_node; p_node = list_next(p_node)) { @@ -650,13 +661,13 @@ tBTM_SEC_DEV_REC *btm_find_oldest_dev (void) continue; /* Device is paired so skip it */ } - if (p_dev_rec->timestamp < ot) { + if (p_dev_rec->timestamp < old_ts) { p_oldest = p_dev_rec; - ot = p_dev_rec->timestamp; + old_ts = p_dev_rec->timestamp; } } - if (ot != 0xFFFFFFFF) { + if (old_ts != 0xFFFFFFFF) { return (p_oldest); } @@ -666,9 +677,9 @@ tBTM_SEC_DEV_REC *btm_find_oldest_dev (void) continue; } - if (p_dev_rec->timestamp < ot) { + if (p_dev_rec->timestamp < old_ts) { p_oldest = p_dev_rec; - ot = p_dev_rec->timestamp; + old_ts = p_dev_rec->timestamp; } } return (p_oldest);