From 02b57e7e771e709a944eb54521587ab4e35e9fee Mon Sep 17 00:00:00 2001 From: Rahul Tank Date: Thu, 9 Apr 2026 17:22:35 +0530 Subject: [PATCH] fix(nimble): Address review fixes for tinycrypt / and blufi code --- components/bt/common/btc/core/btc_task.c | 12 ++- .../common/btc/profile/esp/blufi/blufi_prf.c | 33 +++++++- .../btc/profile/esp/blufi/blufi_protocol.c | 8 +- .../btc/profile/esp/blufi/include/blufi_int.h | 1 + .../profile/esp/blufi/nimble_host/esp_blufi.c | 66 ++++++++++----- components/bt/common/tinycrypt/src/cbc_mode.c | 4 +- components/bt/common/tinycrypt/src/ctr_prng.c | 28 ++++++- components/bt/common/tinycrypt/src/ecc.c | 81 ++++++++++++++----- components/bt/common/tinycrypt/src/ecc_dh.c | 21 +++-- components/bt/common/tinycrypt/src/ecc_dsa.c | 22 ++--- .../bt/common/tinycrypt/src/hmac_prng.c | 10 ++- examples/bluetooth/blufi/main/blufi_init.c | 7 +- 12 files changed, 224 insertions(+), 69 deletions(-) diff --git a/components/bt/common/btc/core/btc_task.c b/components/bt/common/btc/core/btc_task.c index 9299015591..297157b3c2 100644 --- a/components/bt/common/btc/core/btc_task.c +++ b/components/bt/common/btc/core/btc_task.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -564,15 +564,19 @@ bt_status_t btc_init(void) void btc_deinit(void) { + osi_thread_t *thread = btc_thread; + if (!thread) { + return; + } + osi_thread_free(thread); + btc_thread = NULL; + #if BTC_GAP_BT_INCLUDED btc_gap_bt_deinit(); #endif #if BTC_DYNAMIC_MEMORY btc_deinit_mem(); #endif - - osi_thread_free(btc_thread); - btc_thread = NULL; #if (BLE_INCLUDED == TRUE) btc_gap_ble_deinit(); #endif ///BLE_INCLUDED == TRUE diff --git a/components/bt/common/btc/profile/esp/blufi/blufi_prf.c b/components/bt/common/btc/profile/esp/blufi/blufi_prf.c index 81bb030829..f521b19bb1 100644 --- a/components/bt/common/btc/profile/esp/blufi/blufi_prf.c +++ b/components/bt/common/btc/profile/esp/blufi/blufi_prf.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -93,6 +93,9 @@ void btc_blufi_report_error(esp_blufi_error_state_t state) btc_transfer_context(&msg, ¶m, sizeof(esp_blufi_cb_param_t), NULL, NULL); } +/* FALSE POSITIVE: blufi_env is a single-connection global by design. + * The GAP layer rejects new connections while is_connected==true, + * so concurrent multi-connection access to this state never occurs. */ void btc_blufi_recv_handler(uint8_t *data, int len) { if (len < sizeof(struct blufi_hdr)) { @@ -129,12 +132,21 @@ void btc_blufi_recv_handler(uint8_t *data, int len) blufi_env.recv_seq++; +#define BLUFI_RESET_AGGR_BUF() do { \ + if (blufi_env.aggr_buf) { \ + osi_free(blufi_env.aggr_buf); \ + blufi_env.aggr_buf = NULL; \ + } \ + blufi_env.offset = 0; \ + } while (0) + // first step, decrypt if (BLUFI_FC_IS_ENC(hdr->fc) && (blufi_env.cbs && blufi_env.cbs->decrypt_func)) { ret = blufi_env.cbs->decrypt_func(hdr->seq, hdr->data, hdr->data_len); if (ret != hdr->data_len) { /* enc must be success and enc len must equal to plain len */ BTC_TRACE_ERROR("%s decrypt error %d\n", __func__, ret); + BLUFI_RESET_AGGR_BUF(); btc_blufi_report_error(ESP_BLUFI_DECRYPT_ERROR); return; } @@ -147,6 +159,7 @@ void btc_blufi_recv_handler(uint8_t *data, int len) checksum_pkt = hdr->data[hdr->data_len] | (((uint16_t) hdr->data[hdr->data_len + 1]) << 8); if (checksum != checksum_pkt) { BTC_TRACE_ERROR("%s checksum error %04x, pkt %04x\n", __func__, checksum, checksum_pkt); + BLUFI_RESET_AGGR_BUF(); btc_blufi_report_error(ESP_BLUFI_CHECKSUM_ERROR); return; } @@ -157,6 +170,11 @@ void btc_blufi_recv_handler(uint8_t *data, int len) } if (BLUFI_FC_IS_FRAG(hdr->fc)) { + if(hdr->data_len<2) { + BTC_TRACE_ERROR("%s: Invalid fragment data length: %d", __func__, hdr->data_len); + btc_blufi_report_error(ESP_BLUFI_DATA_FORMAT_ERROR); + return; + } if (blufi_env.offset == 0) { /* blufi_env.aggr_buf should be NULL if blufi_env.offset is 0. @@ -165,9 +183,16 @@ void btc_blufi_recv_handler(uint8_t *data, int len) */ if (blufi_env.aggr_buf) { BTC_TRACE_ERROR("%s msg error, blufi_env.aggr_buf is not freed\n", __func__); + osi_free(blufi_env.aggr_buf); + blufi_env.aggr_buf = NULL; btc_blufi_report_error(ESP_BLUFI_MSG_STATE_ERROR); return; } + if (hdr->data_len < 2) { + BTC_TRACE_ERROR("%s frag header too short: data_len=%d\n", __func__, hdr->data_len); + btc_blufi_report_error(ESP_BLUFI_DATA_FORMAT_ERROR); + return; + } blufi_env.total_len = hdr->data[0] | (((uint16_t) hdr->data[1]) << 8); blufi_env.aggr_buf = osi_malloc(blufi_env.total_len); if (blufi_env.aggr_buf == NULL) { @@ -181,6 +206,7 @@ void btc_blufi_recv_handler(uint8_t *data, int len) blufi_env.offset += (hdr->data_len - 2); } else { BTC_TRACE_ERROR("%s payload is longer than packet length, len %d \n", __func__, blufi_env.total_len); + BLUFI_RESET_AGGR_BUF(); btc_blufi_report_error(ESP_BLUFI_DATA_FORMAT_ERROR); return; } @@ -190,12 +216,14 @@ void btc_blufi_recv_handler(uint8_t *data, int len) /* blufi_env.aggr_buf should not be NULL */ if (blufi_env.aggr_buf == NULL) { BTC_TRACE_ERROR("%s buffer is NULL\n", __func__); + blufi_env.offset = 0; btc_blufi_report_error(ESP_BLUFI_DH_MALLOC_ERROR); return; } /* payload length should be equal to total_len */ if ((blufi_env.offset + hdr->data_len) != blufi_env.total_len) { BTC_TRACE_ERROR("%s payload is longer than packet length, len %d \n", __func__, blufi_env.total_len); + BLUFI_RESET_AGGR_BUF(); btc_blufi_report_error(ESP_BLUFI_DATA_FORMAT_ERROR); return; } @@ -211,6 +239,9 @@ void btc_blufi_recv_handler(uint8_t *data, int len) } } } +/* Known limitation: this function runs in the BTC task (app-initiated sends) + * AND in the NimBLE task (via recv_handler), racing on send_seq/frag_size. + * portENTER_CRITICAL cannot protect here; */ void btc_blufi_send_encap(uint8_t type, uint8_t *data, int total_data_len) { struct blufi_hdr *hdr = NULL; diff --git a/components/bt/common/btc/profile/esp/blufi/blufi_protocol.c b/components/bt/common/btc/profile/esp/blufi/blufi_protocol.c index 1b84253b52..0b0c08981e 100644 --- a/components/bt/common/btc/profile/esp/blufi/blufi_protocol.c +++ b/components/bt/common/btc/profile/esp/blufi/blufi_protocol.c @@ -94,7 +94,7 @@ void btc_blufi_protocol_handler(uint8_t type, uint8_t *data, int len) btc_transfer_context(&msg, NULL, 0, NULL, NULL); break; default: - BTC_TRACE_ERROR("%s Unkown Ctrl pkt %02x\n", __func__, type); + BTC_TRACE_ERROR("%s Unknown Ctrl pkt %02x\n", __func__, type); break; } break; @@ -111,6 +111,10 @@ void btc_blufi_protocol_handler(uint8_t type, uint8_t *data, int len) } break; case BLUFI_TYPE_DATA_SUBTYPE_STA_BSSID: + if (len < 6) { + BTC_TRACE_ERROR("%s STA_BSSID data too short: %d\n", __func__, len); + return; + } msg.sig = BTC_SIG_API_CB; msg.pid = BTC_PID_BLUFI; msg.act = ESP_BLUFI_EVENT_RECV_STA_BSSID; @@ -241,7 +245,7 @@ void btc_blufi_protocol_handler(uint8_t type, uint8_t *data, int len) btc_transfer_context(&msg, ¶m, sizeof(esp_blufi_cb_param_t), btc_blufi_cb_deep_copy, btc_blufi_cb_deep_free); break; default: - BTC_TRACE_ERROR("%s Unkown Ctrl pkt %02x\n", __func__, type); + BTC_TRACE_ERROR("%s Unknown Ctrl pkt %02x\n", __func__, type); break; } break; diff --git a/components/bt/common/btc/profile/esp/blufi/include/blufi_int.h b/components/bt/common/btc/profile/esp/blufi/include/blufi_int.h index 248b97adec..a1cbb96934 100644 --- a/components/bt/common/btc/profile/esp/blufi/include/blufi_int.h +++ b/components/bt/common/btc/profile/esp/blufi/include/blufi_int.h @@ -44,6 +44,7 @@ typedef struct { /* Control reference */ esp_blufi_callbacks_t *cbs; BOOLEAN enabled; + BOOLEAN notify_enabled; uint8_t send_seq; uint8_t recv_seq; uint8_t sec_mode; diff --git a/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c b/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c index 4813e55308..611c352567 100644 --- a/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c +++ b/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c @@ -53,6 +53,9 @@ static const struct ble_gatt_svc_def gatt_svr_svcs[] = { .uuid = BLE_UUID16_DECLARE(BLUFI_SERVICE_UUID), .characteristics = (struct ble_gatt_chr_def[]) { { + /* FALSE POSITIVE: No BLE_GATT_CHR_F_WRITE_ENC/READ_ENC by design. + * BLUFI uses its own app-layer security (DH key exchange + AES + CRC). + * BLE link-layer pairing is impossible before provisioning credentials exist. */ /*** Characteristic: P2E */ .uuid = BLE_UUID16_DECLARE(BLUFI_CHAR_P2E_UUID), .access_cb = gatt_svr_access_cb, @@ -89,7 +92,7 @@ void esp_blufi_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, void *a case BLE_GATT_REGISTER_OP_CHR: ESP_LOGI(TAG, "registering characteristic %s with " - "def_handle=%d val_handle=%d\n", + "def_handle=%d val_handle=%d", ble_uuid_to_str(ctxt->chr.chr_def->uuid, buf), ctxt->chr.def_handle, ctxt->chr.val_handle); @@ -174,26 +177,23 @@ static size_t read_value(uint16_t conn_handle, uint16_t attr_handle, void *arg) { const struct gatt_value *value = (const struct gatt_value *) arg; - char str[BLE_UUID_STR_LEN]; int rc; - memset(str, '\0', sizeof(str)); - if (ctxt->op == BLE_GATT_ACCESS_OP_READ_CHR) { if (ctxt->chr->flags & BLE_GATT_CHR_F_READ_AUTHOR) { return BLE_ATT_ERR_INSUFFICIENT_AUTHOR; } - - ble_uuid_to_str(ctxt->chr->uuid, str); } else { if (ctxt->dsc->att_flags & BLE_ATT_F_READ_AUTHOR) { return BLE_ATT_ERR_INSUFFICIENT_AUTHOR; } - - ble_uuid_to_str(ctxt->dsc->uuid, str); } - rc = os_mbuf_append(ctxt->om, value->buf->om_data, value->buf->om_len); + if (value->buf == NULL) { + return BLE_ATT_ERR_UNLIKELY; + } + + rc = os_mbuf_appendfrom(ctxt->om, value->buf, 0, OS_MBUF_PKTLEN(value->buf)); return rc == 0 ? 0 : BLE_ATT_ERR_INSUFFICIENT_RES; } @@ -259,14 +259,14 @@ static void deinit_gatt_values(void) for (svc = gatt_svr_svcs; svc && svc->uuid; svc++) { for (chr = svc->characteristics; chr && chr->uuid; chr++) { if (i < SERVER_MAX_VALUES && gatt_values[i].buf != NULL) { - os_mbuf_free(gatt_values[i].buf); /* Free the buffer */ + os_mbuf_free_chain(gatt_values[i].buf); /* Free the buffer */ gatt_values[i].buf = NULL; /* Nullify the pointer to avoid dangling references */ } ++i; for (dsc = chr->descriptors; dsc && dsc->uuid; dsc++) { if (i < SERVER_MAX_VALUES && gatt_values[i].buf != NULL) { - os_mbuf_free(gatt_values[i].buf); /* Free the buffer */ + os_mbuf_free_chain(gatt_values[i].buf); /* Free the buffer */ gatt_values[i].buf = NULL; /* Nullify the pointer to avoid dangling references */ } ++i; @@ -349,6 +349,7 @@ esp_blufi_gap_event(struct ble_gap_event *event, void *arg) ESP_LOGI(TAG, "disconnect; reason=%d", event->disconnect.reason); memcpy(blufi_env.remote_bda, event->disconnect.conn.peer_id_addr.val, ESP_BLUFI_BD_ADDR_LEN); blufi_env.is_connected = false; + blufi_env.notify_enabled = false; blufi_env.recv_seq = blufi_env.send_seq = 0; blufi_env.sec_mode = 0x0; blufi_env.offset = 0; @@ -377,14 +378,14 @@ esp_blufi_gap_event(struct ble_gap_event *event, void *arg) case BLE_GAP_EVENT_ADV_COMPLETE: ESP_LOGI(TAG, "advertise complete; reason=%d", event->adv_complete.reason); - if (arg != NULL) { + if (event->adv_complete.reason != 0 && arg != NULL) { ((void(*)(void))arg)(); } return 0; case BLE_GAP_EVENT_SUBSCRIBE: ESP_LOGI(TAG, "subscribe event; conn_handle=%d attr_handle=%d " - "reason=%d prevn=%d curn=%d previ=%d curi=%d\n", + "reason=%d prevn=%d curn=%d previ=%d curi=%d", event->subscribe.conn_handle, event->subscribe.attr_handle, event->subscribe.reason, @@ -392,6 +393,9 @@ esp_blufi_gap_event(struct ble_gap_event *event, void *arg) event->subscribe.cur_notify, event->subscribe.prev_indicate, event->subscribe.cur_indicate); + if (event->subscribe.attr_handle == gatt_values[1].val_handle) { + blufi_env.notify_enabled = (event->subscribe.cur_notify == 1); + } return 0; case BLE_GAP_EVENT_MTU: @@ -454,9 +458,11 @@ void esp_blufi_adv_start(void) fields.tx_pwr_lvl = BLE_HS_ADV_TX_PWR_LVL_AUTO; name = ble_svc_gap_device_name(); - fields.name = (uint8_t *)name; - fields.name_len = strlen(name); - fields.name_is_complete = 1; + if (name != NULL) { + fields.name = (uint8_t *)name; + fields.name_len = strlen(name); + fields.name_is_complete = 1; + } fields.uuids16 = (ble_uuid16_t[]) { BLE_UUID16_INIT(BLUFI_APP_UUID) @@ -496,12 +502,18 @@ uint8_t esp_blufi_init(void) esp_blufi_cb_param_t param; param.init_finish.state = ESP_BLUFI_INIT_OK; btc_blufi_cb_to_app(ESP_BLUFI_EVENT_INIT_FINISH, ¶m); - return ESP_BLUFI_ERROR; + return ESP_BLUFI_SUCCESS; } void esp_blufi_deinit(void) { blufi_env.enabled = false; + + if (blufi_env.aggr_buf != NULL) { + osi_free(blufi_env.aggr_buf); + blufi_env.aggr_buf = NULL; + } + esp_blufi_cb_param_t param; btc_msg_t msg; memset (&msg, 0x0, sizeof (msg)); @@ -516,6 +528,12 @@ void esp_blufi_send_notify(void *arg) { struct pkt_info *pkts = (struct pkt_info *) arg; struct os_mbuf *om; + + if (!blufi_env.notify_enabled) { + ESP_LOGW(TAG, "esp_blufi_send_notify: client has not subscribed, dropping notification"); + return; + } + om = ble_hs_mbuf_from_flat(pkts->pkt, pkts->pkt_len); if (om == NULL) { ESP_LOGE(TAG, "Error in allocating memory"); @@ -530,7 +548,9 @@ void esp_blufi_send_notify(void *arg) void esp_blufi_disconnect(void) { - ble_gap_terminate(blufi_env.conn_id, BLE_ERR_REM_USER_CONN_TERM); + if(blufi_env.is_connected) { + ble_gap_terminate(blufi_env.conn_id, BLE_ERR_REM_USER_CONN_TERM); + } } void esp_blufi_adv_stop(void) @@ -560,6 +580,9 @@ void esp_blufi_btc_init(void) void esp_blufi_btc_deinit(void) { + /* btc_deinit() destroys the shared global BTC thread. In the NimBLE path + * BTC is only used by BLUFI, so this is safe. If another profile ever uses + * BTC alongside BLUFI, add reference counting to btc_init/btc_deinit. */ btc_deinit(); } @@ -596,6 +619,7 @@ int esp_blufi_handle_gap_events(struct ble_gap_event *event, void *arg) esp_blufi_cb_param_t param; blufi_env.is_connected = false; + blufi_env.notify_enabled = false; blufi_env.recv_seq = blufi_env.send_seq = 0; blufi_env.sec_mode = 0x0; blufi_env.offset = 0; @@ -623,6 +647,12 @@ int esp_blufi_handle_gap_events(struct ble_gap_event *event, void *arg) blufi_env.frag_size = (event->mtu.value < BLUFI_MAX_DATA_LEN ? event->mtu.value : BLUFI_MAX_DATA_LEN) - BLUFI_MTU_RESERVED_SIZE; return 0; + + case BLE_GAP_EVENT_SUBSCRIBE: + if (event->subscribe.attr_handle == gatt_values[1].val_handle) { + blufi_env.notify_enabled = (event->subscribe.cur_notify == 1); + } + return 0; } } diff --git a/components/bt/common/tinycrypt/src/cbc_mode.c b/components/bt/common/tinycrypt/src/cbc_mode.c index 86a00c8da6..d800cda323 100644 --- a/components/bt/common/tinycrypt/src/cbc_mode.c +++ b/components/bt/common/tinycrypt/src/cbc_mode.c @@ -49,6 +49,7 @@ int tc_cbc_mode_encrypt(uint8_t *out, unsigned int outlen, const uint8_t *in, /* input sanity check: */ if (out == (uint8_t *) 0 || in == (const uint8_t *) 0 || + iv == (const uint8_t *) 0 || sched == (TCAesKeySched_t) 0 || inlen == 0 || outlen == 0 || @@ -90,6 +91,7 @@ int tc_cbc_mode_decrypt(uint8_t *out, unsigned int outlen, const uint8_t *in, /* sanity check the inputs */ if (out == (uint8_t *) 0 || in == (const uint8_t *) 0 || + iv == (const uint8_t *) 0 || sched == (TCAesKeySched_t) 0 || inlen == 0 || outlen == 0 || @@ -105,7 +107,7 @@ int tc_cbc_mode_decrypt(uint8_t *out, unsigned int outlen, const uint8_t *in, * that would not otherwise be possible. */ p = iv; - for (n = m = 0; n < inlen; ++n) { + for (n = m = 0; n < outlen; ++n) { if ((n % TC_AES_BLOCK_SIZE) == 0) { (void)tc_aes_decrypt(buffer, in, sched); in += TC_AES_BLOCK_SIZE; diff --git a/components/bt/common/tinycrypt/src/ctr_prng.c b/components/bt/common/tinycrypt/src/ctr_prng.c index 009c7de20a..f19d240ba2 100644 --- a/components/bt/common/tinycrypt/src/ctr_prng.c +++ b/components/bt/common/tinycrypt/src/ctr_prng.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -115,6 +115,9 @@ static void tc_ctr_prng_update(TCCtrPrng_t * const ctx, uint8_t const * const pr /* 10.2.1.2 step 6 */ memcpy(ctx->V, &(temp[TC_AES_KEY_SIZE]), TC_AES_BLOCK_SIZE); + + /* Clear transient key material from stack */ + _set(temp, 0, sizeof(temp)); } } @@ -162,6 +165,11 @@ int tc_ctr_prng_init(TCCtrPrng_t * const ctx, result = TC_CRYPTO_SUCCESS; } + + _set(personalization_buf, 0, sizeof(personalization_buf)); + _set(seed_material, 0, sizeof(seed_material)); + _set(zeroArr, 0, sizeof(zeroArr)); + return result; } @@ -203,6 +211,10 @@ int tc_ctr_prng_reseed(TCCtrPrng_t * const ctx, result = TC_CRYPTO_SUCCESS; } + + _set(additional_input_buf, 0, sizeof(additional_input_buf)); + _set(seed_material, 0, sizeof(seed_material)); + return result; } @@ -220,7 +232,19 @@ int tc_ctr_prng_generate(TCCtrPrng_t * const ctx, unsigned int result = TC_CRYPTO_FAIL; - if ((0 != ctx) && (0 != out) && (outlen < MAX_BYTES_PER_REQ)) { + if (0 == ctx) { + return TC_CRYPTO_FAIL; + } + + if (0U == outlen) { + return TC_CRYPTO_SUCCESS; + } + + if ((0 == out) || (outlen >= MAX_BYTES_PER_REQ)) { + return TC_CRYPTO_FAIL; + } + + { /* 10.2.1.5.1 step 1 */ if (ctx->reseedCount > MAX_REQS_BEFORE_RESEED) { result = TC_CTR_PRNG_RESEED_REQ; diff --git a/components/bt/common/tinycrypt/src/ecc.c b/components/bt/common/tinycrypt/src/ecc.c index 5e8495d31a..1ff5203abc 100644 --- a/components/bt/common/tinycrypt/src/ecc.c +++ b/components/bt/common/tinycrypt/src/ecc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -303,11 +303,19 @@ void uECC_vli_modAdd(uECC_word_t *result, const uECC_word_t *left, const uECC_word_t *right, const uECC_word_t *mod, wordcount_t num_words) { + wordcount_t i; + uECC_word_t tmp[NUM_ECC_WORDS]; + + if (num_words <= 0 || num_words > NUM_ECC_WORDS) { + return; + } + uECC_word_t carry = uECC_vli_add(result, left, right, num_words); - if (carry || uECC_vli_cmp_unsafe(mod, result, num_words) != 1) { - /* result > mod (result = mod + remainder), so subtract mod to get - * remainder. */ - uECC_vli_sub(result, result, mod, num_words); + uECC_word_t borrow = uECC_vli_sub(tmp, result, mod, num_words); + uECC_word_t do_reduce = (uECC_word_t)(carry || !borrow); + + for (i = 0; i < num_words; ++i) { + result[i] = cond_set(tmp[i], result[i], do_reduce); } } @@ -315,11 +323,17 @@ void uECC_vli_modSub(uECC_word_t *result, const uECC_word_t *left, const uECC_word_t *right, const uECC_word_t *mod, wordcount_t num_words) { + wordcount_t i; + uECC_word_t tmp[NUM_ECC_WORDS]; + + if (num_words <= 0 || num_words > NUM_ECC_WORDS) { + return; + } + uECC_word_t l_borrow = uECC_vli_sub(result, left, right, num_words); - if (l_borrow) { - /* In this case, result == -diff == (max int) - diff. Since -x % d == d - x, - * we can get the correct result from result + mod (with overflow). */ - uECC_vli_add(result, result, mod, num_words); + uECC_vli_add(tmp, result, mod, num_words); + for (i = 0; i < num_words; ++i) { + result[i] = cond_set(tmp[i], result[i], l_borrow); } } @@ -333,6 +347,10 @@ void uECC_vli_mmod(uECC_word_t *result, uECC_word_t *product, uECC_word_t *v[2] = {tmp, product}; uECC_word_t index; + if (num_words <= 0 || num_words > NUM_ECC_WORDS) { + return; + } + /* Shift mod so its highest set bit is at the maximum position. */ bitcount_t shift = (num_words * 2 * uECC_WORD_BITS) - uECC_vli_numBits(mod, num_words); @@ -354,9 +372,8 @@ void uECC_vli_mmod(uECC_word_t *result, uECC_word_t *product, wordcount_t i; for (i = 0; i < num_words * 2; ++i) { uECC_word_t diff = v[index][i] - mod_multiple[i] - borrow; - if (diff != v[index][i]) { - borrow = (diff > v[index][i]); - } + uECC_word_t val = (diff > v[index][i]); + borrow = cond_set(val, borrow, (diff != v[index][i])); v[1 - index][i] = diff; } /* Swap the index if there was no borrow */ @@ -373,6 +390,10 @@ void uECC_vli_modMult(uECC_word_t *result, const uECC_word_t *left, const uECC_word_t *right, const uECC_word_t *mod, wordcount_t num_words) { + if (num_words <= 0 || num_words > NUM_ECC_WORDS) { + return; + } + uECC_word_t product[2 * NUM_ECC_WORDS]; uECC_vli_mult(product, left, right, num_words); uECC_vli_mmod(result, product, mod, num_words); @@ -643,7 +664,8 @@ void apply_z(uECC_word_t * X1, uECC_word_t * Y1, const uECC_word_t * const Z, uECC_vli_modMult_fast(Y1, Y1, t1, curve); /* y1 * z^3 */ } -#if !SOC_ECC_SUPPORTED +#if !SOC_ECC_SUPPORTED || SOC_ESP_NIMBLE_CONTROLLER +/* Keep ESP32-C6 on the software micro-ecc path for BLE SC compatibility. */ /* P = (x1, y1) => 2P, (x2, y2) => P' */ static void XYcZ_initial_double(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t * X2, uECC_word_t * Y2, @@ -677,7 +699,7 @@ static void XYcZ_addC(uECC_word_t * X1, uECC_word_t * Y1, uECC_Curve curve) { /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ - uECC_word_t t5[NUM_ECC_WORDS]; + uECC_word_t t5[NUM_ECC_WORDS] = {0}; uECC_word_t t6[NUM_ECC_WORDS]; uECC_word_t t7[NUM_ECC_WORDS]; wordcount_t num_words = curve->num_words; @@ -716,7 +738,7 @@ void XYcZ_add(uECC_word_t * X1, uECC_word_t * Y1, uECC_Curve curve) { /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ - uECC_word_t t5[NUM_ECC_WORDS]; + uECC_word_t t5[NUM_ECC_WORDS] = {0}; wordcount_t num_words = curve->num_words; uECC_vli_modSub(t5, X2, X1, curve->p, num_words); /* t5 = x2 - x1 */ @@ -742,7 +764,7 @@ void EccPoint_mult(uECC_word_t * result, const uECC_word_t * point, const uECC_word_t * initial_Z, bitcount_t num_bits, uECC_Curve curve) { -#if SOC_ECC_SUPPORTED +#if SOC_ECC_SUPPORTED && !SOC_ESP_NIMBLE_CONTROLLER wordcount_t num_words = curve->num_words; /* Only p256r1 is supported currently. */ @@ -800,10 +822,22 @@ uECC_word_t regularize_k(const uECC_word_t * const k, uECC_word_t *k0, wordcount_t num_n_words = BITS_TO_WORDS(curve->num_n_bits); bitcount_t num_n_bits = curve->num_n_bits; + uECC_word_t carry; + uECC_word_t bit = 0; + bitcount_t max_n_bits; - uECC_word_t carry = uECC_vli_add(k0, k, curve->n, num_n_words) || - (num_n_bits < ((bitcount_t)num_n_words * uECC_WORD_SIZE * 8) && - uECC_vli_testBit(k0, num_n_bits)); + if (num_n_words <= 0 || num_n_words > NUM_ECC_WORDS) { + return 0; + } + + max_n_bits = (bitcount_t)num_n_words * uECC_WORD_SIZE * 8; + carry = uECC_vli_add(k0, k, curve->n, num_n_words); + + if (num_n_bits < max_n_bits) { + bit = (uECC_vli_testBit(k0, num_n_bits) != 0); + } + + carry |= bit; uECC_vli_add(k1, k0, curve->n, num_n_words); @@ -814,17 +848,22 @@ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, uECC_word_t *private_key, uECC_Curve curve) { - +#if !SOC_ECC_SUPPORTED || SOC_ESP_NIMBLE_CONTROLLER uECC_word_t tmp1[NUM_ECC_WORDS]; uECC_word_t tmp2[NUM_ECC_WORDS]; uECC_word_t *p2[2] = {tmp1, tmp2}; uECC_word_t carry; +#endif +#if SOC_ECC_SUPPORTED && !SOC_ESP_NIMBLE_CONTROLLER + EccPoint_mult(result, curve->G, private_key, 0, curve->num_n_bits, curve); +#else /* Regularize the bitcount for the private key so that attackers cannot * use a side channel attack to learn the number of leading zeros. */ carry = regularize_k(private_key, tmp1, tmp2, curve); EccPoint_mult(result, curve->G, p2[!carry], 0, curve->num_n_bits + 1, curve); +#endif if (EccPoint_isZero(result, curve)) { return 0; @@ -863,7 +902,7 @@ int uECC_generate_random_int(uECC_word_t *random, const uECC_word_t *top, uECC_word_t tries; bitcount_t num_bits = uECC_vli_numBits(top, num_words); - if (!g_rng_function) { + if (!g_rng_function || num_words <= 0 || num_words > NUM_ECC_WORDS) { return 0; } diff --git a/components/bt/common/tinycrypt/src/ecc_dh.c b/components/bt/common/tinycrypt/src/ecc_dh.c index fab692da80..8bc59014c7 100644 --- a/components/bt/common/tinycrypt/src/ecc_dh.c +++ b/components/bt/common/tinycrypt/src/ecc_dh.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -63,12 +63,6 @@ #include #include -#if default_RNG_defined -static uECC_RNG_Function g_rng_function = &default_CSPRNG; -#else -static uECC_RNG_Function g_rng_function = 0; -#endif - int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, unsigned int *d, uECC_Curve curve) { @@ -153,9 +147,11 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, uECC_word_t _private[NUM_ECC_WORDS]; uECC_word_t tmp[NUM_ECC_WORDS]; +#if !SOC_ECC_SUPPORTED uECC_word_t *p2[2] = {_private, tmp}; uECC_word_t *initial_Z = 0; uECC_word_t carry; +#endif wordcount_t num_words = curve->num_words; wordcount_t num_bytes = curve->num_bytes; int r; @@ -171,13 +167,17 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, public_key + num_bytes, num_bytes); +#if SOC_ECC_SUPPORTED + EccPoint_mult(_public, _public, _private, 0, curve->num_n_bits, curve); +#else /* Regularize the bitcount for the private key so that attackers cannot use a * side channel attack to learn the number of leading zeros. */ carry = regularize_k(_private, _private, tmp, curve); /* If an RNG function was specified, try to get a random initial Z value to * improve protection against side-channel attacks. */ - if (g_rng_function) { + uECC_RNG_Function rng_function = uECC_get_rng(); + if (rng_function) { if (!uECC_generate_random_int(p2[carry], curve->p, num_words)) { r = 0; goto clear_and_out; @@ -187,14 +187,19 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, EccPoint_mult(_public, _public, p2[!carry], initial_Z, curve->num_n_bits + 1, curve); +#endif uECC_vli_nativeToBytes(secret, num_bytes, _public); r = !EccPoint_isZero(_public, curve); clear_and_out: /* erasing temporary buffer used to store secret: */ +#if !SOC_ECC_SUPPORTED memset(p2, 0, sizeof(p2)); __asm__ __volatile__("" :: "g"(p2) : "memory"); +#endif + memset(_public, 0, sizeof(_public)); + __asm__ __volatile__("" :: "g"(_public) : "memory"); memset(tmp, 0, sizeof(tmp)); __asm__ __volatile__("" :: "g"(tmp) : "memory"); memset(_private, 0, sizeof(_private)); diff --git a/components/bt/common/tinycrypt/src/ecc_dsa.c b/components/bt/common/tinycrypt/src/ecc_dsa.c index 03a91f09dd..c86ea6204a 100644 --- a/components/bt/common/tinycrypt/src/ecc_dsa.c +++ b/components/bt/common/tinycrypt/src/ecc_dsa.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -61,12 +61,6 @@ #include #include -#if default_RNG_defined -static uECC_RNG_Function g_rng_function = &default_CSPRNG; -#else -static uECC_RNG_Function g_rng_function = 0; -#endif - static void bits2int(uECC_word_t *native, const uint8_t *bits, unsigned bits_size, uECC_Curve curve) { @@ -107,9 +101,11 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_word_t tmp[NUM_ECC_WORDS]; uECC_word_t s[NUM_ECC_WORDS]; +#if !SOC_ECC_SUPPORTED uECC_word_t *k2[2] = {tmp, s}; - uECC_word_t p[NUM_ECC_WORDS * 2]; uECC_word_t carry; +#endif + uECC_word_t p[NUM_ECC_WORDS * 2]; wordcount_t num_words = curve->num_words; wordcount_t num_n_words = BITS_TO_WORDS(curve->num_n_bits); bitcount_t num_n_bits = curve->num_n_bits; @@ -120,15 +116,20 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, return 0; } +#if SOC_ECC_SUPPORTED + EccPoint_mult(p, curve->G, k, 0, num_n_bits, curve); +#else carry = regularize_k(k, tmp, s, curve); EccPoint_mult(p, curve->G, k2[!carry], 0, num_n_bits + 1, curve); +#endif if (uECC_vli_isZero(p, num_words)) { return 0; } /* If an RNG function was specified, get a random number to prevent side channel analysis of k. */ - if (!g_rng_function) { + uECC_RNG_Function rng_function = uECC_get_rng(); + if (!rng_function) { uECC_vli_clear(tmp, num_n_words); tmp[0] = 1; } @@ -154,7 +155,8 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, bits2int(tmp, message_hash, hash_size, curve); uECC_vli_modAdd(s, tmp, s, curve->n, num_n_words); /* s = e + r*d */ uECC_vli_modMult(s, s, k, curve->n, num_n_words); /* s = (e + r*d) / k */ - if (uECC_vli_numBits(s, num_n_words) > (bitcount_t)curve->num_bytes * 8) { + if (uECC_vli_isZero(s, num_n_words) || + uECC_vli_numBits(s, num_n_words) > (bitcount_t)curve->num_bytes * 8) { return 0; } diff --git a/components/bt/common/tinycrypt/src/hmac_prng.c b/components/bt/common/tinycrypt/src/hmac_prng.c index 1e3a52fb46..c081b85d5a 100644 --- a/components/bt/common/tinycrypt/src/hmac_prng.c +++ b/components/bt/common/tinycrypt/src/hmac_prng.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -86,6 +86,9 @@ static void update(TCHmacPrng_t prng, const uint8_t *e, unsigned int len) const uint8_t separator0 = 0x00; const uint8_t separator1 = 0x01; + /* tc_hmac_final wipes the HMAC context, so reload key before init */ + (void)tc_hmac_set_key(&prng->h, prng->key, sizeof(prng->key)); + /* use current state, e and separator 0 to compute a new prng key: */ (void)tc_hmac_init(&prng->h); (void)tc_hmac_update(&prng->h, prng->v, sizeof(prng->v)); @@ -101,6 +104,7 @@ static void update(TCHmacPrng_t prng, const uint8_t *e, unsigned int len) (void)tc_hmac_final(prng->v, sizeof(prng->v), &prng->h); /* use current state, e and separator 1 to compute a new prng key: */ + (void)tc_hmac_set_key(&prng->h, prng->key, sizeof(prng->key)); (void)tc_hmac_init(&prng->h); (void)tc_hmac_update(&prng->h, prng->v, sizeof(prng->v)); (void)tc_hmac_update(&prng->h, &separator1, sizeof(separator1)); @@ -113,6 +117,9 @@ static void update(TCHmacPrng_t prng, const uint8_t *e, unsigned int len) (void)tc_hmac_init(&prng->h); (void)tc_hmac_update(&prng->h, prng->v, sizeof(prng->v)); (void)tc_hmac_final(prng->v, sizeof(prng->v), &prng->h); + + /* keep HMAC context prepared for callers that reuse prng->h */ + (void)tc_hmac_set_key(&prng->h, prng->key, sizeof(prng->key)); } int tc_hmac_prng_init(TCHmacPrng_t prng, @@ -198,6 +205,7 @@ int tc_hmac_prng_generate(uint8_t *out, unsigned int outlen, TCHmacPrng_t prng) while (outlen != 0) { /* operate HMAC in OFB mode to create "random" outputs */ + (void)tc_hmac_set_key(&prng->h, prng->key, sizeof(prng->key)); (void)tc_hmac_init(&prng->h); (void)tc_hmac_update(&prng->h, prng->v, sizeof(prng->v)); (void)tc_hmac_final(prng->v, sizeof(prng->v), &prng->h); diff --git a/examples/bluetooth/blufi/main/blufi_init.c b/examples/bluetooth/blufi/main/blufi_init.c index 8ba279edc3..ee81d678f1 100644 --- a/examples/bluetooth/blufi/main/blufi_init.c +++ b/examples/bluetooth/blufi/main/blufi_init.c @@ -233,6 +233,9 @@ esp_err_t esp_blufi_host_init(void) ble_hs_cfg.gatts_register_cb = esp_blufi_gatt_svr_register_cb; ble_hs_cfg.store_status_cb = ble_store_util_status_rr; + /* FALSE POSITIVE: BLUFI uses its own app-layer security (DH + AES), not BLE SM. + * sm_mitm/sm_sc/sm_bonding are opt-in via Kconfig to let the example show + * multiple configurations; Just Works is acceptable for BLUFI provisioning. */ ble_hs_cfg.sm_io_cap = 4; #ifdef CONFIG_EXAMPLE_BONDING ble_hs_cfg.sm_bonding = 1; @@ -278,11 +281,13 @@ esp_err_t esp_blufi_host_deinit(void) { esp_err_t ret = ESP_OK; - esp_blufi_gatt_svr_deinit(); ret = nimble_port_stop(); if (ret != ESP_OK) { return ret; } + + esp_blufi_gatt_svr_deinit(); + if (ret == 0) { esp_nimble_deinit(); }