From 563a333a843f5890658c38ea07996bbbae8d5ace Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Sep 2018 16:41:54 +0200 Subject: [PATCH] CTR_DRBG: add mbedtls_ctr_drbg_update_ret Deprecate mbedtls_ctr_drbg_update (which returns void) in favor of a new function mbedtls_ctr_drbg_update_ret which reports error. The old function is not officially marked as deprecated in this branch because this is a stable maintenance branch. --- include/mbedtls/ctr_drbg.h | 39 +++++++++++++++++----- library/ctr_drbg.c | 40 +++++++++++++++-------- tests/suites/test_suite_ctr_drbg.function | 8 +++-- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/include/mbedtls/ctr_drbg.h b/include/mbedtls/ctr_drbg.h index c91ca58b3..304bc911c 100644 --- a/include/mbedtls/ctr_drbg.h +++ b/include/mbedtls/ctr_drbg.h @@ -237,20 +237,41 @@ int mbedtls_ctr_drbg_reseed( mbedtls_ctr_drbg_context *ctx, const unsigned char *additional, size_t len ); /** - * \brief This function updates the state of the CTR_DRBG context. + * \brief This function updates the state of the CTR_DRBG context. * - * \note If \p add_len is greater than - * #MBEDTLS_CTR_DRBG_MAX_SEED_INPUT, only the first - * #MBEDTLS_CTR_DRBG_MAX_SEED_INPUT Bytes are used. - * The remaining Bytes are silently discarded. + * \param ctx The CTR_DRBG context. + * \param additional The data to update the state with. + * \param add_len Length of \p additional in bytes. This must be at + * most #MBEDTLS_CTR_DRBG_MAX_SEED_INPUT. * - * \param ctx The CTR_DRBG context. - * \param additional The data to update the state with. - * \param add_len Length of \p additional data. + * \return \c 0 on success. + * \return #MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG if + * \p add_len is more than + * #MBEDTLS_CTR_DRBG_MAX_SEED_INPUT. + * \return An error from the underlying AES cipher on failure. + */ +int mbedtls_ctr_drbg_update_ret( mbedtls_ctr_drbg_context *ctx, + const unsigned char *additional, + size_t add_len ); + +/** + * \brief This function updates the state of the CTR_DRBG context. * + * \warning This function cannot report errors. You should use + * mbedtls_ctr_drbg_update_ret() instead. + * + * \note If \p add_len is greater than + * #MBEDTLS_CTR_DRBG_MAX_SEED_INPUT, only the first + * #MBEDTLS_CTR_DRBG_MAX_SEED_INPUT Bytes are used. + * The remaining Bytes are silently discarded. + * + * \param ctx The CTR_DRBG context. + * \param additional The data to update the state with. + * \param add_len Length of \p additional data. */ void mbedtls_ctr_drbg_update( mbedtls_ctr_drbg_context *ctx, - const unsigned char *additional, size_t add_len ); + const unsigned char *additional, + size_t add_len ); /** * \brief This function updates a CTR_DRBG instance with additional diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 559538121..0655e7652 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -331,22 +331,36 @@ exit: * and with outputs * ctx = initial_working_state */ -void mbedtls_ctr_drbg_update( mbedtls_ctr_drbg_context *ctx, - const unsigned char *additional, size_t add_len ) +int mbedtls_ctr_drbg_update_ret( mbedtls_ctr_drbg_context *ctx, + const unsigned char *additional, + size_t add_len ) { unsigned char add_input[MBEDTLS_CTR_DRBG_SEEDLEN]; + int ret; - if( add_len > 0 ) - { - /* MAX_INPUT would be more logical here, but we have to match - * block_cipher_df()'s limits since we can't propagate errors */ - if( add_len > MBEDTLS_CTR_DRBG_MAX_SEED_INPUT ) - add_len = MBEDTLS_CTR_DRBG_MAX_SEED_INPUT; + if( add_len == 0 ) + return( 0 ); - block_cipher_df( add_input, additional, add_len ); - ctr_drbg_update_internal( ctx, add_input ); - mbedtls_platform_zeroize( add_input, sizeof( add_input ) ); - } + if( ( ret = block_cipher_df( add_input, additional, add_len ) ) != 0 ) + goto exit; + if( ( ret = ctr_drbg_update_internal( ctx, add_input ) ) != 0 ) + goto exit; + +exit: + mbedtls_platform_zeroize( add_input, sizeof( add_input ) ); + return( ret ); +} + +/* Deprecated function, kept for backward compatibility. */ +void mbedtls_ctr_drbg_update( mbedtls_ctr_drbg_context *ctx, + const unsigned char *additional, + size_t add_len ) +{ + /* MAX_INPUT would be more logical here, but we have to match + * block_cipher_df()'s limits since we can't propagate errors */ + if( add_len > MBEDTLS_CTR_DRBG_MAX_SEED_INPUT ) + add_len = MBEDTLS_CTR_DRBG_MAX_SEED_INPUT; + (void) mbedtls_ctr_drbg_update_ret( ctx, additional, add_len ); } /* CTR_DRBG_Reseed with derivation function (SP 800-90A §10.2.1.4.2) @@ -573,7 +587,7 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, const char if( fread( buf, 1, n, f ) != n ) ret = MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR; else - mbedtls_ctr_drbg_update( ctx, buf, n ); + ret = mbedtls_ctr_drbg_update_ret( ctx, buf, n ); fclose( f ); diff --git a/tests/suites/test_suite_ctr_drbg.function b/tests/suites/test_suite_ctr_drbg.function index f10e98aa5..4a97826f6 100644 --- a/tests/suites/test_suite_ctr_drbg.function +++ b/tests/suites/test_suite_ctr_drbg.function @@ -244,9 +244,11 @@ void ctr_drbg_entropy_usage( ) } TEST_ASSERT( last_idx == test_offset_idx ); - /* Call update with too much data (sizeof entropy > MAX(_SEED)_INPUT) - * (just make sure it doesn't cause memory corruption) */ - mbedtls_ctr_drbg_update( &ctx, entropy, sizeof( entropy ) ); + /* Call update with too much data (sizeof entropy > MAX(_SEED)_INPUT). + * Make sure it's detected as an error and doesn't cause memory + * corruption. */ + TEST_ASSERT( mbedtls_ctr_drbg_update_ret( + &ctx, entropy, sizeof( entropy ) ) != 0 ); /* Now enable PR, so the next few calls should all reseed */ mbedtls_ctr_drbg_set_prediction_resistance( &ctx, MBEDTLS_CTR_DRBG_PR_ON );