Add support for RESTARTABLE with internal RNG

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 <manuel.pegourie-gonnard@arm.com>
This commit is contained in:
Manuel Pégourié-Gonnard 2020-06-04 09:43:14 +02:00
parent d18f0519a5
commit 047986c2f8
2 changed files with 51 additions and 2 deletions

View File

@ -281,6 +281,10 @@ struct mbedtls_ecp_restart_mul
ecp_rsm_comb_core, /* ecp_mul_comb_core() */ ecp_rsm_comb_core, /* ecp_mul_comb_core() */
ecp_rsm_final_norm, /* do the final normalization */ ecp_rsm_final_norm, /* do the final normalization */
} state; } 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 = NULL;
ctx->T_size = 0; ctx->T_size = 0;
ctx->state = ecp_rsm_init; 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 ); mbedtls_free( ctx->T );
} }
#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG)
ecp_drbg_free( &ctx->drbg_ctx );
#endif
ecp_restart_rsm_init( ctx ); 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 !defined(MBEDTLS_ECP_NO_INTERNAL_RNG)
if( f_rng == NULL ) if( f_rng == NULL )
{ {
MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); /* Adjust pointers */
f_rng = &ecp_drbg_random; 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 */ #endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */

View File

@ -835,6 +835,25 @@ component_test_ecp_no_internal_rng () {
# no SSL tests as they all depend on having a DRBG # 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 () { component_test_small_ssl_out_content_len () {
msg "build: small SSL_OUT_CONTENT_LEN (ASan build)" msg "build: small SSL_OUT_CONTENT_LEN (ASan build)"
scripts/config.pl set MBEDTLS_SSL_IN_CONTENT_LEN 16384 scripts/config.pl set MBEDTLS_SSL_IN_CONTENT_LEN 16384