From f96f98e60e062535baaaa20ffc4735bbb43a27b4 Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Fri, 11 Jun 2021 12:10:39 +0100 Subject: [PATCH 1/6] mbedtls: Use LowPowerTimer/Timer for timing Previously we used `gettimeofday()` for Mbed TLS timing, but its implementation provided by Mbed OS is only precise to seconds. The microsecond component of the output `struct timeval` is always set to zero. But Mbed TLS requires millisecond precision. To provide required timing precision, switch to use `LowPowerTicker` or (microsecond) `Ticker`. `LowPowerTicker` is preferred as it saves power and Mbed TLS does not require microsecond precision. --- .../mbedtls/platform/inc/timing_alt.h | 4 +-- .../mbedtls/platform/src/timing_mbed.cpp | 26 +++++++++++-------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/connectivity/mbedtls/platform/inc/timing_alt.h b/connectivity/mbedtls/platform/inc/timing_alt.h index de61d52814..533eab3d5b 100644 --- a/connectivity/mbedtls/platform/inc/timing_alt.h +++ b/connectivity/mbedtls/platform/inc/timing_alt.h @@ -24,11 +24,9 @@ #include "mbedtls/timing.h" #if defined(MBEDTLS_TIMING_ALT) -#include - struct mbedtls_timing_hr_time { - struct timeval start; + unsigned long start; }; typedef struct mbedtls_timing_delay_context diff --git a/connectivity/mbedtls/platform/src/timing_mbed.cpp b/connectivity/mbedtls/platform/src/timing_mbed.cpp index e26d2d5d77..4ee25fb1f2 100644 --- a/connectivity/mbedtls/platform/src/timing_mbed.cpp +++ b/connectivity/mbedtls/platform/src/timing_mbed.cpp @@ -25,6 +25,8 @@ #endif #include "mbedtls/timing.h" #include "drivers/Timeout.h" +#include "drivers/Timer.h" +#include "drivers/LowPowerTimer.h" #include extern "C" { @@ -46,22 +48,24 @@ extern "C" void mbedtls_set_alarm(int seconds) #if !defined(HAVE_HARDCLOCK) #define HAVE_HARDCLOCK -#include "platform/mbed_rtc_time.h" -static int hardclock_init = 0; -static struct timeval tv_init; +static int timer_init = 0; extern "C" unsigned long mbedtls_timing_hardclock(void) { - struct timeval tv_cur; +#if DEVICE_LPTICKER + static mbed::LowPowerTimer timer; +#elif DEVICE_USTICKER + static mbed::Timer timer; +#else +#error "MBEDTLS_TIMING_C requires either LPTICKER or USTICKER" +#endif - if (hardclock_init == 0) - { - gettimeofday(&tv_init, NULL); - hardclock_init = 1; + if (timer_init == 0) { + timer.reset(); + timer.start(); + timer_init = 1; } - gettimeofday(&tv_cur, NULL); - return((tv_cur.tv_sec - tv_init.tv_sec) * 1000000 - + (tv_cur.tv_usec - tv_init.tv_usec)); + return timer.elapsed_time().count(); } #endif /* !HAVE_HARDCLOCK */ From 17ae051075471b9daf7283c64af3db79822552c5 Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Fri, 11 Jun 2021 12:17:47 +0100 Subject: [PATCH 2/6] mbedtls: Add full platform implementation of timing When MBEDTLS_TIMING_C and MBEDTLS_TIMING_ALT are enabled, the Arm Compiler generates errors like the following (one for each missing symbol): Error: L6218E: Undefined symbol mbedtls_timing_get_delay Reason: The function `mbedtls_timing_self_test()` in the Mbed TLS default `timing.c` always gets compiled, if MBEDTLS_SELF_TEST is defined. And MBEDTLS_SELF_TEST is always defined, as we have a Greentea test to run some of the Mbed TLS self tests. (In the future we should try not to enable MBEDTLS_SELF_TEST except for tests, but it requires a rework in our test flow.) `mbedtls_timing_self_test()` tests (calls) the full API declared in `timing.h`, and the ARM Compiler requires all symbols referenced by all functions to be defined, even those not used by the final application. This is unlike GCC_ARM which resolves what are required. Solution: To fix the "undefined symbol" errors, we add an implementation of `mbedtls_timing_get_timer()` based on Mbed OS `LowPowerTimer` or `Timer` (depending on which one is available), and copy Mbed TLS's default `mbedtls_timing_set_delay()` and `mbedtls_timing_get_delay()` which are built on top of `mbedtls_timing_get_timer()`. This will also benefit user applications that need to enable timing in Mbed TLS. --- .../mbedtls/platform/src/timing_mbed.cpp | 77 +++++++++++++++++-- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/connectivity/mbedtls/platform/src/timing_mbed.cpp b/connectivity/mbedtls/platform/src/timing_mbed.cpp index 4ee25fb1f2..0ba81aa8e1 100644 --- a/connectivity/mbedtls/platform/src/timing_mbed.cpp +++ b/connectivity/mbedtls/platform/src/timing_mbed.cpp @@ -1,6 +1,7 @@ /* * timing.cpp * + * Copyright The Mbed TLS Contributors * Copyright (C) 2021, Arm Limited, All Rights Reserved * SPDX-License-Identifier: Apache-2.0 * @@ -46,20 +47,23 @@ extern "C" void mbedtls_set_alarm(int seconds) t.attach(handle_alarm, std::chrono::seconds(seconds)); } -#if !defined(HAVE_HARDCLOCK) -#define HAVE_HARDCLOCK -static int timer_init = 0; - -extern "C" unsigned long mbedtls_timing_hardclock(void) -{ +// The static Mbed timer here is initialized once only. +// Mbed TLS can have multiple timers (mbedtls_timing_hr_time) derived +// from the Mbed timer. #if DEVICE_LPTICKER - static mbed::LowPowerTimer timer; +static mbed::LowPowerTimer timer; #elif DEVICE_USTICKER - static mbed::Timer timer; +static mbed::Timer timer; #else #error "MBEDTLS_TIMING_C requires either LPTICKER or USTICKER" #endif +static int timer_init = 0; +#if !defined(HAVE_HARDCLOCK) +#define HAVE_HARDCLOCK + +extern "C" unsigned long mbedtls_timing_hardclock(void) +{ if (timer_init == 0) { timer.reset(); timer.start(); @@ -69,3 +73,60 @@ extern "C" unsigned long mbedtls_timing_hardclock(void) return timer.elapsed_time().count(); } #endif /* !HAVE_HARDCLOCK */ + +extern "C" unsigned long mbedtls_timing_get_timer(struct mbedtls_timing_hr_time *val, int reset) +{ + if (timer_init == 0) { + timer.reset(); + timer.start(); + timer_init = 1; + } + + if (reset) { + val->start = std::chrono::duration_cast(timer.elapsed_time()).count(); + return 0; + } else { + return std::chrono::duration_cast(timer.elapsed_time()).count() - val->start; + } +} + +/** + * Note: The following implementations come from the default timing.c + * from Mbed TLS. They are disabled in timing.c when MBEDTLS_TIMING_ALT + * is defined, but the implementation is nonetheless applicable to + * Mbed OS, so we copy them over. + */ + +extern "C" void mbedtls_timing_set_delay(void *data, uint32_t int_ms, uint32_t fin_ms) +{ + mbedtls_timing_delay_context *ctx = (mbedtls_timing_delay_context *) data; + + ctx->int_ms = int_ms; + ctx->fin_ms = fin_ms; + + if (fin_ms != 0) { + (void) mbedtls_timing_get_timer(&ctx->timer, 1); + } +} + +extern "C" int mbedtls_timing_get_delay(void *data) +{ + mbedtls_timing_delay_context *ctx = (mbedtls_timing_delay_context *) data; + unsigned long elapsed_ms; + + if (ctx->fin_ms == 0) { + return -1; + } + + elapsed_ms = mbedtls_timing_get_timer(&ctx->timer, 0); + + if (elapsed_ms >= ctx->fin_ms) { + return 2; + } + + if (elapsed_ms >= ctx->int_ms) { + return 1; + } + + return 0; +} From e16f59a2ee138f03b86ab1a3faa19f12ca320b8f Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Mon, 14 Jun 2021 14:06:35 +0100 Subject: [PATCH 3/6] timing_mbed.cpp: Check MBEDTLS_TIMING_ALT Do not compile the Mbed implementation of Mbed TLS unless MBEDTLS_TIMING_ALT is defined. This prevents a macro check error on devices that do not have LPTICKER or USTICKER when Mbed TLS timing is not enabled. --- connectivity/mbedtls/platform/src/timing_mbed.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/connectivity/mbedtls/platform/src/timing_mbed.cpp b/connectivity/mbedtls/platform/src/timing_mbed.cpp index 0ba81aa8e1..17acf6c9b9 100644 --- a/connectivity/mbedtls/platform/src/timing_mbed.cpp +++ b/connectivity/mbedtls/platform/src/timing_mbed.cpp @@ -24,6 +24,9 @@ #else #include MBEDTLS_CONFIG_FILE #endif + +#if defined(MBEDTLS_TIMING_ALT) + #include "mbedtls/timing.h" #include "drivers/Timeout.h" #include "drivers/Timer.h" @@ -130,3 +133,5 @@ extern "C" int mbedtls_timing_get_delay(void *data) return 0; } + +#endif // MBEDTLS_TIMING_ALT From ca719a96a8d3e61666b70a9f4fb19ab2fb5b6f7d Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Mon, 14 Jun 2021 14:49:31 +0100 Subject: [PATCH 4/6] mbedtls: Use LowPowerTimeout for mbedtls_set_alarm() if available The function `mbedtls_set_alarm()` is only precise to seconds, so `LowPowerTimeout` is enough and saves power. --- connectivity/mbedtls/platform/src/timing_mbed.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/connectivity/mbedtls/platform/src/timing_mbed.cpp b/connectivity/mbedtls/platform/src/timing_mbed.cpp index 17acf6c9b9..dd35cb415e 100644 --- a/connectivity/mbedtls/platform/src/timing_mbed.cpp +++ b/connectivity/mbedtls/platform/src/timing_mbed.cpp @@ -29,6 +29,7 @@ #include "mbedtls/timing.h" #include "drivers/Timeout.h" +#include "drivers/LowPowerTimeout.h" #include "drivers/Timer.h" #include "drivers/LowPowerTimer.h" #include @@ -44,7 +45,14 @@ static void handle_alarm(void) extern "C" void mbedtls_set_alarm(int seconds) { +#if DEVICE_LPTICKER + static mbed::LowPowerTimeout t; +#elif DEVICE_USTICKER static mbed::Timeout t; +#else +#error "MBEDTLS_TIMING_C requires either LPTICKER or USTICKER" +#endif + mbedtls_timing_alarmed = 0; t.attach(handle_alarm, std::chrono::seconds(seconds)); From d6f825ebf0c769c8dc83db1f1b810e1fa4c37d06 Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Fri, 11 Jun 2021 12:32:59 +0100 Subject: [PATCH 5/6] mbedtls: Run mbedtls_timing_self_test if MBEDTLS_TIMING_C This allows us to verify the support for Mbed TLS timing on Mbed OS. Note: The macros MBEDTLS_TIMING_C and MBEDTLS_TIMING_ALT are not enabled by default and need to be additionally enabled to run this test. --- .../mbedtls/tests/TESTS/mbedtls/selftest/main.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp b/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp index 1f98c25755..de8db26cf4 100644 --- a/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp +++ b/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp @@ -35,6 +35,7 @@ using namespace utest::v1; #include "mbedtls/sha512.h" #include "mbedtls/entropy.h" #include "mbedtls/entropy_poll.h" +#include "mbedtls/timing.h" #include @@ -65,6 +66,10 @@ MBEDTLS_SELF_TEST_TEST_CASE(mbedtls_sha512_self_test) MBEDTLS_SELF_TEST_TEST_CASE(mbedtls_entropy_self_test) #endif +#if defined(MBEDTLS_TIMING_C) +MBEDTLS_SELF_TEST_TEST_CASE(mbedtls_timing_self_test) +#endif + #else #warning "MBEDTLS_SELF_TEST not enabled" #endif /* MBEDTLS_SELF_TEST */ @@ -84,6 +89,10 @@ Case cases[] = { Case("mbedtls_entropy_self_test", mbedtls_entropy_self_test_test_case), #endif +#if defined(MBEDTLS_TIMING_C) + Case("mbedtls_timing_self_test", mbedtls_timing_self_test_test_case), +#endif + #endif /* MBEDTLS_SELF_TEST */ }; From 49163f0f33739c5221f0fb7d38a4ee00434d72ce Mon Sep 17 00:00:00 2001 From: Lingkai Dong Date: Mon, 14 Jun 2021 16:25:00 +0100 Subject: [PATCH 6/6] Move Mbed TLS self tests to a separate configuration We potentially save flash space by not enabling Mbed TLS self-tests by default. A new test config file, TESTS/configs/mbedtls.json, is provided to enable self tests. This newly created JSON file also enables timing in Mbed TLS so timing gets tested. --- TESTS/configs/mbedtls.json | 7 +++++++ connectivity/mbedtls/include/mbedtls/config-no-entropy.h | 2 +- connectivity/mbedtls/include/mbedtls/config.h | 2 +- connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp | 4 ++++ connectivity/mbedtls/tools/importer/adjust-config.sh | 3 +++ .../mbedtls/tools/importer/adjust-no-entropy-config.sh | 3 +++ 6 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 TESTS/configs/mbedtls.json diff --git a/TESTS/configs/mbedtls.json b/TESTS/configs/mbedtls.json new file mode 100644 index 0000000000..46be8a650c --- /dev/null +++ b/TESTS/configs/mbedtls.json @@ -0,0 +1,7 @@ +{ + "macros": [ + "MBEDTLS_SELF_TEST", + "MBEDTLS_TIMING_C", + "MBEDTLS_TIMING_ALT" + ] +} diff --git a/connectivity/mbedtls/include/mbedtls/config-no-entropy.h b/connectivity/mbedtls/include/mbedtls/config-no-entropy.h index aa298ba7b7..19df6d1435 100644 --- a/connectivity/mbedtls/include/mbedtls/config-no-entropy.h +++ b/connectivity/mbedtls/include/mbedtls/config-no-entropy.h @@ -48,7 +48,7 @@ #define MBEDTLS_PK_RSA_ALT_SUPPORT #define MBEDTLS_PKCS1_V15 #define MBEDTLS_PKCS1_V21 -#define MBEDTLS_SELF_TEST +//#define MBEDTLS_SELF_TEST #define MBEDTLS_VERSION_FEATURES #define MBEDTLS_X509_CHECK_KEY_USAGE #define MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE diff --git a/connectivity/mbedtls/include/mbedtls/config.h b/connectivity/mbedtls/include/mbedtls/config.h index 249e5e38fb..6201d9910c 100644 --- a/connectivity/mbedtls/include/mbedtls/config.h +++ b/connectivity/mbedtls/include/mbedtls/config.h @@ -1396,7 +1396,7 @@ * * Enable the checkup functions (*_self_test). */ -#define MBEDTLS_SELF_TEST +//#define MBEDTLS_SELF_TEST /** * \def MBEDTLS_SHA256_SMALLER diff --git a/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp b/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp index de8db26cf4..bc39d8584a 100644 --- a/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp +++ b/connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp @@ -31,6 +31,10 @@ using namespace utest::v1; #include MBEDTLS_CONFIG_FILE #endif +#if !defined(MBEDTLS_SELF_TEST) +#error [NOT_SUPPORTED] MBEDTLS_SELF_TEST undefined +#endif + #include "mbedtls/sha256.h" #include "mbedtls/sha512.h" #include "mbedtls/entropy.h" diff --git a/connectivity/mbedtls/tools/importer/adjust-config.sh b/connectivity/mbedtls/tools/importer/adjust-config.sh index 4825661989..143bda1560 100755 --- a/connectivity/mbedtls/tools/importer/adjust-config.sh +++ b/connectivity/mbedtls/tools/importer/adjust-config.sh @@ -117,6 +117,9 @@ conf unset MBEDTLS_SSL_TRUNCATED_HMAC conf unset MBEDTLS_PLATFORM_TIME_TYPE_MACRO +# potentially save flash space by not enabling self-tests by default +conf unset MBEDTLS_SELF_TEST + # The default size of MBEDTLS_MPI_MAX_SIZE is 1024 bytes. # In some cases, this value is set to stack buffers. # Reduce the maximal MBEDTLS_MPI_MAX_SIZE to 512 bytes, diff --git a/connectivity/mbedtls/tools/importer/adjust-no-entropy-config.sh b/connectivity/mbedtls/tools/importer/adjust-no-entropy-config.sh index a6fb8c7403..10abcc264f 100755 --- a/connectivity/mbedtls/tools/importer/adjust-no-entropy-config.sh +++ b/connectivity/mbedtls/tools/importer/adjust-no-entropy-config.sh @@ -37,3 +37,6 @@ add_code() { conf set MBEDTLS_CMAC_C conf unset MBEDTLS_CIPHER_MODE_XTS + +# potentially save flash space by not enabling self-tests by default +conf unset MBEDTLS_SELF_TEST