Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport 2.7: Fix premature fopen() call in mbedtls_entropy_write_seed_file #3821

Closed
wants to merge 1 commit into from
Closed

Backport 2.7: Fix premature fopen() call in mbedtls_entropy_write_seed_file #3821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog.d/bugfix_PR3616.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 13 additions & 5 deletions library/entropy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,20 @@ int mbedtls_entropy_update_nv_seed( mbedtls_entropy_context *ctx )
int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *path )
{
int ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR;
FILE *f;
FILE *f = NULL;
unsigned char buf[MBEDTLS_ENTROPY_BLOCK_SIZE];

if( ( f = fopen( path, "wb" ) ) == NULL )
return( MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR );
if( (mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should check the return value here like we do on the other two branches:

Suggested change
if( (mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 )
if( ( ret = mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value is checked. It isn't assigned to ret, but that doesn't matter.

However please do assign to ret like in the other branches. Keeping the code identical where it's possible makes future backports easier.

{
ret = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED;
goto exit;
}

if( ( ret = mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 )
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 )
{
Expand All @@ -513,7 +519,9 @@ int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *p
exit:
mbedtls_zeroize( buf, sizeof( buf ) );

fclose( f );
if( f != NULL )
fclose( f );

return( ret );
}

Expand Down
3 changes: 3 additions & 0 deletions tests/suites/test_suite_entropy.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ entropy_seed_file:"data_files/entropy_seed":0
Entropy write/update seed file
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 too many sources
entropy_too_many_sources:

Expand Down
15 changes: 15 additions & 0 deletions tests/suites/test_suite_entropy.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,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_too_many_sources( )
{
Expand Down