Merge the 2019-08-01 SPL branch from AOSP-Partner
* security-aosp-nyc-mr2-release:
Revert "DO NOT MERGE Separate SDP procedure from bonding state (1/2)"
DO NOT MERGE Fix for Bluetooth connection being dropped after HCI Read Encryption Key Size
DO NOT MERGE Separate SDP procedure from bonding state (1/2)
DO NOT MERGE Send HCI Read Encryption Key properly
DO NOT MERGE Fix potential OOB read in sdpu_get_len_from_type
DO NOT MERGE Don't persist bonds using sample LTK
DO NOT MERGE Drop Bluetooth connection with weak encryption key
Change-Id: I12dcf97029e0bddd45b8ee1f269aa4f547b47c09
diff --git a/btif/src/btif_storage.c b/btif/src/btif_storage.c
index 9562746..d3784f9 100644
--- a/btif/src/btif_storage.c
+++ b/btif/src/btif_storage.c
@@ -35,6 +35,7 @@
#include <alloca.h>
#include <assert.h>
#include <ctype.h>
+#include <log/log.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
@@ -49,6 +50,7 @@
#include "osi/include/allocator.h"
#include "osi/include/compat.h"
#include "osi/include/config.h"
+#include "osi/include/list.h"
#include "osi/include/log.h"
#include "osi/include/osi.h"
@@ -854,6 +856,47 @@
}
+/* Some devices hardcode sample LTK value from spec, instead of generating one.
+ * Treat such devices as insecure, and remove such bonds when bluetooth restarts.
+ * Removing them after disconnection is handled separately.
+ *
+ * We still allow such devices to bond in order to give the user a chance to update
+ * firmware.
+ */
+static void remove_devices_with_sample_ltk() {
+ list_t *bad_ltk = list_new(osi_free);
+
+ for (const btif_config_section_iter_t *iter = btif_config_section_begin(); iter != btif_config_section_end(); iter = btif_config_section_next(iter)) {
+ const char *name = btif_config_section_name(iter);
+ if (!string_is_bdaddr(name)) {
+ continue;
+ }
+
+ bt_bdaddr_t *bda = osi_malloc(sizeof(bt_bdaddr_t));
+ string_to_bdaddr(name, bda);
+
+ tBTA_LE_KEY_VALUE key;
+ memset(&key, 0, sizeof(key));
+
+ if (btif_storage_get_ble_bonding_key(bda, BTIF_DM_LE_KEY_PENC, (char*)&key, sizeof(tBTM_LE_PENC_KEYS)) ==
+ BT_STATUS_SUCCESS) {
+ if (is_sample_ltk(key.penc_key.ltk)) {
+ list_append(bad_ltk, (void*)bda);
+ }
+ }
+ }
+
+ for (list_node_t *sn = list_begin(bad_ltk); sn != list_end(bad_ltk); sn = list_next(sn)) {
+ android_errorWriteLog(0x534e4554, "128437297");
+ BTIF_TRACE_ERROR("%s: removing bond to device using test TLK", __func__);
+
+ bt_bdaddr_t *bda = (bt_bdaddr_t*)list_node(sn);
+ btif_storage_remove_bonded_device(bda);
+ }
+
+ list_free(bad_ltk);
+}
+
/*******************************************************************************
**
** Function btif_storage_is_device_bonded
@@ -901,6 +944,8 @@
bt_uuid_t local_uuids[BT_MAX_NUM_UUIDS];
bt_uuid_t remote_uuids[BT_MAX_NUM_UUIDS];
+ remove_devices_with_sample_ltk();
+
btif_in_fetch_bonded_devices(&bonded_devices, 1);
/* Now send the adapter_properties_cb with all adapter_properties */
diff --git a/device/src/controller.c b/device/src/controller.c
index d0a1469..bb53eb3 100644
--- a/device/src/controller.c
+++ b/device/src/controller.c
@@ -322,6 +322,8 @@
&number_of_local_supported_codecs, local_supported_codecs);
}
+ assert(HCI_READ_ENCR_KEY_SIZE_SUPPORTED(supported_commands));
+
readable = true;
return future_new_immediate(FUTURE_SUCCESS);
}
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
index ceb46ab..099d512 100644
--- a/stack/btm/btm_sec.c
+++ b/stack/btm/btm_sec.c
@@ -24,6 +24,7 @@
#define LOG_TAG "bt_btm_sec"
+#include <log/log.h>
#include <stdarg.h>
#include <string.h>
@@ -47,6 +48,9 @@
#include "gatt_int.h"
#endif
+#include "bta/sys/bta_sys.h"
+#include "bta/dm/bta_dm_int.h"
+
#define BTM_SEC_MAX_COLLISION_DELAY (5000)
extern fixed_queue_t *btu_general_alarm_queue;
@@ -4859,6 +4863,19 @@
| BTM_SEC_ROLE_SWITCHED | BTM_SEC_16_DIGIT_PIN_AUTHED);
}
+ /* Some devices hardcode sample LTK value from spec, instead of generating
+ * one. Treat such devices as insecure, and remove such bonds on
+ * disconnection.
+ */
+ if (is_sample_ltk(p_dev_rec->ble.keys.pltk)) {
+ android_errorWriteLog(0x534e4554, "128437297");
+ BTM_TRACE_ERROR("%s: removing bond to device that used sample LTK", __func__);
+
+ tBTA_DM_MSG p_data;
+ memcpy(p_data.remove_dev.bd_addr, p_dev_rec->bd_addr, BD_ADDR_LEN);
+ bta_dm_remove_device(&p_data);
+ }
+
#if BLE_INCLUDED == TRUE && SMP_INCLUDED == TRUE
if (p_dev_rec->sec_state == BTM_SEC_STATE_DISCONNECTING_BOTH)
{
diff --git a/stack/btu/btu_hcif.c b/stack/btu/btu_hcif.c
index 28ea831..5476f53 100644
--- a/stack/btu/btu_hcif.c
+++ b/stack/btu/btu_hcif.c
@@ -28,6 +28,7 @@
#define LOG_TAG "bt_btu_hcif"
#include <assert.h>
+#include <log/log.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -628,6 +629,73 @@
btm_sec_rmt_name_request_complete (bd_addr, p, status);
}
+const uint8_t MIN_KEY_SIZE = 7;
+bool read_key_send_from_key_refresh = false;
+
+static void read_encryption_key_size_complete_after_key_refresh(
+ uint8_t status, uint16_t handle, uint8_t key_size) {
+ if (status == HCI_ERR_INSUFFCIENT_SECURITY) {
+ /* If remote device stop the encryption before we call "Read Encryption Key
+ * Size", we might receive Insufficient Security, which means that link is
+ * no longer encrypted. */
+ HCI_TRACE_WARNING("%s encryption stopped on link: 0x%02x", __func__,
+ handle);
+ return;
+ }
+
+ if (status != HCI_SUCCESS) {
+ HCI_TRACE_WARNING("%s: disconnecting, status: 0x%02x", __func__, status);
+ btsnd_hcic_disconnect(handle, HCI_ERR_PEER_USER);
+ return;
+ }
+
+ if (key_size < MIN_KEY_SIZE) {
+ android_errorWriteLog(0x534e4554, "124301137");
+ HCI_TRACE_ERROR(
+ "%s encryption key too short, disconnecting. handle: 0x%02x, key_size: "
+ "%d",
+ __func__, handle, key_size);
+
+ btsnd_hcic_disconnect(handle, HCI_ERR_HOST_REJECT_SECURITY);
+ return;
+ }
+
+ btm_sec_encrypt_change(handle, status, 1 /* enc_enable */);
+}
+
+static void read_encryption_key_size_complete_after_encryption_change(
+ uint8_t status, uint16_t handle, uint8_t key_size) {
+ if (status == HCI_ERR_INSUFFCIENT_SECURITY) {
+ /* If remote device stop the encryption before we call "Read Encryption Key
+ * Size", we might receive Insufficient Security, which means that link is
+ * no longer encrypted. */
+ HCI_TRACE_WARNING("%s encryption stopped on link: 0x%02x", __func__,
+ handle);
+ return;
+ }
+
+ if (status != HCI_SUCCESS) {
+ HCI_TRACE_WARNING("%s: disconnecting, status: 0x%02x", __func__, status);
+ btsnd_hcic_disconnect(handle, HCI_ERR_PEER_USER);
+ return;
+ }
+
+ if (key_size < MIN_KEY_SIZE) {
+ android_errorWriteLog(0x534e4554, "124301137");
+ HCI_TRACE_ERROR(
+ "%s encryption key too short, disconnecting. handle: 0x%02x, key_size: "
+ "%d",
+ __func__, handle, key_size);
+
+ btsnd_hcic_disconnect(handle, HCI_ERR_HOST_REJECT_SECURITY);
+ return;
+ }
+
+ // good key size - succeed
+ btm_acl_encrypt_change(handle, status, 1 /* enable */);
+ btm_sec_encrypt_change(handle, status, 1 /* enable */);
+}
+
/*******************************************************************************
**
** Function btu_hcif_encryption_change_evt
@@ -647,8 +715,14 @@
STREAM_TO_UINT16 (handle, p);
STREAM_TO_UINT8 (encr_enable, p);
- btm_acl_encrypt_change (handle, status, encr_enable);
- btm_sec_encrypt_change (handle, status, encr_enable);
+ if (status != HCI_SUCCESS || encr_enable == 0 ||
+ BTM_IsBleConnection(handle)) {
+ btm_acl_encrypt_change (handle, status, encr_enable);
+ btm_sec_encrypt_change (handle, status, encr_enable);
+ } else {
+ read_key_send_from_key_refresh = false;
+ btsnd_hcic_read_encryption_key_size(handle);
+ }
}
/*******************************************************************************
@@ -851,6 +925,26 @@
btm_read_inq_tx_power_complete(p);
break;
+ case HCI_READ_ENCR_KEY_SIZE: {
+ UINT8 *pp = p;
+
+ UINT8 status;
+ UINT16 handle;
+ UINT8 key_size;
+
+ STREAM_TO_UINT8 (status, pp);
+ STREAM_TO_UINT16 (handle, pp);
+ STREAM_TO_UINT8 (key_size, pp);
+
+ if (read_key_send_from_key_refresh) {
+ read_encryption_key_size_complete_after_encryption_change(status, handle, key_size);
+ } else {
+ read_encryption_key_size_complete_after_key_refresh(status, handle, key_size);
+ }
+
+ }
+ break;
+
#if (BLE_INCLUDED == TRUE)
/* BLE Commands sComplete*/
case HCI_BLE_ADD_WHITE_LIST:
@@ -1657,6 +1751,7 @@
** BLE Events
***********************************************/
#if (defined BLE_INCLUDED) && (BLE_INCLUDED == TRUE)
+
static void btu_hcif_encryption_key_refresh_cmpl_evt (UINT8 *p)
{
UINT8 status;
@@ -1668,7 +1763,12 @@
if (status == HCI_SUCCESS) enc_enable = 1;
- btm_sec_encrypt_change (handle, status, enc_enable);
+ if (status != HCI_SUCCESS || BTM_IsBleConnection(handle)) {
+ btm_sec_encrypt_change (handle, status, enc_enable);
+ } else {
+ read_key_send_from_key_refresh = true;
+ btsnd_hcic_read_encryption_key_size(handle);
+ }
}
static void btu_ble_process_adv_pkt (UINT8 *p)
diff --git a/stack/hcic/hcicmds.c b/stack/hcic/hcicmds.c
index a3dae3e..ba1f6d4 100644
--- a/stack/hcic/hcicmds.c
+++ b/stack/hcic/hcicmds.c
@@ -1371,6 +1371,22 @@
return (TRUE);
}
+BOOLEAN btsnd_hcic_read_encryption_key_size(UINT16 handle) {
+ BT_HDR *p = (BT_HDR *)osi_malloc(HCI_CMD_BUF_SIZE);
+ UINT8 *pp = (UINT8 *)(p + 1);
+
+ p->len = HCIC_PREAMBLE_SIZE + HCIC_PARAM_SIZE_CMD_HANDLE;
+ p->offset = 0;
+
+ UINT16_TO_STREAM (pp, HCI_READ_ENCR_KEY_SIZE);
+ UINT8_TO_STREAM (pp, HCIC_PARAM_SIZE_CMD_HANDLE);
+
+ UINT16_TO_STREAM (pp, handle);
+
+ btu_hcif_send_cmd (LOCAL_BR_EDR_CONTROLLER_ID, p);
+ return (TRUE);
+}
+
BOOLEAN btsnd_hcic_enable_test_mode (void)
{
BT_HDR *p = (BT_HDR *)osi_malloc(HCI_CMD_BUF_SIZE);
diff --git a/stack/include/bt_types.h b/stack/include/bt_types.h
index 93f33b9..ebc1e00 100644
--- a/stack/include/bt_types.h
+++ b/stack/include/bt_types.h
@@ -22,6 +22,7 @@
#include <stdint.h>
#include <stdio.h>
#include <stdbool.h>
+#include <string.h>
#ifndef FALSE
# define FALSE false
@@ -795,4 +796,13 @@
{
bdcpy(a, bd_addr_any);
}
+
+static inline bool is_sample_ltk(const BT_OCTET16 ltk) {
+ /* Sample LTK from BT Spec 5.1 | Vol 6, Part C 1
+ * 0x4C68384139F574D836BCF34E9DFB01BF */
+ const uint8_t SAMPLE_LTK[] = {0xbf, 0x01, 0xfb, 0x9d, 0x4e, 0xf3, 0xbc, 0x36,
+ 0xd8, 0x74, 0xf5, 0x39, 0x41, 0x38, 0x68, 0x4c};
+ return memcmp(ltk, SAMPLE_LTK, BT_OCTET16_LEN) == 0;
+}
+
#endif
diff --git a/stack/include/btm_ble_api.h b/stack/include/btm_ble_api.h
index 4a9517c..e065ae3 100644
--- a/stack/include/btm_ble_api.h
+++ b/stack/include/btm_ble_api.h
@@ -1395,7 +1395,17 @@
extern void BTM_ReadConnectionAddr (BD_ADDR remote_bda, BD_ADDR local_conn_addr,
tBLE_ADDR_TYPE *p_addr_type);
-
+/*******************************************************************************
+ *
+ * Function BTM_IsBleConnection
+ *
+ * Description This function is called to check if the connection handle
+ * for an LE link
+ *
+ * Returns true if connection is LE link, otherwise false.
+ *
+ ******************************************************************************/
+extern bool BTM_IsBleConnection(uint16_t conn_handle);
/*******************************************************************************
**
diff --git a/stack/include/hcimsgs.h b/stack/include/hcimsgs.h
index 26d1f5f..ae2f966 100644
--- a/stack/include/hcimsgs.h
+++ b/stack/include/hcimsgs.h
@@ -609,6 +609,7 @@
extern BOOLEAN btsnd_hcic_get_link_quality (UINT16 handle); /* Get Link Quality */
extern BOOLEAN btsnd_hcic_read_rssi (UINT16 handle); /* Read RSSI */
+extern BOOLEAN btsnd_hcic_read_encryption_key_size (UINT16 handle); /* Read encryption key size */
extern BOOLEAN btsnd_hcic_enable_test_mode (void); /* Enable Device Under Test Mode */
extern BOOLEAN btsnd_hcic_write_pagescan_type(UINT8 type); /* Write Page Scan Type */
extern BOOLEAN btsnd_hcic_write_inqscan_type(UINT8 type); /* Write Inquiry Scan Type */
diff --git a/stack/sdp/sdp_db.c b/stack/sdp/sdp_db.c
index e24cde9..d0d293c 100644
--- a/stack/sdp/sdp_db.c
+++ b/stack/sdp/sdp_db.c
@@ -129,7 +129,11 @@
while (p < p_end)
{
type = *p++;
- p = sdpu_get_len_from_type (p, type, &len);
+ p = sdpu_get_len_from_type(p, p_end, type, &len);
+ if (p == NULL || (p + len) > p_end) {
+ SDP_TRACE_WARNING("%s: bad length", __func__);
+ break;
+ }
type = type >> 3;
if (type == UUID_DESC_TYPE)
{
diff --git a/stack/sdp/sdp_discovery.c b/stack/sdp/sdp_discovery.c
index 0e840f6..9560400 100644
--- a/stack/sdp/sdp_discovery.c
+++ b/stack/sdp/sdp_discovery.c
@@ -371,6 +371,7 @@
unsigned int cpy_len, rem_len;
UINT32 list_len;
UINT8 *p;
+ UINT8 *p_end;
UINT8 type;
#if (SDP_DEBUG_RAW == TRUE)
@@ -390,13 +391,18 @@
cpy_len = p_ccb->p_db->raw_size - p_ccb->p_db->raw_used;
list_len = p_ccb->list_len;
p = &p_ccb->rsp_list[0];
+ p_end = &p_ccb->rsp_list[0] + list_len;
if(offset)
{
cpy_len -= 1;
type = *p++;
uint8_t* old_p = p;
- p = sdpu_get_len_from_type (p, type, &list_len);
+ p = sdpu_get_len_from_type (p, p_end, type, &list_len);
+ if (p == NULL || (p + list_len) > p_end) {
+ SDP_TRACE_WARNING("%s: bad length", __func__);
+ return;
+ }
if ((int)cpy_len < (p - old_p)) {
SDP_TRACE_WARNING("%s: no bytes left for data", __func__);
return;
@@ -746,7 +752,11 @@
SDP_TRACE_WARNING ("SDP - Wrong type: 0x%02x in attr_rsp", type);
return;
}
- p = sdpu_get_len_from_type (p, type, &seq_len);
+ p = sdpu_get_len_from_type (p, p + p_ccb->list_len, type, &seq_len);
+ if (p == NULL || (p + seq_len) > (p + p_ccb->list_len)) {
+ SDP_TRACE_WARNING("%s: bad length", __func__);
+ return;
+ }
p_end = &p_ccb->rsp_list[p_ccb->list_len];
@@ -795,8 +805,8 @@
return (NULL);
}
- p = sdpu_get_len_from_type (p, type, &seq_len);
- if ((p + seq_len) > p_msg_end)
+ p = sdpu_get_len_from_type (p, p_msg_end, type, &seq_len);
+ if (p == NULL || (p + seq_len) > p_msg_end)
{
SDP_TRACE_WARNING ("SDP - Bad len in attr_rsp %d", seq_len);
return (NULL);
@@ -816,7 +826,11 @@
{
/* First get the attribute ID */
type = *p++;
- p = sdpu_get_len_from_type (p, type, &attr_len);
+ p = sdpu_get_len_from_type (p, p_msg_end, type, &attr_len);
+ if (p == NULL || (p + attr_len) > p_seq_end) {
+ SDP_TRACE_WARNING("%s: Bad len in attr_rsp %d", __func__, attr_len);
+ return (NULL);
+ }
if (((type >> 3) != UINT_DESC_TYPE) || (attr_len != 2))
{
SDP_TRACE_WARNING ("SDP - Bad type: 0x%02x or len: %d in attr_rsp", type, attr_len);
@@ -906,7 +920,11 @@
nest_level &= ~(SDP_ADDITIONAL_LIST_MASK);
type = *p++;
- p = sdpu_get_len_from_type (p, type, &attr_len);
+ p = sdpu_get_len_from_type (p, p_end, type, &attr_len);
+ if (p == NULL || (p + attr_len) > p_end) {
+ SDP_TRACE_WARNING("%s: bad length in attr_rsp", __func__);
+ return NULL;
+ }
attr_len &= SDP_DISC_ATTR_LEN_MASK;
attr_type = (type >> 3) & 0x0f;
diff --git a/stack/sdp/sdp_utils.c b/stack/sdp/sdp_utils.c
index 8682154..c5fd811 100644
--- a/stack/sdp/sdp_utils.c
+++ b/stack/sdp/sdp_utils.c
@@ -635,7 +635,7 @@
** Returns void
**
*******************************************************************************/
-UINT8 *sdpu_get_len_from_type (UINT8 *p, UINT8 type, UINT32 *p_len)
+UINT8 *sdpu_get_len_from_type (UINT8 *p, UINT8 *p_end, UINT8 type, UINT32 *p_len)
{
UINT8 u8;
UINT16 u16;
@@ -659,14 +659,26 @@
*p_len = 16;
break;
case SIZE_IN_NEXT_BYTE:
+ if (p + 1 > p_end) {
+ *p_len = 0;
+ return NULL;
+ }
BE_STREAM_TO_UINT8 (u8, p);
*p_len = u8;
break;
case SIZE_IN_NEXT_WORD:
+ if (p + 2 > p_end) {
+ *p_len = 0;
+ return NULL;
+ }
BE_STREAM_TO_UINT16 (u16, p);
*p_len = u16;
break;
case SIZE_IN_NEXT_LONG:
+ if (p + 4 > p_end) {
+ *p_len = 0;
+ return NULL;
+ }
BE_STREAM_TO_UINT32 (u32, p);
*p_len = (UINT16) u32;
break;
diff --git a/stack/sdp/sdpint.h b/stack/sdp/sdpint.h
index 0b2a82b..4763e79 100644
--- a/stack/sdp/sdpint.h
+++ b/stack/sdp/sdpint.h
@@ -284,7 +284,7 @@
extern UINT8 *sdpu_extract_attr_seq (UINT8 *p, UINT16 param_len, tSDP_ATTR_SEQ *p_seq);
extern UINT8 *sdpu_extract_uid_seq (UINT8 *p, UINT16 param_len, tSDP_UUID_SEQ *p_seq);
-extern UINT8 *sdpu_get_len_from_type (UINT8 *p, UINT8 type, UINT32 *p_len);
+extern UINT8 *sdpu_get_len_from_type (UINT8 *p, UINT8 *p_end, UINT8 type, UINT32 *p_len);
extern BOOLEAN sdpu_is_base_uuid (UINT8 *p_uuid);
extern BOOLEAN sdpu_compare_uuid_arrays (UINT8 *p_uuid1, UINT32 len1, UINT8 *p_uuid2, UINT16 len2);
extern BOOLEAN sdpu_compare_bt_uuids (tBT_UUID *p_uuid1, tBT_UUID *p_uuid2);