From 047986c2f8d8227aebb143054250c36d26ac2c3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 4 Jun 2020 09:43:14 +0200 Subject: [PATCH] Add support for RESTARTABLE with internal RNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we draw pseudo-random numbers at the beginning and end of the main loop. With ECP_RESTARTABLE, it's possible that between those two occasions we returned from the multiplication function, hence lost our internal DRBG context that lives in this function's stack frame. This would result in the same pseudo-random numbers being used for blinding in multiple places. While it's not immediately clear that this would give rise to an attack, it's also absolutely not clear that it doesn't. So let's avoid that by using a DRBG context that lives inside the restart context and persists across return/resume cycles. That way the RESTARTABLE case uses exactly the same pseudo-random numbers as the non-restartable case. Testing and compile-time options: - The case ECP_RESTARTABLE && !ECP_NO_INTERNAL_RNG is already tested by component_test_no_use_psa_crypto_full_cmake_asan. - The case ECP_RESTARTABLE && ECP_NO_INTERNAL_RNG didn't have a pre-existing test so a component is added. Testing and runtime options: when ECP_RESTARTABLE is enabled, the test suites already contain cases where restart happens and cases where it doesn't (because the operation is short enough or because restart is disabled (NULL restart context)). Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 34 ++++++++++++++++++++++++++++++++-- tests/scripts/all.sh | 19 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index b120adf7..8e87e60c 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -281,6 +281,10 @@ struct mbedtls_ecp_restart_mul ecp_rsm_comb_core, /* ecp_mul_comb_core() */ ecp_rsm_final_norm, /* do the final normalization */ } state; +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_context drbg_ctx; + unsigned char drbg_seeded; +#endif }; /* @@ -293,6 +297,10 @@ static void ecp_restart_rsm_init( mbedtls_ecp_restart_mul_ctx *ctx ) ctx->T = NULL; ctx->T_size = 0; ctx->state = ecp_rsm_init; +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_init( &ctx->drbg_ctx ); + ctx->drbg_seeded = 0; +#endif } /* @@ -314,6 +322,10 @@ static void ecp_restart_rsm_free( mbedtls_ecp_restart_mul_ctx *ctx ) mbedtls_free( ctx->T ); } +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_free( &ctx->drbg_ctx ); +#endif + ecp_restart_rsm_init( ctx ); } @@ -2154,9 +2166,27 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng == NULL ) { - MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); + /* Adjust pointers */ f_rng = &ecp_drbg_random; - p_rng = &drbg_ctx; +#if defined(MBEDTLS_ECP_RESTARTABLE) + if( rs_ctx != NULL && rs_ctx->rsm != NULL ) + p_rng = &rs_ctx->rsm->drbg_ctx; + else +#endif + p_rng = &drbg_ctx; + + /* Initialize internal DRBG if necessary */ +#if defined(MBEDTLS_ECP_RESTARTABLE) + if( rs_ctx == NULL || rs_ctx->rsm == NULL || + rs_ctx->rsm->drbg_seeded == 0 ) +#endif + { + MBEDTLS_MPI_CHK( ecp_drbg_seed( p_rng, m ) ); + } +#if defined(MBEDTLS_ECP_RESTARTABLE) + if( rs_ctx != NULL && rs_ctx->rsm != NULL ) + rs_ctx->rsm->drbg_seeded = 1; +#endif } #endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index b16fd9a2..039ae158 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -835,6 +835,25 @@ component_test_ecp_no_internal_rng () { # no SSL tests as they all depend on having a DRBG } +component_test_ecp_restartable_no_internal_rng () { + msg "build: Default plus ECP_RESTARTABLE and ECP_NO_INTERNAL_RNG, no DRBG" + scripts/config.pl set MBEDTLS_ECP_NO_INTERNAL_RNG + scripts/config.pl set MBEDTLS_ECP_RESTARTABLE + scripts/config.pl unset MBEDTLS_CTR_DRBG_C + scripts/config.pl unset MBEDTLS_HMAC_DRBG_C + scripts/config.pl unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_C # requires CTR_DRBG + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C # requires PSA Crypto + + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: ECP_RESTARTABLE and ECP_NO_INTERNAL_RNG, no DRBG module" + make test + + # no SSL tests as they all depend on having a DRBG +} + component_test_small_ssl_out_content_len () { msg "build: small SSL_OUT_CONTENT_LEN (ASan build)" scripts/config.pl set MBEDTLS_SSL_IN_CONTENT_LEN 16384