From e79812ed4d9c3186c3e4040217143cd0419a8e4a Mon Sep 17 00:00:00 2001 From: Victor Krasnoshchok Date: Thu, 27 Aug 2020 00:19:55 +0300 Subject: [PATCH 1/4] Fix premature fopen() call in mbedtls_entropy_write_seed_file #3175 Signed-off-by: Victor Krasnoshchok --- library/entropy.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index db61f16d84..519c3aef35 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -466,28 +466,27 @@ int mbedtls_entropy_update_nv_seed( mbedtls_entropy_context *ctx ) #if defined(MBEDTLS_FS_IO) int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *path ) { - int ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; - FILE *f; + int ret; + FILE *f = NULL; unsigned char buf[MBEDTLS_ENTROPY_BLOCK_SIZE]; - if( ( f = fopen( path, "wb" ) ) == NULL ) - return( MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR ); - if( ( ret = mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 ) goto exit; - if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) + ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; + if( ( f = fopen( path, "wb" ) ) != NULL ) { - ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; - goto exit; + if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) + goto exit; + ret = 0; } - ret = 0; - exit: mbedtls_platform_zeroize( buf, sizeof( buf ) ); - fclose( f ); + if( f ) + fclose( f ); + return( ret ); } From b3129ba11930c81fb47b58d81e4c4651a24aaec6 Mon Sep 17 00:00:00 2001 From: Victor Krasnoshchok Date: Sat, 29 Aug 2020 22:54:37 +0300 Subject: [PATCH 2/4] Refactoring after CR and new unit test #3175 Signed-off-by: Victor Krasnoshchok --- library/entropy.c | 25 +++++++++++++++--------- tests/suites/test_suite_entropy.data | 3 +++ tests/suites/test_suite_entropy.function | 15 ++++++++++++++ 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index 519c3aef35..fd2c207f08 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -466,25 +466,32 @@ int mbedtls_entropy_update_nv_seed( mbedtls_entropy_context *ctx ) #if defined(MBEDTLS_FS_IO) int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *path ) { - int ret; + int ret = 0; FILE *f = NULL; unsigned char buf[MBEDTLS_ENTROPY_BLOCK_SIZE]; if( ( ret = mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 ) - goto exit; - - ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; - if( ( f = fopen( path, "wb" ) ) != NULL ) { - if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) - goto exit; - ret = 0; + ret = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; + goto exit; + } + + if( ( f = fopen( path, "wb" ) ) == NULL ) + { + ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; + goto exit; + } + + if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) + { + ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; + goto exit; } exit: mbedtls_platform_zeroize( buf, sizeof( buf ) ); - if( f ) + if( f != NULL ) fclose( f ); return( ret ); diff --git a/tests/suites/test_suite_entropy.data b/tests/suites/test_suite_entropy.data index b2d20b4729..bc077f8158 100644 --- a/tests/suites/test_suite_entropy.data +++ b/tests/suites/test_suite_entropy.data @@ -7,6 +7,9 @@ entropy_seed_file:"data_files/entropy_seed":0 Entropy write/update seed file: nonexistent entropy_seed_file:"no_such_dir/file":MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR +Entropy write/update seed file: base NV seed file +entropy_write_base_seed_file:0 + Entropy no sources entropy_no_sources: diff --git a/tests/suites/test_suite_entropy.function b/tests/suites/test_suite_entropy.function index d9ea441492..a453aadf3c 100644 --- a/tests/suites/test_suite_entropy.function +++ b/tests/suites/test_suite_entropy.function @@ -149,6 +149,21 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_NV_SEED:MBEDTLS_FS_IO */ +void entropy_write_base_seed_file( int ret ) +{ + mbedtls_entropy_context ctx; + + mbedtls_entropy_init( &ctx ); + + TEST_ASSERT( mbedtls_entropy_write_seed_file( &ctx, MBEDTLS_PLATFORM_STD_NV_SEED_FILE ) == ret ); + TEST_ASSERT( mbedtls_entropy_update_seed_file( &ctx, MBEDTLS_PLATFORM_STD_NV_SEED_FILE ) == ret ); + +exit: + mbedtls_entropy_free( &ctx ); +} +/* END_CASE */ + /* BEGIN_CASE */ void entropy_no_sources( ) { From a0c2d19d8451d95b2af7d48a429b1c469c656910 Mon Sep 17 00:00:00 2001 From: Victor Krasnoshchok Date: Thu, 3 Sep 2020 00:07:05 +0300 Subject: [PATCH 3/4] Code style fix #3175 Signed-off-by: Victor Krasnoshchok --- library/entropy.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/entropy.c b/library/entropy.c index fd2c207f08..81b4c509ee 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -466,7 +466,7 @@ int mbedtls_entropy_update_nv_seed( mbedtls_entropy_context *ctx ) #if defined(MBEDTLS_FS_IO) int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *path ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; FILE *f = NULL; unsigned char buf[MBEDTLS_ENTROPY_BLOCK_SIZE]; @@ -488,6 +488,8 @@ int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *p goto exit; } + ret = 0; + exit: mbedtls_platform_zeroize( buf, sizeof( buf ) ); From 6361ad9bc61e1041fb4fd8beb8033d6dd1b10efb Mon Sep 17 00:00:00 2001 From: Victor Krasnoshchok Date: Sun, 27 Sep 2020 23:51:21 +0300 Subject: [PATCH 4/4] Changelog update #3175 Signed-off-by: Victor Krasnoshchok --- ChangeLog.d/bugfix_PR3616.txt | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ChangeLog.d/bugfix_PR3616.txt diff --git a/ChangeLog.d/bugfix_PR3616.txt b/ChangeLog.d/bugfix_PR3616.txt new file mode 100644 index 0000000000..47d1044922 --- /dev/null +++ b/ChangeLog.d/bugfix_PR3616.txt @@ -0,0 +1,5 @@ +Bugfix + * Fix premature fopen() call in mbedtls_entropy_write_seed_file which may + lead to the seed file corruption in case if the path to the seed file is + equal to MBEDTLS_PLATFORM_STD_NV_SEED_FILE. Contributed by Victor + Krasnoshchok in #3616.