diff --git a/ChangeLog.d/use-psa-ecdhe-curve.txt b/ChangeLog.d/use-psa-ecdhe-curve.txt new file mode 100644 index 0000000000..cc432bdaee --- /dev/null +++ b/ChangeLog.d/use-psa-ecdhe-curve.txt @@ -0,0 +1,7 @@ +Bugfix + * Fix a bug in (D)TLS curve negotiation: when MBEDTLS_USE_PSA_CRYPTO was + enabled and an ECDHE-ECDSA or ECDHE-RSA key exchange was used, the + client would fail to check that the curve selected by the server for + ECDHE was indeed one that was offered. As a result, the client would + accept any curve that it supported, even if that curve was not allowed + according to its configuration. diff --git a/include/mbedtls/psa_util.h b/include/mbedtls/psa_util.h index c54c035c37..e38e2e3469 100644 --- a/include/mbedtls/psa_util.h +++ b/include/mbedtls/psa_util.h @@ -258,85 +258,24 @@ static inline int mbedtls_psa_get_ecc_oid_from_id( return( -1 ); } -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH 1 - -#if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 192 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 192 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP192R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 224 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 224 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP224R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 256 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 256 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP256R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 384 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 384 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP384R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 521 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 521 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP521R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 192 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 192 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP192K1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 224 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 224 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP224K1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 256 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 256 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_SECP256K1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 256 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 256 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_BP256R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 384 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 384 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_BP384R1_ENABLED */ - -#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) -#if MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH < ( 2 * ( ( 512 + 7 ) / 8 ) + 1 ) -#undef MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH -#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH ( 2 * ( ( 512 + 7 ) / 8 ) + 1 ) -#endif -#endif /* MBEDTLS_ECP_DP_BP512R1_ENABLED */ +#define MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH \ + PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE( PSA_VENDOR_ECC_MAX_CURVE_BITS ) +/* This function transforms an ECC group identifier from + * https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8 + * into a PSA ECC group identifier. */ +#if defined(MBEDTLS_ECP_C) +static inline psa_key_type_t mbedtls_psa_parse_tls_ecc_group( + uint16_t tls_ecc_grp_reg_id, size_t *bits ) +{ + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_tls_id( tls_ecc_grp_reg_id ); + if( curve_info == NULL ) + return( 0 ); + return( PSA_KEY_TYPE_ECC_KEY_PAIR( + mbedtls_ecc_group_to_psa( curve_info->grp_id, bits ) ) ); +} +#endif /* MBEDTLS_ECP_C */ /* Translations for PK layer */ @@ -366,63 +305,6 @@ static inline int mbedtls_psa_err_translate_pk( psa_status_t status ) } } -/* Translations for ECC */ - -/* This function transforms an ECC group identifier from - * https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8 - * into a PSA ECC group identifier. */ -#if defined(MBEDTLS_ECP_C) -static inline psa_key_type_t mbedtls_psa_parse_tls_ecc_group( - uint16_t tls_ecc_grp_reg_id, size_t *bits ) -{ - const mbedtls_ecp_curve_info *curve_info = - mbedtls_ecp_curve_info_from_tls_id( tls_ecc_grp_reg_id ); - if( curve_info == NULL ) - return( 0 ); - return( PSA_KEY_TYPE_ECC_KEY_PAIR( - mbedtls_ecc_group_to_psa( curve_info->grp_id, bits ) ) ); -} -#endif /* MBEDTLS_ECP_C */ - -/* This function takes a buffer holding an EC public key - * exported through psa_export_public_key(), and converts - * it into an ECPoint structure to be put into a ClientKeyExchange - * message in an ECDHE exchange. - * - * Both the present and the foreseeable future format of EC public keys - * used by PSA have the ECPoint structure contained in the exported key - * as a subbuffer, and the function merely selects this subbuffer instead - * of making a copy. - */ -static inline int mbedtls_psa_tls_psa_ec_to_ecpoint( unsigned char *src, - size_t srclen, - unsigned char **dst, - size_t *dstlen ) -{ - *dst = src; - *dstlen = srclen; - return( 0 ); -} - -/* This function takes a buffer holding an ECPoint structure - * (as contained in a TLS ServerKeyExchange message for ECDHE - * exchanges) and converts it into a format that the PSA key - * agreement API understands. - */ -static inline int mbedtls_psa_tls_ecpoint_to_psa_ec( unsigned char const *src, - size_t srclen, - unsigned char *dst, - size_t dstlen, - size_t *olen ) -{ - if( srclen > dstlen ) - return( MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL ); - - memcpy( dst, src, srclen ); - *olen = srclen; - return( 0 ); -} - #endif /* MBEDTLS_USE_PSA_CRYPTO */ /* Expose whatever RNG the PSA subsystem uses to applications using the diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 825034a8d8..694473f2b0 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2334,12 +2334,7 @@ static int ssl_check_server_ecdh_params( const mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); -#if defined(MBEDTLS_ECP_C) if( mbedtls_ssl_check_curve( ssl, grp_id ) != 0 ) -#else - if( ssl->handshake->ecdh_ctx.grp.nbits < 163 || - ssl->handshake->ecdh_ctx.grp.nbits > 521 ) -#endif return( -1 ); MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, @@ -2366,9 +2361,16 @@ static int ssl_parse_server_ecdh_params_psa( mbedtls_ssl_context *ssl, mbedtls_ssl_handshake_params *handshake = ssl->handshake; /* - * Parse ECC group + * struct { + * ECParameters curve_params; + * ECPoint public; + * } ServerECDHParams; + * + * 1 curve_type (must be "named_curve") + * 2..3 NamedCurve + * 4 ECPoint.len + * 5+ ECPoint contents */ - if( end - *p < 4 ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); @@ -2381,6 +2383,15 @@ static int ssl_parse_server_ecdh_params_psa( mbedtls_ssl_context *ssl, tls_id <<= 8; tls_id |= *(*p)++; + /* Check it's a curve we offered */ + if( mbedtls_ssl_check_curve_tls_id( ssl, tls_id ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "bad server key exchange message (ECDHE curve): %u", + (unsigned) tls_id ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + /* Convert EC group to PSA key type. */ if( ( handshake->ecdh_psa_type = mbedtls_psa_parse_tls_ecc_group( tls_id, &ecdh_bits ) ) == 0 ) @@ -2391,24 +2402,18 @@ static int ssl_parse_server_ecdh_params_psa( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); handshake->ecdh_bits = (uint16_t) ecdh_bits; - /* - * Put peer's ECDH public key in the format understood by PSA. - */ - + /* Keep a copy of the peer's public key */ ecpoint_len = *(*p)++; if( (size_t)( end - *p ) < ecpoint_len ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - if( mbedtls_psa_tls_ecpoint_to_psa_ec( - *p, ecpoint_len, - handshake->ecdh_psa_peerkey, - sizeof( handshake->ecdh_psa_peerkey ), - &handshake->ecdh_psa_peerkey_len ) != 0 ) - { - return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - } + if( ecpoint_len > sizeof( handshake->ecdh_psa_peerkey ) ) + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + memcpy( handshake->ecdh_psa_peerkey, *p, ecpoint_len ); + handshake->ecdh_psa_peerkey_len = ecpoint_len; *p += ecpoint_len; + return( 0 ); } #endif /* MBEDTLS_USE_PSA_CRYPTO && @@ -3373,11 +3378,6 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_params *handshake = ssl->handshake; - unsigned char own_pubkey[MBEDTLS_PSA_MAX_EC_PUBKEY_LENGTH]; - size_t own_pubkey_len; - unsigned char *own_pubkey_ecpoint; - size_t own_pubkey_ecpoint_len; - header_len = 4; MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); @@ -3405,27 +3405,22 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) if( status != PSA_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - /* Export the public part of the ECDH private key from PSA - * and convert it to ECPoint format used in ClientKeyExchange. */ + /* Export the public part of the ECDH private key from PSA. + * The export format is an ECPoint structure as expected by TLS, + * but we just need to add a length byte before that. */ + unsigned char *own_pubkey = ssl->out_msg + header_len + 1; + unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; + size_t own_pubkey_max_len = (size_t)( end - own_pubkey ); + size_t own_pubkey_len; + status = psa_export_public_key( handshake->ecdh_psa_privkey, - own_pubkey, sizeof( own_pubkey ), + own_pubkey, own_pubkey_max_len, &own_pubkey_len ); if( status != PSA_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - if( mbedtls_psa_tls_psa_ec_to_ecpoint( own_pubkey, - own_pubkey_len, - &own_pubkey_ecpoint, - &own_pubkey_ecpoint_len ) != 0 ) - { - return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - } - - /* Copy ECPoint structure to outgoing message buffer. */ - ssl->out_msg[header_len] = (unsigned char) own_pubkey_ecpoint_len; - memcpy( ssl->out_msg + header_len + 1, - own_pubkey_ecpoint, own_pubkey_ecpoint_len ); - content_len = own_pubkey_ecpoint_len + 1; + ssl->out_msg[header_len] = (unsigned char) own_pubkey_len; + content_len = own_pubkey_len + 1; /* The ECDH secret is the premaster secret used for key derivation. */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 8cb9576ff6..4d753c8b7e 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1314,6 +1314,7 @@ mbedtls_md_type_t mbedtls_ssl_md_alg_from_hash( unsigned char hash ); unsigned char mbedtls_ssl_hash_from_md_alg( int md ); int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ); +int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id ); #if defined(MBEDTLS_ECP_C) int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id ); #endif diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 988fafbd6f..df17ca55cd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7216,18 +7216,16 @@ unsigned char mbedtls_ssl_hash_from_md_alg( int md ) } } -#if defined(MBEDTLS_ECP_C) /* * Check if a curve proposed by the peer is in our list. * Return 0 if we're willing to use it, -1 otherwise. */ -int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id ) +int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id ) { const uint16_t *group_list = mbedtls_ssl_get_groups( ssl ); if( group_list == NULL ) return( -1 ); - uint16_t tls_id = mbedtls_ecp_curve_info_from_grp_id(grp_id)->tls_id; for( ; *group_list != 0; group_list++ ) { @@ -7237,6 +7235,16 @@ int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_i return( -1 ); } + +#if defined(MBEDTLS_ECP_C) +/* + * Same as mbedtls_ssl_check_curve_tls_id() but with a mbedtls_ecp_group_id. + */ +int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id ) +{ + uint16_t tls_id = mbedtls_ecp_curve_info_from_grp_id( grp_id )->tls_id; + return mbedtls_ssl_check_curve_tls_id( ssl, tls_id ); +} #endif /* MBEDTLS_ECP_C */ #if defined(MBEDTLS_X509_CRT_PARSE_C)