From 896a2942117514bf0427c187f05d687d08e2b0c9 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 7 Jan 2019 17:27:56 +0000 Subject: [PATCH] Correct deterministic ECDSA behavior We were still reusing the internal HMAC-DRBG of the deterministic ECDSA for blinding. This meant that with cryptographically low likelyhood the result was not the same signature as the one the deterministic ECDSA algorithm has to produce (however it is still a valid ECDSA signature). To correct this we seed a second HMAC-DRBG with the same seed to restore correct behavior. We also apply a label to avoid reusing the bits of the ephemeral key for a different purpose and reduce the chance that they leak. This workaround can't be implemented in the restartable case without penalising the case where external RNG is available or completely defeating the purpose of the restartable feature, therefore in this case the small chance of incorrect behavior remains. --- library/ecdsa.c | 61 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/library/ecdsa.c b/library/ecdsa.c index 3ae8eedf8..bda9262c9 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -478,17 +478,64 @@ sign: mbedtls_hmac_drbg_random, p_rng, f_rng_blind, p_rng_blind, rs_ctx ); else + { + mbedtls_hmac_drbg_context *p_rng_blind_det; + +#if !defined(MBEDTLS_ECP_RESTARTABLE) /* - * Use the same RNG for both blinding and ephemeral key generation. - * Since the RNG output is always the same for the same key and message, - * this limits the efficiency of blinding and leaks information through - * side channels. After mbedtls_ecdsa_sign_det() is removed NULL won't - * be a valid value for f_rng_blind anymore. Therefore it should be - * checked by the caller and this branch and check can be removed. + * To avoid reusing rng_ctx and risking incorrect behavior we seed a + * second HMAC-DRBG with the same seed. We also apply a label to avoid + * reusing the bits of the ephemeral key for blinding and eliminate the + * risk that they leak this way. + */ + const char* blind_label = "BLINDING CONTEXT"; + mbedtls_hmac_drbg_context rng_ctx_blind; + + mbedtls_hmac_drbg_init( &rng_ctx_blind ); + p_rng_blind_det = &rng_ctx_blind; + mbedtls_hmac_drbg_seed_buf( p_rng_blind_det, md_info, + data, 2 * grp_len ); + ret = mbedtls_hmac_drbg_update_ret( p_rng_blind_det, + (const unsigned char*) blind_label, + strlen( blind_label ) ); + if( ret != 0 ) + { + mbedtls_hmac_drbg_free( &rng_ctx_blind ); + goto cleanup; + } +#else + /* + * In the case of restartable computations we would either need to store + * the second RNG in the restart context too or set it up at every + * restart. The first option would penalize the correct application of + * the function and the second would defeat the purpose of the + * restartable feature. + * + * Therefore in this case we reuse the original RNG. This comes with the + * price that the resulting signature might not be a valid deterministic + * ECDSA signature with a very low probability (same magnitude as + * successfully guessing the private key). However even then it is still + * a valid ECDSA signature. + */ + p_rng_blind_det = p_rng; +#endif /* MBEDTLS_ECP_RESTARTABLE */ + + /* + * Since the output of the RNGs is always the same for the same key and + * message, this limits the efficiency of blinding and leaks information + * through side channels. After mbedtls_ecdsa_sign_det() is removed NULL + * won't be a valid value for f_rng_blind anymore. Therefore it should + * be checked by the caller and this branch and check can be removed. */ ret = ecdsa_sign_restartable( grp, r, s, d, buf, blen, mbedtls_hmac_drbg_random, p_rng, - mbedtls_hmac_drbg_random, p_rng, rs_ctx ); + mbedtls_hmac_drbg_random, p_rng_blind_det, + rs_ctx ); + +#if !defined(MBEDTLS_ECP_RESTARTABLE) + mbedtls_hmac_drbg_free( &rng_ctx_blind ); +#endif + } #endif /* MBEDTLS_ECDSA_SIGN_ALT */ cleanup: