From fa9a433f3631ec5131ef922d6849404d3b8a5c50 Mon Sep 17 00:00:00 2001 From: Hansong Zhang Date: Mon, 31 Aug 2020 13:55:43 -0700 Subject: [PATCH] Move rfcomm security requirement to rfcomm module Bug: 159815595 Tag: #refactor Test: compile & verify basic functions working Change-Id: I2b0a15e887da8d3b5e022deaf600a99448976487 --- bta/ag/bta_ag_sdp.cc | 3 +- bta/hf_client/bta_hf_client_sdp.cc | 3 +- bta/jv/bta_jv_act.cc | 5 ++- main/shim/btm_api.cc | 4 +-- main/shim/btm_api.h | 6 ++-- stack/btm/btm_int_types.h | 3 +- stack/btm/btm_sec.cc | 66 +++++++------------------------------ stack/btm/btm_sec.h | 8 ++--- stack/include/port_api.h | 2 ++ stack/rfcomm/port_api.cc | 6 ++-- stack/rfcomm/rfc_int.h | 5 +++ stack/rfcomm/rfc_port_fsm.cc | 17 ++++++++-- stack/rfcomm/rfc_port_if.cc | 1 + stack/test/common/mock_btm_layer.cc | 7 ++-- 14 files changed, 55 insertions(+), 81 deletions(-) diff --git a/bta/ag/bta_ag_sdp.cc b/bta/ag/bta_ag_sdp.cc index 5209c2725..f59338125 100644 --- a/bta/ag/bta_ag_sdp.cc +++ b/bta/ag/bta_ag_sdp.cc @@ -38,6 +38,7 @@ #include "sdp_api.h" #include "stack/btm/btm_sec.h" #include "stack/include/btu.h" +#include "stack/include/port_api.h" #include "utl.h" using bluetooth::Uuid; @@ -273,7 +274,7 @@ void bta_ag_del_records(tBTA_AG_SCB* p_scb) { bta_ag_cb.profile[i].sdp_handle = 0; } BTM_FreeSCN(bta_ag_cb.profile[i].scn); - BTM_ClearRfcommSecurity(bta_ag_cb.profile[i].scn); + RFCOMM_ClearSecurityRecord(bta_ag_cb.profile[i].scn); bta_sys_remove_uuid(bta_ag_uuid[i]); } } diff --git a/bta/hf_client/bta_hf_client_sdp.cc b/bta/hf_client/bta_hf_client_sdp.cc index f1b922d13..9ddda9fa8 100755 --- a/bta/hf_client/bta_hf_client_sdp.cc +++ b/bta/hf_client/bta_hf_client_sdp.cc @@ -35,6 +35,7 @@ #include "osi/include/osi.h" #include "osi/include/properties.h" #include "stack/btm/btm_sec.h" +#include "stack/include/port_api.h" using bluetooth::Uuid; @@ -205,7 +206,7 @@ void bta_hf_client_del_record(tBTA_HF_CLIENT_CB_ARR* client_cb) { SDP_DeleteRecord(client_cb->sdp_handle); client_cb->sdp_handle = 0; BTM_FreeSCN(client_cb->scn); - BTM_ClearRfcommSecurity(client_cb->scn); + RFCOMM_ClearSecurityRecord(client_cb->scn); bta_sys_remove_uuid(UUID_SERVCLASS_HF_HANDSFREE); } } diff --git a/bta/jv/bta_jv_act.cc b/bta/jv/bta_jv_act.cc index b0acedb62..b6bb5fe8f 100644 --- a/bta/jv/bta_jv_act.cc +++ b/bta/jv/bta_jv_act.cc @@ -117,7 +117,6 @@ static void bta_jv_free_sec_id(uint8_t* p_sec_id) { uint8_t sec_id = *p_sec_id; *p_sec_id = 0; if (sec_id >= BTA_JV_FIRST_SERVICE_ID && sec_id <= BTA_JV_LAST_SERVICE_ID) { - BTM_SecClrService(sec_id); bta_jv_cb.sec_id[sec_id - BTA_JV_FIRST_SERVICE_ID] = 0; } } @@ -292,6 +291,7 @@ static tBTA_JV_STATUS bta_jv_free_rfc_cb(tBTA_JV_RFC_CB* p_cb, p_pcb->handle = 0; p_cb->curr_sess--; if (p_cb->curr_sess == 0) { + RFCOMM_ClearSecurityRecord(p_cb->scn); p_cb->scn = 0; bta_jv_free_sec_id(&p_cb->sec_id); p_cb->p_cback = NULL; @@ -299,6 +299,7 @@ static tBTA_JV_STATUS bta_jv_free_rfc_cb(tBTA_JV_RFC_CB* p_cb, p_cb->curr_sess = -1; } if (remove_server) { + RFCOMM_ClearSecurityRecord(p_cb->scn); bta_jv_free_sec_id(&p_cb->sec_id); } } @@ -1323,6 +1324,7 @@ void bta_jv_rfcomm_connect(tBTA_SEC sec_mask, uint8_t remote_scn, p_cback(BTA_JV_RFCOMM_CL_INIT_EVT, &bta_jv, rfcomm_slot_id); if (bta_jv.rfc_cl_init.status == BTA_JV_FAILURE) { if (sec_id) bta_jv_free_sec_id(&sec_id); + RFCOMM_ClearSecurityRecord(remote_scn); if (handle) RFCOMM_RemoveConnection(handle); } } @@ -1634,6 +1636,7 @@ void bta_jv_rfcomm_start_server(tBTA_SEC sec_mask, uint8_t local_scn, PORT_SetDataCOCallback(handle, bta_jv_port_data_co_cback); } else { if (sec_id) bta_jv_free_sec_id(&sec_id); + RFCOMM_ClearSecurityRecord(local_scn); if (handle) RFCOMM_RemoveConnection(handle); } } diff --git a/main/shim/btm_api.cc b/main/shim/btm_api.cc index 437c0c575..06090ea18 100644 --- a/main/shim/btm_api.cc +++ b/main/shim/btm_api.cc @@ -970,8 +970,8 @@ void bluetooth::shim::SendRemoteNameRequest(const RawAddress& raw_address) { } tBTM_STATUS bluetooth::shim::btm_sec_mx_access_request( - const RawAddress& bd_addr, uint16_t psm, bool is_originator, - uint32_t mx_proto_id, uint32_t mx_chan_id, tBTM_SEC_CALLBACK* p_callback, + const RawAddress& bd_addr, bool is_originator, + uint16_t security_requirement, tBTM_SEC_CALLBACK* p_callback, void* p_ref_data) { // Security has already been fulfilled by the l2cap connection, so reply back // that everything is totally fine and legit and definitely not two kids in a diff --git a/main/shim/btm_api.h b/main/shim/btm_api.h index 5a9ccccc7..50e0f135b 100644 --- a/main/shim/btm_api.h +++ b/main/shim/btm_api.h @@ -1986,9 +1986,9 @@ tBTM_STATUS BTM_BleGetEnergyInfo(tBTM_BLE_ENERGY_INFO_CBACK* p_ener_cback); */ void SendRemoteNameRequest(const RawAddress& raw_address); -tBTM_STATUS btm_sec_mx_access_request(const RawAddress& bd_addr, uint16_t psm, - bool is_originator, uint32_t mx_proto_id, - uint32_t mx_chan_id, +tBTM_STATUS btm_sec_mx_access_request(const RawAddress& bd_addr, + bool is_originator, + uint16_t security_requirement, tBTM_SEC_CALLBACK* p_callback, void* p_ref_data); diff --git a/stack/btm/btm_int_types.h b/stack/btm/btm_int_types.h index 372e17033..32e689be0 100644 --- a/stack/btm/btm_int_types.h +++ b/stack/btm/btm_int_types.h @@ -109,8 +109,7 @@ typedef struct { bool is_orig; tBTM_SEC_CALLBACK* p_callback; void* p_ref_data; - uint32_t mx_proto_id; - uint32_t mx_chan_id; + uint16_t rfcomm_security_requirement; tBT_TRANSPORT transport; tBTM_BLE_SEC_ACT sec_act; } tBTM_SEC_QUEUE_ENTRY; diff --git a/stack/btm/btm_sec.cc b/stack/btm/btm_sec.cc index 03592cf9c..43c49a50e 100644 --- a/stack/btm/btm_sec.cc +++ b/stack/btm/btm_sec.cc @@ -501,22 +501,6 @@ bool BTM_SetSecurityLevel(bool is_originator, const char* p_name, return (record_allocated); } -struct RfcommSecurityRecord { - bool need_mitm; - bool need_16_digit_pin; -}; -static std::unordered_map - legacy_stack_rfcomm_security_records; - -void BTM_SetRfcommSecurity(uint32_t scn, bool need_mitm, - bool need_16_digit_pin) { - legacy_stack_rfcomm_security_records[scn] = {need_mitm, need_16_digit_pin}; -} - -void BTM_ClearRfcommSecurity(uint32_t scn) { - legacy_stack_rfcomm_security_records.erase(scn); -} - /******************************************************************************* * * Function BTM_SecClrService @@ -1784,56 +1768,30 @@ tBTM_STATUS btm_sec_l2cap_access_req(const RawAddress& bd_addr, uint16_t psm, * ******************************************************************************/ tBTM_STATUS btm_sec_mx_access_request(const RawAddress& bd_addr, - bool is_originator, uint32_t mx_chan_id, + bool is_originator, + uint16_t security_required, tBTM_SEC_CALLBACK* p_callback, void* p_ref_data) { tBTM_SEC_DEV_REC* p_dev_rec; tBTM_STATUS rc; - uint16_t security_required; bool transport = false; /* should check PSM range in LE connection oriented L2CAP connection */ if (bluetooth::shim::is_gd_shim_enabled()) { return bluetooth::shim::btm_sec_mx_access_request( - bd_addr, BT_PSM_RFCOMM, is_originator, BTM_SEC_PROTO_RFCOMM, mx_chan_id, - p_callback, p_ref_data); - } - - /* If there is no application registered with this PSM do not allow connection - */ - if (legacy_stack_rfcomm_security_records.count(mx_chan_id) == 0) { - if (p_callback) - (*p_callback)(&bd_addr, transport, p_ref_data, BTM_MODE_UNSUPPORTED); - - BTM_TRACE_ERROR("Security Manager: MX service not found SCN:%d", - mx_chan_id); - return BTM_NO_RESOURCES; + bd_addr, is_originator, security_required, p_callback, p_ref_data); } - auto requirement = legacy_stack_rfcomm_security_records[mx_chan_id]; - BTM_TRACE_DEBUG("%s() is_originator: %d", __func__, is_originator); /* Find or get oldest record */ p_dev_rec = btm_find_or_alloc_dev(bd_addr); - if (is_originator) { - security_required = BTM_SEC_OUT_ENCRYPT | BTM_SEC_OUT_AUTHENTICATE; - if (requirement.need_mitm) security_required |= BTM_SEC_OUT_MITM; - if (requirement.need_16_digit_pin) - security_required |= BTM_SEC_IN_MIN_16_DIGIT_PIN; - } else { - security_required = BTM_SEC_IN_ENCRYPT | BTM_SEC_IN_AUTHENTICATE; - if (requirement.need_mitm) security_required |= BTM_SEC_IN_MITM; - if (requirement.need_16_digit_pin) - security_required |= BTM_SEC_IN_MIN_16_DIGIT_PIN; - } - /* there are some devices (moto phone) which connects to several services at * the same time */ /* we will process one after another */ if ((p_dev_rec->p_callback) || (btm_cb.pairing_state != BTM_PAIR_STATE_IDLE)) { - BTM_TRACE_EVENT("%s() service SCN:%d delayed state: %s", __func__, - mx_chan_id, btm_pair_state_descr(btm_cb.pairing_state)); + BTM_TRACE_EVENT("%s() delayed state: %s", __func__, + btm_pair_state_descr(btm_cb.pairing_state)); rc = BTM_CMD_STARTED; @@ -1886,8 +1844,8 @@ tBTM_STATUS btm_sec_mx_access_request(const RawAddress& bd_addr, if (rc == BTM_CMD_STARTED) { BTM_TRACE_EVENT("%s: call btm_sec_queue_mx_request", __func__); btm_sec_queue_mx_request(bd_addr, BT_PSM_RFCOMM, is_originator, - BTM_SEC_PROTO_RFCOMM, mx_chan_id, p_callback, - p_ref_data); + BTM_SEC_PROTO_RFCOMM, security_required, + p_callback, p_ref_data); } else /* rc == BTM_SUCCESS */ { /* access granted */ @@ -2129,11 +2087,11 @@ void btm_sec_check_pending_reqs(void) { /* Check that the ACL is still up before starting security procedures */ if (BTM_IsAclConnectionUp(p_e->bd_addr, p_e->transport)) { if (p_e->psm != 0) { - BTM_TRACE_EVENT( - "%s PSM:0x%04x Is_Orig:%u mx_proto_id:%u mx_chan_id:%u", __func__, - p_e->psm, p_e->is_orig, p_e->mx_proto_id, p_e->mx_chan_id); + BTM_TRACE_EVENT("%s PSM:0x%04x Is_Orig:%u", __func__, p_e->psm, + p_e->is_orig); - btm_sec_mx_access_request(p_e->bd_addr, p_e->is_orig, p_e->mx_chan_id, + btm_sec_mx_access_request(p_e->bd_addr, p_e->is_orig, + p_e->rfcomm_security_requirement, p_e->p_callback, p_e->p_ref_data); } else { BTM_SetEncryption(p_e->bd_addr, p_e->transport, p_e->p_callback, @@ -4764,8 +4722,6 @@ static bool btm_sec_queue_mx_request(const RawAddress& bd_addr, uint16_t psm, p_e->is_orig = is_orig; p_e->p_callback = p_callback; p_e->p_ref_data = p_ref_data; - p_e->mx_proto_id = mx_proto_id; - p_e->mx_chan_id = mx_chan_id; p_e->transport = BT_TRANSPORT_BR_EDR; p_e->sec_act = 0; p_e->bd_addr = bd_addr; diff --git a/stack/btm/btm_sec.h b/stack/btm/btm_sec.h index 9122d9f8f..424fd91b8 100644 --- a/stack/btm/btm_sec.h +++ b/stack/btm/btm_sec.h @@ -137,11 +137,6 @@ bool BTM_SetSecurityLevel(bool is_originator, const char* p_name, uint8_t service_id, uint16_t sec_level, uint16_t psm, uint32_t mx_proto_id, uint32_t mx_chan_id); -// Set the rfcomm security requirement -void BTM_SetRfcommSecurity(uint32_t scn, bool need_mitm, - bool need_16_digit_pin); -void BTM_ClearRfcommSecurity(uint32_t scn); - /******************************************************************************* * * Function BTM_SecClrService @@ -430,7 +425,8 @@ tBTM_STATUS btm_sec_l2cap_access_req(const RawAddress& bd_addr, uint16_t psm, * ******************************************************************************/ tBTM_STATUS btm_sec_mx_access_request(const RawAddress& bd_addr, - bool is_originator, uint32_t mx_chan_id, + bool is_originator, + uint16_t security_requirement, tBTM_SEC_CALLBACK* p_callback, void* p_ref_data); diff --git a/stack/include/port_api.h b/stack/include/port_api.h index 79245ed02..1ef25a82e 100644 --- a/stack/include/port_api.h +++ b/stack/include/port_api.h @@ -186,6 +186,8 @@ extern int RFCOMM_CreateConnectionWithSecurity( const RawAddress& bd_addr, uint16_t* p_handle, tPORT_CALLBACK* p_mgmt_cb, uint8_t service_id, uint16_t sec_mask); +extern void RFCOMM_ClearSecurityRecord(uint32_t scn); + extern int RFCOMM_CreateConnection(uint16_t uuid, uint8_t scn, bool is_server, uint16_t mtu, const RawAddress& bd_addr, uint16_t* p_handle, diff --git a/stack/rfcomm/port_api.cc b/stack/rfcomm/port_api.cc index 38a4cd1da..3613186df 100644 --- a/stack/rfcomm/port_api.cc +++ b/stack/rfcomm/port_api.cc @@ -79,13 +79,14 @@ int RFCOMM_CreateConnectionWithSecurity(uint16_t uuid, uint8_t scn, uint16_t* p_handle, tPORT_CALLBACK* p_mgmt_cb, uint8_t service_id, uint16_t sec_mask) { - BTM_SetRfcommSecurity(scn, sec_mask & (BTM_SEC_OUT_MITM | BTM_SEC_IN_MITM), - sec_mask & BTM_SEC_IN_MIN_16_DIGIT_PIN); + rfcomm_security_records[scn] = sec_mask; return RFCOMM_CreateConnection(uuid, scn, is_server, mtu, bd_addr, p_handle, p_mgmt_cb); } +extern void RFCOMM_ClearSecurityRecord(uint32_t scn) {} + /******************************************************************************* * * Function RFCOMM_CreateConnection @@ -1107,6 +1108,7 @@ int PORT_WriteData(uint16_t handle, const char* p_data, uint16_t max_len, ******************************************************************************/ void RFCOMM_Init(void) { memset(&rfc_cb, 0, sizeof(tRFC_CB)); /* Init RFCOMM control block */ + rfcomm_security_records = {}; rfc_cb.rfc.last_mux = MAX_BD_CONNECTIONS; diff --git a/stack/rfcomm/rfc_int.h b/stack/rfcomm/rfc_int.h index 22aab6640..29206ce19 100644 --- a/stack/rfcomm/rfc_int.h +++ b/stack/rfcomm/rfc_int.h @@ -28,6 +28,8 @@ #include "l2c_api.h" #include "port_int.h" +#include + /* * Define RFCOMM result codes */ @@ -238,6 +240,9 @@ typedef struct { extern tRFC_CB rfc_cb; +extern std::unordered_map + rfcomm_security_records; + /* Timer running on the multiplexor channel while no DLCI connection is open */ #define RFC_MCB_INIT_INACT_TIMER 60 /* in seconds */ diff --git a/stack/rfcomm/rfc_port_fsm.cc b/stack/rfcomm/rfc_port_fsm.cc index d60705d7a..0e00ce397 100644 --- a/stack/rfcomm/rfc_port_fsm.cc +++ b/stack/rfcomm/rfc_port_fsm.cc @@ -117,11 +117,17 @@ void rfc_port_sm_execute(tPORT* p_port, uint16_t event, void* p_data) { * ******************************************************************************/ void rfc_port_sm_state_closed(tPORT* p_port, uint16_t event, void* p_data) { + uint32_t scn = (uint32_t)(p_port->dlci / 2); switch (event) { case RFC_EVENT_OPEN: p_port->rfc.state = RFC_STATE_ORIG_WAIT_SEC_CHECK; + if (rfcomm_security_records.count(scn) == 0) { + rfc_sec_check_complete(nullptr, BT_TRANSPORT_BR_EDR, p_port, + BTM_NO_RESOURCES); + return; + } btm_sec_mx_access_request(p_port->rfc.p_mcb->bd_addr, true, - (uint32_t)(p_port->dlci / 2), + rfcomm_security_records[scn], &rfc_sec_check_complete, p_port); return; @@ -142,8 +148,13 @@ void rfc_port_sm_state_closed(tPORT* p_port, uint16_t event, void* p_data) { /* Open will be continued after security checks are passed */ p_port->rfc.state = RFC_STATE_TERM_WAIT_SEC_CHECK; - btm_sec_mx_access_request(p_port->rfc.p_mcb->bd_addr, false, - (uint32_t)(p_port->dlci / 2), + if (rfcomm_security_records.count(scn) == 0) { + rfc_sec_check_complete(nullptr, BT_TRANSPORT_BR_EDR, p_port, + BTM_NO_RESOURCES); + return; + } + btm_sec_mx_access_request(p_port->rfc.p_mcb->bd_addr, true, + rfcomm_security_records[scn], &rfc_sec_check_complete, p_port); return; diff --git a/stack/rfcomm/rfc_port_if.cc b/stack/rfcomm/rfc_port_if.cc index 097d7745f..2abd32c59 100644 --- a/stack/rfcomm/rfc_port_if.cc +++ b/stack/rfcomm/rfc_port_if.cc @@ -35,6 +35,7 @@ #include "rfcdefs.h" tRFC_CB rfc_cb; +std::unordered_map rfcomm_security_records; /******************************************************************************* * diff --git a/stack/test/common/mock_btm_layer.cc b/stack/test/common/mock_btm_layer.cc index f42607b6e..74b5a5a45 100644 --- a/stack/test/common/mock_btm_layer.cc +++ b/stack/test/common/mock_btm_layer.cc @@ -31,11 +31,11 @@ void btm_sec_abort_access_req(const RawAddress& bd_addr) { } tBTM_STATUS btm_sec_mx_access_request(const RawAddress& bd_addr, - bool is_originator, uint32_t mx_chan_id, + bool is_originator, uint16_t requirement, tBTM_SEC_CALLBACK* p_callback, void* p_ref_data) { return btm_security_internal_interface->MultiplexingProtocolAccessRequest( - bd_addr, BT_PSM_RFCOMM, is_originator, BTM_SEC_PROTO_RFCOMM, mx_chan_id, + bd_addr, BT_PSM_RFCOMM, is_originator, BTM_SEC_PROTO_RFCOMM, 0, p_callback, p_ref_data); } @@ -45,9 +45,6 @@ bool BTM_SetSecurityLevel(bool is_originator, const char* p_name, return true; } -void BTM_SetRfcommSecurity(uint32_t scn, bool need_mitm, - bool need_16_digit_pin) {} - uint16_t BTM_GetMaxPacketSize(const RawAddress& addr) { return RFCOMM_DEFAULT_MTU; } -- 2.11.0