fix(ble/bluedroid): Null/range checks, crypto cleanup and API consistency

- smp_api.h/smp_int.h: SMP_OPCODE_ARRAY_SIZE and SecureConnectionOobDataReply
  declaration alignment
- p_256_ecc_pp/p_256_multprecision: bounds and overflow fixes in ECC/multiprecision
- smp_act: init le_key; p_dev_rec null check in smp_key_distribution;
  smp_compute_dhkey failure notify in smp_both_have_public_keys
- smp_api: early state/cb_evt check in SMP_SecureConnectionOobDataReply
- smp_cmac: input/length validation in cmac_aes_k_calculate and
  aes_cipher_msg_auth_code
- smp_keys: smp_gen_p2_4_confirm return and smp_calculate_comfirm_cont;
  smp_process_private_key/smp_compute_dhkey cleanup and peer_pub_be clear
- smp_l2c: fix callback param types with L2CAP
- smp_main: event/state bounds in smp_sm_event; smp_get_event_name default string
- smp_utils: cmd_code<SMP_OPCODE_ARRAY_SIZE and smp_cmd_build_act check;
  smp_mask_enc_key/smp_command_has_invalid_parameters bounds
This commit is contained in:
zhiweijian
2026-03-18 16:49:09 +08:00
parent 16d523e9bf
commit 50747e4f63
11 changed files with 86 additions and 36 deletions
@@ -48,6 +48,7 @@
#define SMP_OPCODE_MAX SMP_OPCODE_PAIR_KEYPR_NOTIF
#define SMP_OPCODE_MIN SMP_OPCODE_PAIRING_REQ
#define SMP_OPCODE_PAIR_COMMITM 0x0F
#define SMP_OPCODE_ARRAY_SIZE (SMP_OPCODE_PAIR_COMMITM + 1)
// #endif
/* SMP event type */
@@ -465,10 +466,10 @@ extern void SMP_SecureConnectionOobDataReply(UINT8 *p_data);
** Description This function is called to encrypt the data with the specified
** key
**
** Parameters: key - Pointer to key key[0] conatins the MSB
** Parameters: key - Pointer to key key[0] contains the MSB
** key_len - key length
** plain_text - Pointer to data to be encrypted
** plain_text[0] conatins the MSB
** plain_text[0] contains the MSB
** pt_len - plain text length
** p_out - pointer to the encrypted outputs
**
@@ -510,7 +510,7 @@ extern void smp_generate_passkey (tSMP_CB *p_cb, tSMP_INT_DATA *p_data);
extern void smp_generate_rand_cont(tSMP_CB *p_cb, tSMP_INT_DATA *p_data);
extern void smp_create_private_key(tSMP_CB *p_cb, tSMP_INT_DATA *p_data);
extern void smp_use_oob_private_key(tSMP_CB *p_cb, tSMP_INT_DATA *p_data);
extern void smp_compute_dhkey(tSMP_CB *p_cb);
extern BOOLEAN smp_compute_dhkey(tSMP_CB *p_cb);
extern void smp_calculate_local_commitment(tSMP_CB *p_cb);
extern void smp_calculate_peer_commitment(tSMP_CB *p_cb, BT_OCTET16 output_buf);
extern void smp_calculate_numeric_comparison_display_number(tSMP_CB *p_cb, tSMP_INT_DATA *p_data);
@@ -274,7 +274,9 @@ bool ECC_CheckPointIsInElliCur_P256(Point *p)
/* The function of the elliptic curve is y^2 = x^3 - 3x + b (mod q) ==>
y^2 = (x^2 - 3)*x + b (mod q),
so we calculate the x^2 - 3 value here */
x_x[0] -= 3;
DWORD three[2 * KEY_LENGTH_DWORDS_P256] = {0};
three[0] = 3;
multiprecision_sub(x_x, x_x, three, 2 * KEY_LENGTH_DWORDS_P256);
/* Using math relations. (a*b) % q = ((a%q)*(b%q)) % q ==>
(x^2 - 3)*x = (((x^2 - 3) % q) * x % q) % q */
multiprecision_fast_mod_P256(x_x_q, x_x);
@@ -262,7 +262,7 @@ void multiprecision_mult(DWORD *c, DWORD *a, DWORD *b, uint32_t keyLength)
DWORD V;
U = V = W = 0;
multiprecision_init(c, keyLength);
multiprecision_init(c, 2 * keyLength);
//assume little endian right now
for (uint32_t i = 0; i < keyLength; i++) {
@@ -340,7 +340,7 @@ void multiprecision_fast_mod(DWORD *c, DWORD *a)
c[2] += V;
V = c[2] < V;
c[2] += U;
V = c[2] < U;
V += c[2] < U;
c[3] += V;
V = c[3] < V;
c[4] += V;
@@ -594,6 +594,7 @@ void multiprecision_fast_mod_P256(DWORD *c, DWORD *a)
void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength)
{
DWORD u_local[KEY_LENGTH_DWORDS_P256];
DWORD v[KEY_LENGTH_DWORDS_P256];
DWORD A[KEY_LENGTH_DWORDS_P256 + 1];
DWORD C[KEY_LENGTH_DWORDS_P256 + 1];
@@ -605,14 +606,15 @@ void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength)
modp = curve.p;
}
multiprecision_copy(u_local, u, keyLength);
multiprecision_copy(v, modp, keyLength);
multiprecision_init(A, keyLength);
multiprecision_init(C, keyLength);
A[0] = 1;
while (!multiprecision_iszero(u, keyLength)) {
while (!(u[0] & 0x01)) { // u is even
multiprecision_rshift(u, u, keyLength);
while (!multiprecision_iszero(u_local, keyLength)) {
while (!(u_local[0] & 0x01)) { // u is even
multiprecision_rshift(u_local, u_local, keyLength);
if (!(A[0] & 0x01)) { // A is even
multiprecision_rshift(A, A, keyLength);
} else {
@@ -633,11 +635,11 @@ void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength)
}
}
if (multiprecision_compare(u, v, keyLength) >= 0) {
multiprecision_sub(u, u, v, keyLength);
if (multiprecision_compare(u_local, v, keyLength) >= 0) {
multiprecision_sub(u_local, u_local, v, keyLength);
multiprecision_sub_mod(A, A, C, keyLength);
} else {
multiprecision_sub(v, v, u, keyLength);
multiprecision_sub(v, v, u_local, keyLength);
multiprecision_sub_mod(C, C, A, keyLength);
}
}
@@ -406,7 +406,7 @@ void smp_send_id_info(tSMP_CB *p_cb, tSMP_INT_DATA *p_data)
smp_send_cmd(SMP_OPCODE_ID_ADDR, p_cb);
#if (BLE_INCLUDED == TRUE)
tBTM_LE_KEY_VALUE le_key;
tBTM_LE_KEY_VALUE le_key = {0};
if ((p_cb->peer_auth_req & SMP_AUTH_BOND) && (p_cb->loc_auth_req & SMP_AUTH_BOND)) {
btm_sec_save_le_key(p_cb->pairing_bda, BTM_LE_KEY_LID,
&le_key, TRUE);
@@ -1420,7 +1420,9 @@ void smp_key_distribution(tSMP_CB *p_cb, tSMP_INT_DATA *p_data)
if (smp_get_state() == SMP_STATE_BOND_PENDING) {
if (p_cb->derive_lk) {
tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev(p_cb->pairing_bda);
if (!(p_dev_rec->sec_flags & BTM_SEC_LE_LINK_KEY_AUTHED) &&
if (p_dev_rec == NULL) {
SMP_TRACE_WARNING("%s device record not found, skip LK derivation", __func__);
} else if (!(p_dev_rec->sec_flags & BTM_SEC_LE_LINK_KEY_AUTHED) &&
(p_dev_rec->sec_flags & BTM_SEC_LINK_KEY_AUTHED)) {
SMP_TRACE_DEBUG("%s BREDR key is higher security than existing LE keys, "
"don't derive LK from LTK", __func__);
@@ -1678,7 +1680,11 @@ void smp_both_have_public_keys(tSMP_CB *p_cb, tSMP_INT_DATA *p_data)
SMP_TRACE_DEBUG("%s\n", __func__);
/* invokes DHKey computation */
smp_compute_dhkey(p_cb);
if (!smp_compute_dhkey(p_cb)) {
UINT8 reason = SMP_PAIR_INTERNAL_ERR;
smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &reason);
return;
}
/* on slave side invokes sending local public key to the peer */
if (p_cb->role == HCI_ROLE_SLAVE) {
@@ -483,6 +483,10 @@ void SMP_SecureConnectionOobDataReply(UINT8 *p_data)
return;
}
if (p_cb->state != SMP_STATE_WAIT_APP_RSP || p_cb->cb_evt != SMP_SC_OOB_REQ_EVT) {
return;
}
/* Set local oob data when req_oob_type = SMP_OOB_BOTH */
memcpy(&p_oob->loc_oob_data, smp_get_local_oob_data(), sizeof(tSMP_LOC_OOB_DATA));
@@ -491,10 +495,6 @@ void SMP_SecureConnectionOobDataReply(UINT8 *p_data)
__FUNCTION__, p_cb->req_oob_type, p_oob->loc_oob_data.present,
p_oob->peer_oob_data.present);
if (p_cb->state != SMP_STATE_WAIT_APP_RSP || p_cb->cb_evt != SMP_SC_OOB_REQ_EVT) {
return;
}
BOOLEAN data_missing = FALSE;
switch (p_cb->req_oob_type) {
case SMP_OOB_PEER:
@@ -153,6 +153,11 @@ static BOOLEAN cmac_aes_k_calculate(BT_OCTET16 key, UINT8 *p_signature, UINT16 t
SMP_TRACE_EVENT ("cmac_aes_k_calculate ");
if (cmac_cb.round == 0) {
SMP_TRACE_ERROR("%s round is 0", __func__);
return FALSE;
}
while (i <= cmac_cb.round) {
smp_xor_128(&cmac_cb.text[(cmac_cb.round - i)*BT_OCTET16_LEN], x); /* Mi' := Mi (+) X */
@@ -304,6 +309,11 @@ BOOLEAN aes_cipher_msg_auth_code(BT_OCTET16 key, UINT8 *input, UINT16 length,
SMP_TRACE_EVENT ("%s", __func__);
if (tlen == 0 || tlen > BT_OCTET16_LEN) {
SMP_TRACE_ERROR("%s invalid tlen=%d", __func__, tlen);
return FALSE;
}
#if (SMP_CRYPTO_MBEDTLS == TRUE)
{
/*
@@ -672,10 +672,10 @@ BOOLEAN smp_gen_p1_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p1)
** Description Generate Confirm/Compare Step2:
** p2 = padding || ia || ra
**
** Returns void
** Returns FALSE if remote address unavailable, TRUE otherwise
**
*******************************************************************************/
void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2)
BOOLEAN smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2)
{
UINT8 *p = (UINT8 *)p2;
BD_ADDR remote_bda;
@@ -683,7 +683,7 @@ void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2)
SMP_TRACE_DEBUG ("smp_gen_p2_4_confirm\n");
if (!BTM_ReadRemoteConnectionAddr(p_cb->pairing_bda, remote_bda, &addr_type)) {
SMP_TRACE_ERROR("can not generate confirm p2 for unknown device\n");
return;
return FALSE;
}
SMP_TRACE_DEBUG ("smp_gen_p2_4_confirm\n");
@@ -705,6 +705,7 @@ void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2)
SMP_TRACE_DEBUG("p2 = padding || ia || ra");
smp_debug_print_nbyte_little_endian(p2, (const UINT8 *)"p2", 16);
#endif
return TRUE;
}
/*******************************************************************************
@@ -768,7 +769,10 @@ static void smp_calculate_comfirm_cont(tSMP_CB *p_cb, tSMP_ENC *p)
smp_debug_print_nbyte_little_endian (p->param_buf, (const UINT8 *)"C1", 16);
#endif
smp_gen_p2_4_confirm(p_cb, p2);
if (!smp_gen_p2_4_confirm(p_cb, p2)) {
smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &status);
return;
}
/* calculate p2 = (p1' XOR p2) */
smp_xor_128(p2, p->param_buf);
@@ -1217,6 +1221,7 @@ void smp_process_private_key(tSMP_CB *p_cb)
UINT8 priv_be[BT_OCTET32_LEN];
UINT8 pub_be[BT_OCTET32_LEN + BT_OCTET32_LEN + 1]; /* 0x04 || X (32 bytes) || Y (32 bytes) */
size_t pub_len = 0;
BOOLEAN psa_ok = FALSE;
/* Convert private key from little-endian to big-endian */
for (int i = 0; i < BT_OCTET32_LEN; i++) {
@@ -1250,11 +1255,16 @@ void smp_process_private_key(tSMP_CB *p_cb)
p_cb->loc_publ_key.x[i] = pub_be[1 + BT_OCTET32_LEN - 1 - i];
p_cb->loc_publ_key.y[i] = pub_be[33 + BT_OCTET32_LEN - 1 - i];
}
psa_ok = TRUE;
psa_pubkey_cleanup:
psa_destroy_key(key_id);
/* Clear sensitive data from stack */
memset(priv_be, 0, sizeof(priv_be));
memset(pub_be, 0, sizeof(pub_be));
if (!psa_ok) {
return;
}
#elif (SMP_CRYPTO_TINYCRYPT == TRUE)
{
UINT8 pub_key[64]; /* TinyCrypt format: X (32 bytes) || Y (32 bytes), no prefix */
@@ -1315,10 +1325,10 @@ psa_pubkey_cleanup:
** key and peer public key;
** - saves the new public key x-coordinate as DHKey.
**
** Returns void
** Returns TRUE if DHKey was computed successfully, FALSE otherwise.
**
*******************************************************************************/
void smp_compute_dhkey (tSMP_CB *p_cb)
BOOLEAN smp_compute_dhkey (tSMP_CB *p_cb)
{
SMP_TRACE_DEBUG ("%s\n", __FUNCTION__);
@@ -1330,6 +1340,7 @@ void smp_compute_dhkey (tSMP_CB *p_cb)
UINT8 peer_pub_be[BT_OCTET32_LEN + BT_OCTET32_LEN + 1]; /* 0x04 || X (32 bytes) || Y (32 bytes) */
UINT8 shared_secret[BT_OCTET32_LEN];
size_t output_len = 0;
BOOLEAN psa_ok = FALSE;
/* Convert private key from little-endian to big-endian */
for (int i = 0; i < BT_OCTET32_LEN; i++) {
@@ -1369,12 +1380,17 @@ void smp_compute_dhkey (tSMP_CB *p_cb)
for (int i = 0; i < BT_OCTET32_LEN; i++) {
p_cb->dhkey[i] = shared_secret[BT_OCTET32_LEN - 1 - i];
}
psa_ok = TRUE;
psa_dhkey_cleanup:
psa_destroy_key(key_id);
/* Clear sensitive data from stack */
memset(priv_be, 0, sizeof(priv_be));
memset(peer_pub_be, 0, sizeof(peer_pub_be));
memset(shared_secret, 0, sizeof(shared_secret));
if (!psa_ok) {
return FALSE;
}
#elif (SMP_CRYPTO_TINYCRYPT == TRUE)
{
UINT8 priv_be[BT_OCTET32_LEN];
@@ -1395,11 +1411,11 @@ psa_dhkey_cleanup:
/* Validate peer public key */
/* uECC_valid_public_key returns 0 if valid, negative value if invalid */
if (uECC_valid_public_key(peer_pub_be, uECC_secp256r1()) < 0) {
if (uECC_valid_public_key(peer_pub_be, uECC_secp256r1()) != 0) {
SMP_TRACE_ERROR("%s Invalid peer public key\n", __FUNCTION__);
memset(priv_be, 0, sizeof(priv_be));
memset(peer_pub_be, 0, sizeof(peer_pub_be));
return;
return FALSE;
}
/* Compute ECDH shared secret */
@@ -1409,7 +1425,7 @@ psa_dhkey_cleanup:
memset(priv_be, 0, sizeof(priv_be));
memset(peer_pub_be, 0, sizeof(peer_pub_be));
memset(shared_secret, 0, sizeof(shared_secret));
return;
return FALSE;
}
/* Convert shared secret from big-endian to little-endian for DHKey */
@@ -1430,9 +1446,16 @@ psa_dhkey_cleanup:
memcpy(peer_publ_key.x, p_cb->peer_publ_key.x, BT_OCTET32_LEN);
memcpy(peer_publ_key.y, p_cb->peer_publ_key.y, BT_OCTET32_LEN);
if (!ECC_CheckPointIsInElliCur_P256(&peer_publ_key)) {
SMP_TRACE_ERROR("%s Invalid peer public key\n", __FUNCTION__);
memset(private_key, 0, sizeof(private_key));
return FALSE;
}
ECC_PointMult(&new_publ_key, &peer_publ_key, (DWORD *) private_key, KEY_LENGTH_DWORDS_P256);
memcpy(p_cb->dhkey, new_publ_key.x, BT_OCTET32_LEN);
memset(private_key, 0, sizeof(private_key));
#endif /* SMP_CRYPTO_MBEDTLS */
smp_debug_print_nbyte_little_endian (p_cb->dhkey, (const UINT8 *)"DHKey",
@@ -1444,6 +1467,7 @@ psa_dhkey_cleanup:
BT_OCTET32_LEN);
smp_debug_print_nbyte_little_endian (p_cb->peer_publ_key.y, (const UINT8 *)"rem public(y)",
BT_OCTET32_LEN);
return TRUE;
}
/*******************************************************************************
@@ -118,8 +118,8 @@ static void smp_connect_callback (UINT16 channel, BD_ADDR bd_addr, BOOLEAN conne
if (memcmp(bd_addr, p_cb->pairing_bda, BD_ADDR_LEN) == 0) {
SMP_TRACE_EVENT ("%s() for pairing BDA: %08x%04x Event: %s\n",
__FUNCTION__,
(bd_addr[0] << 24) + (bd_addr[1] << 16) + (bd_addr[2] << 8) + bd_addr[3],
(bd_addr[4] << 8) + bd_addr[5],
((UINT32)bd_addr[0] << 24) + ((UINT32)bd_addr[1] << 16) + ((UINT32)bd_addr[2] << 8) + bd_addr[3],
((UINT32)bd_addr[4] << 8) + bd_addr[5],
(connected) ? "connected" : "disconnected");
if (connected) {
@@ -283,8 +283,8 @@ static void smp_br_connect_callback(UINT16 channel, BD_ADDR bd_addr, BOOLEAN con
SMP_TRACE_EVENT ("%s for pairing BDA: %08x%04x Event: %s\n",
__func__,
(bd_addr[0] << 24) + (bd_addr[1] << 16) + (bd_addr[2] << 8) + bd_addr[3],
(bd_addr[4] << 8) + bd_addr[5],
((UINT32)bd_addr[0] << 24) + ((UINT32)bd_addr[1] << 16) + ((UINT32)bd_addr[2] << 8) + bd_addr[3],
((UINT32)bd_addr[4] << 8) + bd_addr[5],
(connected) ? "connected" : "disconnected");
if (connected) {
@@ -746,7 +746,7 @@ void smp_sm_event(tSMP_CB *p_cb, tSMP_EVENT event, void *p_data)
/* lookup entry /w event & curr_state */
/* If entry is ignore, return.
* Otherwise, get state table (according to curr_state or all_state) */
if ((event <= SMP_MAX_EVT) && ( (entry = entry_table[event - 1][curr_state]) != SMP_SM_IGNORE )) {
if ((event >= 1 && event <= SMP_MAX_EVT) && ( (entry = entry_table[event - 1][curr_state]) != SMP_SM_IGNORE )) {
if (entry & SMP_ALL_TBL_MASK) {
entry &= ~SMP_ALL_TBL_MASK;
state_table = smp_all_table;
@@ -803,7 +803,7 @@ const char *smp_get_event_name(tSMP_EVENT event)
{
const char *p_str = smp_event_name[SMP_MAX_EVT];
if (event <= SMP_MAX_EVT) {
if (event >= 1 && event <= SMP_MAX_EVT) {
p_str = smp_event_name[event - 1];
}
return p_str;
@@ -351,7 +351,7 @@ BOOLEAN smp_send_cmd(UINT8 cmd_code, tSMP_CB *p_cb)
BOOLEAN sent = FALSE;
UINT8 failure = SMP_PAIR_INTERNAL_ERR;
SMP_TRACE_EVENT("smp_send_cmd on l2cap cmd_code=0x%x\n", cmd_code);
if ( cmd_code <= (SMP_OPCODE_MAX + 1 /* for SMP_OPCODE_PAIR_COMMITM */) &&
if ( cmd_code < SMP_OPCODE_ARRAY_SIZE &&
smp_cmd_build_act[cmd_code] != NULL) {
p_buf = (*smp_cmd_build_act[cmd_code])(cmd_code, p_cb);
@@ -861,6 +861,11 @@ void smp_convert_string_to_tk(BT_OCTET16 tk, UINT32 passkey)
void smp_mask_enc_key(UINT8 loc_enc_size, UINT8 *p_data)
{
SMP_TRACE_EVENT("smp_mask_enc_key\n");
if (loc_enc_size < SMP_ENCR_KEY_SIZE_MIN) {
SMP_TRACE_ERROR("smp_mask_enc_key: loc_enc_size %d below minimum %d\n",
loc_enc_size, SMP_ENCR_KEY_SIZE_MIN);
return;
}
if (loc_enc_size < BT_OCTET16_LEN) {
for (; loc_enc_size < BT_OCTET16_LEN; loc_enc_size ++) {
* (p_data + loc_enc_size) = 0;
@@ -1060,7 +1065,7 @@ BOOLEAN smp_command_has_invalid_parameters(tSMP_CB *p_cb)
SMP_TRACE_DEBUG("%s for cmd code 0x%02x\n", __func__, cmd_code);
if ((cmd_code > (SMP_OPCODE_MAX + 1 /* for SMP_OPCODE_PAIR_COMMITM */)) ||
if ((cmd_code >= SMP_OPCODE_ARRAY_SIZE) ||
(cmd_code < SMP_OPCODE_MIN)) {
SMP_TRACE_WARNING("Somehow received command with the RESERVED code 0x%02x\n", cmd_code);
return TRUE;