Merge pull request #952 from gilles-peskine-arm/stdio_buffering-setbuf
Turn off stdio buffering with setbuf()
diff --git a/ChangeLog.d/add_mbedtls_setbuf.txt b/ChangeLog.d/add_mbedtls_setbuf.txt
new file mode 100644
index 0000000..d14cd18
--- /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 bdc32e1..1c3d6ab 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 9c8ec11..060973f 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 <stdlib.h> /**< 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 a598434..a5a43ac 100644
--- a/include/mbedtls/platform.h
+++ b/include/mbedtls/platform.h
@@ -91,6 +91,9 @@
#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
@@ -277,6 +280,56 @@
#endif /* MBEDTLS_PLATFORM_VSNPRINTF_ALT */
/*
+ * The function pointers for setbuf
+ */
+#if defined(MBEDTLS_PLATFORM_SETBUF_ALT)
+#include <stdio.h>
+/**
+ * \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
*/
#if defined(MBEDTLS_PLATFORM_EXIT_ALT)
diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c
index 23ea07b..43f490e 100644
--- a/library/ctr_drbg.c
+++ b/library/ctr_drbg.c
@@ -607,6 +607,9 @@
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 @@
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 2ce0ed4..1e95bda 100644
--- a/library/dhm.c
+++ b/library/dhm.c
@@ -620,6 +620,7 @@
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 9e31f84..08c5bd7 100644
--- a/library/entropy.c
+++ b/library/entropy.c
@@ -457,6 +457,9 @@
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 @@
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 058c307..2ae57fd 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 @@
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 ab353bf..8b13a86 100644
--- a/library/hmac_drbg.c
+++ b/library/hmac_drbg.c
@@ -436,6 +436,9 @@
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 @@
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 f2c1a90..a387da5 100644
--- a/library/md.c
+++ b/library/md.c
@@ -605,6 +605,9 @@
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 68727ec..722abd7 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -82,6 +82,9 @@
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 e742fde..6151e6c 100644
--- a/library/platform.c
+++ b/library/platform.c
@@ -226,6 +226,28 @@
}
#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 @@
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 @@
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 f058720..b7c2e6b 100644
--- a/library/psa_its_file.c
+++ b/library/psa_its_file.c
@@ -102,6 +102,9 @@
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_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 7288548..136e25b 100644
--- a/programs/aes/crypt_and_hash.c
+++ b/programs/aes/crypt_and_hash.c
@@ -166,6 +166,10 @@
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 cad875e..1303719 100644
--- a/programs/psa/key_ladder_demo.c
+++ b/programs/psa/key_ladder_demo.c
@@ -56,6 +56,7 @@
#include <stdio.h>
#include <string.h>
+#include "mbedtls/platform.h" // for mbedtls_setbuf
#include "mbedtls/platform_util.h" // for mbedtls_platform_zeroize
#include <psa/crypto.h>
@@ -177,6 +178,8 @@
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 @@
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 @@
/* 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 @@
/* 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 @@
/* 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 @@
/* 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 511ede9..24e1f3e 100644
--- a/programs/ssl/ssl_test_common_source.c
+++ b/programs/ssl/ssl_test_common_source.c
@@ -101,6 +101,10 @@
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 03349ba..c368f57 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 27c211f..458fe8f 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -2253,6 +2253,7 @@
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
