From 33406b645dea9a10a8ebf08e98b1c5d8face1b98 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 18:48:39 +0100 Subject: [PATCH 01/21] Add a metatest program This program can be used to validate that things that should be detected as test failures are indeed caught, either by setting the test result to MBEDTLS_TEST_RESULT_FAILED or by aborting the program. Signed-off-by: Gilles Peskine --- programs/.gitignore | 1 + programs/Makefile | 5 +++ programs/test/CMakeLists.txt | 1 + programs/test/metatest.c | 81 ++++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 programs/test/metatest.c diff --git a/programs/.gitignore b/programs/.gitignore index a641c31c45..2826a18a49 100644 --- a/programs/.gitignore +++ b/programs/.gitignore @@ -56,6 +56,7 @@ test/cpp_dummy_build test/cpp_dummy_build.cpp test/dlopen test/ecp-bench +test/metatest test/query_compile_time_config test/query_included_headers test/selftest diff --git a/programs/Makefile b/programs/Makefile index 116883b836..7b538da4b8 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -123,6 +123,7 @@ APPS = \ ssl/ssl_server \ ssl/ssl_server2 \ test/benchmark \ + test/metatest \ test/query_compile_time_config \ test/query_included_headers \ test/selftest \ @@ -413,6 +414,10 @@ test/dlopen$(EXEXT): test/dlopen.c $(DEP) $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/dlopen.c $(LDFLAGS) $(DLOPEN_LDFLAGS) -o $@ endif +test/metatest$(EXEXT): test/metatest.c $(DEP) + echo " CC test/metatest.c" + $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ + test/query_config.o: test/query_config.c test/query_config.h $(DEP) echo " CC test/query_config.c" $(CC) $(LOCAL_CFLAGS) $(CFLAGS) -c test/query_config.c -o $@ diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index a75f8d9239..8469d064d2 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -3,6 +3,7 @@ set(libs ) set(executables_libs + metatest query_included_headers selftest udp_proxy diff --git a/programs/test/metatest.c b/programs/test/metatest.c new file mode 100644 index 0000000000..f03e5e7038 --- /dev/null +++ b/programs/test/metatest.c @@ -0,0 +1,81 @@ +/** \file metatest.c + * + * \brief Test features of the test framework. + */ + +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#define MBEDTLS_ALLOW_PRIVATE_ACCESS + +#include +#include "test/helpers.h" + +#include +#include + + +/****************************************************************/ +/* Command line entry point */ +/****************************************************************/ + +typedef struct { + const char *name; + const char *platform; + void (*entry_point)(const char *name); +} metatest_t; + +metatest_t metatests[] = { + { NULL, NULL, NULL } +}; + +static void help(FILE *out, const char *argv0) +{ + mbedtls_fprintf(out, "Usage: %s list|TEST\n", argv0); + mbedtls_fprintf(out, "Run a meta-test that should cause a test failure.\n"); + mbedtls_fprintf(out, "With 'list', list the available tests and their platform requirement.\n"); +} + +int main(int argc, char *argv[]) +{ + const char *argv0 = argc > 0 ? argv[0] : "metatest"; + if (argc != 2) { + help(stderr, argv0); + mbedtls_exit(MBEDTLS_EXIT_FAILURE); + } + + /* Support "-help", "--help", "--list", etc. */ + const char *command = argv[1]; + while (*command == '-') { + ++command; + } + + if (strcmp(argv[1], "help") == 0) { + help(stdout, argv0); + mbedtls_exit(MBEDTLS_EXIT_SUCCESS); + } + if (strcmp(argv[1], "list") == 0) { + for (const metatest_t *p = metatests; p->name != NULL; p++) { + mbedtls_printf("%s %s\n", p->name, p->platform); + } + mbedtls_exit(MBEDTLS_EXIT_SUCCESS); + } + + for (const metatest_t *p = metatests; p->name != NULL; p++) { + if (strcmp(argv[1], p->name) == 0) { + mbedtls_printf("Running metatest %s...\n", argv[1]); + p->entry_point(argv[1]); + mbedtls_printf("Running metatest %s... done, result=%d\n", + argv[1], (int) mbedtls_test_info.result); + mbedtls_exit(mbedtls_test_info.result == MBEDTLS_TEST_RESULT_SUCCESS ? + MBEDTLS_EXIT_SUCCESS : + MBEDTLS_EXIT_FAILURE); + } + } + + mbedtls_fprintf(stderr, "%s: FATAL: No such metatest: %s\n", + argv0, command); + mbedtls_exit(MBEDTLS_EXIT_FAILURE); +} From f309fbf0d5c6da465ecda4b1ce563d0c59dc5534 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 18:49:52 +0100 Subject: [PATCH 02/21] Validate that test_fail causes a test failure Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index f03e5e7038..fab3b1f53f 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -17,6 +17,17 @@ #include +/****************************************************************/ +/* Test framework features */ +/****************************************************************/ + +void meta_test_fail(const char *name) +{ + (void) name; + mbedtls_test_fail("Forced test failure", __LINE__, __FILE__); +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -28,6 +39,7 @@ typedef struct { } metatest_t; metatest_t metatests[] = { + { "test_fail", "any", meta_test_fail }, { NULL, NULL, NULL } }; From 80ba832be6a1c909aecf031f42a8b0a53cef9089 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:23:26 +0100 Subject: [PATCH 03/21] Metatests for null pointer dereference Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index fab3b1f53f..ba3ec94439 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -28,6 +28,29 @@ void meta_test_fail(const char *name) } +/****************************************************************/ +/* Platform features */ +/****************************************************************/ + +void null_pointer_dereference(const char *name) +{ + (void) name; + char *p; + memset(&p, 0, sizeof(p)); + volatile char c; + c = *p; + (void) c; +} + +void null_pointer_call(const char *name) +{ + (void) name; + void (*p)(void); + memset(&p, 0, sizeof(p)); + p(); +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -40,6 +63,8 @@ typedef struct { metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, + { "null_dereference", "any", null_pointer_dereference }, + { "null_call", "any", null_pointer_call }, { NULL, NULL, NULL } }; From f109664448dc71c134521f2b3aa317c05908bc78 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:23:41 +0100 Subject: [PATCH 04/21] Script to run all the metatests (with platform filtering) Signed-off-by: Gilles Peskine --- tests/scripts/run-metatests.sh | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100755 tests/scripts/run-metatests.sh diff --git a/tests/scripts/run-metatests.sh b/tests/scripts/run-metatests.sh new file mode 100755 index 0000000000..182bf0410a --- /dev/null +++ b/tests/scripts/run-metatests.sh @@ -0,0 +1,81 @@ +#!/bin/sh + +help () { + cat <&2 "$0: FATAL: programs/test/metatest not found" + exit 120 +fi + +LIST_ONLY= +while getopts hl OPTLET; do + case $OPTLET in + h) help; exit;; + l) LIST_ONLY=1;; + \?) help >&2; exit 120;; + esac +done +shift $((OPTIND - 1)) + +list_matches () { + while read name platform junk; do + for pattern; do + case $platform in + $pattern) echo "$name"; break;; + esac + done + done +} + +count=0 +errors=0 +run_metatest () { + ret=0 + "$METATEST_PROGRAM" "$1" || ret=$? + if [ $ret -eq 0 ]; then + echo >&2 "$0: Unexpected success: $1" + errors=$((errors + 1)) + fi + count=$((count + 1)) +} + +# Don't pipe the output of metatest so that if it fails, this script exits +# immediately with a failure status. +full_list=$("$METATEST_PROGRAM" list) +matching_list=$(printf '%s\n' "$full_list" | list_matches "$@") + +if [ -n "$LIST_ONLY" ]; then + printf '%s\n' $matching_list + exit +fi + +for name in $matching_list; do + run_metatest "$name" +done + +if [ $errors -eq 0 ]; then + echo "Ran $count metatests, all good." + exit 0 +else + echo "Ran $count metatests, $errors unexpected successes." + exit 1 +fi From b0f0a64de058950127396e52ff3a58581feca0e8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:42:13 +0100 Subject: [PATCH 05/21] Metatests for basic Asan and Msan features Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index ba3ec94439..91a1e2a7d9 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -51,6 +51,50 @@ void null_pointer_call(const char *name) } +/****************************************************************/ +/* Sanitizers */ +/****************************************************************/ + +void read_after_free(const char *name) +{ + (void) name; + volatile char *p = mbedtls_calloc(1, 1); + *p = 'a'; + mbedtls_free((void *) p); + mbedtls_printf("%u\n", (unsigned) *p); +} + +void double_free(const char *name) +{ + (void) name; + volatile char *p = mbedtls_calloc(1, 1); + *p = 'a'; + mbedtls_free((void *) p); + mbedtls_free((void *) p); +} + +void read_uninitialized_stack(const char *name) +{ + (void) name; + volatile char buf[1]; + static int false_but_the_compiler_does_not_know = 0; + if (false_but_the_compiler_does_not_know) { + buf[0] = '!'; + } + if (*buf != 0) { + mbedtls_printf("%u\n", (unsigned) *buf); + } +} + +void memory_leak(const char *name) +{ + (void) name; + volatile char *p = mbedtls_calloc(1, 1); + /* Hint to the compiler that calloc must not be optimized away. */ + (void) *p; +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -65,6 +109,10 @@ metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, { "null_dereference", "any", null_pointer_dereference }, { "null_call", "any", null_pointer_call }, + { "read_after_free", "asan", read_after_free }, + { "double_free", "asan", double_free }, + { "read_uninitialized_stack", "msan", read_uninitialized_stack }, + { "memory_leak", "asan", memory_leak }, { NULL, NULL, NULL } }; From 69e8db036639ab2293501676c7f4f5d1ac7df536 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:52:32 +0100 Subject: [PATCH 06/21] Strengthen against Clang optimizations Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 91a1e2a7d9..eb3bb76aff 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -11,12 +11,21 @@ #define MBEDTLS_ALLOW_PRIVATE_ACCESS #include +#include #include "test/helpers.h" #include #include +/* This is an external variable, so the compiler doesn't know that we're never + * changing its value. + * + * TODO: LTO (link-time-optimization) would defeat this. + */ +int false_but_the_compiler_does_not_know = 0; + + /****************************************************************/ /* Test framework features */ /****************************************************************/ @@ -35,19 +44,17 @@ void meta_test_fail(const char *name) void null_pointer_dereference(const char *name) { (void) name; - char *p; - memset(&p, 0, sizeof(p)); - volatile char c; - c = *p; - (void) c; + volatile char *p; + mbedtls_platform_zeroize(&p, sizeof(p)); + mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } void null_pointer_call(const char *name) { (void) name; - void (*p)(void); - memset(&p, 0, sizeof(p)); - p(); + unsigned (*p)(void); + mbedtls_platform_zeroize(&p, sizeof(p)); + mbedtls_printf("%p() -> %u\n", p, p()); } @@ -77,7 +84,6 @@ void read_uninitialized_stack(const char *name) { (void) name; volatile char buf[1]; - static int false_but_the_compiler_does_not_know = 0; if (false_but_the_compiler_does_not_know) { buf[0] = '!'; } From 6848d1709baf6f63c1901dbab3e8af2dde1c2167 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 19:58:03 +0100 Subject: [PATCH 07/21] Run metatests in selected components Run metatests in some components, covering both GCC and Clang, with ASan, MSan or neither. Note that this commit does not cover constant-flow testing builds or Valgrind. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 086677a600..5f5d2fd233 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1065,6 +1065,9 @@ component_test_default_cmake_gcc_asan () { msg "test: selftest (ASan build)" # ~ 10s programs/test/selftest + msg "test: metatests (GCC, ASan build)" + tests/scripts/run-metatests.sh any asan + msg "test: ssl-opt.sh (ASan build)" # ~ 1 min tests/ssl-opt.sh @@ -1830,6 +1833,9 @@ component_test_everest () { msg "test: Everest ECDH context - main suites (inc. selftests) (ASan build)" # ~ 50s make test + msg "test: metatests (clang, ASan)" + tests/scripts/run-metatests.sh any asan + msg "test: Everest ECDH context - ECDH-related part of ssl-opt.sh (ASan build)" # ~ 5s tests/ssl-opt.sh -f ECDH @@ -1918,6 +1924,9 @@ component_test_full_cmake_clang () { msg "test: cpp_dummy_build (full config, clang)" # ~ 1s programs/test/cpp_dummy_build + msg "test: metatests (clang)" + tests/scripts/run-metatests.sh any + msg "program demos (full config, clang)" # ~10s tests/scripts/run_demos.py @@ -5403,6 +5412,9 @@ component_test_memsan () { msg "test: main suites (MSan)" # ~ 10s make test + msg "test: metatests (MSan)" + tests/scripts/run-metatests.sh any msan + msg "program demos (MSan)" # ~20s tests/scripts/run_demos.py From 6aa9f321246519690ec4c50fe04a2b256376c84a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 21:31:07 +0100 Subject: [PATCH 08/21] Use casts when doing nonstandard pointer conversions Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index eb3bb76aff..36119f68d3 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -45,7 +45,7 @@ void null_pointer_dereference(const char *name) { (void) name; volatile char *p; - mbedtls_platform_zeroize(&p, sizeof(p)); + mbedtls_platform_zeroize((void *) &p, sizeof(p)); mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -54,7 +54,7 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%p() -> %u\n", p, p()); + mbedtls_printf("%p() -> %u\n", (void *) p, p()); } From ee8109541a433d01add3be695e78653898c24e08 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 2 Nov 2023 22:32:50 +0100 Subject: [PATCH 09/21] Don't cast a function pointer to a data pointer That's nonstandard. Instead, convert to an integer. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 36119f68d3..872dbf0f80 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -54,7 +54,7 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%p() -> %u\n", (void *) p, p()); + mbedtls_printf("%llx() -> %u\n", (unsigned long long) p, p()); } From a1dfa14c066962ac5f8780e2e4fdbc97a9acad72 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 10:31:56 +0100 Subject: [PATCH 10/21] Fix cast from pointer to integer of different size Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 872dbf0f80..1a638b5958 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -54,7 +54,7 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%llx() -> %u\n", (unsigned long long) p, p()); + mbedtls_printf("%llx() -> %u\n", (unsigned long long) (uintptr_t) p, p()); } From f0d5cf9a0c7fe8742b3a8ddaf26d4e00a5a564c3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 10:58:57 +0100 Subject: [PATCH 11/21] Don't use %llx in printf We still do MinGW builds on our CI whose printf doesn't support it! Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 1a638b5958..d3173f3d40 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -54,7 +54,10 @@ void null_pointer_call(const char *name) (void) name; unsigned (*p)(void); mbedtls_platform_zeroize(&p, sizeof(p)); - mbedtls_printf("%llx() -> %u\n", (unsigned long long) (uintptr_t) p, p()); + /* The pointer representation may be truncated, but we don't care: + * the only point of printing it is to have some use of the pointer + * to dissuade the compiler from optimizing it away. */ + mbedtls_printf("%lx() -> %u\n", (unsigned long) (uintptr_t) p, p()); } From 102aea2ba834e584fc73ae308868187b9c102a39 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 18:05:38 +0100 Subject: [PATCH 12/21] Add metatests for mutex usage Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 100 ++++++++++++++++++++++++++++++++++++++- tests/scripts/all.sh | 2 +- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index d3173f3d40..7e173ee270 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -13,10 +13,15 @@ #include #include #include "test/helpers.h" +#include "test/macros.h" #include #include +#if defined(MBEDTLS_THREADING_C) +#include +#endif + /* This is an external variable, so the compiler doesn't know that we're never * changing its value. @@ -62,7 +67,7 @@ void null_pointer_call(const char *name) /****************************************************************/ -/* Sanitizers */ +/* Memory */ /****************************************************************/ void read_after_free(const char *name) @@ -104,6 +109,84 @@ void memory_leak(const char *name) } +/****************************************************************/ +/* Threading */ +/****************************************************************/ + +void mutex_lock_not_initialized(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_PTHREAD) + mbedtls_threading_mutex_t mutex; + memset(&mutex, 0, sizeof(mutex)); + TEST_ASSERT(mbedtls_mutex_lock(&mutex) == 0); +exit: + ; +#endif +} + +void mutex_unlock_not_initialized(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + memset(&mutex, 0, sizeof(mutex)); + TEST_ASSERT(mbedtls_mutex_unlock(&mutex) == 0); +exit: + ; +#endif +} + +void mutex_free_not_initialized(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + memset(&mutex, 0, sizeof(mutex)); + mbedtls_mutex_free(&mutex); +#endif +} + +void mutex_double_init(const char *name) +{ + (void) name; +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + mbedtls_mutex_init(&mutex); + mbedtls_mutex_init(&mutex); + mbedtls_mutex_free(&mutex); +#endif +} + +void mutex_double_free(const char *name) +{ + (void) name; +#if defined(MBEDTLS_THREADING_C) + mbedtls_threading_mutex_t mutex; + mbedtls_mutex_init(&mutex); + mbedtls_mutex_free(&mutex); + mbedtls_mutex_free(&mutex); +#endif +} + +void mutex_leak(const char *name) +{ + (void) name; + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ +#if defined(MBEDTLS_THREADING_PTHREAD) + mbedtls_threading_mutex_t mutex; + mbedtls_mutex_init(&mutex); +#endif +} + + /****************************************************************/ /* Command line entry point */ /****************************************************************/ @@ -122,6 +205,14 @@ metatest_t metatests[] = { { "double_free", "asan", double_free }, { "read_uninitialized_stack", "msan", read_uninitialized_stack }, { "memory_leak", "asan", memory_leak }, + /* Mutex usage verification is only done with pthread, not with other + * threading implementations. See tests/src/threading_helpers.c. */ + { "mutex_lock_not_initialized", "pthread", mutex_lock_not_initialized }, + { "mutex_unlock_not_initialized", "pthread", mutex_unlock_not_initialized }, + { "mutex_free_not_initialized", "pthread", mutex_free_not_initialized }, + { "mutex_double_init", "pthread", mutex_double_init }, + { "mutex_double_free", "pthread", mutex_double_free }, + { "mutex_leak", "pthread", mutex_leak }, { NULL, NULL, NULL } }; @@ -157,10 +248,17 @@ int main(int argc, char *argv[]) mbedtls_exit(MBEDTLS_EXIT_SUCCESS); } +#if defined(MBEDTLS_TEST_MUTEX_USAGE) + mbedtls_test_mutex_usage_init(); +#endif + for (const metatest_t *p = metatests; p->name != NULL; p++) { if (strcmp(argv[1], p->name) == 0) { mbedtls_printf("Running metatest %s...\n", argv[1]); p->entry_point(argv[1]); +#if defined(MBEDTLS_TEST_MUTEX_USAGE) + mbedtls_test_mutex_usage_check(); +#endif mbedtls_printf("Running metatest %s... done, result=%d\n", argv[1], (int) mbedtls_test_info.result); mbedtls_exit(mbedtls_test_info.result == MBEDTLS_TEST_RESULT_SUCCESS ? diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 5f5d2fd233..04626c39f1 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1925,7 +1925,7 @@ component_test_full_cmake_clang () { programs/test/cpp_dummy_build msg "test: metatests (clang)" - tests/scripts/run-metatests.sh any + tests/scripts/run-metatests.sh any pthread msg "program demos (full config, clang)" # ~10s tests/scripts/run_demos.py From 4bc873f0a189ed906a77ee839869419ca1815e84 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 18:57:06 +0100 Subject: [PATCH 13/21] Add missing program to .gitignore Signed-off-by: Gilles Peskine --- programs/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/.gitignore b/programs/.gitignore index 2826a18a49..e0c49873ee 100644 --- a/programs/.gitignore +++ b/programs/.gitignore @@ -38,6 +38,7 @@ psa/crypto_examples psa/hmac_demo psa/key_ladder_demo psa/psa_constant_names +psa/psa_hash random/gen_entropy random/gen_random_ctr_drbg ssl/dtls_client From a1023e2bd665233cc9562817f9f6222570d10c4c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 3 Nov 2023 19:16:56 +0100 Subject: [PATCH 14/21] programs/test/metatest indirectly includes library/common.h Signed-off-by: Gilles Peskine --- programs/Makefile | 2 +- programs/test/CMakeLists.txt | 1 + scripts/generate_visualc_files.pl | 5 ++++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 7b538da4b8..a3fa81679f 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -416,7 +416,7 @@ endif test/metatest$(EXEXT): test/metatest.c $(DEP) echo " CC test/metatest.c" - $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ + $(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I ../library test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ test/query_config.o: test/query_config.c test/query_config.h $(DEP) echo " CC test/query_config.c" diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index 8469d064d2..0778731125 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -73,6 +73,7 @@ foreach(exe IN LISTS executables_libs executables_mbedcrypto) add_executable(${exe} ${exe}.c $ ${extra_sources}) target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../tests/include) + target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../library) if(exe STREQUAL "query_compile_time_config") target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) endif() diff --git a/scripts/generate_visualc_files.pl b/scripts/generate_visualc_files.pl index 7f5609820b..96ade2fa76 100755 --- a/scripts/generate_visualc_files.pl +++ b/scripts/generate_visualc_files.pl @@ -144,6 +144,7 @@ sub gen_app { my $guid = gen_app_guid( $path ); $path =~ s!/!\\!g; (my $appname = $path) =~ s/.*\\//; + my $is_test_app = ($path =~ m/^test\\/); my $srcs = ""; if( $appname eq "ssl_client2" or $appname eq "ssl_server2" or @@ -158,7 +159,9 @@ sub gen_app { $content =~ s//$srcs/g; $content =~ s//$appname/g; $content =~ s//$guid/g; - $content =~ s/INCLUDE_DIRECTORIES\n/$include_directories/g; + $content =~ s/INCLUDE_DIRECTORIES\n/($is_test_app ? + $library_include_directories : + $include_directories)/ge; content_to_file( $content, "$dir/$appname.$ext" ); } From d2fa6981550e0d0515a1d70bfc7840045f71bb62 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 9 Nov 2023 21:46:24 +0100 Subject: [PATCH 15/21] Strengthen against possible compiler optimizations Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 7e173ee270..805de2d305 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -25,10 +25,15 @@ /* This is an external variable, so the compiler doesn't know that we're never * changing its value. - * - * TODO: LTO (link-time-optimization) would defeat this. */ -int false_but_the_compiler_does_not_know = 0; +volatile int false_but_the_compiler_does_not_know = 0; + +/* Set n bytes at the address p to all-bits-zero, in such a way that + * the compiler should not know that p is all-bits-zero. */ +static void set_to_zero_but_the_compiler_does_not_know(void *p, size_t n) +{ + memset(p, false_but_the_compiler_does_not_know, n); +} /****************************************************************/ @@ -50,7 +55,7 @@ void null_pointer_dereference(const char *name) { (void) name; volatile char *p; - mbedtls_platform_zeroize((void *) &p, sizeof(p)); + set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -58,7 +63,7 @@ void null_pointer_call(const char *name) { (void) name; unsigned (*p)(void); - mbedtls_platform_zeroize(&p, sizeof(p)); + set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); /* The pointer representation may be truncated, but we don't care: * the only point of printing it is to have some use of the pointer * to dissuade the compiler from optimizing it away. */ @@ -104,8 +109,7 @@ void memory_leak(const char *name) { (void) name; volatile char *p = mbedtls_calloc(1, 1); - /* Hint to the compiler that calloc must not be optimized away. */ - (void) *p; + mbedtls_printf("%u\n", (unsigned) *p); } From da6e7a2ac27a3a3d579645e946b30d0928b2dbb3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Nov 2023 10:09:27 +0100 Subject: [PATCH 16/21] More consistent usage of volatile Fix MSVC warning C4090. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 805de2d305..ce866edecc 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -30,9 +30,9 @@ volatile int false_but_the_compiler_does_not_know = 0; /* Set n bytes at the address p to all-bits-zero, in such a way that * the compiler should not know that p is all-bits-zero. */ -static void set_to_zero_but_the_compiler_does_not_know(void *p, size_t n) +static void set_to_zero_but_the_compiler_does_not_know(volatile void *p, size_t n) { - memset(p, false_but_the_compiler_does_not_know, n); + memset((void *) p, false_but_the_compiler_does_not_know, n); } @@ -54,7 +54,7 @@ void meta_test_fail(const char *name) void null_pointer_dereference(const char *name) { (void) name; - volatile char *p; + volatile char *volatile p; set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -62,7 +62,7 @@ void null_pointer_dereference(const char *name) void null_pointer_call(const char *name) { (void) name; - unsigned (*p)(void); + unsigned(*volatile p)(void); set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); /* The pointer representation may be truncated, but we don't care: * the only point of printing it is to have some use of the pointer From ccb121500d84567c30127b792128bf7e0f00f350 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Nov 2023 11:35:36 +0100 Subject: [PATCH 17/21] Uninitialized read: make the pointer non-volatile rather than the buffer Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index ce866edecc..5e6a15f1d7 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -96,12 +96,13 @@ void double_free(const char *name) void read_uninitialized_stack(const char *name) { (void) name; - volatile char buf[1]; + char buf[1]; if (false_but_the_compiler_does_not_know) { buf[0] = '!'; } - if (*buf != 0) { - mbedtls_printf("%u\n", (unsigned) *buf); + char *volatile p = buf; + if (*p != 0) { + mbedtls_printf("%u\n", (unsigned) *p); } } From cce0012463180e56ac85ab0cdea45900b1d60fae Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Nov 2023 15:36:15 +0100 Subject: [PATCH 18/21] Add documentation Explain the goals of metatests, how to write them, and how to read their output. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 48 ++++++++++++++++++++++++++++++++++ tests/scripts/run-metatests.sh | 8 ++++++ 2 files changed, 56 insertions(+) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 5e6a15f1d7..c35e9a9524 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -1,6 +1,24 @@ /** \file metatest.c * * \brief Test features of the test framework. + * + * When you run this program, it runs a single "meta-test". A meta-test + * performs an operation which should be caught as a failure by our + * test framework. The meta-test passes if this program calls `exit` with + * a nonzero status, or aborts, or is terminated by a signal, or if the + * framework running the program considers the run an error (this happens + * with Valgrind for a memory leak). The non-success of the meta-test + * program means that the test failure has been caught correctly. + * + * Some failures are purely functional: the logic of the code causes the + * test result to be set to FAIL. Other failures come from extra + * instrumentation which is not present in a normal build; for example, + * Asan or Valgrind to detect memory leaks. This is reflected by the + * "platform" associated with each meta-test. + * + * Use the companion script `tests/scripts/run-metatests.sh` to run all + * the meta-tests for a given platform and validate that they trigger a + * detected failure as expected. */ /* @@ -197,11 +215,41 @@ void mutex_leak(const char *name) /****************************************************************/ typedef struct { + /** Command line argument that will trigger that metatest. + * + * Conventionally matches "[a-z0-9_]+". */ const char *name; + + /** Platform under which that metatest is valid. + * + * - "any": should work anywhere. + * - "asan": triggers ASan (Address Sanitizer). + * - "msan": triggers MSan (Memory Sanitizer). + * - "pthread": requires MBEDTLS_THREADING_PTHREAD and MBEDTLS_TEST_HOOKS. + */ const char *platform; + + /** Function that performs the metatest. + * + * The function receives the name as an argument. This allows using the + * same function to perform multiple variants of a test based on the name. + * + * When executed on a conforming platform, the function is expected to + * either cause a test failure (mbedtls_test_fail()), or cause the + * program to abort in some way (e.g. by causing a segfault or by + * triggering a sanitizer). + * + * When executed on a non-conforming platform, the function may return + * normally or may have unpredictable behavior. + */ void (*entry_point)(const char *name); } metatest_t; +/* The list of availble meta-tests. Remember to register new functions here! + * + * Note that we always compile all the functions, so that `metatest --list` + * will always list all the available meta-tests. + */ metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, { "null_dereference", "any", null_pointer_dereference }, diff --git a/tests/scripts/run-metatests.sh b/tests/scripts/run-metatests.sh index 182bf0410a..09a6f8a4f9 100755 --- a/tests/scripts/run-metatests.sh +++ b/tests/scripts/run-metatests.sh @@ -6,6 +6,14 @@ Usage: $0 [OPTION] [PLATFORM]... Run all the metatests whose platform matches any of the given PLATFORM. A PLATFORM can contain shell wildcards. +Expected output: a lot of scary-looking error messages, since each +metatest is expected to report a failure. The final line should be +"Ran N metatests, all good." + +If something goes wrong: the final line should be +"Ran N metatests, X unexpected successes". Look for "Unexpected success" +in the logs above. + -l List the available metatests, don't run them. EOF } From e7fc8a232fd34ec6fd74ffb93a1d200307ba9588 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 15 Nov 2023 16:56:26 +0100 Subject: [PATCH 19/21] Readability improvement Signed-off-by: Gilles Peskine --- tests/scripts/run-metatests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/run-metatests.sh b/tests/scripts/run-metatests.sh index 09a6f8a4f9..22a302c62f 100755 --- a/tests/scripts/run-metatests.sh +++ b/tests/scripts/run-metatests.sh @@ -46,7 +46,7 @@ shift $((OPTIND - 1)) list_matches () { while read name platform junk; do - for pattern; do + for pattern in "$@"; do case $platform in $pattern) echo "$name"; break;; esac From ad2a17eb6038d9e31a0f3aa1a24774e3fec40315 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Nov 2023 15:09:48 +0100 Subject: [PATCH 20/21] Uniformly use MBEDTLS_THREADING_C guards Since the code compiles with MBEDTLS_THREADING_C, not just with MBEDTLS_THREADING_PTHREAD, use MBEDTLS_THREADING_C as the guard. The runtime behavior is only as desired under certain conditions that imply MBEDTLS_THREADING_PTHREAD, but that's fine: no metatest is expected to pass in all scenarios, only under specific build- and run-time conditions. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index c35e9a9524..68e8da6fa0 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -141,7 +141,7 @@ void mutex_lock_not_initialized(const char *name) (void) name; /* Mutex usage verification is only done with pthread, not with other * threading implementations. See tests/src/threading_helpers.c. */ -#if defined(MBEDTLS_THREADING_PTHREAD) +#if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); TEST_ASSERT(mbedtls_mutex_lock(&mutex) == 0); @@ -203,7 +203,7 @@ void mutex_leak(const char *name) (void) name; /* Mutex usage verification is only done with pthread, not with other * threading implementations. See tests/src/threading_helpers.c. */ -#if defined(MBEDTLS_THREADING_PTHREAD) +#if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); #endif From 2f40cc05f01a34c6e091a8014b76e6f7be644c6f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Nov 2023 15:11:44 +0100 Subject: [PATCH 21/21] Improve explanations of what bad thing a metatest does Especially clarify the situation with respect to mutex usage. Signed-off-by: Gilles Peskine --- programs/test/metatest.c | 47 ++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/programs/test/metatest.c b/programs/test/metatest.c index 68e8da6fa0..2973cce3fa 100644 --- a/programs/test/metatest.c +++ b/programs/test/metatest.c @@ -74,6 +74,7 @@ void null_pointer_dereference(const char *name) (void) name; volatile char *volatile p; set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); + /* Undefined behavior (read from null data pointer) */ mbedtls_printf("%p -> %u\n", p, (unsigned) *p); } @@ -82,6 +83,7 @@ void null_pointer_call(const char *name) (void) name; unsigned(*volatile p)(void); set_to_zero_but_the_compiler_does_not_know(&p, sizeof(p)); + /* Undefined behavior (execute null function pointer) */ /* The pointer representation may be truncated, but we don't care: * the only point of printing it is to have some use of the pointer * to dissuade the compiler from optimizing it away. */ @@ -99,6 +101,7 @@ void read_after_free(const char *name) volatile char *p = mbedtls_calloc(1, 1); *p = 'a'; mbedtls_free((void *) p); + /* Undefined behavior (read after free) */ mbedtls_printf("%u\n", (unsigned) *p); } @@ -108,6 +111,7 @@ void double_free(const char *name) volatile char *p = mbedtls_calloc(1, 1); *p = 'a'; mbedtls_free((void *) p); + /* Undefined behavior (double free) */ mbedtls_free((void *) p); } @@ -120,6 +124,7 @@ void read_uninitialized_stack(const char *name) } char *volatile p = buf; if (*p != 0) { + /* Unspecified result (read from uninitialized memory) */ mbedtls_printf("%u\n", (unsigned) *p); } } @@ -129,6 +134,7 @@ void memory_leak(const char *name) (void) name; volatile char *p = mbedtls_calloc(1, 1); mbedtls_printf("%u\n", (unsigned) *p); + /* Leak of a heap object */ } @@ -139,11 +145,13 @@ void memory_leak(const char *name) void mutex_lock_not_initialized(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ TEST_ASSERT(mbedtls_mutex_lock(&mutex) == 0); exit: ; @@ -153,11 +161,13 @@ exit: void mutex_unlock_not_initialized(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ TEST_ASSERT(mbedtls_mutex_unlock(&mutex) == 0); exit: ; @@ -167,11 +177,13 @@ exit: void mutex_free_not_initialized(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; memset(&mutex, 0, sizeof(mutex)); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ mbedtls_mutex_free(&mutex); #endif } @@ -182,6 +194,10 @@ void mutex_double_init(const char *name) #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ mbedtls_mutex_init(&mutex); mbedtls_mutex_free(&mutex); #endif @@ -194,6 +210,10 @@ void mutex_double_free(const char *name) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); mbedtls_mutex_free(&mutex); + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ mbedtls_mutex_free(&mutex); #endif } @@ -201,12 +221,14 @@ void mutex_double_free(const char *name) void mutex_leak(const char *name) { (void) name; - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ #if defined(MBEDTLS_THREADING_C) mbedtls_threading_mutex_t mutex; mbedtls_mutex_init(&mutex); #endif + /* This mutex usage error is detected by our test framework's mutex usage + * verification framework. See tests/src/threading_helpers.c. Other + * threading implementations (e.g. pthread without our instrumentation) + * might consider this normal usage. */ } @@ -225,7 +247,9 @@ typedef struct { * - "any": should work anywhere. * - "asan": triggers ASan (Address Sanitizer). * - "msan": triggers MSan (Memory Sanitizer). - * - "pthread": requires MBEDTLS_THREADING_PTHREAD and MBEDTLS_TEST_HOOKS. + * - "pthread": requires MBEDTLS_THREADING_PTHREAD and MBEDTLS_TEST_HOOKS, + * which enables MBEDTLS_TEST_MUTEX_USAGE internally in the test + * framework (see tests/src/threading_helpers.c). */ const char *platform; @@ -249,6 +273,9 @@ typedef struct { * * Note that we always compile all the functions, so that `metatest --list` * will always list all the available meta-tests. + * + * See the documentation of metatest_t::platform for the meaning of + * platform values. */ metatest_t metatests[] = { { "test_fail", "any", meta_test_fail }, @@ -258,8 +285,6 @@ metatest_t metatests[] = { { "double_free", "asan", double_free }, { "read_uninitialized_stack", "msan", read_uninitialized_stack }, { "memory_leak", "asan", memory_leak }, - /* Mutex usage verification is only done with pthread, not with other - * threading implementations. See tests/src/threading_helpers.c. */ { "mutex_lock_not_initialized", "pthread", mutex_lock_not_initialized }, { "mutex_unlock_not_initialized", "pthread", mutex_unlock_not_initialized }, { "mutex_free_not_initialized", "pthread", mutex_free_not_initialized },