From 72eaaff9fe93cca6751778d667a57e46845f86c2 Mon Sep 17 00:00:00 2001 From: yulong Date: Sun, 5 Aug 2018 21:18:31 +0800 Subject: [PATCH] component/bt: Fixed the vulnerability released by Bluetooth org when using public key not check in the process of ECDH encryption. 1. Add the 100 times test when the private key is generated by the random number; 2. Add the bt components to the unit-test-app/config directory. 3. Added the bt unit test case to CI. --- .gitlab-ci.yml | 6 + .../stack/smp/include/p_256_ecc_pp.h | 2 + .../bt/bluedroid/stack/smp/p_256_ecc_pp.c | 36 ++++++ components/bt/bluedroid/stack/smp/smp_act.c | 7 ++ components/bt/test/component.mk | 5 + components/bt/test/test_smp.c | 107 ++++++++++++++++++ tools/unit-test-app/configs/bt | 3 + tools/unit-test-app/configs/default | 2 +- tools/unit-test-app/configs/libsodium | 1 + tools/unit-test-app/configs/psram | 2 +- tools/unit-test-app/configs/release | 1 + tools/unit-test-app/configs/single_core | 2 +- 12 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 components/bt/test/component.mk create mode 100644 components/bt/test/test_smp.c create mode 100644 tools/unit-test-app/configs/bt diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 22b64aac1..ea363158c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1117,6 +1117,12 @@ UT_010_04: - UT_T1_RMT - psram +UT_601_01: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + IT_001_01: <<: *test_template tags: diff --git a/components/bt/bluedroid/stack/smp/include/p_256_ecc_pp.h b/components/bt/bluedroid/stack/smp/include/p_256_ecc_pp.h index 029a79ff1..f91d6056b 100644 --- a/components/bt/bluedroid/stack/smp/include/p_256_ecc_pp.h +++ b/components/bt/bluedroid/stack/smp/include/p_256_ecc_pp.h @@ -58,6 +58,8 @@ extern elliptic_curve_t curve_p256; void ECC_PointMult_Bin_NAF(Point *q, Point *p, DWORD *n, uint32_t keyLength); +bool ECC_CheckPointIsInElliCur_P256(Point *p); + #define ECC_PointMult(q, p, n, keyLength) ECC_PointMult_Bin_NAF(q, p, n, keyLength) void p_256_init_curve(UINT32 keyLength); diff --git a/components/bt/bluedroid/stack/smp/p_256_ecc_pp.c b/components/bt/bluedroid/stack/smp/p_256_ecc_pp.c index 991b6fd75..0f7ab3ec4 100644 --- a/components/bt/bluedroid/stack/smp/p_256_ecc_pp.c +++ b/components/bt/bluedroid/stack/smp/p_256_ecc_pp.c @@ -240,4 +240,40 @@ void ECC_PointMult_Bin_NAF(Point *q, Point *p, DWORD *n, uint32_t keyLength) multiprecision_mersenns_mult_mod(q->y, q->y, q->z, keyLength); } +bool ECC_CheckPointIsInElliCur_P256(Point *p) +{ + /* y^2 % q */ + DWORD y_y_q[KEY_LENGTH_DWORDS_P256] = {0x0}; + /* x^2 % q */ + DWORD x_x_q[KEY_LENGTH_DWORDS_P256] = {0x0}; + /* x % q */ + DWORD x_q[KEY_LENGTH_DWORDS_P256] = {0x0}; + /* x^2, To prevent overflow, the length of the x square here needs to + be expanded to two times the original one. */ + DWORD x_x[2*KEY_LENGTH_DWORDS_P256] = {0x0}; + /* y_y_q =(p->y)^2(mod q) */ + multiprecision_mersenns_squa_mod(y_y_q, p->y, KEY_LENGTH_DWORDS_P256); + /* Calculate the value of p->x square, x_x = (p->x)^2 */ + multiprecision_mult(x_x, p->x, p->x, KEY_LENGTH_DWORDS_P256); + /* The function of the elliptic curve is y^2 = x^3 - 3x + b (mod q) ==> + y^2 = (x^2 - 3)*x + b (mod q), + so we calculate the x^2 - 3 value here */ + x_x[0] -= 3; + /* Using math relations. (a*b) % q = ((a%q)*(b%q)) % q ==> + (x^2 - 3)*x = (((x^2 - 3) % q) * x % q) % q */ + multiprecision_fast_mod_P256(x_x_q, x_x); + /* x_x = x_x_q * x_q */ + multiprecision_mult(x_x, x_x_q, p->x, KEY_LENGTH_DWORDS_P256); + /* x_q = x_x % q */ + multiprecision_fast_mod_P256(x_q, x_x); + /* Save the result in x_x_q */ + multiprecision_add_mod(x_x_q, x_q, curve_p256.b, KEY_LENGTH_DWORDS_P256); + /* compare the y_y_q and x_x_q, see if they are on a given elliptic curve. */ + if (multiprecision_compare(y_y_q, x_x_q, KEY_LENGTH_DWORDS_P256)) { + return false; + } else { + return true; + } +} + diff --git a/components/bt/bluedroid/stack/smp/smp_act.c b/components/bt/bluedroid/stack/smp/smp_act.c index 0b5d50be2..fee4034e6 100644 --- a/components/bt/bluedroid/stack/smp/smp_act.c +++ b/components/bt/bluedroid/stack/smp/smp_act.c @@ -22,6 +22,7 @@ #include "btm_int.h" #include "stack/l2c_api.h" #include "smp_int.h" +#include "p_256_ecc_pp.h" //#include "utils/include/bt_utils.h" #if SMP_INCLUDED == TRUE @@ -668,6 +669,12 @@ 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); + /* 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)) { + SMP_TRACE_ERROR("%s, Invalid Public key.", __func__); + smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &reason); + } p_cb->flags |= SMP_PAIR_FLAG_HAVE_PEER_PUBL_KEY; smp_wait_for_both_public_keys(p_cb, NULL); diff --git a/components/bt/test/component.mk b/components/bt/test/component.mk new file mode 100644 index 000000000..5dd172bdb --- /dev/null +++ b/components/bt/test/component.mk @@ -0,0 +1,5 @@ +# +#Component Makefile +# + +COMPONENT_ADD_LDFLAGS = -Wl,--whole-archive -l$(COMPONENT_NAME) -Wl,--no-whole-archive diff --git a/components/bt/test/test_smp.c b/components/bt/test/test_smp.c new file mode 100644 index 000000000..8758f9ccf --- /dev/null +++ b/components/bt/test/test_smp.c @@ -0,0 +1,107 @@ +/* + Tests for the BLE SMP implementation +*/ + +#include +#include +#include +#include +#include +#include + +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "freertos/semphr.h" +#include "freertos/queue.h" +#include "freertos/xtensa_api.h" +#include "unity.h" +#include "esp_heap_caps.h" +#include "esp_log.h" +#include "freertos/ringbuf.h" +#include "esp_system.h" +#include "nvs_flash.h" + +#include "esp_bt.h" +#include "esp_bt_main.h" +#include "esp_bt_device.h" +#include "esp_gap_ble_api.h" + +#define TAG "ble_smp_test" + +#define KEY_LENGTH_DWORDS_P256 8 + +typedef unsigned long DWORD; +typedef uint32_t UINT32; + +typedef struct { +DWORD x[KEY_LENGTH_DWORDS_P256]; + DWORD y[KEY_LENGTH_DWORDS_P256]; + DWORD z[KEY_LENGTH_DWORDS_P256]; +} Point; + +typedef struct { + // curve's coefficients + DWORD a[KEY_LENGTH_DWORDS_P256]; + DWORD b[KEY_LENGTH_DWORDS_P256]; + + //whether a is -3 + int a_minus3; + + // prime modulus + DWORD p[KEY_LENGTH_DWORDS_P256]; + + // Omega, p = 2^m -omega + DWORD omega[KEY_LENGTH_DWORDS_P256]; + + // base point, a point on E of order r + Point G; + +} elliptic_curve_t; + +extern void ECC_PointMult_Bin_NAF(Point *q, Point *p, DWORD *n, uint32_t keyLength); +extern bool ECC_CheckPointIsInElliCur_P256(Point *p); +extern void p_256_init_curve(UINT32 keyLength); +extern elliptic_curve_t curve_p256; + +static void bt_rand(void *buf, size_t len) +{ + if (!len) { + return; + } + // Reset the buf value to the fixed value. + memset(buf, 0x55, len); + + for (int i = 0; i < (int)(len / sizeof(uint32_t)); i++) { + uint32_t rand = esp_random(); + memcpy(buf + i*sizeof(uint32_t), &rand, sizeof(uint32_t)); + } + + return; +} + +TEST_CASE("ble_smp_public_key_check", "[ble_smp]") +{ + /* We wait init finish 200ms here */ + vTaskDelay(200 / portTICK_PERIOD_MS); + Point public_key; + DWORD private_key[KEY_LENGTH_DWORDS_P256] = {[0 ... (KEY_LENGTH_DWORDS_P256 - 1)] = 0x12345678}; + p_256_init_curve(KEY_LENGTH_DWORDS_P256); + ECC_PointMult_Bin_NAF(&public_key, &(curve_p256.G), private_key, KEY_LENGTH_DWORDS_P256); + /* Check Is the public key generated by the system on the given elliptic curve */ + TEST_ASSERT(ECC_CheckPointIsInElliCur_P256(&public_key)); + /* We simulate the attacker and set the y coordinate of the public key to 0. */ + for (int i = 0; i < KEY_LENGTH_DWORDS_P256; i++) { + public_key.y[i] = 0x0; + } + /* At this point the public key should not be on the given elliptic curve. */ + TEST_ASSERT(!ECC_CheckPointIsInElliCur_P256(&public_key)); + /* Test whether the G point on the protocol is on a given elliptic curve */ + TEST_ASSERT(ECC_CheckPointIsInElliCur_P256(&(curve_p256.G))); + /* test 100 times when the private key is generated by the random number. */ + for (int j = 0; j < 100; j++) { + bt_rand(private_key, sizeof(DWORD)*KEY_LENGTH_DWORDS_P256); + ECC_PointMult_Bin_NAF(&public_key, &(curve_p256.G), private_key, KEY_LENGTH_DWORDS_P256); + /* Check Is the public key generated by the system on the given elliptic curve */ + TEST_ASSERT(ECC_CheckPointIsInElliCur_P256(&public_key)); + } +} diff --git a/tools/unit-test-app/configs/bt b/tools/unit-test-app/configs/bt new file mode 100644 index 000000000..cc83ba16d --- /dev/null +++ b/tools/unit-test-app/configs/bt @@ -0,0 +1,3 @@ +TEST_COMPONENTS=bt +CONFIG_BT_ENABLED=y +CONFIG_UNITY_FREERTOS_STACK_SIZE=12288 diff --git a/tools/unit-test-app/configs/default b/tools/unit-test-app/configs/default index a01e42947..b83aec589 100644 --- a/tools/unit-test-app/configs/default +++ b/tools/unit-test-app/configs/default @@ -1 +1 @@ -EXCLUDE_COMPONENTS=libsodium +EXCLUDE_COMPONENTS=libsodium bt diff --git a/tools/unit-test-app/configs/libsodium b/tools/unit-test-app/configs/libsodium index f43b22a4c..7828480c9 100644 --- a/tools/unit-test-app/configs/libsodium +++ b/tools/unit-test-app/configs/libsodium @@ -1,2 +1,3 @@ TEST_COMPONENTS=libsodium +EXCLUDE_COMPONENTS=bt CONFIG_UNITY_FREERTOS_STACK_SIZE=12288 diff --git a/tools/unit-test-app/configs/psram b/tools/unit-test-app/configs/psram index 34f865f04..dc74b5a2d 100644 --- a/tools/unit-test-app/configs/psram +++ b/tools/unit-test-app/configs/psram @@ -1,2 +1,2 @@ -EXCLUDE_COMPONENTS=libsodium +EXCLUDE_COMPONENTS=libsodium bt CONFIG_SPIRAM_SUPPORT=y diff --git a/tools/unit-test-app/configs/release b/tools/unit-test-app/configs/release index 6e178910f..370039d16 100644 --- a/tools/unit-test-app/configs/release +++ b/tools/unit-test-app/configs/release @@ -1,2 +1,3 @@ +EXCLUDE_COMPONENTS=bt CONFIG_OPTIMIZATION_LEVEL_RELEASE=y CONFIG_OPTIMIZATION_ASSERTIONS_SILENT=y diff --git a/tools/unit-test-app/configs/single_core b/tools/unit-test-app/configs/single_core index a985c851a..29d0350f5 100644 --- a/tools/unit-test-app/configs/single_core +++ b/tools/unit-test-app/configs/single_core @@ -1,3 +1,3 @@ -EXCLUDE_COMPONENTS=libsodium +EXCLUDE_COMPONENTS=libsodium bt CONFIG_MEMMAP_SMP=n CONFIG_FREERTOS_UNICORE=y