Backport 2.7: Fix premature fopen() call in mbedtls_entropy_write_seed_file #3821
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…S#3175 Signed-off-by: Victor Krasnoshchok <ct3da21164@protonmail.ch>
militant-daos
force-pushed
the
mbedtls-2.7
branch
from
6916cf4 to
d9d484c
Compare
2 tasks
yanesca
requested changes
Oct 26, 2020
| 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 ) |
There was a problem hiding this comment.
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 ) |
There was a problem hiding this comment.
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.
yanesca
added
Community
needs-reviewer
|
Also, Travis CI is failing, could you please check out what the issue is and fix it? |
gilles-peskine-arm
changed the title
|
The 2.7 branch is no longer being maintained, so I am closing this pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Victor Krasnoshchok ct3da21164@protonmail.ch
Description
The PR fixes potential seed file corruption in case if
pathparam ofmbedtls_entropy_write_seed_fileis equal toMBEDTLS_PLATFORM_STD_NV_SEED_FILE(or vice versa; i.e. the same file is used to read from and to write to).Status
READY
Requires Backporting
No
Migrations
NO
Steps to test
MBEDTLS_ENTROPY_NV_SEEDin the active config (config.h);test_suite_entropy.datain such a way to makeentropy_seed_filedefine the same name asMBEDTLS_PLATFORM_STD_NV_SEED_FILE(seedfileby default);test_suite_entropytarget - NV-file related tests should pass.