From 8f7921ec4b8207550a765070edbca815e7374a02 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 4 Oct 2019 11:47:35 +0200 Subject: [PATCH 1/3] HMAC_DRBG: support set_entropy_len() before seed() mbedtls_hmac_drbg_seed() always set the entropy length to the default, so a call to mbedtls_hmac_drbg_set_entropy_len() before seed() had no effect. Change this to the more intuitive behavior that set_entropy_len() sets the entropy length and seed() respects that and only uses the default entropy length if there was no call to set_entropy_len(). --- include/mbedtls/hmac_drbg.h | 8 +++----- library/hmac_drbg.c | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/hmac_drbg.h b/include/mbedtls/hmac_drbg.h index 519d692fb..8ac227caa 100644 --- a/include/mbedtls/hmac_drbg.h +++ b/include/mbedtls/hmac_drbg.h @@ -141,11 +141,9 @@ void mbedtls_hmac_drbg_init( mbedtls_hmac_drbg_context *ctx ); * entropy length is set with * mbedtls_hmac_drbg_set_entropy_len() afterwards. * - * \note The entropy length for the initial seeding is - * the security strength (converted from bits to bytes). - * You can set a different entropy length for subsequent - * seeding by calling mbedtls_hmac_drbg_set_entropy_len() - * after this function. + * \note The default entropy length is the security strength + * (converted from bits to bytes). You can override + * it by calling mbedtls_hmac_drbg_set_entropy_len(). * * \note During the initial seeding, this function calls * the entropy source to obtain a nonce diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index 50d88bd54..284c9b4e9 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -273,16 +273,19 @@ int mbedtls_hmac_drbg_seed( mbedtls_hmac_drbg_context *ctx, ctx->reseed_interval = MBEDTLS_HMAC_DRBG_RESEED_INTERVAL; - /* - * See SP800-57 5.6.1 (p. 65-66) for the security strength provided by - * each hash function, then according to SP800-90A rev1 10.1 table 2, - * min_entropy_len (in bits) is security_strength. - * - * (This also matches the sizes used in the NIST test vectors.) - */ - ctx->entropy_len = md_size <= 20 ? 16 : /* 160-bits hash -> 128 bits */ - md_size <= 28 ? 24 : /* 224-bits hash -> 192 bits */ - 32; /* better (256+) -> 256 bits */ + if( ctx->entropy_len == 0 ) + { + /* + * See SP800-57 5.6.1 (p. 65-66) for the security strength provided by + * each hash function, then according to SP800-90A rev1 10.1 table 2, + * min_entropy_len (in bits) is security_strength. + * + * (This also matches the sizes used in the NIST test vectors.) + */ + ctx->entropy_len = md_size <= 20 ? 16 : /* 160-bits hash -> 128 bits */ + md_size <= 28 ? 24 : /* 224-bits hash -> 192 bits */ + 32; /* better (256+) -> 256 bits */ + } if( ( ret = hmac_drbg_reseed_core( ctx, custom, len, 1 /* add nonce */ ) ) != 0 ) @@ -303,7 +306,7 @@ void mbedtls_hmac_drbg_set_prediction_resistance( mbedtls_hmac_drbg_context *ctx } /* - * Set entropy length grabbed for reseeds + * Set entropy length grabbed for seeding */ void mbedtls_hmac_drbg_set_entropy_len( mbedtls_hmac_drbg_context *ctx, size_t len ) { From 8bf5613336f159c70560d3d93cd6c8362e102472 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Oct 2019 20:31:54 +0200 Subject: [PATCH 2/3] CTR_DRBG: Don't use functions before they're defined Move the definitions of mbedtls_ctr_drbg_seed_entropy_len() and mbedtls_ctr_drbg_seed() to after they are used. This makes the code easier to read and to maintain. --- library/ctr_drbg.c | 128 ++++++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 0db7beb29..c986b7019 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -62,70 +62,6 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx ) #endif } -/* - * Non-public function wrapped by mbedtls_ctr_drbg_seed(). Necessary to allow - * NIST tests to succeed (which require known length fixed entropy) - */ -/* CTR_DRBG_Instantiate with derivation function (SP 800-90A §10.2.1.3.2) - * mbedtls_ctr_drbg_seed_entropy_len(ctx, f_entropy, p_entropy, - * custom, len, entropy_len) - * implements - * CTR_DRBG_Instantiate(entropy_input, nonce, personalization_string, - * security_strength) -> initial_working_state - * with inputs - * custom[:len] = nonce || personalization_string - * where entropy_input comes from f_entropy for entropy_len bytes - * and with outputs - * ctx = initial_working_state - */ -int mbedtls_ctr_drbg_seed_entropy_len( - mbedtls_ctr_drbg_context *ctx, - int (*f_entropy)(void *, unsigned char *, size_t), - void *p_entropy, - const unsigned char *custom, - size_t len, - size_t entropy_len ) -{ - int ret; - unsigned char key[MBEDTLS_CTR_DRBG_KEYSIZE]; - - memset( key, 0, MBEDTLS_CTR_DRBG_KEYSIZE ); - - mbedtls_aes_init( &ctx->aes_ctx ); - - ctx->f_entropy = f_entropy; - ctx->p_entropy = p_entropy; - - ctx->entropy_len = entropy_len; - ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL; - - /* - * Initialize with an empty key - */ - if( ( ret = mbedtls_aes_setkey_enc( &ctx->aes_ctx, key, - MBEDTLS_CTR_DRBG_KEYBITS ) ) != 0 ) - { - return( ret ); - } - - if( ( ret = mbedtls_ctr_drbg_reseed( ctx, custom, len ) ) != 0 ) - { - return( ret ); - } - return( 0 ); -} - -int mbedtls_ctr_drbg_seed( mbedtls_ctr_drbg_context *ctx, - int (*f_entropy)(void *, unsigned char *, size_t), - void *p_entropy, - const unsigned char *custom, - size_t len ) -{ - return( mbedtls_ctr_drbg_seed_entropy_len( ctx, f_entropy, p_entropy, - custom, len, - MBEDTLS_CTR_DRBG_ENTROPY_LEN ) ); -} - void mbedtls_ctr_drbg_free( mbedtls_ctr_drbg_context *ctx ) { if( ctx == NULL ) @@ -445,6 +381,70 @@ exit: return( ret ); } +/* + * Non-public function wrapped by mbedtls_ctr_drbg_seed(). Necessary to allow + * NIST tests to succeed (which require known length fixed entropy) + */ +/* CTR_DRBG_Instantiate with derivation function (SP 800-90A §10.2.1.3.2) + * mbedtls_ctr_drbg_seed_entropy_len(ctx, f_entropy, p_entropy, + * custom, len, entropy_len) + * implements + * CTR_DRBG_Instantiate(entropy_input, nonce, personalization_string, + * security_strength) -> initial_working_state + * with inputs + * custom[:len] = nonce || personalization_string + * where entropy_input comes from f_entropy for entropy_len bytes + * and with outputs + * ctx = initial_working_state + */ +int mbedtls_ctr_drbg_seed_entropy_len( + mbedtls_ctr_drbg_context *ctx, + int (*f_entropy)(void *, unsigned char *, size_t), + void *p_entropy, + const unsigned char *custom, + size_t len, + size_t entropy_len ) +{ + int ret; + unsigned char key[MBEDTLS_CTR_DRBG_KEYSIZE]; + + memset( key, 0, MBEDTLS_CTR_DRBG_KEYSIZE ); + + mbedtls_aes_init( &ctx->aes_ctx ); + + ctx->f_entropy = f_entropy; + ctx->p_entropy = p_entropy; + + ctx->entropy_len = entropy_len; + ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL; + + /* + * Initialize with an empty key + */ + if( ( ret = mbedtls_aes_setkey_enc( &ctx->aes_ctx, key, + MBEDTLS_CTR_DRBG_KEYBITS ) ) != 0 ) + { + return( ret ); + } + + if( ( ret = mbedtls_ctr_drbg_reseed( ctx, custom, len ) ) != 0 ) + { + return( ret ); + } + return( 0 ); +} + +int mbedtls_ctr_drbg_seed( mbedtls_ctr_drbg_context *ctx, + int (*f_entropy)(void *, unsigned char *, size_t), + void *p_entropy, + const unsigned char *custom, + size_t len ) +{ + return( mbedtls_ctr_drbg_seed_entropy_len( ctx, f_entropy, p_entropy, + custom, len, + MBEDTLS_CTR_DRBG_ENTROPY_LEN ) ); +} + /* CTR_DRBG_Generate with derivation function (SP 800-90A §10.2.1.5.2) * mbedtls_ctr_drbg_random_with_add(ctx, output, output_len, additional, add_len) * implements From 50ed86b6b92021969c6c84b99cee11e7e9121042 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 4 Oct 2019 12:15:55 +0200 Subject: [PATCH 3/3] CTR_DRBG: support set_entropy_len() before seed() mbedtls_ctr_drbg_seed() always set the entropy length to the default, so a call to mbedtls_ctr_drbg_set_entropy_len() before seed() had no effect. Change this to the more intuitive behavior that set_entropy_len() sets the entropy length and seed() respects that and only uses the default entropy length if there was no call to set_entropy_len(). This removes the need for the test-only function mbedtls_ctr_drbg_seed_entropy_len(). Just call mbedtls_ctr_drbg_set_entropy_len() followed by mbedtls_ctr_drbg_seed(), it works now. --- include/mbedtls/ctr_drbg.h | 16 +++------ library/ctr_drbg.c | 44 +++++++++-------------- tests/suites/test_suite_ctr_drbg.function | 6 ++-- 3 files changed, 25 insertions(+), 41 deletions(-) diff --git a/include/mbedtls/ctr_drbg.h b/include/mbedtls/ctr_drbg.h index 2db402133..676b96e49 100644 --- a/include/mbedtls/ctr_drbg.h +++ b/include/mbedtls/ctr_drbg.h @@ -214,11 +214,8 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx ); * with mbedtls_entropy_init() (which registers the platform's default * entropy sources). * - * \p f_entropy is always called with a buffer size equal to the entropy - * length. The entropy length is initially #MBEDTLS_CTR_DRBG_ENTROPY_LEN - * and this value is always used for the initial seeding. You can change - * the entropy length for subsequent seeding by calling - * mbedtls_ctr_drbg_set_entropy_len() after this function. + * The entropy length is #MBEDTLS_CTR_DRBG_ENTROPY_LEN by default. + * You can override it by calling mbedtls_ctr_drbg_set_entropy_len(). * * You can provide a personalization string in addition to the * entropy source, to make this instantiation as unique as possible. @@ -255,6 +252,8 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx ); * \param f_entropy The entropy callback, taking as arguments the * \p p_entropy context, the buffer to fill, and the * length of the buffer. + * \p f_entropy is always called with a buffer size + * equal to the entropy length. * \param p_entropy The entropy context to pass to \p f_entropy. * \param custom The personalization string. * This can be \c NULL, in which case the personalization @@ -298,7 +297,7 @@ void mbedtls_ctr_drbg_set_prediction_resistance( mbedtls_ctr_drbg_context *ctx, /** * \brief This function sets the amount of entropy grabbed on each - * subsequent reseed. + * seed or reseed. * * The default value is #MBEDTLS_CTR_DRBG_ENTROPY_LEN. * @@ -499,11 +498,6 @@ int mbedtls_ctr_drbg_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ -/* Internal functions (do not call directly) */ -int mbedtls_ctr_drbg_seed_entropy_len( mbedtls_ctr_drbg_context *, - int (*)(void *, unsigned char *, size_t), void *, - const unsigned char *, size_t, size_t ); - #ifdef __cplusplus } #endif diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index c986b7019..ae51d5467 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -381,10 +381,6 @@ exit: return( ret ); } -/* - * Non-public function wrapped by mbedtls_ctr_drbg_seed(). Necessary to allow - * NIST tests to succeed (which require known length fixed entropy) - */ /* CTR_DRBG_Instantiate with derivation function (SP 800-90A §10.2.1.3.2) * mbedtls_ctr_drbg_seed_entropy_len(ctx, f_entropy, p_entropy, * custom, len, entropy_len) @@ -397,13 +393,11 @@ exit: * and with outputs * ctx = initial_working_state */ -int mbedtls_ctr_drbg_seed_entropy_len( - mbedtls_ctr_drbg_context *ctx, - int (*f_entropy)(void *, unsigned char *, size_t), - void *p_entropy, - const unsigned char *custom, - size_t len, - size_t entropy_len ) +int mbedtls_ctr_drbg_seed( mbedtls_ctr_drbg_context *ctx, + int (*f_entropy)(void *, unsigned char *, size_t), + void *p_entropy, + const unsigned char *custom, + size_t len ) { int ret; unsigned char key[MBEDTLS_CTR_DRBG_KEYSIZE]; @@ -415,7 +409,8 @@ int mbedtls_ctr_drbg_seed_entropy_len( ctx->f_entropy = f_entropy; ctx->p_entropy = p_entropy; - ctx->entropy_len = entropy_len; + if( ctx->entropy_len == 0 ) + ctx->entropy_len = MBEDTLS_CTR_DRBG_ENTROPY_LEN; ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL; /* @@ -434,17 +429,6 @@ int mbedtls_ctr_drbg_seed_entropy_len( return( 0 ); } -int mbedtls_ctr_drbg_seed( mbedtls_ctr_drbg_context *ctx, - int (*f_entropy)(void *, unsigned char *, size_t), - void *p_entropy, - const unsigned char *custom, - size_t len ) -{ - return( mbedtls_ctr_drbg_seed_entropy_len( ctx, f_entropy, p_entropy, - custom, len, - MBEDTLS_CTR_DRBG_ENTROPY_LEN ) ); -} - /* CTR_DRBG_Generate with derivation function (SP 800-90A §10.2.1.5.2) * mbedtls_ctr_drbg_random_with_add(ctx, output, output_len, additional, add_len) * implements @@ -708,8 +692,11 @@ int mbedtls_ctr_drbg_self_test( int verbose ) mbedtls_printf( " CTR_DRBG (PR = TRUE) : " ); test_offset = 0; - CHK( mbedtls_ctr_drbg_seed_entropy_len( &ctx, ctr_drbg_self_test_entropy, - (void *) entropy_source_pr, nonce_pers_pr, 16, 32 ) ); + mbedtls_ctr_drbg_set_entropy_len( &ctx, 32 ); + CHK( mbedtls_ctr_drbg_seed( &ctx, + ctr_drbg_self_test_entropy, + (void *) entropy_source_pr, + nonce_pers_pr, 16 ) ); mbedtls_ctr_drbg_set_prediction_resistance( &ctx, MBEDTLS_CTR_DRBG_PR_ON ); CHK( mbedtls_ctr_drbg_random( &ctx, buf, MBEDTLS_CTR_DRBG_BLOCKSIZE ) ); CHK( mbedtls_ctr_drbg_random( &ctx, buf, MBEDTLS_CTR_DRBG_BLOCKSIZE ) ); @@ -729,8 +716,11 @@ int mbedtls_ctr_drbg_self_test( int verbose ) mbedtls_ctr_drbg_init( &ctx ); test_offset = 0; - CHK( mbedtls_ctr_drbg_seed_entropy_len( &ctx, ctr_drbg_self_test_entropy, - (void *) entropy_source_nopr, nonce_pers_nopr, 16, 32 ) ); + mbedtls_ctr_drbg_set_entropy_len( &ctx, 32 ); + CHK( mbedtls_ctr_drbg_seed( &ctx, + ctr_drbg_self_test_entropy, + (void *) entropy_source_nopr, + nonce_pers_nopr, 16 ) ); CHK( mbedtls_ctr_drbg_random( &ctx, buf, 16 ) ); CHK( mbedtls_ctr_drbg_reseed( &ctx, NULL, 0 ) ); CHK( mbedtls_ctr_drbg_random( &ctx, buf, 16 ) ); diff --git a/tests/suites/test_suite_ctr_drbg.function b/tests/suites/test_suite_ctr_drbg.function index 4a97826f6..01050d92d 100644 --- a/tests/suites/test_suite_ctr_drbg.function +++ b/tests/suites/test_suite_ctr_drbg.function @@ -44,11 +44,11 @@ static void ctr_drbg_validate_internal( int reseed_mode, data_t * nonce, /* CTR_DRBG_Instantiate(entropy[:entropy->len], nonce, perso, ) * where nonce||perso = nonce[nonce->len] */ - TEST_ASSERT( mbedtls_ctr_drbg_seed_entropy_len( + mbedtls_ctr_drbg_set_entropy_len( &ctx, entropy_chunk_len ); + TEST_ASSERT( mbedtls_ctr_drbg_seed( &ctx, mbedtls_test_entropy_func, entropy->x, - nonce->x, nonce->len, - entropy_chunk_len ) == 0 ); + nonce->x, nonce->len ) == 0 ); if( reseed_mode == RESEED_ALWAYS ) mbedtls_ctr_drbg_set_prediction_resistance( &ctx,