Merge pull request #10607 from gilles-peskine-arm/timing-use-mstime

Simplify MBEDTLS_TIMING_C to use mbedtls_ms_time()

ChangeLog.d/timing.txt

@@ -0,0 +1,13 @@
+API changes
+   * MBEDTLS_TIMING_C now requires MBEDTLS_HAVE_TIME to be enabled in the
+     TF-PSA-Crypto configuration, unless MBEDTLS_TIMING_ALT is enabled.
+     As a benefit, platforms where the default implementation is not
+     supported now only need to implement MBEDTLS_PLATFORM_MS_TIME_ALT.
+   * When MBEDTLS_TIMING_ALT is enabled, the function
+     mbedtls_timing_get_timer() now returns unsigned long long instead
+     of unsigned long.
+
+Bugfix
+   * mbedtls_timing_get_delay() now correctly treats a timer as expired
+     after more than 2^32 ms (about 49 days) on platforms where long is
+     a 32-bit type. Fixes #10613.

configs/config-ccm-psk-dtls1_2.h

@@ -29,7 +29,7 @@
 #define MBEDTLS_SSL_COOKIE_C
 #define MBEDTLS_SSL_SRV_C
 #define MBEDTLS_SSL_TLS_C
-#define MBEDTLS_TIMING_C
+#define MBEDTLS_TIMING_C //Only used by test programs
 
 /* TLS protocol feature support */
 #define MBEDTLS_KEY_EXCHANGE_PSK_ENABLED

configs/config-symmetric-only.h

@@ -12,5 +12,5 @@
 #define MBEDTLS_ERROR_STRERROR_DUMMY
 #define MBEDTLS_VERSION_FEATURES
 
-#define MBEDTLS_TIMING_C
+#define MBEDTLS_TIMING_C //Only for benchmarking
 #define MBEDTLS_VERSION_C

configs/crypto-config-ccm-psk-tls1_2.h

@@ -26,7 +26,8 @@
 #define MBEDTLS_PSA_CRYPTO_C
 
 /* System support */
-//#define MBEDTLS_HAVE_TIME /* Optionally used in Hello messages */
+/* Optionally used in Hello messages. Needed for DTLS testing. */
+#define MBEDTLS_HAVE_TIME
 /* Other MBEDTLS_HAVE_XXX flags irrelevant for this configuration */
 
 #define MBEDTLS_CTR_DRBG_C

configs/crypto-config-thread.h

@@ -48,6 +48,7 @@
 
 /* System support */
 #define MBEDTLS_HAVE_ASM
+#define MBEDTLS_HAVE_TIME //Only used by test programs
 
 #define MBEDTLS_AES_ROM_TABLES
 #define MBEDTLS_ECP_NIST_OPTIM

include/mbedtls/mbedtls_config.h

@@ -63,22 +63,9 @@
 /**
  * \def MBEDTLS_TIMING_C
  *
- * Enable the semi-portable timing interface.
+ * Enable a timer interface used by some sample and test programs.
  *
- * \note The provided implementation only works on POSIX/Unix (including Linux,
- * BSD and OS X) and Windows. On other platforms, you can either disable that
- * module and provide your own implementations of the callbacks needed by
- * \c mbedtls_ssl_set_timer_cb() for DTLS, or leave it enabled and provide
- * your own implementation of the whole module by setting
- * \c MBEDTLS_TIMING_ALT in the current file.
- *
- * \note The timing module will include time.h on suitable platforms
- *       regardless of the setting of MBEDTLS_HAVE_TIME, unless
- *       MBEDTLS_TIMING_ALT is used. See timing.c for more information.
- *
- * \note See also our Knowledge Base article about porting to a new
- * environment:
- * https://mbed-tls.readthedocs.io/en/latest/kb/how-to/how-do-i-port-mbed-tls-to-a-new-environment-OS
+ * Requires: MBEDTLS_HAVE_TIME or MBEDTLS_TIMING_ALT
  *
  * Module:  library/timing.c
  */

include/mbedtls/timing.h

@@ -23,11 +23,22 @@ extern "C" {
 // Regular implementation
 //
 
+#if defined(MBEDTLS_HAVE_TIME)
+#include <mbedtls/platform_time.h>
+#endif
+
 /**
  * \brief          timer structure
  */
 struct mbedtls_timing_hr_time {
-    uint64_t MBEDTLS_PRIVATE(opaque)[4];
+#if defined(MBEDTLS_HAVE_TIME)
+    mbedtls_ms_time_t ms;
+#else
+    /* Without MBEDTLS_HAVE_TIME, we expose the type definitions and
+     * function declarations, but they can't be implemented. We do
+     * need to write something here. */
+    unsigned MBEDTLS_PRIVATE(unused);
+#endif
 };
 
 /**
@@ -44,7 +55,7 @@ typedef struct mbedtls_timing_delay_context {
 #endif /* MBEDTLS_TIMING_ALT */
 
 /* Internal use */
-unsigned long mbedtls_timing_get_timer(struct mbedtls_timing_hr_time *val, int reset);
+unsigned long long mbedtls_timing_get_timer(struct mbedtls_timing_hr_time *val, int reset);
 
 /**
  * \brief          Set a pair of delays to watch

library/mbedtls_check_config.h

@@ -358,5 +358,10 @@
 #error  "MBEDTLS_PKCS7_C is defined, but not all prerequisites"
 #endif
 
+#if defined(MBEDTLS_TIMING_C) && \
+    !(defined(MBEDTLS_HAVE_TIME) || defined(MBEDTLS_TIMING_ALT))
+#error "MBEDTLS_TIMING_C requires either MBEDTLS_HAVE_TIME or MBEDTLS_TIMING_ALT"
+#endif
+
 /* *INDENT-ON* */
 #endif /* MBEDTLS_CHECK_CONFIG_H */

library/timing.c

@@ -13,95 +13,17 @@
 
 #if !defined(MBEDTLS_TIMING_ALT)
 
-#if !defined(unix) && !defined(__unix__) && !defined(__unix) && \
-    !defined(__APPLE__) && !defined(_WIN32) && !defined(__QNXNTO__) && \
-    !defined(__HAIKU__) && !defined(__midipix__)
-#error "This module only works on Unix and Windows, see MBEDTLS_TIMING_C in mbedtls_config.h"
-#endif
-
-#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
-
-#include <windows.h>
-#include <process.h>
-
-struct _hr_time {
-    LARGE_INTEGER start;
-};
-
-#else
-
-#include <unistd.h>
-#include <sys/types.h>
-#include <signal.h>
-/* time.h should be included independently of MBEDTLS_HAVE_TIME. If the
- * platform matches the ifdefs above, it will be used. */
-#include <time.h>
-#include <sys/time.h>
-struct _hr_time {
-    struct timeval start;
-};
-#endif /* _WIN32 && !EFIX64 && !EFI32 */
-
-/**
- * \brief          Return the elapsed time in milliseconds
- *
- * \warning        May change without notice
- *
- * \param val      points to a timer structure
- * \param reset    If 0, query the elapsed time. Otherwise (re)start the timer.
- *
- * \return         Elapsed time since the previous reset in ms. When
- *                 restarting, this is always 0.
- *
- * \note           To initialize a timer, call this function with reset=1.
- *
- *                 Determining the elapsed time and resetting the timer is not
- *                 atomic on all platforms, so after the sequence
- *                 `{ get_timer(1); ...; time1 = get_timer(1); ...; time2 =
- *                 get_timer(0) }` the value time1+time2 is only approximately
- *                 the delay since the first reset.
- */
-#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
-
-unsigned long mbedtls_timing_get_timer(struct mbedtls_timing_hr_time *val, int reset)
+unsigned long long mbedtls_timing_get_timer(struct mbedtls_timing_hr_time *val, int reset)
 {
-    struct _hr_time *t = (struct _hr_time *) val;
-
     if (reset) {
-        QueryPerformanceCounter(&t->start);
+        val->ms = mbedtls_ms_time();
         return 0;
     } else {
-        unsigned long delta;
-        LARGE_INTEGER now, hfreq;
-        QueryPerformanceCounter(&now);
-        QueryPerformanceFrequency(&hfreq);
-        delta = (unsigned long) ((now.QuadPart - t->start.QuadPart) * 1000ul
-                                 / hfreq.QuadPart);
-        return delta;
+        mbedtls_ms_time_t now = mbedtls_ms_time();
+        return now - val->ms;
     }
 }
 
-#else /* _WIN32 && !EFIX64 && !EFI32 */
-
-unsigned long mbedtls_timing_get_timer(struct mbedtls_timing_hr_time *val, int reset)
-{
-    struct _hr_time *t = (struct _hr_time *) val;
-
-    if (reset) {
-        gettimeofday(&t->start, NULL);
-        return 0;
-    } else {
-        unsigned long delta;
-        struct timeval now;
-        gettimeofday(&now, NULL);
-        delta = (now.tv_sec  - t->start.tv_sec) * 1000ul
-                + (now.tv_usec - t->start.tv_usec) / 1000;
-        return delta;
-    }
-}
-
-#endif /* _WIN32 && !EFIX64 && !EFI32 */
-
 /*
  * Set delays to watch
  */
@@ -123,7 +45,7 @@ void mbedtls_timing_set_delay(void *data, uint32_t int_ms, uint32_t fin_ms)
 int mbedtls_timing_get_delay(void *data)
 {
     mbedtls_timing_delay_context *ctx = (mbedtls_timing_delay_context *) data;
-    unsigned long elapsed_ms;
+    unsigned long long elapsed_ms;
 
     if (ctx->fin_ms == 0) {
         return -1;

programs/test/udp_proxy.c

@@ -360,7 +360,9 @@ static unsigned elapsed_time(void)
         return 0;
     }
 
-    return mbedtls_timing_get_timer(&hires, 0);
+    /* Wraps after ~49.7 days (assuming 32-bit int).
+     * Don't run udp_proxy that long! */
+    return (unsigned) mbedtls_timing_get_timer(&hires, 0);
 }
 
 typedef struct {

programs/x509/load_roots.c

@@ -150,7 +150,9 @@ int main(int argc, char *argv[])
         }
         mbedtls_printf(".");
     }
-    ms = mbedtls_timing_get_timer(&timer, 0);
+    /* On 64-bit Windows and 32-bit platforms, this wraps after about
+     * 49.7 days. This shouldn't be a problem in practice. */
+    ms = (unsigned long) mbedtls_timing_get_timer(&timer, 0);
     mbedtls_printf("\n%u iterations -> %lu ms\n", opt.iterations, ms);
     exit_code = MBEDTLS_EXIT_SUCCESS;
 

tests/scripts/components-configuration-crypto.sh

@@ -528,7 +528,7 @@ component_test_crypto_for_psa_service () {
 component_build_crypto_baremetal () {
   msg "build: make, crypto only, baremetal config"
   scripts/config.py crypto_baremetal
-  CFLAGS="-O1 -I$PWD/framework/tests/include/baremetal-override/" cmake .
+  CFLAGS="-O1 -I$PWD/framework/tests/include/baremetal-override/ -DMBEDTLS_TEST_PLATFORM_IS_NOT_UNIXLIKE" cmake .
   cmake --build .
   ctest
   are_empty_libraries library/libmbedx509.* library/libmbedtls.*

tests/scripts/components-configuration.sh

@@ -220,7 +220,7 @@ component_test_full_deprecated_warning () {
 component_build_baremetal () {
   msg "build: make, baremetal config"
   scripts/config.py baremetal
-  $MAKE_COMMAND CFLAGS="-O1 -Werror -I$PWD/framework/tests/include/baremetal-override/"
+  $MAKE_COMMAND CFLAGS="-O1 -Werror -I$PWD/framework/tests/include/baremetal-override/ -DMBEDTLS_TEST_PLATFORM_IS_NOT_UNIXLIKE"
 }
 
 support_build_baremetal () {