From e09d2f8261723acfecaf22d6fcde723913ee8ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 2 Sep 2013 14:29:09 +0200 Subject: [PATCH] Change ecp_mul() prototype to allow randomization (Also improve an error code while at it.) --- include/polarssl/ecdh.h | 16 ++++++++++++-- include/polarssl/ecp.h | 22 +++++++++++++++----- library/ecdh.c | 16 +++++++++----- library/ecdsa.c | 7 +++++-- library/ecp.c | 20 +++++++++++------- library/ssl_cli.c | 3 ++- library/ssl_srv.c | 3 ++- tests/suites/test_suite_ecdh.function | 15 ++++++++------ tests/suites/test_suite_ecp.data | 4 ++-- tests/suites/test_suite_ecp.function | 30 ++++++++++++++++++++++----- 10 files changed, 99 insertions(+), 37 deletions(-) diff --git a/include/polarssl/ecdh.h b/include/polarssl/ecdh.h index 2184ab95f..d91aea58d 100644 --- a/include/polarssl/ecdh.h +++ b/include/polarssl/ecdh.h @@ -70,12 +70,20 @@ int ecdh_gen_public( const ecp_group *grp, mpi *d, ecp_point *Q, * \param z Destination MPI (shared secret) * \param Q Public key from other party * \param d Our secret exponent + * \param f_rng RNG function (see notes) + * \param p_rng RNG parameter * * \return 0 if successful, * or a POLARSSL_ERR_ECP_XXX or POLARSSL_MPI_XXX error code + * + * \note If f_rng is not NULL, it is used to implement + * countermeasures against potential elaborate timing + * attacks, see \c ecp_mul() for details. */ int ecdh_compute_shared( const ecp_group *grp, mpi *z, - const ecp_point *Q, const mpi *d ); + const ecp_point *Q, const mpi *d, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ); /** * \brief Initialize context @@ -156,11 +164,15 @@ int ecdh_read_public( ecdh_context *ctx, * \param olen number of bytes written * \param buf destination buffer * \param blen buffer length + * \param f_rng RNG function, see notes for \c ecdh_compute_shared() + * \param p_rng RNG parameter * * \return 0 if successful, or an POLARSSL_ERR_ECP_XXX error code */ int ecdh_calc_secret( ecdh_context *ctx, size_t *olen, - unsigned char *buf, size_t blen ); + unsigned char *buf, size_t blen, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ); /** * \brief Checkup routine diff --git a/include/polarssl/ecp.h b/include/polarssl/ecp.h index ad31bff66..594223133 100644 --- a/include/polarssl/ecp.h +++ b/include/polarssl/ecp.h @@ -411,17 +411,29 @@ int ecp_sub( const ecp_group *grp, ecp_point *R, * \param R Destination point * \param m Integer by which to multiply * \param P Point to multiply + * \param f_rng RNG function (see notes) + * \param p_rng RNG parameter * * \return 0 if successful, * POLARSSL_ERR_MPI_MALLOC_FAILED if memory allocation failed - * POLARSSL_ERR_ECP_GENERIC if m < 0 of m has greater bit - * length than N, the number of points in the group. + * POLARSSL_ERR_ECP_BAD_INPUT_DATA if m < 0 of m has greater + * bit length than N, the number of points in the group. * - * \note This function executes a constant number of operations - * for random m in the allowed range. + * \note In order to prevent simple timing attacks, this function + * executes a constant number of operations (that is, point + * doubling and addition of distinct points) for random m in + * the allowed range. + * + * \note If f_rng is not NULL, it is used to randomize projective + * coordinates of indermediate results, in order to prevent + * more elaborate timing attacks relying on intermediate + * operations. (This is a prophylactic measure since so such + * attack has been published yet.) */ int ecp_mul( const ecp_group *grp, ecp_point *R, - const mpi *m, const ecp_point *P ); + const mpi *m, const ecp_point *P, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); + /** * \brief Check that a point is a valid public key on this curve diff --git a/library/ecdh.c b/library/ecdh.c index d76596eb2..8ef02f54b 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -50,7 +50,9 @@ int ecdh_gen_public( const ecp_group *grp, mpi *d, ecp_point *Q, * Compute shared secret (SEC1 3.3.1) */ int ecdh_compute_shared( const ecp_group *grp, mpi *z, - const ecp_point *Q, const mpi *d ) + const ecp_point *Q, const mpi *d, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { int ret; ecp_point P; @@ -62,7 +64,7 @@ int ecdh_compute_shared( const ecp_group *grp, mpi *z, */ MPI_CHK( ecp_check_pubkey( grp, Q ) ); - MPI_CHK( ecp_mul( grp, &P, d, Q ) ); + MPI_CHK( ecp_mul( grp, &P, d, Q, f_rng, p_rng ) ); if( ecp_is_zero( &P ) ) { @@ -202,16 +204,20 @@ int ecdh_read_public( ecdh_context *ctx, * Derive and export the shared secret */ int ecdh_calc_secret( ecdh_context *ctx, size_t *olen, - unsigned char *buf, size_t blen ) + unsigned char *buf, size_t blen, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { int ret; if( ctx == NULL ) return( POLARSSL_ERR_ECP_BAD_INPUT_DATA ); - if( ( ret = ecdh_compute_shared( &ctx->grp, &ctx->z, &ctx->Qp, &ctx->d ) ) - != 0 ) + if( ( ret = ecdh_compute_shared( &ctx->grp, &ctx->z, &ctx->Qp, &ctx->d, + f_rng, p_rng ) ) != 0 ) + { return( ret ); + } if( mpi_size( &ctx->z ) > blen ) return( POLARSSL_ERR_ECP_BAD_INPUT_DATA ); diff --git a/library/ecdsa.c b/library/ecdsa.c index 67774c9d0..bbdb5d57b 100644 --- a/library/ecdsa.c +++ b/library/ecdsa.c @@ -161,9 +161,12 @@ int ecdsa_verify( const ecp_group *grp, /* * Step 5: R = u1 G + u2 Q + * + * Since we're not using any secret data, no need to pass a RNG to + * ecp_mul() for countermesures. */ - MPI_CHK( ecp_mul( grp, &R, &u1, &grp->G ) ); - MPI_CHK( ecp_mul( grp, &P, &u2, Q ) ); + MPI_CHK( ecp_mul( grp, &R, &u1, &grp->G, NULL, NULL ) ); + MPI_CHK( ecp_mul( grp, &P, &u2, Q, NULL, NULL ) ); MPI_CHK( ecp_add( grp, &R, &R, &P ) ); if( ecp_is_zero( &R ) ) diff --git a/library/ecp.c b/library/ecp.c index 09a021bf8..b4ee042d2 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1166,7 +1166,8 @@ cleanup: * random m in the range 0 .. 2^nbits - 1. */ int ecp_mul( const ecp_group *grp, ecp_point *R, - const mpi *m, const ecp_point *P ) + const mpi *m, const ecp_point *P, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret; unsigned char w, m_is_odd; @@ -1175,18 +1176,21 @@ int ecp_mul( const ecp_group *grp, ecp_point *R, ecp_point Q, T[ MAX_PRE_LEN ]; mpi M; + ((void) f_rng); + ((void) p_rng); + if( mpi_cmp_int( m, 0 ) < 0 || mpi_msb( m ) > grp->nbits ) - return( POLARSSL_ERR_ECP_GENERIC ); + return( POLARSSL_ERR_ECP_BAD_INPUT_DATA ); w = grp->nbits >= 521 ? 6 : grp->nbits >= 224 ? 5 : - 4; + 4; /* * Make sure w is within the limits. * The last test ensures that none of the precomputed points is zero, * which wouldn't be handled correctly by ecp_normalize_many(). - * It is only useful for small curves, as used in the test suite. + * It is only useful for very small curves, as used in the test suite. */ if( w > POLARSSL_ECP_WINDOW_SIZE ) w = POLARSSL_ECP_WINDOW_SIZE; @@ -1348,7 +1352,7 @@ int ecp_gen_keypair( const ecp_group *grp, mpi *d, ecp_point *Q, } while( mpi_cmp_int( d, 1 ) < 0 ); - return( ecp_mul( grp, Q, d, &grp->G ) ); + return( ecp_mul( grp, Q, d, &grp->G, f_rng, p_rng ) ); } #if defined(POLARSSL_SELF_TEST) @@ -1402,12 +1406,12 @@ int ecp_self_test( int verbose ) #endif /* POLARSSL_ECP_DP_SECP192R1_ENABLED */ if( verbose != 0 ) - printf( " ECP test #1 (SPA resistance): " ); + printf( " ECP test #1 (resistance to simple timing attacks): " ); add_count = 0; dbl_count = 0; MPI_CHK( mpi_read_string( &m, 16, exponents[0] ) ); - MPI_CHK( ecp_mul( &grp, &R, &m, &grp.G ) ); + MPI_CHK( ecp_mul( &grp, &R, &m, &grp.G, NULL, NULL ) ); for( i = 1; i < sizeof( exponents ) / sizeof( exponents[0] ); i++ ) { @@ -1417,7 +1421,7 @@ int ecp_self_test( int verbose ) dbl_count = 0; MPI_CHK( mpi_read_string( &m, 16, exponents[i] ) ); - MPI_CHK( ecp_mul( &grp, &R, &m, &grp.G ) ); + MPI_CHK( ecp_mul( &grp, &R, &m, &grp.G, NULL, NULL ) ); if( add_count != add_c_prev || dbl_count != dbl_c_prev ) { diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 0fccf3443..3b9d14246 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1748,7 +1748,8 @@ static int ssl_write_client_key_exchange( ssl_context *ssl ) if( ( ret = ecdh_calc_secret( &ssl->handshake->ecdh_ctx, &ssl->handshake->pmslen, ssl->handshake->premaster, - POLARSSL_MPI_MAX_SIZE ) ) != 0 ) + POLARSSL_MPI_MAX_SIZE, + ssl->f_rng, ssl->p_rng ) ) != 0 ) { SSL_DEBUG_RET( 1, "ecdh_calc_secret", ret ); return( ret ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 5bedcadce..adf5a623f 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2410,7 +2410,8 @@ static int ssl_parse_client_key_exchange( ssl_context *ssl ) if( ( ret = ecdh_calc_secret( &ssl->handshake->ecdh_ctx, &ssl->handshake->pmslen, ssl->handshake->premaster, - POLARSSL_MPI_MAX_SIZE ) ) != 0 ) + POLARSSL_MPI_MAX_SIZE, + ssl->f_rng, ssl->p_rng ) ) != 0 ) { SSL_DEBUG_RET( 1, "ecdh_calc_secret", ret ); return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE_CS ); diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function index 5bfd63db7..ba35c76a7 100644 --- a/tests/suites/test_suite_ecdh.function +++ b/tests/suites/test_suite_ecdh.function @@ -27,8 +27,10 @@ void ecdh_primitive_random( int id ) == 0 ); TEST_ASSERT( ecdh_gen_public( &grp, &dB, &qB, &rnd_pseudo_rand, &rnd_info ) == 0 ); - TEST_ASSERT( ecdh_compute_shared( &grp, &zA, &qB, &dA ) == 0 ); - TEST_ASSERT( ecdh_compute_shared( &grp, &zB, &qA, &dB ) == 0 ); + TEST_ASSERT( ecdh_compute_shared( &grp, &zA, &qB, &dA, + &rnd_pseudo_rand, &rnd_info ) == 0 ); + TEST_ASSERT( ecdh_compute_shared( &grp, &zB, &qA, &dB, + NULL, NULL ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &zA, &zB ) == 0 ); @@ -70,9 +72,9 @@ void ecdh_primitive_testvec( int id, char *dA_str, char *xA_str, char *yA_str, TEST_ASSERT( mpi_cmp_mpi( &qB.Y, &check ) == 0 ); TEST_ASSERT( mpi_read_string( &check, 16, z_str ) == 0 ); - TEST_ASSERT( ecdh_compute_shared( &grp, &zA, &qB, &dA ) == 0 ); + TEST_ASSERT( ecdh_compute_shared( &grp, &zA, &qB, &dA, NULL, NULL ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &zA, &check ) == 0 ); - TEST_ASSERT( ecdh_compute_shared( &grp, &zB, &qA, &dB ) == 0 ); + TEST_ASSERT( ecdh_compute_shared( &grp, &zB, &qA, &dB, NULL, NULL ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &zB, &check ) == 0 ); ecp_group_free( &grp ); @@ -107,8 +109,9 @@ void ecdh_exchange( int id ) &rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( ecdh_read_public( &srv, buf, len ) == 0 ); - TEST_ASSERT( ecdh_calc_secret( &srv, &len, buf, 1000 ) == 0 ); - TEST_ASSERT( ecdh_calc_secret( &cli, &len, buf, 1000 ) == 0 ); + TEST_ASSERT( ecdh_calc_secret( &srv, &len, buf, 1000, + &rnd_pseudo_rand, &rnd_info ) == 0 ); + TEST_ASSERT( ecdh_calc_secret( &cli, &len, buf, 1000, NULL, NULL ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &srv.z, &cli.z ) == 0 ); ecdh_free( &srv ); diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index ce9263364..55a34539f 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -50,7 +50,7 @@ ECP small subtraction #9 ecp_small_sub:0:"14":"11":0:"14":"36":0:27:30 ECP small multiplication negative -ecp_small_mul:-1:0:0:0:POLARSSL_ERR_ECP_GENERIC +ecp_small_mul:-1:0:0:0:POLARSSL_ERR_ECP_BAD_INPUT_DATA ECP small multiplication #0 ecp_small_mul:0:1:0:0:0 @@ -101,7 +101,7 @@ ECP small multiplication #15 ecp_small_mul:2:0:20:01:0 ECP small multiplication too big -ecp_small_mul:-1:0:0:0:POLARSSL_ERR_ECP_GENERIC +ecp_small_mul:-1:0:0:0:POLARSSL_ERR_ECP_BAD_INPUT_DATA ECP small check pubkey #1 ecp_small_check_pub:1:1:0:POLARSSL_ERR_ECP_GENERIC diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 62c5de2d4..5b1ab601d 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -101,17 +101,33 @@ void ecp_small_mul( int m_str, int r_zero, int x_r, int y_r, int ret ) ecp_group grp; ecp_point R; mpi m; + rnd_pseudo_info rnd_info; ecp_group_init( &grp ); ecp_point_init( &R ); mpi_init( &m ); + memset( &rnd_info, 0x00, sizeof( rnd_pseudo_info ) ); TEST_ASSERT( ecp_group_read_string( &grp, 10, "47", "4", "17", "42", "13" ) == 0 ); TEST_ASSERT( mpi_lset( &m, m_str ) == 0 ); - TEST_ASSERT( ecp_mul( &grp, &R, &m, &grp.G ) == ret ); + TEST_ASSERT( ecp_mul( &grp, &R, &m, &grp.G, NULL, NULL ) == ret ); + + if( r_zero ) + TEST_ASSERT( mpi_cmp_int( &R.Z, 0 ) == 0 ); + else + { + TEST_ASSERT( mpi_cmp_int( &R.X, x_r ) == 0 ); + TEST_ASSERT( mpi_cmp_int( &R.Y, y_r ) == 0 ); + } + + /* try again with randomization */ + ecp_point_free( &R ); + + TEST_ASSERT( ecp_mul( &grp, &R, &m, &grp.G, + &rnd_pseudo_rand, &rnd_info ) == ret ); if( r_zero ) TEST_ASSERT( mpi_cmp_int( &R.Z, 0 ) == 0 ); @@ -158,10 +174,12 @@ void ecp_test_vect( int id, char *dA_str, char *xA_str, char *yA_str, ecp_group grp; ecp_point R; mpi dA, xA, yA, dB, xB, yB, xZ, yZ; + rnd_pseudo_info rnd_info; ecp_group_init( &grp ); ecp_point_init( &R ); mpi_init( &dA ); mpi_init( &xA ); mpi_init( &yA ); mpi_init( &dB ); mpi_init( &xB ); mpi_init( &yB ); mpi_init( &xZ ); mpi_init( &yZ ); + memset( &rnd_info, 0x00, sizeof( rnd_pseudo_info ) ); TEST_ASSERT( ecp_use_known_dp( &grp, id ) == 0 ); @@ -176,20 +194,22 @@ void ecp_test_vect( int id, char *dA_str, char *xA_str, char *yA_str, TEST_ASSERT( mpi_read_string( &xZ, 16, xZ_str ) == 0 ); TEST_ASSERT( mpi_read_string( &yZ, 16, yZ_str ) == 0 ); - TEST_ASSERT( ecp_mul( &grp, &R, &dA, &grp.G ) == 0 ); + TEST_ASSERT( ecp_mul( &grp, &R, &dA, &grp.G, + &rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.X, &xA ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.Y, &yA ) == 0 ); TEST_ASSERT( ecp_check_pubkey( &grp, &R ) == 0 ); - TEST_ASSERT( ecp_mul( &grp, &R, &dB, &R ) == 0 ); + TEST_ASSERT( ecp_mul( &grp, &R, &dB, &R, NULL, NULL ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.X, &xZ ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.Y, &yZ ) == 0 ); TEST_ASSERT( ecp_check_pubkey( &grp, &R ) == 0 ); - TEST_ASSERT( ecp_mul( &grp, &R, &dB, &grp.G ) == 0 ); + TEST_ASSERT( ecp_mul( &grp, &R, &dB, &grp.G, NULL, NULL ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.X, &xB ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.Y, &yB ) == 0 ); TEST_ASSERT( ecp_check_pubkey( &grp, &R ) == 0 ); - TEST_ASSERT( ecp_mul( &grp, &R, &dA, &R ) == 0 ); + TEST_ASSERT( ecp_mul( &grp, &R, &dA, &R, + &rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.X, &xZ ) == 0 ); TEST_ASSERT( mpi_cmp_mpi( &R.Y, &yZ ) == 0 ); TEST_ASSERT( ecp_check_pubkey( &grp, &R ) == 0 );