diff --git a/ChangeLog b/ChangeLog index af7ee46e7..2c031a189 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,14 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 2.7.x branch released xxxx-xx-xx +Security + * Make mbedtls_ecdh_get_params return an error if the second key + belongs to a different group from the first. Before, if an application + passed keys that belonged to different group, the first key's data was + interpreted according to the second group, which could lead to either + an error or a meaningless output from mbedtls_ecdh_get_params. In the + latter case, this could expose at most 5 bits of the private key. + Bugfix * Server's RSA certificate in certs.c was SHA-1 signed. In the default mbedTLS configuration only SHA-2 signed certificates are accepted. diff --git a/library/ecdh.c b/library/ecdh.c index 61380b693..75630bd35 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -179,8 +179,20 @@ int mbedtls_ecdh_get_params( mbedtls_ecdh_context *ctx, const mbedtls_ecp_keypai { int ret; - if( ( ret = mbedtls_ecp_group_copy( &ctx->grp, &key->grp ) ) != 0 ) - return( ret ); + if( ctx->grp.id == MBEDTLS_ECP_DP_NONE ) + { + /* This is the first call to get_params(). Copy the group information + * into the context. */ + if( ( ret = mbedtls_ecp_group_copy( &ctx->grp, &key->grp ) ) != 0 ) + return( ret ); + } + else + { + /* This is not the first call to get_params(). Check that the group + * is the same as the first time. */ + if( ctx->grp.id != key->grp.id ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + } /* If it's not our key, just import the public part as Qp */ if( side == MBEDTLS_ECDH_THEIRS ) diff --git a/tests/suites/test_suite_ecdh.data b/tests/suites/test_suite_ecdh.data index f7119de41..4ed32219b 100644 --- a/tests/suites/test_suite_ecdh.data +++ b/tests/suites/test_suite_ecdh.data @@ -37,3 +37,19 @@ ecdh_exchange:MBEDTLS_ECP_DP_SECP192R1 ECDH exchange #2 depends_on:MBEDTLS_ECP_DP_SECP521R1_ENABLED ecdh_exchange:MBEDTLS_ECP_DP_SECP521R1 + +ECDH calc_secret: ours first, SECP256R1 (RFC 5903) +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_SECP256R1:"c6ef9c5d78ae012a011164acb397ce2088685d8f06bf9be0b283ab46476bee53":"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":0:"d6840f6b42f6edafd13116e0e12565202fef8e9ece7dce03812464d04b9442de" + +ECDH calc_secret: theirs first, SECP256R1 (RFC 5903) +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_SECP256R1:"c6ef9c5d78ae012a011164acb397ce2088685d8f06bf9be0b283ab46476bee53":"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":1:"d6840f6b42f6edafd13116e0e12565202fef8e9ece7dce03812464d04b9442de" + +ECDH get_params with mismatched groups: our BP256R1, their SECP256R1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_BP256R1_ENABLED +ecdh_exchange_get_params_fail:MBEDTLS_ECP_DP_BP256R1:"1234567812345678123456781234567812345678123456781234567812345678":MBEDTLS_ECP_DP_SECP256R1:"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":0:MBEDTLS_ERR_ECP_BAD_INPUT_DATA + +ECDH get_params with mismatched groups: their SECP256R1, our BP256R1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_BP256R1_ENABLED +ecdh_exchange_get_params_fail:MBEDTLS_ECP_DP_BP256R1:"1234567812345678123456781234567812345678123456781234567812345678":MBEDTLS_ECP_DP_SECP256R1:"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":1:MBEDTLS_ERR_ECP_BAD_INPUT_DATA diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function index 4c6a97baf..0645ce798 100644 --- a/tests/suites/test_suite_ecdh.function +++ b/tests/suites/test_suite_ecdh.function @@ -1,5 +1,47 @@ /* BEGIN_HEADER */ #include "mbedtls/ecdh.h" + +static int load_public_key( int grp_id, const char *point_str, + mbedtls_ecp_keypair *ecp ) +{ + int ok = 0; + unsigned char point_buf[MBEDTLS_ECP_MAX_PT_LEN]; + size_t point_len = unhexify( point_buf, point_str ); + + TEST_ASSERT( mbedtls_ecp_group_load( &ecp->grp, grp_id ) == 0 ); + TEST_ASSERT( mbedtls_ecp_point_read_binary( &ecp->grp, + &ecp->Q, + point_buf, + point_len ) == 0 ); + TEST_ASSERT( mbedtls_ecp_check_pubkey( &ecp->grp, + &ecp->Q ) == 0 ); + ok = 1; +exit: + return( ok ); +} + +static int load_private_key( int grp_id, const char *private_key_str, + mbedtls_ecp_keypair *ecp, + rnd_pseudo_info *rnd_info ) +{ + int ok = 0; + unsigned char private_key_buf[MBEDTLS_ECP_MAX_BYTES]; + size_t private_key_len = unhexify( private_key_buf, private_key_str ); + + TEST_ASSERT( mbedtls_ecp_group_load( &ecp->grp, grp_id ) == 0 ); + TEST_ASSERT( mbedtls_mpi_read_binary( &ecp->d, + private_key_buf, + private_key_len ) == 0 ); + TEST_ASSERT( mbedtls_ecp_check_privkey( &ecp->grp, &ecp->d ) == 0 ); + /* Calculate the public key from the private key. */ + TEST_ASSERT( mbedtls_ecp_mul( &ecp->grp, &ecp->Q, &ecp->d, + &ecp->grp.G, + &rnd_pseudo_rand, rnd_info ) == 0 ); + ok = 1; +exit: + return( ok ); +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -158,3 +200,111 @@ exit: mbedtls_ecdh_free( &cli ); } /* END_CASE */ + +/* BEGIN_CASE */ +void ecdh_exchange_calc_secret( int grp_id, + char *our_private_key, + char *their_point, + int ours_first, + char *expected_str ) +{ + rnd_pseudo_info rnd_info; + unsigned char expected_buf[MBEDTLS_ECP_MAX_BYTES]; + size_t expected_len; + mbedtls_ecp_keypair our_key; + mbedtls_ecp_keypair their_key; + mbedtls_ecdh_context ecdh; + unsigned char shared_secret[MBEDTLS_ECP_MAX_BYTES]; + size_t shared_secret_length = 0; + + memset( &rnd_info, 0x00, sizeof( rnd_pseudo_info ) ); + mbedtls_ecdh_init( &ecdh ); + mbedtls_ecp_keypair_init( &our_key ); + mbedtls_ecp_keypair_init( &their_key ); + + expected_len = unhexify( expected_buf, expected_str ); + + if( ! load_private_key( grp_id, our_private_key, &our_key, &rnd_info ) ) + goto exit; + if( ! load_public_key( grp_id, their_point, &their_key ) ) + goto exit; + + /* Import the keys to the ECDH calculation. */ + if( ours_first ) + { + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &our_key, MBEDTLS_ECDH_OURS ) == 0 ); + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &their_key, MBEDTLS_ECDH_THEIRS ) == 0 ); + } + else + { + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &their_key, MBEDTLS_ECDH_THEIRS ) == 0 ); + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &our_key, MBEDTLS_ECDH_OURS ) == 0 ); + } + + /* Perform the ECDH calculation. */ + TEST_ASSERT( mbedtls_ecdh_calc_secret( + &ecdh, + &shared_secret_length, + shared_secret, sizeof( shared_secret ), + &rnd_pseudo_rand, &rnd_info ) == 0 ); + TEST_ASSERT( shared_secret_length == expected_len ); + TEST_ASSERT( memcmp( expected_buf, shared_secret, + shared_secret_length ) == 0 ); + +exit: + mbedtls_ecdh_free( &ecdh ); + mbedtls_ecp_keypair_free( &our_key ); + mbedtls_ecp_keypair_free( &their_key ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void ecdh_exchange_get_params_fail( int our_grp_id, + char *our_private_key, + int their_grp_id, + char *their_point, + int ours_first, + int expected_ret ) +{ + rnd_pseudo_info rnd_info; + mbedtls_ecp_keypair our_key; + mbedtls_ecp_keypair their_key; + mbedtls_ecdh_context ecdh; + + memset( &rnd_info, 0x00, sizeof( rnd_pseudo_info ) ); + mbedtls_ecdh_init( &ecdh ); + mbedtls_ecp_keypair_init( &our_key ); + mbedtls_ecp_keypair_init( &their_key ); + + if( ! load_private_key( our_grp_id, our_private_key, &our_key, &rnd_info ) ) + goto exit; + if( ! load_public_key( their_grp_id, their_point, &their_key ) ) + goto exit; + + if( ours_first ) + { + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &our_key, MBEDTLS_ECDH_OURS ) == 0 ); + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &their_key, MBEDTLS_ECDH_THEIRS ) == + expected_ret ); + } + else + { + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &their_key, MBEDTLS_ECDH_THEIRS ) == 0 ); + TEST_ASSERT( mbedtls_ecdh_get_params( + &ecdh, &our_key, MBEDTLS_ECDH_OURS ) == + expected_ret ); + } + +exit: + mbedtls_ecdh_free( &ecdh ); + mbedtls_ecp_keypair_free( &our_key ); + mbedtls_ecp_keypair_free( &their_key ); +} +/* END_CASE */