diff --git a/ChangeLog.d/add_mbedtls_setbuf.txt b/ChangeLog.d/add_mbedtls_setbuf.txt new file mode 100644 index 000000000..d14cd18aa --- /dev/null +++ b/ChangeLog.d/add_mbedtls_setbuf.txt @@ -0,0 +1,11 @@ +Security + * Add the platform function mbedtls_setbuf() to allow buffering to be + disabled on stdio files, to stop secrets loaded from said files being + potentially left in memory after file operations. Reported by + Glenn Strauss. +Requirement changes + * The library will no longer compile out of the box on a platform without + setbuf(). If your platform does not have setbuf(), you can configure an + alternative function by enabling MBEDTLS_PLATFORM_SETBUF_ALT or + MBEDTLS_PLATFORM_SETBUF_MACRO. + diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index bdc32e183..1c3d6ab40 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -385,6 +385,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 9c8ec11a7..060973f22 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 @@ -3318,6 +3319,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 */ @@ -3335,6 +3337,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..a5a43ac6d 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,56 @@ 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. + * + * The library always calls this function with + * `buf` equal to `NULL`. + */ +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/ctr_drbg.c b/library/ctr_drbg.c index 23ea07b23..43f490e83 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -607,6 +607,9 @@ int mbedtls_ctr_drbg_write_seed_file( mbedtls_ctr_drbg_context *ctx, if( ( f = fopen( path, "wb" ) ) == NULL ) return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( ( ret = mbedtls_ctr_drbg_random( ctx, buf, MBEDTLS_CTR_DRBG_MAX_INPUT ) ) != 0 ) goto exit; @@ -640,6 +643,9 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + n = fread( buf, 1, sizeof( buf ), f ); if( fread( &c, 1, 1, f ) != 0 ) { diff --git a/library/dhm.c b/library/dhm.c index 2ce0ed4fd..1e95bdab0 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -620,6 +620,7 @@ static int load_file( const char *path, unsigned char **buf, size_t *n ) if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_DHM_FILE_IO_ERROR ); + /* The data loaded here is public, so don't bother disabling buffering. */ fseek( f, 0, SEEK_END ); if( ( size = ftell( f ) ) == -1 ) diff --git a/library/entropy.c b/library/entropy.c index 9e31f8491..08c5bd7d1 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -457,6 +457,9 @@ int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *p goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) { ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; @@ -484,6 +487,9 @@ int mbedtls_entropy_update_seed_file( mbedtls_entropy_context *ctx, const char * if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + fseek( f, 0, SEEK_END ); n = (size_t) ftell( f ); fseek( f, 0, SEEK_SET ); diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 058c307df..2ae57fdc0 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -35,7 +35,7 @@ #if defined(MBEDTLS_TIMING_C) #include "mbedtls/timing.h" #endif -#if defined(MBEDTLS_ENTROPY_NV_SEED) +#if defined(MBEDTLS_ENTROPY_NV_SEED) || !defined(HAVE_SYSCTL_ARND) #include "mbedtls/platform.h" #endif @@ -195,6 +195,9 @@ int mbedtls_platform_entropy_poll( void *data, if( file == NULL ) return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + read_len = fread( output, 1, len, file ); if( read_len != len ) { diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index ab353bfd5..8b13a860f 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -436,6 +436,9 @@ int mbedtls_hmac_drbg_write_seed_file( mbedtls_hmac_drbg_context *ctx, const cha if( ( f = fopen( path, "wb" ) ) == NULL ) return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( ( ret = mbedtls_hmac_drbg_random( ctx, buf, sizeof( buf ) ) ) != 0 ) goto exit; @@ -465,6 +468,9 @@ int mbedtls_hmac_drbg_update_seed_file( mbedtls_hmac_drbg_context *ctx, const ch if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + n = fread( buf, 1, sizeof( buf ), f ); if( fread( &c, 1, 1, f ) != 0 ) { diff --git a/library/md.c b/library/md.c index f2c1a90f8..a387da50a 100644 --- a/library/md.c +++ b/library/md.c @@ -605,6 +605,9 @@ int mbedtls_md_file( const mbedtls_md_info_t *md_info, const char *path, unsigne if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_MD_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + mbedtls_md_init( &ctx ); if( ( ret = mbedtls_md_setup( &ctx, md_info, 0 ) ) != 0 ) diff --git a/library/pkparse.c b/library/pkparse.c index 68727ec7e..722abd741 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -82,6 +82,9 @@ int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n ) if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_PK_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + fseek( f, 0, SEEK_END ); if( ( size = ftell( f ) ) == -1 ) { diff --git a/library/platform.c b/library/platform.c index e742fde7c..6151e6c49 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/library/psa_its_file.c b/library/psa_its_file.c index f05872095..b7c2e6b04 100644 --- a/library/psa_its_file.c +++ b/library/psa_its_file.c @@ -102,6 +102,9 @@ static psa_status_t psa_its_read_file( psa_storage_uid_t uid, if( *p_stream == NULL ) return( PSA_ERROR_DOES_NOT_EXIST ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( *p_stream, NULL ); + n = fread( &header, 1, sizeof( header ), *p_stream ); if( n != sizeof( header ) ) return( PSA_ERROR_DATA_CORRUPT ); @@ -201,9 +204,13 @@ psa_status_t psa_its_set( psa_storage_uid_t uid, psa_its_fill_filename( uid, filename ); stream = fopen( PSA_ITS_STORAGE_TEMP, "wb" ); + if( stream == NULL ) goto exit; + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( stream, NULL ); + status = PSA_ERROR_INSUFFICIENT_STORAGE; n = fwrite( &header, 1, sizeof( header ), stream ); if( n != sizeof( header ) ) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index 728854875..136e25ba4 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -166,6 +166,10 @@ int main( int argc, char *argv[] ) goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( fin, NULL ); + mbedtls_setbuf( fout, NULL ); + /* * Read the Cipher and MD from the command line */ diff --git a/programs/psa/key_ladder_demo.c b/programs/psa/key_ladder_demo.c index cad875e01..13037190d 100644 --- a/programs/psa/key_ladder_demo.c +++ b/programs/psa/key_ladder_demo.c @@ -56,6 +56,7 @@ #include #include +#include "mbedtls/platform.h" // for mbedtls_setbuf #include "mbedtls/platform_util.h" // for mbedtls_platform_zeroize #include @@ -177,6 +178,8 @@ static psa_status_t save_key( psa_key_id_t key, key_data, sizeof( key_data ), &key_size ) ); SYS_CHECK( ( key_file = fopen( output_file_name, "wb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( key_file, NULL ); SYS_CHECK( fwrite( key_data, 1, key_size, key_file ) == key_size ); SYS_CHECK( fclose( key_file ) == 0 ); key_file = NULL; @@ -231,6 +234,8 @@ static psa_status_t import_key_from_file( psa_key_usage_t usage, unsigned char extra_byte; SYS_CHECK( ( key_file = fopen( key_file_name, "rb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( key_file, NULL ); SYS_CHECK( ( key_size = fread( key_data, 1, sizeof( key_data ), key_file ) ) != 0 ); if( fread( &extra_byte, 1, 1, key_file ) != 0 ) @@ -372,6 +377,8 @@ static psa_status_t wrap_data( const char *input_file_name, /* Find the size of the data to wrap. */ SYS_CHECK( ( input_file = fopen( input_file_name, "rb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( input_file, NULL ); SYS_CHECK( fseek( input_file, 0, SEEK_END ) == 0 ); SYS_CHECK( ( input_position = ftell( input_file ) ) != -1 ); #if LONG_MAX > SIZE_MAX @@ -418,6 +425,8 @@ static psa_status_t wrap_data( const char *input_file_name, /* Write the output. */ SYS_CHECK( ( output_file = fopen( output_file_name, "wb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( output_file, NULL ); SYS_CHECK( fwrite( &header, 1, sizeof( header ), output_file ) == sizeof( header ) ); SYS_CHECK( fwrite( buffer, 1, ciphertext_size, @@ -453,6 +462,8 @@ static psa_status_t unwrap_data( const char *input_file_name, /* Load and validate the header. */ SYS_CHECK( ( input_file = fopen( input_file_name, "rb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( input_file, NULL ); SYS_CHECK( fread( &header, 1, sizeof( header ), input_file ) == sizeof( header ) ); if( memcmp( &header.magic, WRAPPED_DATA_MAGIC, @@ -509,6 +520,8 @@ static psa_status_t unwrap_data( const char *input_file_name, /* Write the output. */ SYS_CHECK( ( output_file = fopen( output_file_name, "wb" ) ) != NULL ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( output_file, NULL ); SYS_CHECK( fwrite( buffer, 1, plaintext_size, output_file ) == plaintext_size ); SYS_CHECK( fclose( output_file ) == 0 ); diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index 511ede9cb..24e1f3eee 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -101,6 +101,10 @@ void nss_keylog_export( void *p_expkey, goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be + * wiped. */ + mbedtls_setbuf( f, NULL ); + if( fwrite( nss_keylog_line, 1, len, f ) != len ) { fclose( f ); diff --git a/programs/ssl/ssl_test_lib.h b/programs/ssl/ssl_test_lib.h index 03349babb..c368f573a 100644 --- a/programs/ssl/ssl_test_lib.h +++ b/programs/ssl/ssl_test_lib.h @@ -34,6 +34,7 @@ #define mbedtls_printf printf #define mbedtls_fprintf fprintf #define mbedtls_snprintf snprintf +#define mbedtls_setbuf setbuf #define mbedtls_exit exit #define MBEDTLS_EXIT_SUCCESS EXIT_SUCCESS #define MBEDTLS_EXIT_FAILURE EXIT_FAILURE diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 27c211fbf..458fe8f5b 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2253,6 +2253,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