From 574d0cf84606494f6371740312542671e02efed7 Mon Sep 17 00:00:00 2001 From: Yulong Date: Mon, 26 Jun 2017 05:02:57 -0400 Subject: [PATCH 1/2] component/bt: Fixed a very important bug of the SMP security module when use SMP to connected after bonding. --- .../bt/bluedroid/btc/core/btc_ble_storage.c | 50 ++++++-- components/bt/bluedroid/btc/core/btc_config.c | 119 +++++++++--------- components/bt/bluedroid/btc/core/btc_dm.c | 15 ++- components/bt/bluedroid/btc/core/btc_main.c | 2 +- .../bluedroid/btc/include/btc_ble_storage.h | 5 + .../bt/bluedroid/btc/include/btc_config.h | 1 + components/bt/bluedroid/include/bt_target.h | 6 +- components/bt/bluedroid/osi/config.c | 25 +++- components/bt/bluedroid/osi/include/config.h | 4 + 9 files changed, 152 insertions(+), 75 deletions(-) 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 From b60b58a333089d6e8fe7118b75f8e005de313fe5 Mon Sep 17 00:00:00 2001 From: Yulong Date: Mon, 26 Jun 2017 05:39:14 -0400 Subject: [PATCH 2/2] component/bt: Added the bta gattc write ccc judgment. --- components/bt/bluedroid/bta/gatt/bta_gattc_act.c | 8 +++++++- components/bt/bluedroid/btc/core/btc_ble_storage.c | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/components/bt/bluedroid/bta/gatt/bta_gattc_act.c b/components/bt/bluedroid/bta/gatt/bta_gattc_act.c index 292b3dfaf6..617aa02cdf 100644 --- a/components/bt/bluedroid/bta/gatt/bta_gattc_act.c +++ b/components/bt/bluedroid/bta/gatt/bta_gattc_act.c @@ -33,6 +33,7 @@ #include "bta_gattc_int.h" #include "l2c_api.h" #include "l2c_int.h" +#include "gatt_int.h" #if (defined BTA_HH_LE_INCLUDED && BTA_HH_LE_INCLUDED == TRUE) #include "bta_hh_int.h" @@ -2410,7 +2411,12 @@ tBTA_GATTC_FIND_SERVICE_CB bta_gattc_register_service_change_notify(UINT16 conn_ ccc_value.len = 2; ccc_value.value[0] = GATT_CLT_CONFIG_INDICATION; ccc_value.auth_req = GATT_AUTH_REQ_NONE; - write_status = GATTC_Write (conn_id, GATT_WRITE, &ccc_value); + if (gatt_is_clcb_allocated(conn_id)) { + APPL_TRACE_DEBUG("%s, GATTC_Write GATT_BUSY conn_id = %d", __func__, conn_id); + write_status = GATT_BUSY; + } else { + write_status = GATTC_Write (conn_id, GATT_WRITE, &ccc_value); + } if (write_status != GATT_SUCCESS) { start_find_ccc_timer = TRUE; result = SERVICE_CHANGE_WRITE_CCC_FAILED; diff --git a/components/bt/bluedroid/btc/core/btc_ble_storage.c b/components/bt/bluedroid/btc/core/btc_ble_storage.c index 5e588012ea..3bafbc1cea 100644 --- a/components/bt/bluedroid/btc/core/btc_ble_storage.c +++ b/components/bt/bluedroid/btc/core/btc_ble_storage.c @@ -238,7 +238,7 @@ 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; + char *key_type_str; switch (key_type) { case BTM_LE_KEY_PENC: key_type_str = "LE_KEY_PENC";