Make RNG parameters mandatory in ECP functions

Fix trivial faulty calls in ECP test suite and ECP/ECJPAKE self-tests (by
adding a dummy RNG).

Several tests suites are not passing yet, as a couple of library
function do call ecp_mul() with a NULL RNG. The complexity of the fixes
range from "simple refactoring" to "requires API changes", so these will
be addressed in separate commits.

This makes the option MBEDTLS_ECP_NO_INTERNAL_RNG, as well as the whole
"internal RNG" code, obsolete. This will be addressed in a future
commit, after getting the test suites to pass again.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This commit is contained in:
Manuel Pégourié-Gonnard 2021-06-15 11:29:26 +02:00
parent 7861ecf838
commit aa3ed6f987
4 changed files with 76 additions and 27 deletions

View File

@ -911,15 +911,8 @@ int mbedtls_ecp_tls_write_group( const mbedtls_ecp_group *grp,
* \note To prevent timing attacks, this function
* executes the exact same sequence of base-field
* operations for any valid \p m. It avoids any if-branch or
* array index depending on the value of \p m.
*
* \note If \p f_rng is not NULL, it is used to randomize
* intermediate results to prevent potential timing attacks
* targeting these results. We recommend always providing
* a non-NULL \p f_rng. The overhead is negligible.
* Note: unless #MBEDTLS_ECP_NO_INTERNAL_RNG is defined, when
* \p f_rng is NULL, an internal RNG (seeded from the value
* of \p m) will be used instead.
* array index depending on the value of \p m. If also uses
* \p f_rng to randomize some intermediate results.
*
* \param grp The ECP group to use.
* This must be initialized and have group parameters
@ -928,9 +921,9 @@ int mbedtls_ecp_tls_write_group( const mbedtls_ecp_group *grp,
* This must be initialized.
* \param m The integer by which to multiply. This must be initialized.
* \param P The point to multiply. This must be initialized.
* \param f_rng The RNG function. This may be \c NULL if randomization
* of intermediate results isn't desired (discouraged).
* \param p_rng The RNG context to be passed to \p p_rng.
* \param f_rng The RNG function. This must not be \c NULL.
* \param p_rng The RNG context to be passed to \p f_rng. This may be \c
* NULL if \p f_rng doesn't need a context.
*
* \return \c 0 on success.
* \return #MBEDTLS_ERR_ECP_INVALID_KEY if \p m is not a valid private
@ -959,9 +952,9 @@ int mbedtls_ecp_mul( mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
* This must be initialized.
* \param m The integer by which to multiply. This must be initialized.
* \param P The point to multiply. This must be initialized.
* \param f_rng The RNG function. This may be \c NULL if randomization
* of intermediate results isn't desired (discouraged).
* \param p_rng The RNG context to be passed to \p p_rng.
* \param f_rng The RNG function. This must not be \c NULL.
* \param p_rng The RNG context to be passed to \p f_rng. This may be \c
* NULL if \p f_rng doesn't need a context.
* \param rs_ctx The restart context (NULL disables restart).
*
* \return \c 0 on success.

View File

@ -962,6 +962,28 @@ static const unsigned char ecjpake_test_pms[] = {
0xb4, 0x38, 0xf7, 0x19, 0xd3, 0xc4, 0xf3, 0x51
};
/*
* PRNG for test - !!!INSECURE NEVER USE IN PRODUCTION!!!
*
* This is the linear congruential generator from numerical recipes,
* except we only use the low byte as the output. See
* https://en.wikipedia.org/wiki/Linear_congruential_generator#Parameters_in_common_use
*/
static int self_test_rng( void *ctx, unsigned char *out, size_t len )
{
static uint32_t state = 42;
(void) ctx;
for( size_t i = 0; i < len; i++ )
{
state = state * 1664525u + 1013904223u;
out[i] = (unsigned char) state;
}
return( 0 );
}
/* Load my private keys and generate the corresponding public keys */
static int ecjpake_test_load( mbedtls_ecjpake_context *ctx,
const unsigned char *xm1, size_t len1,
@ -972,9 +994,9 @@ static int ecjpake_test_load( mbedtls_ecjpake_context *ctx,
MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &ctx->xm1, xm1, len1 ) );
MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &ctx->xm2, xm2, len2 ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &ctx->grp, &ctx->Xm1, &ctx->xm1,
&ctx->grp.G, NULL, NULL ) );
&ctx->grp.G, self_test_rng, NULL ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &ctx->grp, &ctx->Xm2, &ctx->xm2,
&ctx->grp.G, NULL, NULL ) );
&ctx->grp.G, self_test_rng, NULL ) );
cleanup:
return( ret );

View File

@ -2684,6 +2684,9 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
ECP_VALIDATE_RET( m != NULL );
ECP_VALIDATE_RET( P != NULL );
if( f_rng == NULL )
return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA );
#if defined(MBEDTLS_ECP_RESTARTABLE)
/* reset ops count for this call if top-level */
if( rs_ctx != NULL && rs_ctx->depth++ == 0 )
@ -3315,6 +3318,28 @@ cleanup:
#if defined(MBEDTLS_SELF_TEST)
/*
* PRNG for test - !!!INSECURE NEVER USE IN PRODUCTION!!!
*
* This is the linear congruential generator from numerical recipes,
* except we only use the low byte as the output. See
* https://en.wikipedia.org/wiki/Linear_congruential_generator#Parameters_in_common_use
*/
static int self_test_rng( void *ctx, unsigned char *out, size_t len )
{
static uint32_t state = 42;
(void) ctx;
for( size_t i = 0; i < len; i++ )
{
state = state * 1664525u + 1013904223u;
out[i] = (unsigned char) state;
}
return( 0 );
}
/* Adjust the exponent to be a valid private point for the specified curve.
* This is sometimes necessary because we use a single set of exponents
* for all curves but the validity of values depends on the curve. */
@ -3370,7 +3395,7 @@ static int self_test_point( int verbose,
MBEDTLS_MPI_CHK( mbedtls_mpi_read_string( m, 16, exponents[0] ) );
MBEDTLS_MPI_CHK( self_test_adjust_exponent( grp, m ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, NULL, NULL ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, self_test_rng, NULL ) );
for( i = 1; i < n_exponents; i++ )
{
@ -3383,7 +3408,7 @@ static int self_test_point( int verbose,
MBEDTLS_MPI_CHK( mbedtls_mpi_read_string( m, 16, exponents[i] ) );
MBEDTLS_MPI_CHK( self_test_adjust_exponent( grp, m ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, NULL, NULL ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( grp, R, m, P, self_test_rng, NULL ) );
if( add_count != add_c_prev ||
dbl_count != dbl_c_prev ||
@ -3461,7 +3486,7 @@ int mbedtls_ecp_self_test( int verbose )
mbedtls_printf( " ECP SW test #1 (constant op_count, base point G): " );
/* Do a dummy multiplication first to trigger precomputation */
MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &m, 2 ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &grp, &P, &m, &grp.G, NULL, NULL ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_mul( &grp, &P, &m, &grp.G, self_test_rng, NULL ) );
ret = self_test_point( verbose,
&grp, &R, &m, &grp.G,
sw_exponents,

View File

@ -124,12 +124,14 @@ void ecp_test_vect_restart( int id,
mbedtls_mpi dA, xA, yA, dB, xZ, yZ;
int cnt_restarts;
int ret;
mbedtls_test_rnd_pseudo_info rnd_info;
mbedtls_ecp_restart_init( &ctx );
mbedtls_ecp_group_init( &grp );
mbedtls_ecp_point_init( &R ); mbedtls_ecp_point_init( &P );
mbedtls_mpi_init( &dA ); mbedtls_mpi_init( &xA ); mbedtls_mpi_init( &yA );
mbedtls_mpi_init( &dB ); mbedtls_mpi_init( &xZ ); mbedtls_mpi_init( &yZ );
memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) );
TEST_ASSERT( mbedtls_ecp_group_load( &grp, id ) == 0 );
@ -147,7 +149,8 @@ void ecp_test_vect_restart( int id,
cnt_restarts = 0;
do {
ECP_PT_RESET( &R );
ret = mbedtls_ecp_mul_restartable( &grp, &R, &dA, &grp.G, NULL, NULL, &ctx );
ret = mbedtls_ecp_mul_restartable( &grp, &R, &dA, &grp.G,
&mbedtls_test_rnd_pseudo_rand, &rnd_info, &ctx );
} while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts );
TEST_ASSERT( ret == 0 );
@ -162,7 +165,8 @@ void ecp_test_vect_restart( int id,
cnt_restarts = 0;
do {
ECP_PT_RESET( &R );
ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P, NULL, NULL, &ctx );
ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P,
&mbedtls_test_rnd_pseudo_rand, &rnd_info, &ctx );
} while( ret == MBEDTLS_ERR_ECP_IN_PROGRESS && ++cnt_restarts );
TEST_ASSERT( ret == 0 );
@ -176,7 +180,8 @@ void ecp_test_vect_restart( int id,
* This test only makes sense when we actually restart */
if( min_restarts > 0 )
{
ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P, NULL, NULL, &ctx );
ret = mbedtls_ecp_mul_restartable( &grp, &R, &dB, &P,
&mbedtls_test_rnd_pseudo_rand, &rnd_info, &ctx );
TEST_ASSERT( ret == MBEDTLS_ERR_ECP_IN_PROGRESS );
}
@ -294,12 +299,14 @@ void ecp_test_vect( int id, char * dA_str, char * xA_str, char * yA_str,
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xA ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yA ) == 0 );
TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &R, NULL, NULL ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &R,
&mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xZ ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yZ ) == 0 );
TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G, NULL, NULL ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G,
&mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xB ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.Y, &yB ) == 0 );
TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 );
@ -351,11 +358,13 @@ void ecp_test_vec_x( int id, char * dA_hex, char * xA_hex, char * dB_hex,
TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xS ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G, NULL, NULL ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dB, &grp.G,
&mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 );
TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xB ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dA, &R, NULL, NULL ) == 0 );
TEST_ASSERT( mbedtls_ecp_mul( &grp, &R, &dA, &R,
&mbedtls_test_rnd_pseudo_rand, &rnd_info ) == 0 );
TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &R ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &R.X, &xS ) == 0 );