From f55182d2bf1ede94f716697f8672e5156e6f761d Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 17:42:43 +0000 Subject: [PATCH 01/17] Use platform-provided secure zeroization call Signed-off-by: Dave Rodgman --- library/platform_util.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/library/platform_util.c b/library/platform_util.c index f935b900e8..47feb645a5 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -33,8 +33,27 @@ #include "mbedtls/threading.h" #include + +#ifndef __STDC_WANT_LIB_EXT1__ +#define __STDC_WANT_LIB_EXT1__ 1 +#endif #include +#if defined(_WIN32) +#include +#endif + +// Detect platforms known to support explicit_bzero() +#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) +#define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 +#endif +#if defined(__FreeBSD__) && __FreeBSD_version >= 1100037 +#define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 +#endif +#if defined(__NEWLIB__) +#define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 +#endif + #if !defined(MBEDTLS_PLATFORM_ZEROIZE_ALT) /* * This implementation should never be optimized out by the compiler @@ -69,7 +88,15 @@ void mbedtls_platform_zeroize(void *buf, size_t len) MBEDTLS_INTERNAL_VALIDATE(len == 0 || buf != NULL); if (len > 0) { +#if defined(MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO) + explicit_bzero(buf, len); +#elif(__STDC_LIB_EXT1__) + memset_s(buf, len, 0, len); +#elif defined(_WIN32) + SecureZeroMemory(buf, len); +#else memset_func(buf, 0, len); +#endif } } #endif /* MBEDTLS_PLATFORM_ZEROIZE_ALT */ From 4daca637340e033c2d7fb2043b050a2977ec6182 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 17:43:00 +0000 Subject: [PATCH 02/17] Documentation Signed-off-by: Dave Rodgman --- library/platform_util.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index 47feb645a5..60b77e84e8 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -56,7 +56,12 @@ #if !defined(MBEDTLS_PLATFORM_ZEROIZE_ALT) /* - * This implementation should never be optimized out by the compiler + * Where possible, we try to detect the presence of a platform-provided + * secure memset, such as explicit_bzero(), that is safe against being optimized + * out, and use that. + * + * For other platforms, we provide an implementation that aims not to be + * optimized out by the compiler. * * This implementation for mbedtls_platform_zeroize() was inspired from Colin * Percival's blog article at: @@ -71,11 +76,11 @@ * (refer to http://www.daemonology.net/blog/2014-09-05-erratum.html for * details), optimizations of the following form are still possible: * - * if( memset_func != memset ) - * memset_func( buf, 0, len ); + * if(memset_func != memset) + * memset_func(buf, 0, len); * * Note that it is extremely difficult to guarantee that - * mbedtls_platform_zeroize() will not be optimized out by aggressive compilers + * the memset() call will not be optimized out by aggressive compilers * in a portable way. For this reason, Mbed TLS also provides the configuration * option MBEDTLS_PLATFORM_ZEROIZE_ALT, which allows users to configure * mbedtls_platform_zeroize() to use a suitable implementation for their From bf0597f804701ca11a089354339a022a71a8a556 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 17:45:41 +0000 Subject: [PATCH 03/17] Changelog Signed-off-by: Dave Rodgman --- ChangeLog.d/platform-zeroization.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/platform-zeroization.txt diff --git a/ChangeLog.d/platform-zeroization.txt b/ChangeLog.d/platform-zeroization.txt new file mode 100644 index 0000000000..f17fbbb968 --- /dev/null +++ b/ChangeLog.d/platform-zeroization.txt @@ -0,0 +1,3 @@ +Security + * Use platform-provided secure zeroization function where possible, such as + explicit_bzero(). From 8b6eded03dc40986861c03f31897abc06d9cb872 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 18:07:05 +0000 Subject: [PATCH 04/17] Tidy-up comment Co-authored-by: Tom Cosgrove Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index 60b77e84e8..35e29ae884 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -76,7 +76,7 @@ * (refer to http://www.daemonology.net/blog/2014-09-05-erratum.html for * details), optimizations of the following form are still possible: * - * if(memset_func != memset) + * if (memset_func != memset) * memset_func(buf, 0, len); * * Note that it is extremely difficult to guarantee that From 8a7d26f12c21ce80ffe32133f174cd04353b6f1a Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 18:19:08 +0000 Subject: [PATCH 05/17] Typo fix Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index 35e29ae884..25f5c575f1 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -95,7 +95,7 @@ void mbedtls_platform_zeroize(void *buf, size_t len) if (len > 0) { #if defined(MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO) explicit_bzero(buf, len); -#elif(__STDC_LIB_EXT1__) +#elif defined(__STDC_LIB_EXT1__) memset_s(buf, len, 0, len); #elif defined(_WIN32) SecureZeroMemory(buf, len); From a6fda16a41d92ec3a591f281cfe0a2f7126c7b26 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 19:00:38 +0000 Subject: [PATCH 06/17] Fix re-definition of __STDC_WANT_LIB_EXT1__ Signed-off-by: Dave Rodgman --- library/platform_util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index 25f5c575f1..d0abc179fd 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -35,7 +35,7 @@ #include #ifndef __STDC_WANT_LIB_EXT1__ -#define __STDC_WANT_LIB_EXT1__ 1 +#define __STDC_WANT_LIB_EXT1__ 1 /* Ask for the C11 gmtime_s() and memset_s() if available */ #endif #include @@ -107,7 +107,6 @@ void mbedtls_platform_zeroize(void *buf, size_t len) #endif /* MBEDTLS_PLATFORM_ZEROIZE_ALT */ #if defined(MBEDTLS_HAVE_TIME_DATE) && !defined(MBEDTLS_PLATFORM_GMTIME_R_ALT) -#define __STDC_WANT_LIB_EXT1__ 1 /* Ask for the C11 gmtime_s() if it's available */ #include #if !defined(_WIN32) && (defined(unix) || \ defined(__unix) || defined(__unix__) || (defined(__APPLE__) && \ From f0a0e43053d9741a7531a4063c7abce56a7c042e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 24 Feb 2023 19:01:48 +0000 Subject: [PATCH 07/17] explicit_bzero is not available on arm-none-eabi Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index d0abc179fd..8141dd84bb 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -44,7 +44,7 @@ #endif // Detect platforms known to support explicit_bzero() -#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) +#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) && defined(__unix__) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #endif #if defined(__FreeBSD__) && __FreeBSD_version >= 1100037 From 828ec905dbbeeb01d93f5cf847aec0508ee07a72 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 13:32:26 +0000 Subject: [PATCH 08/17] Improve explicit_bzero detection Signed-off-by: Dave Rodgman --- library/platform_util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index 8141dd84bb..69d3d7a220 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -44,7 +44,8 @@ #endif // Detect platforms known to support explicit_bzero() -#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) && defined(__unix__) +#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) \ + && !defined(__ARM_EABI__) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #endif #if defined(__FreeBSD__) && __FreeBSD_version >= 1100037 From 82f3de55b22b7edd16e032747747d38da91c43b1 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 14:08:22 +0000 Subject: [PATCH 09/17] tidy up brackets Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index 69d3d7a220..2884444348 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -48,7 +48,7 @@ && !defined(__ARM_EABI__) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #endif -#if defined(__FreeBSD__) && __FreeBSD_version >= 1100037 +#if defined(__FreeBSD__) && (__FreeBSD_version >= 1100037) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #endif #if defined(__NEWLIB__) From fe57a2e008bd724d3ac98e95efc9bc9bf0b3ea67 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 14:16:34 +0000 Subject: [PATCH 10/17] Remove newlib detection Signed-off-by: Dave Rodgman --- library/platform_util.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index 2884444348..9ac57e141d 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -51,9 +51,6 @@ #if defined(__FreeBSD__) && (__FreeBSD_version >= 1100037) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #endif -#if defined(__NEWLIB__) -#define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 -#endif #if !defined(MBEDTLS_PLATFORM_ZEROIZE_ALT) /* From 703f805f0990e8c4a661aa2d7d1d30cdd2601934 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 15:19:52 +0000 Subject: [PATCH 11/17] Improve explicit_bzero detection Signed-off-by: Dave Rodgman --- library/platform_util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/platform_util.c b/library/platform_util.c index 9ac57e141d..d8499bddc3 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -26,6 +26,11 @@ #define _POSIX_C_SOURCE 200112L #endif +#if !defined (_GNU_SOURCE) +/* Clang requires this to get support for explicit_bzero */ +#define _GNU_SOURCE +#endif + #include "common.h" #include "mbedtls/platform_util.h" @@ -84,7 +89,10 @@ * mbedtls_platform_zeroize() to use a suitable implementation for their * platform and needs. */ + #if !defined(MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO) && !defined(__STDC_LIB_EXT1__) \ + && !defined(_WIN32) static void *(*const volatile memset_func)(void *, int, size_t) = memset; +#endif void mbedtls_platform_zeroize(void *buf, size_t len) { From 21dfce7a5c336e42ba00e95ab341d48061f4c787 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 17:10:38 +0000 Subject: [PATCH 12/17] Add tests Signed-off-by: Dave Rodgman --- tests/suites/test_suite_platform_util.data | 23 +++++++++++ .../suites/test_suite_platform_util.function | 41 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/suites/test_suite_platform_util.data create mode 100644 tests/suites/test_suite_platform_util.function diff --git a/tests/suites/test_suite_platform_util.data b/tests/suites/test_suite_platform_util.data new file mode 100644 index 0000000000..948543a6fc --- /dev/null +++ b/tests/suites/test_suite_platform_util.data @@ -0,0 +1,23 @@ +Zeroize len 0, null +mbedtls_platform_zeroize:0:1 + +Zeroize len 0, non-null +mbedtls_platform_zeroize:0:0 + +Zeroize len 1 +mbedtls_platform_zeroize:1:0 + +Zeroize len 4 +mbedtls_platform_zeroize:1:0 + +Zeroize len 5 +mbedtls_platform_zeroize:1:0 + +Zeroize len 32 +mbedtls_platform_zeroize:32:0 + +Zeroize len 127 +mbedtls_platform_zeroize:127:0 + +Zeroize len 128 +mbedtls_platform_zeroize:128:0 diff --git a/tests/suites/test_suite_platform_util.function b/tests/suites/test_suite_platform_util.function new file mode 100644 index 0000000000..e5464e0ec3 --- /dev/null +++ b/tests/suites/test_suite_platform_util.function @@ -0,0 +1,41 @@ +/* BEGIN_HEADER */ +#include "mbedtls/platform_util.h" +/* END_HEADER */ + +/* BEGIN_CASE */ +void mbedtls_platform_zeroize(int len, int null) +{ + char buf[130]; + char *p = NULL; + + TEST_ASSERT(len <= 128); + + /* Write sentinel values */ + buf[0] = 2; + buf[len + 1] = 2; + + /* Write non-zero content */ + if (!null) { + p = &buf[1]; + for (int i = 0; i < len; i++) { + p[i] = 1; + } + } + + /* Check content is non-zero */ + TEST_EQUAL(buf[0], 2); + for (int i = 0; i < len; i++) { + TEST_ASSERT(p[i] == 1); + } + TEST_EQUAL(buf[len + 1], 2); + + mbedtls_platform_zeroize(p, len); + + /* Check content is zero and sentinels un-changed */ + TEST_EQUAL(buf[0], 2); + for (int i = 0; i < len; i++) { + TEST_ASSERT(p[i] == 0); + } + TEST_EQUAL(buf[len + 1], 2); +} +/* END_CASE */ From f5e531a87b34f17f46e789456a05fb75655da14b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 17:17:15 +0000 Subject: [PATCH 13/17] Fix code style Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index d8499bddc3..9878cc2e19 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -26,7 +26,7 @@ #define _POSIX_C_SOURCE 200112L #endif -#if !defined (_GNU_SOURCE) +#if !defined(_GNU_SOURCE) /* Clang requires this to get support for explicit_bzero */ #define _GNU_SOURCE #endif From 096e72959b640d7ef71ae3c06642833796117a38 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Sat, 25 Feb 2023 17:17:35 +0000 Subject: [PATCH 14/17] Fix case of include header for mingw Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index 9878cc2e19..f9fe4f5abf 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -45,7 +45,7 @@ #include #if defined(_WIN32) -#include +#include #endif // Detect platforms known to support explicit_bzero() From 6d6a720603f6051ea04d0c578098e1d88055cedd Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 1 Mar 2023 15:09:40 +0000 Subject: [PATCH 15/17] Protect against possible macro redefinition warning Signed-off-by: Dave Rodgman --- library/platform_util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index f9fe4f5abf..0ff7e1d6a2 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -52,8 +52,7 @@ #if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) \ && !defined(__ARM_EABI__) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 -#endif -#if defined(__FreeBSD__) && (__FreeBSD_version >= 1100037) +#elif defined(__FreeBSD__) && (__FreeBSD_version >= 1100037) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #endif From 528bfa640c50835e70ecc22bf986ffe8c8fa40ef Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 2 Mar 2023 13:54:43 +0000 Subject: [PATCH 16/17] Whitespace fix Signed-off-by: Dave Rodgman --- library/platform_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/platform_util.c b/library/platform_util.c index 0ff7e1d6a2..4e1af31dc6 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -88,7 +88,7 @@ * mbedtls_platform_zeroize() to use a suitable implementation for their * platform and needs. */ - #if !defined(MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO) && !defined(__STDC_LIB_EXT1__) \ +#if !defined(MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO) && !defined(__STDC_LIB_EXT1__) \ && !defined(_WIN32) static void *(*const volatile memset_func)(void *, int, size_t) = memset; #endif From b0d96a23a9c8af854154929509e2a594bc792827 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 3 Mar 2023 17:06:09 +0000 Subject: [PATCH 17/17] Remove not-needed EABI exclusion Signed-off-by: Dave Rodgman --- library/platform_util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/platform_util.c b/library/platform_util.c index 4e1af31dc6..d525acc84b 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -49,8 +49,7 @@ #endif // Detect platforms known to support explicit_bzero() -#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) \ - && !defined(__ARM_EABI__) +#if defined(__GLIBC__) && (__GLIBC__ >= 2) && (__GLIBC_MINOR__ >= 25) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1 #elif defined(__FreeBSD__) && (__FreeBSD_version >= 1100037) #define MBEDTLS_PLATFORM_HAS_EXPLICIT_BZERO 1