Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve implementation of Mbed TLS timing #14772

Merged
merged 6 commits into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions TESTS/configs/mbedtls.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"macros": [
"MBEDTLS_SELF_TEST",
"MBEDTLS_TIMING_C",
"MBEDTLS_TIMING_ALT"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ALT should be enabled only for HW crypto targets ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mbed TLS has a number of _ALT macros, but MBEDTLS_TIMING_ALT is only to enable timing based on Mbed OS API (i.e. LowPowerTicker/Ticker). This one is not related to crypto capabilities.

]
}
2 changes: 1 addition & 1 deletion connectivity/mbedtls/include/mbedtls/config-no-entropy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion connectivity/mbedtls/include/mbedtls/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -1396,7 +1396,7 @@
*
* Enable the checkup functions (*_self_test).
*/
#define MBEDTLS_SELF_TEST
//#define MBEDTLS_SELF_TEST
Patater marked this conversation as resolved.
Show resolved Hide resolved

/**
* \def MBEDTLS_SHA256_SMALLER
Expand Down
4 changes: 1 addition & 3 deletions connectivity/mbedtls/platform/inc/timing_alt.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@
#include "mbedtls/timing.h"
#if defined(MBEDTLS_TIMING_ALT)

#include <time.h>

struct mbedtls_timing_hr_time
{
struct timeval start;
unsigned long start;
Patater marked this conversation as resolved.
Show resolved Hide resolved
};

typedef struct mbedtls_timing_delay_context
Expand Down
102 changes: 90 additions & 12 deletions connectivity/mbedtls/platform/src/timing_mbed.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
* timing.cpp
*
* Copyright The Mbed TLS Contributors
Patater marked this conversation as resolved.
Show resolved Hide resolved
* Copyright (C) 2021, Arm Limited, All Rights Reserved
Patater marked this conversation as resolved.
Show resolved Hide resolved
* SPDX-License-Identifier: Apache-2.0
*
Expand All @@ -23,8 +24,14 @@
#else
#include MBEDTLS_CONFIG_FILE
#endif

#if defined(MBEDTLS_TIMING_ALT)
Patater marked this conversation as resolved.
Show resolved Hide resolved

#include "mbedtls/timing.h"
#include "drivers/Timeout.h"
#include "drivers/LowPowerTimeout.h"
#include "drivers/Timer.h"
#include "drivers/LowPowerTimer.h"
#include <chrono>

extern "C" {
Expand All @@ -38,30 +45,101 @@ 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));
}

// 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;
#elif DEVICE_USTICKER
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
#include "platform/mbed_rtc_time.h"
static int hardclock_init = 0;
static struct timeval tv_init;

extern "C" unsigned long mbedtls_timing_hardclock(void)
{
struct timeval tv_cur;

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 */

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<std::chrono::milliseconds>(timer.elapsed_time()).count();
return 0;
} else {
return std::chrono::duration_cast<std::chrono::milliseconds>(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)
Patater marked this conversation as resolved.
Show resolved Hide resolved
{
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;
}

#endif // MBEDTLS_TIMING_ALT
13 changes: 13 additions & 0 deletions connectivity/mbedtls/tests/TESTS/mbedtls/selftest/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,15 @@ 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"
#include "mbedtls/entropy_poll.h"
#include "mbedtls/timing.h"

#include <string.h>

Expand Down Expand Up @@ -65,6 +70,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)
Patater marked this conversation as resolved.
Show resolved Hide resolved
MBEDTLS_SELF_TEST_TEST_CASE(mbedtls_timing_self_test)
#endif

#else
#warning "MBEDTLS_SELF_TEST not enabled"
#endif /* MBEDTLS_SELF_TEST */
Expand All @@ -84,6 +93,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 */
};

Expand Down
3 changes: 3 additions & 0 deletions connectivity/mbedtls/tools/importer/adjust-config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Patater marked this conversation as resolved.
Show resolved Hide resolved
conf unset MBEDTLS_SELF_TEST
Patater marked this conversation as resolved.
Show resolved Hide resolved

# 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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