Make entropy double-free work

Although the library documentation does not guarantee that calling
mbedtls_entropy_free() twice works, it's a plausible assumption and it's
natural to write code that frees an object twice. While this is uncommon for
an entropy context, which is usually a global variable, it came up in our
own unit tests (random_twice tests in test_suite_random).

Announce this in the same changelog entry as for RSA because it's the same
bug in the two modules.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2021-02-22 21:26:54 +01:00
parent 3d979f781e
commit b15832160b
3 changed files with 14 additions and 7 deletions

View File

@ -1,8 +1,8 @@
Bugfix Bugfix
* Ensure that calling mbedtls_rsa_free() twice is safe. This happens * Ensure that calling mbedtls_rsa_free() or mbedtls_entropy_free()
when some Mbed TLS library functions fail. Such a double-free was twice is safe. This happens for RSA when some Mbed TLS library functions
not safe when MBEDTLS_THREADING_C was enabled on platforms where fail. Such a double-free was not safe when MBEDTLS_THREADING_C was
freeing a mutex twice is not safe. enabled on platforms where freeing a mutex twice is not safe.
* Fix a resource leak in a bad-arguments case of mbedtls_rsa_gen_key() * Fix a resource leak in a bad-arguments case of mbedtls_rsa_gen_key()
when MBEDTLS_THREADING_C is enabled on platforms where initializing when MBEDTLS_THREADING_C is enabled on platforms where initializing
a mutex allocates resources. a mutex allocates resources.

View File

@ -120,13 +120,15 @@ mbedtls_entropy_source_state;
*/ */
typedef struct mbedtls_entropy_context typedef struct mbedtls_entropy_context
{ {
int accumulator_started; int accumulator_started; /* 0 after init.
* 1 after the first update.
* -1 after free. */
#if defined(MBEDTLS_ENTROPY_SHA512_ACCUMULATOR) #if defined(MBEDTLS_ENTROPY_SHA512_ACCUMULATOR)
mbedtls_sha512_context accumulator; mbedtls_sha512_context accumulator;
#else #else
mbedtls_sha256_context accumulator; mbedtls_sha256_context accumulator;
#endif #endif
int source_count; int source_count; /* Number of entries used in source. */
mbedtls_entropy_source_state source[MBEDTLS_ENTROPY_MAX_SOURCES]; mbedtls_entropy_source_state source[MBEDTLS_ENTROPY_MAX_SOURCES];
#if defined(MBEDTLS_HAVEGE_C) #if defined(MBEDTLS_HAVEGE_C)
mbedtls_havege_state havege_data; mbedtls_havege_state havege_data;

View File

@ -116,6 +116,11 @@ void mbedtls_entropy_init( mbedtls_entropy_context *ctx )
void mbedtls_entropy_free( mbedtls_entropy_context *ctx ) void mbedtls_entropy_free( mbedtls_entropy_context *ctx )
{ {
/* If the context was already free, don't call free() again.
* This is important for mutexes which don't allow double-free. */
if( ctx->accumulator_started == -1 )
return;
#if defined(MBEDTLS_HAVEGE_C) #if defined(MBEDTLS_HAVEGE_C)
mbedtls_havege_free( &ctx->havege_data ); mbedtls_havege_free( &ctx->havege_data );
#endif #endif
@ -132,7 +137,7 @@ void mbedtls_entropy_free( mbedtls_entropy_context *ctx )
#endif #endif
ctx->source_count = 0; ctx->source_count = 0;
mbedtls_platform_zeroize( ctx->source, sizeof( ctx->source ) ); mbedtls_platform_zeroize( ctx->source, sizeof( ctx->source ) );
ctx->accumulator_started = 0; ctx->accumulator_started = -1;
} }
int mbedtls_entropy_add_source( mbedtls_entropy_context *ctx, int mbedtls_entropy_add_source( mbedtls_entropy_context *ctx,