From eab304caf5156b8506246fe512d6dc5382975217 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 26 Aug 2019 15:29:14 +0100 Subject: [PATCH 1/4] HMAC DRBG: Split entropy-gathering requests to reduce request sizes According to SP800-90A, the DRBG seeding process should use a nonce of length `security_strength / 2` bits as part of the DRBG seed. It further notes that this nonce may be drawn from the same source of entropy that is used for the first `security_strength` bits of the DRBG seed. The present HMAC DRBG implementation does that, requesting `security_strength * 3 / 2` bits of entropy from the configured entropy source in total to form the initial part of the DRBG seed. However, some entropy sources may have thresholds in terms of how much entropy they can provide in a single call to their entropy gathering function which may be exceeded by the present HMAC DRBG implementation even if the threshold is not smaller than `security_strength` bits. Specifically, this is the case for our own entropy module implementation which only allows requesting at most 32 Bytes of entropy at a time in configurations disabling SHA-512, and this leads to runtime failure of HMAC DRBG when used with Mbed TLS' own entropy callbacks in such configurations. This commit fixes this by splitting the seed entropy acquisition into two calls, one requesting `security_strength` bits first, and another one requesting `security_strength / 2` bits for the nonce. --- library/hmac_drbg.c | 82 ++++++++++++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index c50330e7d8..50ac66044a 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -148,21 +148,28 @@ int mbedtls_hmac_drbg_seed_buf( mbedtls_hmac_drbg_context *ctx, return( 0 ); } -/* - * HMAC_DRBG reseeding: 10.1.2.4 (arabic) + 9.2 (Roman) - */ -int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx, - const unsigned char *additional, size_t len ) +static int hmac_drbg_reseed_internal( mbedtls_hmac_drbg_context *ctx, + const unsigned char *additional, size_t len, + int use_nonce ) { unsigned char seed[MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT]; - size_t seedlen; + size_t seedlen = 0; int ret; - /* III. Check input length */ - if( len > MBEDTLS_HMAC_DRBG_MAX_INPUT || - ctx->entropy_len + len > MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT ) { - return( MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG ); + size_t total_entropy_len; + + if( use_nonce == 0 ) + total_entropy_len = ctx->entropy_len; + else + total_entropy_len = ctx->entropy_len * 3 / 2; + + /* III. Check input length */ + if( len > MBEDTLS_HMAC_DRBG_MAX_INPUT || + total_entropy_len + len > MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT ) + { + return( MBEDTLS_ERR_HMAC_DRBG_INPUT_TOO_BIG ); + } } memset( seed, 0, MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT ); @@ -170,9 +177,32 @@ int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx, /* IV. Gather entropy_len bytes of entropy for the seed */ if( ( ret = ctx->f_entropy( ctx->p_entropy, seed, ctx->entropy_len ) ) != 0 ) + { return( MBEDTLS_ERR_HMAC_DRBG_ENTROPY_SOURCE_FAILED ); + } + seedlen += ctx->entropy_len; + + /* IV'. For initial seeding, allow adding of nonce generated + * from the entropy source. See Sect 8.6.7 in SP800-90A. */ + if( use_nonce ) + { + /* Note: We don't merge the two calls to f_entropy() in order + * to avoid requesting too much entropy from f_entropy() + * at once. Specifically, if the underlying digest is not + * SHA-1, 3 / 2 * entropy_len is at least 36 Bytes, which + * is larger than the maximum of 32 Bytes that our own + * entropy source implementation can emit in a single + * call in configurations disabling SHA-512. */ + if( ( ret = ctx->f_entropy( ctx->p_entropy, + seed + seedlen, + ctx->entropy_len / 2 ) ) != 0 ) + { + return( MBEDTLS_ERR_HMAC_DRBG_ENTROPY_SOURCE_FAILED ); + } + + seedlen += ctx->entropy_len / 2; + } - seedlen = ctx->entropy_len; /* 1. Concatenate entropy and additional data if any */ if( additional != NULL && len != 0 ) @@ -194,6 +224,15 @@ exit: return( ret ); } +/* + * HMAC_DRBG reseeding: 10.1.2.4 (arabic) + 9.2 (Roman) + */ +int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx, + const unsigned char *additional, size_t len ) +{ + return( hmac_drbg_reseed_internal( ctx, additional, len, 0 ) ); +} + /* * HMAC_DRBG initialisation (10.1.2.3 + 9.1) */ @@ -205,7 +244,7 @@ int mbedtls_hmac_drbg_seed( mbedtls_hmac_drbg_context *ctx, size_t len ) { int ret; - size_t entropy_len, md_size; + size_t md_size; if( ( ret = mbedtls_md_setup( &ctx->md_ctx, md_info, 1 ) ) != 0 ) return( ret ); @@ -233,20 +272,15 @@ int mbedtls_hmac_drbg_seed( mbedtls_hmac_drbg_context *ctx, * * (This also matches the sizes used in the NIST test vectors.) */ - 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 */ + 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 */ - /* - * For initialisation, use more entropy to emulate a nonce - * (Again, matches test vectors.) - */ - ctx->entropy_len = entropy_len * 3 / 2; - - if( ( ret = mbedtls_hmac_drbg_reseed( ctx, custom, len ) ) != 0 ) + if( ( ret = hmac_drbg_reseed_internal( ctx, custom, len, + 1 /* add nonce */ ) ) != 0 ) + { return( ret ); - - ctx->entropy_len = entropy_len; + } return( 0 ); } From 213ae2c7a8e406a26da5f4cb8f1f9dafd9d84dc6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 26 Aug 2019 15:45:33 +0100 Subject: [PATCH 2/4] Add ChangeLog entry --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 0c579a8b2b..7fdd90f385 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,8 @@ Bugfix * Enable Suite B with subset of ECP curves. Make sure the code compiles even if some curves are not defined. Fixes #1591 reported by dbedev. * Fix misuse of signed arithmetic in the HAVEGE module. #2598 + * Fix incompatibility of HMAC DRBG with Mbed TLS' own entropy module that + lead to HMAC DRBG seeding failure in configurations disabling SHA-512. Changes * Make it easier to define MBEDTLS_PARAM_FAILED as assert (which config.h From b3a06e66d81416a54e148ebadb608d4f169c4fe9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 27 Aug 2019 09:21:44 +0100 Subject: [PATCH 3/4] hmac_drbg.c: Rename hmac_drbg_reseed_internal->hmac_drbg_reseed_core --- library/hmac_drbg.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index 50ac66044a..02ce7f78a5 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -148,9 +148,9 @@ int mbedtls_hmac_drbg_seed_buf( mbedtls_hmac_drbg_context *ctx, return( 0 ); } -static int hmac_drbg_reseed_internal( mbedtls_hmac_drbg_context *ctx, - const unsigned char *additional, size_t len, - int use_nonce ) +static int hmac_drbg_reseed_core( mbedtls_hmac_drbg_context *ctx, + const unsigned char *additional, size_t len, + int use_nonce ) { unsigned char seed[MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT]; size_t seedlen = 0; @@ -230,7 +230,8 @@ exit: int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx, const unsigned char *additional, size_t len ) { - return( hmac_drbg_reseed_internal( ctx, additional, len, 0 ) ); + return( hmac_drbg_reseed_core( ctx, additional, len, + 0 /* no nonce */ ) ); } /* @@ -276,8 +277,8 @@ int mbedtls_hmac_drbg_seed( mbedtls_hmac_drbg_context *ctx, md_size <= 28 ? 24 : /* 224-bits hash -> 192 bits */ 32; /* better (256+) -> 256 bits */ - if( ( ret = hmac_drbg_reseed_internal( ctx, custom, len, - 1 /* add nonce */ ) ) != 0 ) + if( ( ret = hmac_drbg_reseed_core( ctx, custom, len, + 1 /* add nonce */ ) ) != 0 ) { return( ret ); } From 31c95e1e9403b61b1a05d450e042c2a3d644e5cb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 27 Aug 2019 09:22:09 +0100 Subject: [PATCH 4/4] Fix and improve documentation of HMAC DRBG - a comment regarding the implementation of hmac_drbg_reseed_core() was misplaced. - add more references to the standard, and add details on how the comments in the code refer to various parts of the standard. --- library/hmac_drbg.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index 02ce7f78a5..34f18155e6 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -148,6 +148,11 @@ int mbedtls_hmac_drbg_seed_buf( mbedtls_hmac_drbg_context *ctx, return( 0 ); } +/* + * Internal function used both for seeding and reseeding the DRBG. + * Comments starting with arabic numbers refer to section 10.1.2.4 + * of SP800-90A, while roman numbers refer to section 9.2. + */ static int hmac_drbg_reseed_core( mbedtls_hmac_drbg_context *ctx, const unsigned char *additional, size_t len, int use_nonce ) @@ -182,8 +187,8 @@ static int hmac_drbg_reseed_core( mbedtls_hmac_drbg_context *ctx, } seedlen += ctx->entropy_len; - /* IV'. For initial seeding, allow adding of nonce generated - * from the entropy source. See Sect 8.6.7 in SP800-90A. */ + /* For initial seeding, allow adding of nonce generated + * from the entropy source. See Sect 8.6.7 in SP800-90A. */ if( use_nonce ) { /* Note: We don't merge the two calls to f_entropy() in order @@ -225,7 +230,7 @@ exit: } /* - * HMAC_DRBG reseeding: 10.1.2.4 (arabic) + 9.2 (Roman) + * HMAC_DRBG reseeding: 10.1.2.4 + 9.2 */ int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx, const unsigned char *additional, size_t len ) @@ -236,6 +241,9 @@ int mbedtls_hmac_drbg_reseed( mbedtls_hmac_drbg_context *ctx, /* * HMAC_DRBG initialisation (10.1.2.3 + 9.1) + * + * The nonce is not passed as a separate parameter but extracted + * from the entropy source as suggested in 8.6.7. */ int mbedtls_hmac_drbg_seed( mbedtls_hmac_drbg_context *ctx, const mbedtls_md_info_t * md_info,