From: Yulong Date: Mon, 26 Jun 2017 09:02:57 +0000 (-0400) Subject: component/bt: Fixed a very important bug of the SMP security module when use SMP... X-Git-Tag: v3.1-dev~508^2~1 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=574d0cf84606494f6371740312542671e02efed7;p=esp-idf component/bt: Fixed a very important bug of the SMP security module when use SMP to connected after bonding. --- diff --git a/components/bt/bluedroid/btc/core/btc_ble_storage.c b/components/bt/bluedroid/btc/core/btc_ble_storage.c index 1c885f4b5c..5e588012ea 100644 --- a/components/bt/bluedroid/btc/core/btc_ble_storage.c +++ b/components/bt/bluedroid/btc/core/btc_ble_storage.c @@ -19,6 +19,7 @@ #include "bdaddr.h" #include "btc_ble_storage.h" #include "bta_gatts_co.h" +#include "btc_util.h" #if (SMP_INCLUDED == TRUE) @@ -55,9 +56,8 @@ bt_status_t btc_in_fetch_bonded_ble_devices(int add) continue; } - if (!(btc_in_fetch_bonded_ble_device(name, add, &bonded_devices)) ) { + if (btc_in_fetch_bonded_ble_device(name, add, &bonded_devices) != BT_STATUS_SUCCESS) { LOG_DEBUG("Remote device:%s, no link key or ble key found", name); - return BT_STATUS_FAIL; } } @@ -78,6 +78,9 @@ void btc_save_ble_bonding_keys(void) bt_bdaddr_t bd_addr; bdcpy(bd_addr.address, pairing_cb.bd_addr); + bdstr_t bdstr; + bdaddr_to_string(&bd_addr, bdstr, sizeof(bdstr)); + btc_config_set_int(bdstr, "DevType", BT_DEVICE_TYPE_BLE); if (pairing_cb.ble.is_penc_key_rcvd) { btc_storage_add_ble_bonding_key(&bd_addr, (char *) &pairing_cb.ble.penc_key, @@ -131,7 +134,6 @@ static void btc_read_le_key(const uint8_t key_type, const size_t key_len, bt_bda char buffer[100]; memset(buffer, 0, sizeof(buffer)); - if (btc_storage_get_ble_bonding_key(&bd_addr, key_type, buffer, key_len) == BT_STATUS_SUCCESS) { if (add_key) { BD_ADDR bta_bd_addr; @@ -152,13 +154,12 @@ static void btc_read_le_key(const uint8_t key_type, const size_t key_len, bt_bda } } - bt_status_t btc_storage_add_ble_bonding_key(bt_bdaddr_t *remote_bd_addr, char *key, uint8_t key_type, uint8_t key_length) { - char bdstr[6] = {0}; + bdstr_t bdstr; bdaddr_to_string(remote_bd_addr, bdstr, sizeof(bdstr)); const char* name; switch (key_type) { @@ -205,7 +206,7 @@ bt_status_t btc_storage_get_ble_bonding_key(bt_bdaddr_t *remote_bd_addr, char *key_value, int key_length) { - char bdstr[6] = {0}; + bdstr_t bdstr; bdaddr_to_string(remote_bd_addr, bdstr, sizeof(bdstr)); const char* name; switch (key_type) { @@ -235,6 +236,35 @@ bt_status_t btc_storage_get_ble_bonding_key(bt_bdaddr_t *remote_bd_addr, } +bool btc_storage_compare_address_key_value(uint8_t key_type, void *key_value, int key_length) +{ + const char *key_type_str; + switch (key_type) { + case BTM_LE_KEY_PENC: + key_type_str = "LE_KEY_PENC"; + break; + case BTM_LE_KEY_PID: + key_type_str = "LE_KEY_PID"; + break; + case BTM_LE_KEY_PCSRK: + key_type_str = "LE_KEY_PCSRK"; + break; + case BTM_LE_KEY_LENC: + key_type_str = "LE_KEY_LENC"; + break; + case BTM_LE_KEY_LCSRK: + key_type_str = "LE_KEY_LCSRK"; + break; + case BTM_LE_KEY_LID: + key_type_str = "LE_KEY_LID"; + default: + return false; + } + + return btc_compare_address_key_value(key_type_str, key_value, key_length); +} + + /******************************************************************************* ** ** Function btc_storage_remove_ble_bonding_keys @@ -247,7 +277,7 @@ bt_status_t btc_storage_get_ble_bonding_key(bt_bdaddr_t *remote_bd_addr, *******************************************************************************/ bt_status_t btc_storage_remove_ble_bonding_keys(bt_bdaddr_t *remote_bd_addr) { - char bdstr[6] = {0}; + bdstr_t bdstr; bdaddr_to_string(remote_bd_addr, bdstr, sizeof(bdstr)); BTIF_TRACE_DEBUG(" %s in bd addr:%s",__FUNCTION__, bdstr); int ret = 1; @@ -382,7 +412,7 @@ bt_status_t btc_in_fetch_bonded_ble_device(const char *remote_bd_addr, int add, bool device_added = false; bool key_found = false; - if (!btc_config_get_int(remote_bd_addr, "AddrType", &device_type)) { + if (!btc_config_get_int(remote_bd_addr, "DevType", &device_type)) { LOG_ERROR("%s, device_type = %x", __func__, device_type); return BT_STATUS_FAIL; } @@ -423,7 +453,7 @@ bt_status_t btc_in_fetch_bonded_ble_device(const char *remote_bd_addr, int add, bt_status_t btc_storage_set_remote_addr_type(bt_bdaddr_t *remote_bd_addr, uint8_t addr_type) { - char bdstr[6] = {0}; + bdstr_t bdstr; bdaddr_to_string(remote_bd_addr, bdstr, sizeof(bt_bdaddr_t)); int ret = btc_config_set_int(bdstr, "AddrType", (int)addr_type); btc_config_save(); @@ -443,7 +473,7 @@ bt_status_t btc_storage_set_remote_addr_type(bt_bdaddr_t *remote_bd_addr, bt_status_t btc_storage_get_remote_addr_type(bt_bdaddr_t *remote_bd_addr, int*addr_type) { - char bdstr[6] = {0}; + bdstr_t bdstr; bdaddr_to_string(remote_bd_addr, bdstr, sizeof(bdstr)); int ret = btc_config_get_int(bdstr, "AddrType", addr_type); return ret ? BT_STATUS_SUCCESS : BT_STATUS_FAIL; diff --git a/components/bt/bluedroid/btc/core/btc_config.c b/components/bt/bluedroid/btc/core/btc_config.c index bd0bd03187..e2c30291b5 100644 --- a/components/bt/bluedroid/btc/core/btc_config.c +++ b/components/bt/bluedroid/btc/core/btc_config.c @@ -31,7 +31,7 @@ static const char *CONFIG_FILE_PATH = "bt_config.conf"; static const period_ms_t CONFIG_SETTLE_PERIOD_MS = 3000; -static void timer_config_save(void *data); +static void btc_key_value_to_string(uint8_t *key_vaule, char *value_str, int key_length); // TODO(zachoverflow): Move these two functions out, because they are too specific for this file // {grumpy-cat/no, monty-python/you-make-me-sad} @@ -77,7 +77,36 @@ bool btc_get_address_type(const BD_ADDR bd_addr, int *p_addr_type) static pthread_mutex_t lock; // protects operations on |config|. static config_t *config; -static osi_alarm_t *alarm_timer; + +bool btc_compare_address_key_value(char *key_type, void *key_value, int key_length) +{ + assert(key_value != NULL); + bool status = false; + char value_str[100] = {0}; + if(key_length > sizeof(value_str)/2) { + return false; + } + btc_key_value_to_string((uint8_t *)key_value, value_str, key_length); + pthread_mutex_lock(&lock); + status = config_has_key_in_section(config, key_type, value_str); + pthread_mutex_unlock(&lock); + return status; +} + +static void btc_key_value_to_string(uint8_t *key_vaule, char *value_str, int key_length) +{ + const char *lookup = "0123456789abcdef"; + + assert(key_vaule != NULL); + assert(value_str != NULL); + + for (size_t i = 0; i < key_length; ++i) { + value_str[(i * 2) + 0] = lookup[(key_vaule[i] >> 4) & 0x0F]; + value_str[(i * 2) + 1] = lookup[key_vaule[i] & 0x0F]; + } + + return; +} // Module lifecycle functions @@ -93,27 +122,15 @@ bool btc_config_init(void) goto error; } } - if (config_save(config, CONFIG_FILE_PATH)) { // unlink(LEGACY_CONFIG_FILE_PATH); } - // TODO(sharvil): use a non-wake alarm for this once we have - // API support for it. There's no need to wake the system to - // write back to disk. - alarm_timer = osi_alarm_new("btc_config", timer_config_save, NULL, CONFIG_SETTLE_PERIOD_MS); - if (!alarm_timer) { - LOG_ERROR("%s unable to create alarm.\n", __func__); - goto error; - } - return true; error:; - osi_alarm_free(alarm_timer); config_free(config); pthread_mutex_destroy(&lock); - alarm_timer = NULL; config = NULL; LOG_ERROR("%s failed\n", __func__); return false; @@ -129,10 +146,8 @@ bool btc_config_clean_up(void) { btc_config_flush(); - osi_alarm_free(alarm_timer); config_free(config); pthread_mutex_destroy(&lock); - alarm_timer = NULL; config = NULL; return true; } @@ -352,49 +367,7 @@ bool btc_config_remove(const char *section, const char *key) void btc_config_save(void) { - assert(alarm_timer != NULL); - assert(config != NULL); - - osi_alarm_set(alarm_timer, CONFIG_SETTLE_PERIOD_MS); -} - -void btc_config_flush(void) -{ - assert(config != NULL); - assert(alarm_timer != NULL); - osi_alarm_cancel(alarm_timer); - - pthread_mutex_lock(&lock); - config_save(config, CONFIG_FILE_PATH); - pthread_mutex_unlock(&lock); -} - -int btc_config_clear(void) -{ - assert(config != NULL); - assert(alarm_timer != NULL); - - osi_alarm_cancel(alarm_timer); - - pthread_mutex_lock(&lock); - config_free(config); - - config = config_new_empty(); - if (config == NULL) { - pthread_mutex_unlock(&lock); - return false; - } - - int ret = config_save(config, CONFIG_FILE_PATH); - pthread_mutex_unlock(&lock); - return ret; -} - -static void timer_config_save(UNUSED_ATTR void *data) -{ - assert(config != NULL); - assert(alarm_timer != NULL); - + assert(config != NULL); // Garbage collection process: the config file accumulates // cached information about remote devices during regular // inquiry scans. We remove some of these junk entries @@ -433,7 +406,33 @@ static void timer_config_save(UNUSED_ATTR void *data) while (num_keys > 0) { config_remove_section(config, keys[--num_keys]); } + config_save(config, CONFIG_FILE_PATH); + pthread_mutex_unlock(&lock); +} +void btc_config_flush(void) +{ + assert(config != NULL); + pthread_mutex_lock(&lock); config_save(config, CONFIG_FILE_PATH); pthread_mutex_unlock(&lock); } + +int btc_config_clear(void) +{ + assert(config != NULL); + + + pthread_mutex_lock(&lock); + config_free(config); + + config = config_new_empty(); + if (config == NULL) { + pthread_mutex_unlock(&lock); + return false; + } + int ret = config_save(config, CONFIG_FILE_PATH); + pthread_mutex_unlock(&lock); + return ret; +} + diff --git a/components/bt/bluedroid/btc/core/btc_dm.c b/components/bt/bluedroid/btc/core/btc_dm.c index 1f98d17da9..2e0d1675b3 100644 --- a/components/bt/bluedroid/btc/core/btc_dm.c +++ b/components/bt/bluedroid/btc/core/btc_dm.c @@ -125,10 +125,21 @@ static void btc_dm_ble_auth_cmpl_evt (tBTA_DM_AUTH_CMPL *p_auth_cmpl) bt_bdaddr_t bdaddr; bdcpy(bdaddr.address, p_auth_cmpl->bd_addr); bdcpy(pairing_cb.bd_addr, p_auth_cmpl->bd_addr); + LOG_DEBUG ("%s, - p_auth_cmpl->bd_addr: %08x%04x", __func__, + (p_auth_cmpl->bd_addr[0] << 24) + (p_auth_cmpl->bd_addr[1] << 16) + (p_auth_cmpl->bd_addr[2] << 8) + p_auth_cmpl->bd_addr[3], + (p_auth_cmpl->bd_addr[4] << 8) + p_auth_cmpl->bd_addr[5]); + LOG_DEBUG ("%s, - pairing_cb.bd_addr: %08x%04x", __func__, + (pairing_cb.bd_addr[0] << 24) + (pairing_cb.bd_addr[1] << 16) + (pairing_cb.bd_addr[2] << 8) + pairing_cb.bd_addr[3], + (pairing_cb.bd_addr[4] << 8) + pairing_cb.bd_addr[5]); if (btc_storage_get_remote_addr_type(&bdaddr, &addr_type) != BT_STATUS_SUCCESS) { btc_storage_set_remote_addr_type(&bdaddr, p_auth_cmpl->addr_type); } - + /* check the irk has been save in the flash or not, if the irk has already save, means that the peer device has bonding + before. */ + if(pairing_cb.ble.is_pid_key_rcvd) { + btc_storage_compare_address_key_value(BTM_LE_KEY_PID, + (void *)&pairing_cb.ble.pid_key, sizeof(tBTM_LE_PID_KEYS)); + } btc_save_ble_bonding_keys(); } else { /*Map the HCI fail reason to bt status */ @@ -312,6 +323,8 @@ void btc_dm_sec_cb_handler(btc_msg_t *msg) #if (SMP_INCLUDED == TRUE) //load the ble local key whitch has been store in the flash btc_dm_load_ble_local_keys(); + //load the bonding device to the btm layer + btc_storage_load_bonded_ble_devices(); #endif ///SMP_INCLUDED == TRUE btc_enable_bluetooth_evt(p_data->enable.status); break; diff --git a/components/bt/bluedroid/btc/core/btc_main.c b/components/bt/bluedroid/btc/core/btc_main.c index d251782e40..fc1f56829b 100644 --- a/components/bt/bluedroid/btc/core/btc_main.c +++ b/components/bt/bluedroid/btc/core/btc_main.c @@ -54,8 +54,8 @@ static void btc_init_bluetooth(void) { osi_alarm_create_mux(); osi_alarm_init(); - btc_config_init(); bte_main_boot_entry(btc_init_callback); + btc_config_init(); } diff --git a/components/bt/bluedroid/btc/include/btc_ble_storage.h b/components/bt/bluedroid/btc/include/btc_ble_storage.h index 2884d6acf3..cbb6229ca1 100644 --- a/components/bt/bluedroid/btc/include/btc_ble_storage.h +++ b/components/bt/bluedroid/btc/include/btc_ble_storage.h @@ -89,6 +89,9 @@ bt_status_t btc_storage_add_ble_bonding_key( bt_bdaddr_t *remote_bd_addr, uint8_t key_type, uint8_t key_length); +bool btc_compare_le_key_value(const uint8_t key_type, const size_t key_len, const tBTA_LE_KEY_VALUE *key_vaule, + bt_bdaddr_t bd_addr); + void btc_save_ble_bonding_keys(void); bt_status_t btc_in_fetch_bonded_ble_device(const char *remote_bd_addr, int add, @@ -99,6 +102,8 @@ bt_status_t btc_storage_get_ble_bonding_key(bt_bdaddr_t *remote_bd_addr, char *key_value, int key_length); +bool btc_storage_compare_address_key_value(uint8_t key_type, void *key_value, int key_length); + bt_status_t btc_storage_add_ble_local_key(char *key, uint8_t key_type, uint8_t key_length); diff --git a/components/bt/bluedroid/btc/include/btc_config.h b/components/bt/bluedroid/btc/include/btc_config.h index 1472cc83c4..e0d6d6ec08 100644 --- a/components/bt/bluedroid/btc/include/btc_config.h +++ b/components/bt/bluedroid/btc/include/btc_config.h @@ -49,6 +49,7 @@ int btc_config_clear(void); // TODO(zachoverflow): Eww...we need to move these out. These are peer specific, not config general. bool btc_get_address_type(const BD_ADDR bd_addr, int *p_addr_type); +bool btc_compare_address_key_value(char *key_type, void *key_value, int key_length); bool btc_get_device_type(const BD_ADDR bd_addr, int *p_device_type); #endif diff --git a/components/bt/bluedroid/include/bt_target.h b/components/bt/bluedroid/include/bt_target.h index a7ed5bd3b8..a426d69fe6 100644 --- a/components/bt/bluedroid/include/bt_target.h +++ b/components/bt/bluedroid/include/bt_target.h @@ -577,7 +577,11 @@ /* The number of security records for peer devices. 100 AS Default*/ #ifndef BTM_SEC_MAX_DEVICE_RECORDS -#define BTM_SEC_MAX_DEVICE_RECORDS 8 // 100 +#if SMP_INCLUDED == TRUE +#define BTM_SEC_MAX_DEVICE_RECORDS 15 // 100 +#else +#define BTM_SEC_MAX_DEVICE_RECORDS 8 +#endif /* SMP_INCLUDED == TRUE */ #endif /* The number of security records for services. 32 AS Default*/ diff --git a/components/bt/bluedroid/osi/config.c b/components/bt/bluedroid/osi/config.c index 7c876b8430..38281ed9da 100644 --- a/components/bt/bluedroid/osi/config.c +++ b/components/bt/bluedroid/osi/config.c @@ -28,7 +28,7 @@ #include "list.h" #include "bt_trace.h" -#define CONFIG_FILE_MAX_SIZE (1024) +#define CONFIG_FILE_MAX_SIZE (2048) #define CONFIG_KEY "bt_cfg_key" typedef struct { char *key; @@ -128,6 +128,26 @@ bool config_has_key(const config_t *config, const char *section, const char *key return (entry_find(config, section, key) != NULL); } +bool config_has_key_in_section(config_t *config, char *key, char *key_value) +{ + LOG_DEBUG("key = %s, value = %s", key, key_value); + for (const list_node_t *node = list_begin(config->sections); node != list_end(config->sections); node = list_next(node)) { + const section_t *section = (const section_t *)list_node(node); + + for (const list_node_t *node = list_begin(section->entries); node != list_end(section->entries); node = list_next(node)) { + entry_t *entry = list_node(node); + LOG_DEBUG("entry->key = %s, entry->value = %s", entry->key, entry->value); + if (!strcmp(entry->key, key) && !strcmp(entry->value, key_value)) { + LOG_DEBUG("%s, the irk aready in the flash.", __func__); + section_free((void *)section); + return true; + } + } + } + + return false; +} + int config_get_int(const config_t *config, const char *section, const char *key, int def_value) { assert(config != NULL); @@ -303,8 +323,8 @@ bool config_save(const config_t *config, const char *filename) int w_cnt, w_cnt_total = 0; for (const list_node_t *node = list_begin(config->sections); node != list_end(config->sections); node = list_next(node)) { const section_t *section = (const section_t *)list_node(node); - LOG_DEBUG("section name: %s\n", section->name); w_cnt = snprintf(line, 1024, "[%s]\n", section->name); + LOG_DEBUG("section name: %s, w_cnt + w_cnt_total = %d\n", section->name, w_cnt + w_cnt_total); if (w_cnt + w_cnt_total < CONFIG_FILE_MAX_SIZE) { memcpy(buf + w_cnt_total, line, w_cnt); w_cnt_total += w_cnt; @@ -316,6 +336,7 @@ bool config_save(const config_t *config, const char *filename) const entry_t *entry = (const entry_t *)list_node(enode); LOG_DEBUG("(key, val): (%s, %s)\n", entry->key, entry->value); w_cnt = snprintf(line, 1024, "%s = %s\n", entry->key, entry->value); + LOG_DEBUG("%s, w_cnt + w_cnt_total = %d", __func__, w_cnt + w_cnt_total); if (w_cnt + w_cnt_total < CONFIG_FILE_MAX_SIZE) { memcpy(buf + w_cnt_total, line, w_cnt); w_cnt_total += w_cnt; diff --git a/components/bt/bluedroid/osi/include/config.h b/components/bt/bluedroid/osi/include/config.h index 4f0e2cd8ae..41f5ddb18a 100644 --- a/components/bt/bluedroid/osi/include/config.h +++ b/components/bt/bluedroid/osi/include/config.h @@ -66,6 +66,10 @@ bool config_has_section(const config_t *config, const char *section); // Returns false otherwise. |config|, |section|, and |key| must not be NULL. bool config_has_key(const config_t *config, const char *section, const char *key); +// Returns true if the config file has a key named |key| and the key_value. +// Returns false otherwise. |config|, |key|, and |key_value| must not be NULL. +bool config_has_key_in_section(config_t *config, char *key, char *key_value); + // Returns the integral value for a given |key| in |section|. If |section| // or |key| do not exist, or the value cannot be fully converted to an integer, // this function returns |def_value|. |config|, |section|, and |key| must not