From fa8dc3280021e96952ec582d0a853c795fe8ba37 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 691a3b7bd1..dc93b39434 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1009,6 +1009,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 029a79ff16..f91d6056b2 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 991b6fd757..0f7ab3ec41 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 ee73e9289f..bf3fe7def6 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 0000000000..5dd172bdb7 --- /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 0000000000..8758f9ccf4 --- /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 0000000000..cc83ba16df --- /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 a01e42947d..b83aec589e 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 f43b22a4c7..7828480c97 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 34f865f04a..dc74b5a2d5 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 6e178910f7..370039d160 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 a985c851aa..29d0350f5f 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