Skip to content

Commit

Permalink
Add support for checking security downgrade
Browse files Browse the repository at this point in the history
As a guard against the BLUFFS attack, we will need to check the security
parameters of incoming connections against cached values and disallow
connection if these parameters are downgraded or changed from their
cached values.

Future CLs will add checks during connection.  This CL adds the
functions that will be needed to perform those checks and the necessary
mocks.
Currently supported checks are : IO capabilities (must be an exact match),
Secure Connections capability (must not be a downgrade), and session key
length (must not be a downgrade).  Maximum session key length, which was
previously not cached, has been added to the device security manager
cache.

To QA: This CL is a logical no-op by itself.  Tests should be performed as described in ag/25815924 and ag/25815925/

Bug: 314331379
Test: m libbluetooth
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3cf3d9d98909787748e6135733b42be0c67e9333)
Merged-In: I972fd4a3a4d4566968d097df9f27396a821fb24f
Change-Id: I972fd4a3a4d4566968d097df9f27396a821fb24f
  • Loading branch information
Brian Delwiche authored and aoleary committed Sep 17, 2024
1 parent 1935a8a commit 415dfb5
Show file tree
Hide file tree
Showing 7 changed files with 184 additions and 0 deletions.
31 changes: 31 additions & 0 deletions system/btif/src/btif_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,13 @@ using bluetooth::groups::DeviceGroups;
#define BTIF_STORAGE_KEY_ADAPTER_SCANMODE "ScanMode"
#define BTIF_STORAGE_KEY_LOCAL_IO_CAPS "LocalIOCaps"
#define BTIF_STORAGE_KEY_LOCAL_IO_CAPS_BLE "LocalIOCapsBLE"
#define BTIF_STORAGE_KEY_MAX_SESSION_KEY_SIZE "MaxSessionKeySize"
#define BTIF_STORAGE_KEY_ADAPTER_DISC_TIMEOUT "DiscoveryTimeout"
#define BTIF_STORAGE_KEY_GATT_CLIENT_SUPPORTED "GattClientSupportedFeatures"
#define BTIF_STORAGE_KEY_GATT_CLIENT_DB_HASH "GattClientDatabaseHash"
#define BTIF_STORAGE_KEY_GATT_SERVER_SUPPORTED "GattServerSupportedFeatures"
#define BTIF_STORAGE_KEY_SECURE_CONNECTIONS_SUPPORTED \
"SecureConnectionsSupported"
#define BTIF_STORAGE_DEVICE_GROUP_BIN "DeviceGroupBin"
#define BTIF_STORAGE_CSIS_AUTOCONNECT "CsisAutoconnect"
#define BTIF_STORAGE_CSIS_SET_INFO_BIN "CsisSetInfoBin"
Expand Down Expand Up @@ -291,6 +294,14 @@ static int prop2cfg(const RawAddress* remote_bd_addr, bt_property_t* prop) {
btif_config_set_int(bdstr, BT_CONFIG_KEY_REMOTE_VER_SUBVER,
info->sub_ver);
} break;
case BT_PROPERTY_REMOTE_SECURE_CONNECTIONS_SUPPORTED:
btif_config_set_int(bdstr, BTIF_STORAGE_KEY_SECURE_CONNECTIONS_SUPPORTED,
*(uint8_t*)prop->val);
break;
case BT_PROPERTY_REMOTE_MAX_SESSION_KEY_SIZE:
btif_config_set_int(bdstr, BTIF_STORAGE_KEY_MAX_SESSION_KEY_SIZE,
*(uint8_t*)prop->val);
break;

default:
BTIF_TRACE_ERROR("Unknown prop type:%d", prop->type);
Expand Down Expand Up @@ -417,6 +428,26 @@ static int cfg2prop(const RawAddress* remote_bd_addr, bt_property_t* prop) {
}
} break;

case BT_PROPERTY_REMOTE_SECURE_CONNECTIONS_SUPPORTED: {
int val;

if (prop->len >= (int)sizeof(uint8_t)) {
ret = btif_config_get_int(
bdstr, BTIF_STORAGE_KEY_SECURE_CONNECTIONS_SUPPORTED, &val);
*(uint8_t*)prop->val = (uint8_t)val;
}
} break;

case BT_PROPERTY_REMOTE_MAX_SESSION_KEY_SIZE: {
int val;

if (prop->len >= (int)sizeof(uint8_t)) {
ret = btif_config_get_int(bdstr, BTIF_STORAGE_KEY_MAX_SESSION_KEY_SIZE,
&val);
*(uint8_t*)prop->val = (uint8_t)val;
}
} break;

default:
BTIF_TRACE_ERROR("Unknow prop type:%d", prop->type);
return false;
Expand Down
14 changes: 14 additions & 0 deletions system/include/hardware/bluetooth.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,20 @@ typedef enum {
*/
BT_PROPERTY_REMOTE_IS_COORDINATED_SET_MEMBER,

/**
* Description - Whether remote device supports Secure Connections mode
* Access mode - GET and SET.
* Data Type - uint8_t.
*/
BT_PROPERTY_REMOTE_SECURE_CONNECTIONS_SUPPORTED,

/**
* Description - Maximum observed session key for remote device
* Access mode - GET and SET.
* Data Type - uint8_t.
*/
BT_PROPERTY_REMOTE_MAX_SESSION_KEY_SIZE,

BT_PROPERTY_REMOTE_DEVICE_TIMESTAMP = 0xFF,
} bt_property_type_t;

Expand Down
2 changes: 2 additions & 0 deletions system/service/logging_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ const char* BtPropertyText(const bt_property_type_t prop) {
CASE_RETURN_TEXT(BT_PROPERTY_REMOTE_VERSION_INFO);
CASE_RETURN_TEXT(BT_PROPERTY_LOCAL_LE_FEATURES);
CASE_RETURN_TEXT(BT_PROPERTY_REMOTE_DEVICE_TIMESTAMP);
CASE_RETURN_TEXT(BT_PROPERTY_REMOTE_SECURE_CONNECTIONS_SUPPORTED);
CASE_RETURN_TEXT(BT_PROPERTY_REMOTE_MAX_SESSION_KEY_SIZE);
default:
return "Invalid property";
}
Expand Down
103 changes: 103 additions & 0 deletions system/stack/btm/btm_sec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,109 @@ static bool btm_dev_16_digit_authenticated(tBTM_SEC_DEV_REC* p_dev_rec) {
return (false);
}

/*******************************************************************************
*
* Function btm_sec_is_device_sc_downgrade
*
* Description Check for a stored device record matching the candidate
* device, and return true if the stored device has reported
* that it supports Secure Connections mode and the candidate
* device reports that it does not. Otherwise, return false.
*
* Returns bool
*
******************************************************************************/
static bool btm_sec_is_device_sc_downgrade(uint16_t hci_handle,
bool secure_connections_supported) {
if (secure_connections_supported) return false;

tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev_by_handle(hci_handle);
if (p_dev_rec == nullptr) return false;

uint8_t property_val = 0;
bt_property_t property = {
.type = BT_PROPERTY_REMOTE_SECURE_CONNECTIONS_SUPPORTED,
.len = sizeof(uint8_t),
.val = &property_val};

bt_status_t cached =
btif_storage_get_remote_device_property(&p_dev_rec->bd_addr, &property);

if (cached == BT_STATUS_FAIL) return false;

return (bool)property_val;
}

/*******************************************************************************
*
* Function btm_sec_store_device_sc_support
*
* Description Save Secure Connections support for this device to file
*
******************************************************************************/

static void btm_sec_store_device_sc_support(uint16_t hci_handle,
bool secure_connections_supported) {
tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev_by_handle(hci_handle);
if (p_dev_rec == nullptr) return;

uint8_t property_val = (uint8_t)secure_connections_supported;
bt_property_t property = {
.type = BT_PROPERTY_REMOTE_SECURE_CONNECTIONS_SUPPORTED,
.len = sizeof(uint8_t),
.val = &property_val};

btif_storage_set_remote_device_property(&p_dev_rec->bd_addr, &property);
}

/*******************************************************************************
*
* Function btm_sec_is_session_key_size_downgrade
*
* Description Check if there is a stored device record matching this
* handle, and return true if the stored record has a lower
* session key size than the candidate device.
*
* Returns bool
*
******************************************************************************/
bool btm_sec_is_session_key_size_downgrade(uint16_t hci_handle,
uint8_t key_size) {
tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev_by_handle(hci_handle);
if (p_dev_rec == nullptr) return false;

uint8_t property_val = 0;
bt_property_t property = {.type = BT_PROPERTY_REMOTE_MAX_SESSION_KEY_SIZE,
.len = sizeof(uint8_t),
.val = &property_val};

bt_status_t cached =
btif_storage_get_remote_device_property(&p_dev_rec->bd_addr, &property);

if (cached == BT_STATUS_FAIL) return false;

return property_val > key_size;
}

/*******************************************************************************
*
* Function btm_sec_update_session_key_size
*
* Description Store the max session key size to disk, if possible.
*
******************************************************************************/
void btm_sec_update_session_key_size(uint16_t hci_handle, uint8_t key_size) {
tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev_by_handle(hci_handle);
if (p_dev_rec == nullptr) return;

uint8_t property_val = key_size;
bt_property_t property = {.type = BT_PROPERTY_REMOTE_MAX_SESSION_KEY_SIZE,
.len = sizeof(uint8_t),
.val = &property_val};

btif_storage_set_remote_device_property(&p_dev_rec->bd_addr, &property);
}

/*******************************************************************************
*
* Function access_secure_service_from_temp_bond
Expand Down
23 changes: 23 additions & 0 deletions system/stack/btm/btm_sec.h
Original file line number Diff line number Diff line change
Expand Up @@ -805,5 +805,28 @@ void btm_sec_set_peer_sec_caps(uint16_t hci_handle, bool ssp_supported,
void btm_sec_cr_loc_oob_data_cback_event(const RawAddress& address,
tSMP_LOC_OOB_DATA loc_oob_data);

/*******************************************************************************
*
* Function btm_sec_is_session_key_size_downgrade
*
* Description Check if there is a stored device record matching this
* handle, and return true if the stored record has a lower
* session key size than the candidate device.
*
* Returns bool
*
******************************************************************************/
bool btm_sec_is_session_key_size_downgrade(uint16_t hci_handle,
uint8_t key_size);

/*******************************************************************************
*
* Function btm_sec_update_session_key_size
*
* Description Store the max session key size to disk, if possible.
*
******************************************************************************/
void btm_sec_update_session_key_size(uint16_t hci_handle, uint8_t key_size);

// Return DEV_CLASS (uint8_t[3]) of bda. If record doesn't exist, create one.
const uint8_t* btm_get_dev_class(const RawAddress& bda);
3 changes: 3 additions & 0 deletions system/stack/include/sec_hci_link_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ void btm_sec_auth_complete(uint16_t handle, tHCI_STATUS status);
void btm_sec_disconnected(uint16_t handle, tHCI_STATUS reason, std::string);
void btm_sec_encrypt_change(uint16_t handle, tHCI_STATUS status,
uint8_t encr_enable);
bool btm_sec_is_session_key_size_downgrade(uint16_t hci_handle,
uint8_t key_size);
void btm_sec_link_key_notification(const RawAddress& p_bda,
const Octet16& link_key, uint8_t key_type);
void btm_sec_link_key_request(const uint8_t* p_event);
Expand All @@ -46,4 +48,5 @@ void btm_sec_rmt_name_request_complete(const RawAddress* bd_addr,
const uint8_t* bd_name,
tHCI_STATUS status);
void btm_sec_update_clock_offset(uint16_t handle, uint16_t clock_offset);
void btm_sec_update_session_key_size(uint16_t hci_handle, uint8_t key_size);
void btm_simple_pair_complete(const uint8_t* p);
8 changes: 8 additions & 0 deletions system/test/mock/mock_stack_btm_sec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ bool btm_sec_is_a_bonded_dev(const RawAddress& bda) {
mock_function_count_map[__func__]++;
return false;
}
bool btm_sec_is_session_key_size_downgrade(uint16_t hci_handle,
uint8_t key_size) {
mock_function_count_map[__func__]++;
return false;
}
bool is_sec_state_equal(void* data, void* context) {
mock_function_count_map[__func__]++;
return false;
Expand Down Expand Up @@ -313,6 +318,9 @@ void btm_sec_set_peer_sec_caps(uint16_t hci_handle, bool ssp_supported,
void btm_sec_update_clock_offset(uint16_t handle, uint16_t clock_offset) {
mock_function_count_map[__func__]++;
}
void btm_sec_update_session_key_size(uint16_t hci_handle, uint8_t key_size) {
mock_function_count_map[__func__]++;
}
void btm_simple_pair_complete(const uint8_t* p) {
mock_function_count_map[__func__]++;
}

0 comments on commit 415dfb5

Please sign in to comment.