From 3335205a2118d80b25bd40ca0f5a13f8dee8c8d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jun 2015 16:17:08 +0100 Subject: [PATCH] Avoid in-out length in dhm_calc_secret() --- include/mbedtls/dhm.h | 6 +++--- library/dhm.c | 4 ++-- library/ssl_cli.c | 3 +-- library/ssl_srv.c | 3 +-- library/ssl_tls.c | 4 ++-- programs/pkey/dh_client.c | 3 +-- programs/pkey/dh_server.c | 3 +-- programs/test/benchmark.c | 6 ++---- tests/suites/test_suite_dhm.function | 16 +++++++--------- 9 files changed, 20 insertions(+), 28 deletions(-) diff --git a/include/mbedtls/dhm.h b/include/mbedtls/dhm.h index 83aad1e59..10fda5beb 100644 --- a/include/mbedtls/dhm.h +++ b/include/mbedtls/dhm.h @@ -241,8 +241,8 @@ int mbedtls_dhm_make_public( mbedtls_dhm_context *ctx, int x_size, * * \param ctx DHM context * \param output destination buffer - * \param olen on entry, must hold the size of the destination buffer - * on exit, holds the actual number of bytes written + * \param output_size size of the destination buffer + * \param olen on exit, holds the actual number of bytes written * \param f_rng RNG function, for blinding purposes * \param p_rng RNG parameter * @@ -255,7 +255,7 @@ int mbedtls_dhm_make_public( mbedtls_dhm_context *ctx, int x_size, * to always pass a non-NULL f_rng argument. */ int mbedtls_dhm_calc_secret( mbedtls_dhm_context *ctx, - unsigned char *output, size_t *olen, + unsigned char *output, size_t output_size, size_t *olen, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); diff --git a/library/dhm.c b/library/dhm.c index 6b4e29fd5..979fd07fe 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -344,14 +344,14 @@ cleanup: * Derive and export the shared secret (G^Y)^X mod P */ int mbedtls_dhm_calc_secret( mbedtls_dhm_context *ctx, - unsigned char *output, size_t *olen, + unsigned char *output, size_t output_size, size_t *olen, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret; mbedtls_mpi GYb; - if( ctx == NULL || *olen < ctx->len ) + if( ctx == NULL || output_size < ctx->len ) return( MBEDTLS_ERR_DHM_BAD_INPUT_DATA ); if( ( ret = dhm_check_range( &ctx->GY, &ctx->P ) ) != 0 ) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index cff887110..6eb190c57 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2477,10 +2477,9 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: X ", &ssl->handshake->dhm_ctx.X ); MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); - ssl->handshake->pmslen = MBEDTLS_PREMASTER_SIZE; - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, ssl->handshake->premaster, + MBEDTLS_PREMASTER_SIZE, &ssl->handshake->pmslen, ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 72f9eeea2..7db5a3ca6 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3145,10 +3145,9 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); } - ssl->handshake->pmslen = MBEDTLS_PREMASTER_SIZE; - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, ssl->handshake->premaster, + MBEDTLS_PREMASTER_SIZE, &ssl->handshake->pmslen, ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 443b42162..ee32502ce 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1114,11 +1114,11 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch if( key_ex == MBEDTLS_KEY_EXCHANGE_DHE_PSK ) { int ret; - size_t len = end - ( p + 2 ); + size_t len; /* Write length only when we know the actual value */ if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, - p + 2, &len, + p + 2, end - ( p + 2 ), &len, ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); diff --git a/programs/pkey/dh_client.c b/programs/pkey/dh_client.c index 21c379633..546b02ebc 100644 --- a/programs/pkey/dh_client.c +++ b/programs/pkey/dh_client.c @@ -243,8 +243,7 @@ int main( void ) mbedtls_printf( "\n . Shared secret: " ); fflush( stdout ); - n = dhm.len; - if( ( ret = mbedtls_dhm_calc_secret( &dhm, buf, &n, + if( ( ret = mbedtls_dhm_calc_secret( &dhm, buf, sizeof( buf ), &n, mbedtls_ctr_drbg_random, &ctr_drbg ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_dhm_calc_secret returned %d\n\n", ret ); diff --git a/programs/pkey/dh_server.c b/programs/pkey/dh_server.c index 9ed6e36a4..81c0c9f9e 100644 --- a/programs/pkey/dh_server.c +++ b/programs/pkey/dh_server.c @@ -228,7 +228,6 @@ int main( void ) fflush( stdout ); memset( buf, 0, sizeof( buf ) ); - n = dhm.len; if( ( ret = mbedtls_net_recv( &client_fd, buf, n ) ) != (int) n ) { @@ -248,7 +247,7 @@ int main( void ) mbedtls_printf( "\n . Shared secret: " ); fflush( stdout ); - if( ( ret = mbedtls_dhm_calc_secret( &dhm, buf, &n, + if( ( ret = mbedtls_dhm_calc_secret( &dhm, buf, sizeof( buf ), &n, mbedtls_ctr_drbg_random, &ctr_drbg ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_dhm_calc_secret returned %d\n\n", ret ); diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index 6262d339b..67797c64a 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -646,15 +646,13 @@ int main( int argc, char *argv[] ) mbedtls_snprintf( title, sizeof( title ), "DHE-%d", dhm_sizes[i] ); TIME_PUBLIC( title, "handshake", - olen = sizeof( buf ); ret |= mbedtls_dhm_make_public( &dhm, (int) dhm.len, buf, dhm.len, myrand, NULL ); - ret |= mbedtls_dhm_calc_secret( &dhm, buf, &olen, myrand, NULL ) ); + ret |= mbedtls_dhm_calc_secret( &dhm, buf, sizeof( buf ), &olen, myrand, NULL ) ); mbedtls_snprintf( title, sizeof( title ), "DH-%d", dhm_sizes[i] ); TIME_PUBLIC( title, "handshake", - olen = sizeof( buf ); - ret |= mbedtls_dhm_calc_secret( &dhm, buf, &olen, myrand, NULL ) ); + ret |= mbedtls_dhm_calc_secret( &dhm, buf, sizeof( buf ), &olen, myrand, NULL ) ); mbedtls_dhm_free( &dhm ); } diff --git a/tests/suites/test_suite_dhm.function b/tests/suites/test_suite_dhm.function index ae814d352..002c20bf4 100644 --- a/tests/suites/test_suite_dhm.function +++ b/tests/suites/test_suite_dhm.function @@ -20,8 +20,8 @@ void dhm_do_dhm( int radix_P, char *input_P, unsigned char sec_cli[1000]; size_t ske_len = 0; size_t pub_cli_len = 0; - size_t sec_srv_len = 1000; - size_t sec_cli_len = 1000; + size_t sec_srv_len; + size_t sec_cli_len; int x_size, i; rnd_pseudo_info rnd_info; @@ -52,8 +52,8 @@ void dhm_do_dhm( int radix_P, char *input_P, TEST_ASSERT( mbedtls_dhm_make_public( &ctx_cli, x_size, pub_cli, pub_cli_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_dhm_read_public( &ctx_srv, pub_cli, pub_cli_len ) == 0 ); - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_srv, sec_srv, &sec_srv_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, &sec_cli_len, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_srv, sec_srv, sizeof( sec_srv ), &sec_srv_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, sizeof( sec_cli ), &sec_cli_len, NULL, NULL ) == 0 ); TEST_ASSERT( sec_srv_len == sec_cli_len ); TEST_ASSERT( sec_srv_len != 0 ); @@ -63,7 +63,7 @@ void dhm_do_dhm( int radix_P, char *input_P, for( i = 0; i < 3; i++ ) { sec_srv_len = 1000; - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_srv, sec_srv, &sec_srv_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_srv, sec_srv, sizeof( sec_srv ), &sec_srv_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( sec_srv_len == sec_cli_len ); TEST_ASSERT( sec_srv_len != 0 ); @@ -73,8 +73,6 @@ void dhm_do_dhm( int radix_P, char *input_P, /* * Second key exchange to test change of blinding values on server */ - sec_cli_len = 1000; - sec_srv_len = 1000; p = ske; TEST_ASSERT( mbedtls_dhm_make_params( &ctx_srv, x_size, ske, &ske_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); @@ -85,8 +83,8 @@ void dhm_do_dhm( int radix_P, char *input_P, TEST_ASSERT( mbedtls_dhm_make_public( &ctx_cli, x_size, pub_cli, pub_cli_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); TEST_ASSERT( mbedtls_dhm_read_public( &ctx_srv, pub_cli, pub_cli_len ) == 0 ); - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_srv, sec_srv, &sec_srv_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, &sec_cli_len, NULL, NULL ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_srv, sec_srv, sizeof( sec_srv ), &sec_srv_len, &rnd_pseudo_rand, &rnd_info ) == 0 ); + TEST_ASSERT( mbedtls_dhm_calc_secret( &ctx_cli, sec_cli, sizeof( sec_cli ), &sec_cli_len, NULL, NULL ) == 0 ); TEST_ASSERT( sec_srv_len == sec_cli_len ); TEST_ASSERT( sec_srv_len != 0 );