From 53fb66db12fa3ff7a8cd5bfe7f5707cb55aa626a 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 0af3f47462..b8ce020be4 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -282,6 +282,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 }; /* @@ -294,6 +298,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 } /* @@ -315,6 +323,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 ); } @@ -2234,9 +2246,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 44c5fa27d5..34c03c3fa8 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -883,6 +883,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.py set MBEDTLS_ECP_NO_INTERNAL_RNG + scripts/config.py set MBEDTLS_ECP_RESTARTABLE + scripts/config.py unset MBEDTLS_CTR_DRBG_C + scripts/config.py unset MBEDTLS_HMAC_DRBG_C + scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG + scripts/config.py unset MBEDTLS_PSA_CRYPTO_C # requires CTR_DRBG + scripts/config.py 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_new_ecdh_context () { msg "build: new ECDH context (ASan build)" # ~ 6 min scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT