Don't store client-supported ECs in heap-allocated buffer
So far, the client-proposed list of elliptic curves was stored for the
duration of the entire handshake in a heap-allocated buffer referenced
from mbedtls_ssl_handshake_params::curves. It is used in the following
places:
1) When the server chooses a suitable ciphersuite, it checks that
it has a certificate matching the ciphersuite; in particular, if
the ciphersuite involves ECDHE, the server needs an EC certificate
with a curve suitable for the client.
2) When performing the ECDHE key exchange, the server choose one
curve among those proposed by the client which matches the server's
own supported curve configuration.
This commit removes the hold back the entire client-side curve list
during the handshake, by performing (1) and (2) on during ClientHello
parsing, and in case of (2) only remembering the curve chosen for ECDHE
within mbedtls_ssl_handshake_params.
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 66f25ea..ccc32d7 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -279,12 +279,11 @@
#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \
defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED)
static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl,
- const unsigned char *buf,
- size_t len )
+ const unsigned char *buf, size_t len,
+ mbedtls_ecp_group_id curve_ids[ MBEDTLS_ECP_DP_MAX ] )
{
size_t list_size, our_size;
const unsigned char *p;
- const mbedtls_ecp_curve_info *curve_info, **curves;
if ( len < 2 ) {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
@@ -302,44 +301,38 @@
return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
}
- /* Should never happen unless client duplicates the extension */
- if( ssl->handshake->curves != NULL )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
- mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
- MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
- return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
- }
-
- /* Don't allow our peer to make us allocate too much memory,
- * and leave room for a final 0 */
- our_size = list_size / 2 + 1;
- if( our_size > MBEDTLS_ECP_DP_MAX )
- our_size = MBEDTLS_ECP_DP_MAX;
-
- if( ( curves = mbedtls_calloc( our_size, sizeof( *curves ) ) ) == NULL )
- {
- mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
- MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR );
- return( MBEDTLS_ERR_SSL_ALLOC_FAILED );
- }
-
- ssl->handshake->curves = curves;
-
p = buf + 2;
+ our_size = MBEDTLS_ECP_DP_MAX;
+
+ /* Leave room for final 0-entry */
while( list_size > 0 && our_size > 1 )
{
- curve_info = mbedtls_ecp_curve_info_from_tls_id( ( p[0] << 8 ) | p[1] );
+ uint16_t const tls_id = ( p[0] << 8 ) | p[1];
+ mbedtls_ecp_curve_info const * const info =
+ mbedtls_ecp_curve_info_from_tls_id( tls_id );
- if( curve_info != NULL )
+ if( info != NULL )
{
- *curves++ = curve_info;
+ mbedtls_ecp_group_id const *gid;
+ /* Remember the first curve that we also support. */
+ for( gid = ssl->conf->curve_list;
+ *gid != MBEDTLS_ECP_DP_NONE; gid++ )
+ {
+ if( info->grp_id != *gid )
+ continue;
+
+ if( ssl->handshake->curve_info == NULL )
+ ssl->handshake->curve_info = info;
+ }
+
+ *curve_ids++ = info->grp_id;
our_size--;
}
list_size -= 2;
p += 2;
}
+ *curve_ids = MBEDTLS_ECP_DP_NONE;
return( 0 );
}
@@ -743,16 +736,18 @@
*/
#if defined(MBEDTLS_ECDSA_C)
static int ssl_check_key_curve( mbedtls_pk_context *pk,
- const mbedtls_ecp_curve_info **curves )
+ mbedtls_ecp_group_id const *acceptable_ec_grp_ids )
{
- const mbedtls_ecp_curve_info **crv = curves;
mbedtls_ecp_group_id grp_id = mbedtls_pk_ec( *pk )->grp.id;
- while( *crv != NULL )
+ if( acceptable_ec_grp_ids == NULL )
+ return( -1 );
+
+ while( *acceptable_ec_grp_ids != MBEDTLS_ECP_DP_NONE )
{
- if( (*crv)->grp_id == grp_id )
+ if( *acceptable_ec_grp_ids == grp_id )
return( 0 );
- crv++;
+ acceptable_ec_grp_ids++;
}
return( -1 );
@@ -764,7 +759,9 @@
* return 0 on success and -1 on failure.
*/
static int ssl_pick_cert( mbedtls_ssl_context *ssl,
- const mbedtls_ssl_ciphersuite_t * ciphersuite_info )
+ const mbedtls_ssl_ciphersuite_t * ciphersuite_info,
+ mbedtls_ecp_group_id const *acceptable_ec_grp_ids )
+
{
mbedtls_ssl_key_cert *cur, *list, *fallback = NULL;
mbedtls_pk_type_t pk_alg =
@@ -829,11 +826,13 @@
#if defined(MBEDTLS_ECDSA_C)
if( pk_alg == MBEDTLS_PK_ECDSA &&
- ssl_check_key_curve( pk, ssl->handshake->curves ) != 0 )
+ ssl_check_key_curve( pk, acceptable_ec_grp_ids ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate mismatch: elliptic curve" ) );
match = 0;
}
+#else
+ ((void) acceptable_ec_grp_ids);
#endif
#if defined(MBEDTLS_SSL_ASYNC_PRIVATE)
@@ -921,7 +920,8 @@
* Sets ciphersuite_info only if the suite matches.
*/
static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id,
- const mbedtls_ssl_ciphersuite_t **ciphersuite_info )
+ const mbedtls_ssl_ciphersuite_t **ciphersuite_info,
+ mbedtls_ecp_group_id const *acceptable_ec_grp_ids )
{
const mbedtls_ssl_ciphersuite_t *suite_info;
@@ -971,11 +971,9 @@
}
#endif
-
#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C)
if( mbedtls_ssl_ciphersuite_uses_ec( suite_info ) &&
- ( ssl->handshake->curves == NULL ||
- ssl->handshake->curves[0] == NULL ) )
+ ssl->handshake->curve_info == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: "
"no common elliptic curve" ) );
@@ -1023,12 +1021,14 @@
* - try the next ciphersuite if we don't
* This must be done last since we modify the key_cert list.
*/
- if( ssl_pick_cert( ssl, suite_info ) != 0 )
+ if( ssl_pick_cert( ssl, suite_info, acceptable_ec_grp_ids ) != 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: "
"no suitable certificate" ) );
return( 0 );
}
+#else
+ ((void) acceptable_ec_grp_ids);
#endif
*ciphersuite_info = suite_info;
@@ -1252,8 +1252,11 @@
got_common_suite = 1;
if( ( ret = ssl_ciphersuite_match( ssl, ciphersuites[i],
- &ciphersuite_info ) ) != 0 )
+ &ciphersuite_info,
+ NULL ) ) )
+ {
return( ret );
+ }
if( ciphersuite_info != NULL )
goto have_ciphersuite_v2;
@@ -1323,6 +1326,13 @@
const mbedtls_ssl_ciphersuite_t *ciphersuite_info;
int major, minor;
+#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \
+ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED)
+ mbedtls_ecp_group_id acceptable_ec_grp_ids[ MBEDTLS_ECP_DP_MAX ];
+#else
+ mbedtls_ecp_group_id * acceptable_ec_grp_ids = NULL;
+#endif
+
/* If there is no signature-algorithm extension present,
* we need to fall back to the default values for allowed
* signature-hash pairs. */
@@ -1862,7 +1872,9 @@
case MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES:
MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported elliptic curves extension" ) );
- ret = ssl_parse_supported_elliptic_curves( ssl, ext + 4, ext_size );
+ ret = ssl_parse_supported_elliptic_curves( ssl, ext + 4,
+ ext_size,
+ acceptable_ec_grp_ids );
if( ret != 0 )
return( ret );
break;
@@ -2133,8 +2145,11 @@
got_common_suite = 1;
if( ( ret = ssl_ciphersuite_match( ssl, ciphersuites[i],
- &ciphersuite_info ) ) != 0 )
+ &ciphersuite_info,
+ acceptable_ec_grp_ids ) ) != 0 )
+ {
return( ret );
+ }
if( ciphersuite_info != NULL )
goto have_ciphersuite;
@@ -3241,28 +3256,19 @@
* ECPoint public;
* } ServerECDHParams;
*/
- const mbedtls_ecp_curve_info **curve = NULL;
- const mbedtls_ecp_group_id *gid;
+ const mbedtls_ecp_curve_info *curve = ssl->handshake->curve_info;
int ret;
size_t len = 0;
- /* Match our preference list against the offered curves */
- for( gid = ssl->conf->curve_list; *gid != MBEDTLS_ECP_DP_NONE; gid++ )
- for( curve = ssl->handshake->curves; *curve != NULL; curve++ )
- if( (*curve)->grp_id == *gid )
- goto curve_matching_done;
-
-curve_matching_done:
- if( curve == NULL || *curve == NULL )
+ if( curve == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching curve for ECDHE" ) );
return( MBEDTLS_ERR_SSL_NO_CIPHER_CHOSEN );
}
-
- MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDHE curve: %s", (*curve)->name ) );
+ MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDHE curve: %s", curve->name ) );
if( ( ret = mbedtls_ecdh_setup( &ssl->handshake->ecdh_ctx,
- (*curve)->grp_id ) ) != 0 )
+ curve->grp_id ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecp_group_load", ret );
return( ret );