From 5dd8bf4fe553594e715cebc1b6ceb5ba6e7e15fc 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 9f62367ba0..431133a6dc 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 @@ -1129,6 +1129,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)) {