From 6497b5a1d10e59caa45c77bdcfacfeada1337ac0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 17:01:40 +0200 Subject: [PATCH] Add setbuf platform function Add a platform function mbedtls_setbuf(), defaulting to setbuf(). The intent is to allow disabling stdio buffering when reading or writing files with sensitive data, because this exposes the sensitive data to a subsequent memory disclosure vulnerability. Signed-off-by: Gilles Peskine --- include/mbedtls/check_config.h | 14 ++++++++ include/mbedtls/mbedtls_config.h | 3 ++ include/mbedtls/platform.h | 55 ++++++++++++++++++++++++++++++++ library/platform.c | 28 ++++++++++++++++ tests/scripts/all.sh | 1 + 5 files changed, 101 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 1ced6e578..0ac9ccac4 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -368,6 +368,20 @@ #error "MBEDTLS_PLATFORM_EXIT_MACRO and MBEDTLS_PLATFORM_STD_EXIT/MBEDTLS_PLATFORM_EXIT_ALT cannot be defined simultaneously" #endif +#if defined(MBEDTLS_PLATFORM_SETBUF_ALT) && !defined(MBEDTLS_PLATFORM_C) +#error "MBEDTLS_PLATFORM_SETBUF_ALT defined, but not all prerequisites" +#endif + +#if defined(MBEDTLS_PLATFORM_SETBUF_MACRO) && !defined(MBEDTLS_PLATFORM_C) +#error "MBEDTLS_PLATFORM_SETBUF_MACRO defined, but not all prerequisites" +#endif + +#if defined(MBEDTLS_PLATFORM_SETBUF_MACRO) &&\ + ( defined(MBEDTLS_PLATFORM_STD_SETBUF) ||\ + defined(MBEDTLS_PLATFORM_SETBUF_ALT) ) +#error "MBEDTLS_PLATFORM_SETBUF_MACRO and MBEDTLS_PLATFORM_STD_SETBUF/MBEDTLS_PLATFORM_SETBUF_ALT cannot be defined simultaneously" +#endif + #if defined(MBEDTLS_PLATFORM_TIME_ALT) &&\ ( !defined(MBEDTLS_PLATFORM_C) ||\ !defined(MBEDTLS_HAVE_TIME) ) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 2d32f67cc..ad9696135 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -225,6 +225,7 @@ * Uncomment a macro to enable alternate implementation of specific base * platform function */ +//#define MBEDTLS_PLATFORM_SETBUF_ALT //#define MBEDTLS_PLATFORM_EXIT_ALT //#define MBEDTLS_PLATFORM_TIME_ALT //#define MBEDTLS_PLATFORM_FPRINTF_ALT @@ -3288,6 +3289,7 @@ //#define MBEDTLS_PLATFORM_STD_MEM_HDR /**< Header to include if MBEDTLS_PLATFORM_NO_STD_FUNCTIONS is defined. Don't define if no header is needed. */ //#define MBEDTLS_PLATFORM_STD_CALLOC calloc /**< Default allocator to use, can be undefined */ //#define MBEDTLS_PLATFORM_STD_FREE free /**< Default free to use, can be undefined */ +//#define MBEDTLS_PLATFORM_STD_SETBUF setbuf /**< Default setbuf to use, can be undefined */ //#define MBEDTLS_PLATFORM_STD_EXIT exit /**< Default exit to use, can be undefined */ //#define MBEDTLS_PLATFORM_STD_TIME time /**< Default time to use, can be undefined. MBEDTLS_HAVE_TIME must be enabled */ //#define MBEDTLS_PLATFORM_STD_FPRINTF fprintf /**< Default fprintf to use, can be undefined */ @@ -3305,6 +3307,7 @@ //#define MBEDTLS_PLATFORM_CALLOC_MACRO calloc /**< Default allocator macro to use, can be undefined */ //#define MBEDTLS_PLATFORM_FREE_MACRO free /**< Default free macro to use, can be undefined */ //#define MBEDTLS_PLATFORM_EXIT_MACRO exit /**< Default exit macro to use, can be undefined */ +//#define MBEDTLS_PLATFORM_SETBUF_MACRO setbuf /**< Default setbuf macro to use, can be undefined */ //#define MBEDTLS_PLATFORM_TIME_MACRO time /**< Default time macro to use, can be undefined. MBEDTLS_HAVE_TIME must be enabled */ //#define MBEDTLS_PLATFORM_TIME_TYPE_MACRO time_t /**< Default time macro to use, can be undefined. MBEDTLS_HAVE_TIME must be enabled */ //#define MBEDTLS_PLATFORM_FPRINTF_MACRO fprintf /**< Default fprintf macro to use, can be undefined */ diff --git a/include/mbedtls/platform.h b/include/mbedtls/platform.h index a5984345b..bad442e72 100644 --- a/include/mbedtls/platform.h +++ b/include/mbedtls/platform.h @@ -91,6 +91,9 @@ extern "C" { #if !defined(MBEDTLS_PLATFORM_STD_FREE) #define MBEDTLS_PLATFORM_STD_FREE free /**< The default \c free function to use. */ #endif +#if !defined(MBEDTLS_PLATFORM_STD_SETBUF) +#define MBEDTLS_PLATFORM_STD_SETBUF setbuf /**< The default \c setbuf function to use. */ +#endif #if !defined(MBEDTLS_PLATFORM_STD_EXIT) #define MBEDTLS_PLATFORM_STD_EXIT exit /**< The default \c exit function to use. */ #endif @@ -276,6 +279,58 @@ int mbedtls_platform_set_vsnprintf( int (*vsnprintf_func)( char * s, size_t n, #endif /* MBEDTLS_PLATFORM_VSNPRINTF_MACRO */ #endif /* MBEDTLS_PLATFORM_VSNPRINTF_ALT */ +/* + * The function pointers for setbuf + */ +#if defined(MBEDTLS_PLATFORM_SETBUF_ALT) +#include +/** + * \brief Function pointer to call for `setbuf()` functionality + * (changing the internal buffering on stdio calls). + * + * \note The library calls this function to disable + * buffering when reading or writing sensitive data, + * to avoid having extra copies of sensitive data + * remaining in stdio buffers after the file is + * closed. If this is not a concern, for example if + * your platform's stdio doesn't have any buffering, + * you can set mbedtls_setbuf to a function that + * does nothing. + * + * \param setbuf_func The \c setbuf function implementation. + * It is always called with `buf` equal to `NULL`. + * + * \return \c 0 on success, negative on error. + */ +extern void (*mbedtls_setbuf)( FILE *stream, char *buf ); + +/** + * \brief Dynamically configure the function that is called + * when the mbedtls_setbuf() function is called by the + * library. + * + * \param setbuf_func The \c setbuf function implementation + * + * \return \c 0 + */ +int mbedtls_platform_set_setbuf( void (*setbuf_func)( + FILE *stream, char *buf ) ); +#elif defined(MBEDTLS_PLATFORM_SETBUF_MACRO) +/** + * \brief Macro defining the function for the library to + * call for `setbuf` functionality (changing the + * internal buffering on stdio calls). + * + * \note See extra comments on the mbedtls_setbuf() function + * pointer above. + * + * \return \c 0 on success, negative on error. + */ +#define mbedtls_setbuf MBEDTLS_PLATFORM_SETBUF_MACRO +#else +#define mbedtls_setbuf setbuf +#endif /* MBEDTLS_PLATFORM_SETBUF_ALT / MBEDTLS_PLATFORM_SETBUF_MACRO */ + /* * The function pointers for exit */ diff --git a/library/platform.c b/library/platform.c index e742fde7c..f3a1f9847 100644 --- a/library/platform.c +++ b/library/platform.c @@ -226,6 +226,28 @@ int mbedtls_platform_set_fprintf( int (*fprintf_func)( FILE *, const char *, ... } #endif /* MBEDTLS_PLATFORM_FPRINTF_ALT */ +#if defined(MBEDTLS_PLATFORM_SETBUF_ALT) +#if !defined(MBEDTLS_PLATFORM_STD_SETBUF) +/* + * Make dummy function to prevent NULL pointer dereferences + */ +static void platform_setbuf_uninit( FILE *stream, char *buf ) +{ + (( void ) stream); + (( void ) buf); +} + +#define MBEDTLS_PLATFORM_STD_SETBUF platform_setbuf_uninit +#endif /* !MBEDTLS_PLATFORM_STD_SETBUF */ +void (*mbedtls_setbuf)( FILE *stream, char *buf ) = MBEDTLS_PLATFORM_STD_SETBUF; + +int mbedtls_platform_set_setbuf( void (*setbuf_func)( FILE *stream, char *buf ) ) +{ + mbedtls_setbuf = setbuf_func; + return( 0 ); +} +#endif /* MBEDTLS_PLATFORM_SETBUF_ALT */ + #if defined(MBEDTLS_PLATFORM_EXIT_ALT) #if !defined(MBEDTLS_PLATFORM_STD_EXIT) /* @@ -288,6 +310,9 @@ int mbedtls_platform_std_nv_seed_read( unsigned char *buf, size_t buf_len ) if( ( file = fopen( MBEDTLS_PLATFORM_STD_NV_SEED_FILE, "rb" ) ) == NULL ) return( -1 ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + if( ( n = fread( buf, 1, buf_len, file ) ) != buf_len ) { fclose( file ); @@ -307,6 +332,9 @@ int mbedtls_platform_std_nv_seed_write( unsigned char *buf, size_t buf_len ) if( ( file = fopen( MBEDTLS_PLATFORM_STD_NV_SEED_FILE, "w" ) ) == NULL ) return -1; + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + if( ( n = fwrite( buf, 1, buf_len, file ) ) != buf_len ) { fclose( file ); diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1e5cd65ee..1a3a04b82 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2178,6 +2178,7 @@ component_test_no_platform () { scripts/config.py unset MBEDTLS_PLATFORM_SNPRINTF_ALT scripts/config.py unset MBEDTLS_PLATFORM_TIME_ALT scripts/config.py unset MBEDTLS_PLATFORM_EXIT_ALT + scripts/config.py unset MBEDTLS_PLATFORM_SETBUF_ALT scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED scripts/config.py unset MBEDTLS_FS_IO