From d73ebb570bcc2d529b0ff64d97804b70723fbb99 Mon Sep 17 00:00:00 2001 From: Chinmay Chhajed Date: Tue, 22 Dec 2020 12:36:07 +0530 Subject: [PATCH] Bluedroid: Fixes for some vulnerabilities. This commit fixes 'Impersonation in Passkey entry protocol' (CVE-2020-26558) and suggests fixes for other vulnerabilites like 'Impersonation in the Pin Pairing Protocol' (CVE-2020-26555) and 'Authentication of the LE Legacy Pairing Protocol' CVE-2020-26558 can be easily implemented if the peer device can impersonate our public key. This commit adds a check by comparing our and received public key and returns failed pairing if keys are same. This commit also adds comments suggesting to use secure connection when supported by all devices. --- .../bluedroid/api/include/api/esp_gap_ble_api.h | 14 ++++++++++++++ components/bt/host/bluedroid/stack/smp/smp_act.c | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/components/bt/host/bluedroid/api/include/api/esp_gap_ble_api.h b/components/bt/host/bluedroid/api/include/api/esp_gap_ble_api.h index 4bee1667b7..f3dfaa9505 100644 --- a/components/bt/host/bluedroid/api/include/api/esp_gap_ble_api.h +++ b/components/bt/host/bluedroid/api/include/api/esp_gap_ble_api.h @@ -1139,6 +1139,20 @@ esp_err_t esp_ble_gap_clean_duplicate_scan_exceptional_list(esp_duplicate_scan_e /** * @brief Set a GAP security parameter value. Overrides the default value. * +* Secure connection is highly recommended to avoid some major +* vulnerabilities like 'Impersonation in the Pin Pairing Protocol' +* (CVE-2020-26555) and 'Authentication of the LE Legacy Pairing +* Protocol'. +* +* To accept only `secure connection mode`, it is necessary do as following: +* +* 1. Set bit `ESP_LE_AUTH_REQ_SC_ONLY` (`param_type` is +* `ESP_BLE_SM_AUTHEN_REQ_MODE`), bit `ESP_LE_AUTH_BOND` and bit +* `ESP_LE_AUTH_REQ_MITM` is optional as required. +* +* 2. Set to `ESP_BLE_ONLY_ACCEPT_SPECIFIED_AUTH_ENABLE` (`param_type` is +* `ESP_BLE_SM_ONLY_ACCEPT_SPECIFIED_SEC_AUTH`). +* * @param[in] param_type : the type of the param which to be set * @param[in] value : the param value * @param[in] len : the length of the param value diff --git a/components/bt/host/bluedroid/stack/smp/smp_act.c b/components/bt/host/bluedroid/stack/smp/smp_act.c index 9f821b591e..0ff45ad4aa 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_act.c +++ b/components/bt/host/bluedroid/stack/smp/smp_act.c @@ -760,6 +760,19 @@ void smp_process_pairing_public_key(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) STREAM_TO_ARRAY(p_cb->peer_publ_key.x, p, BT_OCTET32_LEN); STREAM_TO_ARRAY(p_cb->peer_publ_key.y, p, BT_OCTET32_LEN); + + /* Check if the peer device's and own public key are not same. If they are same then + * return pairing fail. This check is needed to avoid 'Impersonation in Passkey entry + * protocol' vulnerability (CVE-2020-26558).*/ + if ((memcmp(p_cb->loc_publ_key.x, p_cb->peer_publ_key.x, sizeof(BT_OCTET32)) == 0) && + (memcmp(p_cb->loc_publ_key.y, p_cb->peer_publ_key.y, sizeof(BT_OCTET32)) == 0)) { + p_cb->status = SMP_PAIR_AUTH_FAIL; + p_cb->failure = SMP_PAIR_AUTH_FAIL; + reason = SMP_PAIR_AUTH_FAIL; + SMP_TRACE_ERROR("%s, Peer and own device cannot have same public key.", __func__); + smp_sm_event(p_cb, SMP_PAIRING_FAILED_EVT, &reason); + return ; + } /* In order to prevent the x and y coordinates of the public key from being modified, we need to check whether the x and y coordinates are on the given elliptic curve. */ if (!ECC_CheckPointIsInElliCur_P256((Point *)&p_cb->peer_publ_key)) {