From b47d0f893ed984efb039389989f8b44641b95425 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Dec 2021 17:34:40 +0800 Subject: [PATCH 01/16] Replace SUPPORTED_ELLIPTIC_CURVES with SUPPORTED_GROUPS According to RFC7919 and RFC8442 , they are same. Signed-off-by: Jerry Yu --- library/ssl_cli.c | 16 ++++++++-------- library/ssl_srv.c | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 32d4969132..32befc24c7 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -302,10 +302,10 @@ static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) -static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - const unsigned char *end, - size_t *olen ) +static int ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; unsigned char *elliptic_curve_list = p + 6; @@ -351,7 +351,7 @@ static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, if( elliptic_curve_len == 0 ) return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES, p, 0 ); + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_GROUPS, p, 0 ); p += 2; MBEDTLS_PUT_UINT16_BE( elliptic_curve_len + 2, p, 0 ); @@ -1206,10 +1206,10 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if( uses_ec ) { - if( ( ret = ssl_write_supported_elliptic_curves_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) + if( ( ret = ssl_write_supported_groups_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_elliptic_curves_ext", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_groups_ext", ret ); return( ret ); } ext_len += olen; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index f34f2de30f..977ecc8f33 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -317,7 +317,7 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, #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, +static int ssl_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -1646,10 +1646,10 @@ read_record_header: #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - case MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES: + case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: 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_groups_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); break; From ba07342cd6b2e95f58f1bf9e8298d4a54c429fda Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Dec 2021 22:22:15 +0800 Subject: [PATCH 02/16] Add generic write_supported-groups_ext Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 1 + library/ssl_misc.h | 52 +++++++++++------ library/ssl_tls.c | 133 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 17 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 072ebbe460..45b991b4ca 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -191,6 +191,7 @@ /* Elliptic Curve Groups (ECDHE) */ #define MBEDTLS_SSL_IANA_TLS_GROUP_NONE 0 +#define MBEDTLS_SSL_IANA_TLS_GROUP_SECT163K1 0x0001 /* RFC 4492 */ #define MBEDTLS_SSL_IANA_TLS_GROUP_SECP192K1 0x0012 #define MBEDTLS_SSL_IANA_TLS_GROUP_SECP192R1 0x0013 #define MBEDTLS_SSL_IANA_TLS_GROUP_SECP224K1 0x0014 diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 40e4aaff88..b74b0d9427 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1626,23 +1626,6 @@ static inline int mbedtls_ssl_tls13_some_psk_enabled( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ALL ) ); } -/* - * Helper functions for NamedGroup. - */ -static inline int mbedtls_ssl_tls13_named_group_is_ecdhe( uint16_t named_group ) -{ - return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP384R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X448 ); -} - -static inline int mbedtls_ssl_tls13_named_group_is_dhe( uint16_t named_group ) -{ - return( named_group >= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE2048 && - named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE8192 ); -} static inline void mbedtls_ssl_handshake_set_state( mbedtls_ssl_context *ssl, mbedtls_ssl_states state ) @@ -1743,4 +1726,39 @@ static inline const void *mbedtls_ssl_get_groups( const mbedtls_ssl_context *ssl #endif } +/* + * Helper functions for NamedGroup. + */ +static inline int mbedtls_ssl_named_group_is_ecdhe( uint16_t named_group ) +{ + /* + * RFC 4492 section 5.1.1 + */ + return( named_group >= MBEDTLS_SSL_IANA_TLS_GROUP_SECT163K1 && + named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 ); +} + +static inline int mbedtls_ssl_tls13_named_group_is_ecdhe( uint16_t named_group ) +{ + return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP384R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X448 ); +} + +static inline int mbedtls_ssl_tls13_named_group_is_dhe( uint16_t named_group ) +{ + return( named_group >= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE2048 && + named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE8192 ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ); + +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d868e49650..11d3c4136d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7189,4 +7189,137 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, } #endif /* !MBEDTLS_USE_PSA_CRYPTO */ +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +/* + * Functions for writing supported_groups extension. + * + * Stucture of supported_groups from RFC8446: + * enum { + * secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019), + * x25519(0x001D), x448(0x001E), + * ffdhe2048(0x0100), ffdhe3072(0x0101), ffdhe4096(0x0102), + * ffdhe6144(0x0103), ffdhe8192(0x0104), + * ffdhe_private_use(0x01FC..0x01FF), + * ecdhe_private_use(0xFE00..0xFEFF), + * (0xFFFF) + * } NamedGroup; + * struct { + * NamedGroup named_group_list<2..2^16-1>; + * } NamedGroupList; + */ + +#define SSL_GROUP_IS_UNSUPPORTED 0 +#define SSL_GROUP_IS_ECDHE 1 +#define SSL_GROUP_IS_DHE 2 +static int ssl_check_group_type( const mbedtls_ssl_config *conf, + const uint16_t group ) +{ +#if defined(MBEDTLS_ECDH_C) +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( mbedtls_ssl_conf_is_tls12_only( conf ) + && mbedtls_ssl_named_group_is_ecdhe( group ) ) + return( SSL_GROUP_IS_ECDHE ); +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( mbedtls_ssl_conf_is_tls13_only( conf ) + && mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) + return( SSL_GROUP_IS_ECDHE ); +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + +#endif /* MBEDTLS_ECDH_C */ + if( mbedtls_ssl_tls13_named_group_is_dhe( group ) ) + return( SSL_GROUP_IS_DHE ); + + return( SSL_GROUP_IS_UNSUPPORTED ); +} + +int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + unsigned char *p = buf ; + unsigned char *named_group_list; /* Start of named_group_list */ + size_t named_group_list_len; /* Length of named_group_list */ + const uint16_t *group_list = mbedtls_ssl_get_groups( ssl ); + + *out_len = 0; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( mbedtls_ssl_conf_is_tls13_only( ssl->conf ) + && !mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + return( 0 ); +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_groups extension" ) ); + + /* Check if we have space for header and length fields: + * - extension_type (2 bytes) + * - extension_data_length (2 bytes) + * - named_group_list_length (2 bytes) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 ); + p += 6; + + named_group_list = p; + + if( group_list == NULL ) + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + + for ( ; *group_list != 0; group_list++ ) + { + int group_type = ssl_check_group_type( ssl->conf, *group_list ); + if( group_type == SSL_GROUP_IS_UNSUPPORTED ) + continue; +#if defined(MBEDTLS_ECDH_C) + if( group_type == SSL_GROUP_IS_ECDHE ) + { + const mbedtls_ecp_curve_info *curve_info; + curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); + if( curve_info == NULL ) + continue; + MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", + curve_info->name, *group_list ) ); + } +#endif /* MBEDTLS_ECDH_C */ + + if( group_type == SSL_GROUP_IS_DHE ) + { + /* Implement DHE group here. */ + continue; + } + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); + MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); + p += 2; + + } + + /* Length of named_group_list*/ + named_group_list_len = p - named_group_list; + if( named_group_list_len == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "No group available." ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + /* Write extension_type */ + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_GROUPS, buf, 0 ); + /* Write extension_data_length */ + MBEDTLS_PUT_UINT16_BE( named_group_list_len + 2, buf, 2 ); + /* Write length of named_group_list */ + MBEDTLS_PUT_UINT16_BE( named_group_list_len, buf, 4 ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "Supported groups extension", buf + 4, named_group_list_len + 2 ); + + *out_len = p - buf; + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SUPPORTED_GROUPS; +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + + return( 0 ); +} + +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + #endif /* MBEDTLS_SSL_TLS_C */ From 7581c11fc76a9001b2f26d5ba39d9937663cd206 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Dec 2021 22:25:41 +0800 Subject: [PATCH 03/16] Remove tls13_write_supported_groups_ext Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 159 +------------------------------------ 1 file changed, 1 insertion(+), 158 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 31d7dafdb9..5b6aee1f83 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -115,163 +115,6 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -/* - * Functions for writing supported_groups extension. - * - * Stucture of supported_groups: - * enum { - * secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019), - * x25519(0x001D), x448(0x001E), - * ffdhe2048(0x0100), ffdhe3072(0x0101), ffdhe4096(0x0102), - * ffdhe6144(0x0103), ffdhe8192(0x0104), - * ffdhe_private_use(0x01FC..0x01FF), - * ecdhe_private_use(0xFE00..0xFEFF), - * (0xFFFF) - * } NamedGroup; - * struct { - * NamedGroup named_group_list<2..2^16-1>; - * } NamedGroupList; - */ -#if defined(MBEDTLS_ECDH_C) -/* - * In versions of TLS prior to TLS 1.3, this extension was named - * 'elliptic_curves' and only contained elliptic curve groups. - */ -static int ssl_tls13_write_named_group_list_ecdhe( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - unsigned char *p = buf; - - *out_len = 0; - - const uint16_t *group_list = mbedtls_ssl_get_groups( ssl ); - - if( group_list == NULL ) - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - - for ( ; *group_list != 0; group_list++ ) - { - const mbedtls_ecp_curve_info *curve_info; - curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); - if( curve_info == NULL ) - continue; - - if( !mbedtls_ssl_tls13_named_group_is_ecdhe( *group_list ) ) - continue; - - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); - MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); - p += 2; - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", - curve_info->name, *group_list ) ); - } - - *out_len = p - buf; - - return( 0 ); -} -#else -static int ssl_tls13_write_named_group_list_ecdhe( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - ((void) ssl); - ((void) buf); - ((void) end); - *out_len = 0; - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); -} -#endif /* MBEDTLS_ECDH_C */ - -static int ssl_tls13_write_named_group_list_dhe( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - ((void) ssl); - ((void) buf); - ((void) end); - *out_len = 0; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "write_named_group_dhe is not implemented" ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); -} - -static int ssl_tls13_write_supported_groups_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - unsigned char *p = buf ; - unsigned char *named_group_list; /* Start of named_group_list */ - size_t named_group_list_len; /* Length of named_group_list */ - size_t output_len = 0; - int ret_ecdhe, ret_dhe; - - *out_len = 0; - - if( !mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - return( 0 ); - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_groups extension" ) ); - - /* Check if we have space for header and length fields: - * - extension_type (2 bytes) - * - extension_data_length (2 bytes) - * - named_group_list_length (2 bytes) - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 ); - p += 6; - - named_group_list = p; - ret_ecdhe = ssl_tls13_write_named_group_list_ecdhe( ssl, p, end, &output_len ); - if( ret_ecdhe != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_named_group_list_ecdhe", ret_ecdhe ); - } - p += output_len; - - ret_dhe = ssl_tls13_write_named_group_list_dhe( ssl, p, end, &output_len ); - if( ret_dhe != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_named_group_list_dhe", ret_dhe ); - } - p += output_len; - - /* Both ECDHE and DHE failed. */ - if( ret_ecdhe != 0 && ret_dhe != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Both ECDHE and DHE groups are fail. " ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* Length of named_group_list*/ - named_group_list_len = p - named_group_list; - if( named_group_list_len == 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "No group available." ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* Write extension_type */ - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_GROUPS, buf, 0 ); - /* Write extension_data_length */ - MBEDTLS_PUT_UINT16_BE( named_group_list_len + 2, buf, 2 ); - /* Write length of named_group_list */ - MBEDTLS_PUT_UINT16_BE( named_group_list_len, buf, 4 ); - - MBEDTLS_SSL_DEBUG_BUF( 3, "Supported groups extension", buf + 4, named_group_list_len + 2 ); - - *out_len = p - buf; - - ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SUPPORTED_GROUPS; - - return( 0 ); -} - /* * Functions for writing key_share extension. */ @@ -777,7 +620,7 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, * * It is REQUIRED for ECDHE cipher_suites. */ - ret = ssl_tls13_write_supported_groups_ext( ssl, p, end, &output_len ); + ret = mbedtls_ssl_write_supported_groups_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; From 9d555ac003e301ee603a4f86a69008acbbdb0881 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Dec 2021 22:27:58 +0800 Subject: [PATCH 04/16] Remove TLS12 version write_supported_group_ext Signed-off-by: Jerry Yu --- library/ssl_cli.c | 66 ++--------------------------------------------- 1 file changed, 2 insertions(+), 64 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 32befc24c7..888523f183 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -302,68 +302,6 @@ static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) -static int ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - const unsigned char *end, - size_t *olen ) -{ - unsigned char *p = buf; - unsigned char *elliptic_curve_list = p + 6; - size_t elliptic_curve_len = 0; - const mbedtls_ecp_curve_info *info; - const uint16_t *group_list = mbedtls_ssl_get_groups( ssl ); - *olen = 0; - - /* Check there is room for header */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 ); - - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "client hello, adding supported_elliptic_curves extension" ) ); - - if( group_list == NULL ) - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - - for( ; *group_list != 0; group_list++ ) - { - info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); - if( info == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "invalid curve in ssl configuration" ) ); - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - } - - /* Check there is room for another curve */ - MBEDTLS_SSL_CHK_BUF_PTR( elliptic_curve_list, end, elliptic_curve_len + 2 ); - - MBEDTLS_PUT_UINT16_BE( *group_list, elliptic_curve_list, elliptic_curve_len ); - elliptic_curve_len += 2; - - if( elliptic_curve_len > MBEDTLS_SSL_MAX_CURVE_LIST_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "malformed supported_elliptic_curves extension in config" ) ); - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - } - } - - /* Empty elliptic curve list, this is a configuration error. */ - if( elliptic_curve_len == 0 ) - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_GROUPS, p, 0 ); - p += 2; - - MBEDTLS_PUT_UINT16_BE( elliptic_curve_len + 2, p, 0 ); - p += 2; - - MBEDTLS_PUT_UINT16_BE( elliptic_curve_len, p, 0 ); - p += 2; - - *olen = 6 + elliptic_curve_len; - - return( 0 ); -} static int ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -1206,8 +1144,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if( uses_ec ) { - if( ( ret = ssl_write_supported_groups_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_supported_groups_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_groups_ext", ret ); return( ret ); From 175326108316d93656e3b34cc5fc36f6bf8ab974 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 20 Dec 2021 22:32:09 +0800 Subject: [PATCH 05/16] change write_supported_groups_ext prototype Signed-off-by: Jerry Yu --- library/ssl_misc.h | 2 +- library/ssl_tls.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index b74b0d9427..598c3efc52 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1756,7 +1756,7 @@ static inline int mbedtls_ssl_tls13_named_group_is_dhe( uint16_t named_group ) #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, unsigned char *buf, - unsigned char *end, + const unsigned char *end, size_t *out_len ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 11d3c4136d..bab83ea2c7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7236,7 +7236,7 @@ static int ssl_check_group_type( const mbedtls_ssl_config *conf, int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, unsigned char *buf, - unsigned char *end, + const unsigned char *end, size_t *out_len ) { unsigned char *p = buf ; From 1ea9d1068771af612ef7ee8467e6950fdabe40e8 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 21 Dec 2021 13:41:49 +0800 Subject: [PATCH 06/16] fix test_ref_configs build fail Signed-off-by: Jerry Yu --- library/ssl_misc.h | 8 ++++++-- library/ssl_tls.c | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 598c3efc52..cea8ab53af 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1753,12 +1753,16 @@ static inline int mbedtls_ssl_tls13_named_group_is_dhe( uint16_t named_group ) named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE8192 ); } -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) || \ + defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED || + MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C || + MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bab83ea2c7..e2ad47b8cd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7189,7 +7189,9 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, } #endif /* !MBEDTLS_USE_PSA_CRYPTO */ -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) || \ + defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) /* * Functions for writing supported_groups extension. * @@ -7214,7 +7216,7 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, static int ssl_check_group_type( const mbedtls_ssl_config *conf, const uint16_t group ) { -#if defined(MBEDTLS_ECDH_C) +#if defined(MBEDTLS_ECP_C) #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( mbedtls_ssl_conf_is_tls12_only( conf ) && mbedtls_ssl_named_group_is_ecdhe( group ) ) @@ -7226,8 +7228,10 @@ static int ssl_check_group_type( const mbedtls_ssl_config *conf, && mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) return( SSL_GROUP_IS_ECDHE ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ - +#else + ((void) conf); #endif /* MBEDTLS_ECDH_C */ + if( mbedtls_ssl_tls13_named_group_is_dhe( group ) ) return( SSL_GROUP_IS_DHE ); @@ -7267,10 +7271,12 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, for ( ; *group_list != 0; group_list++ ) { + MBEDTLS_SSL_DEBUG_MSG( 1, ("got supported group(%04x)",*group_list)); int group_type = ssl_check_group_type( ssl->conf, *group_list ); if( group_type == SSL_GROUP_IS_UNSUPPORTED ) continue; -#if defined(MBEDTLS_ECDH_C) + MBEDTLS_SSL_DEBUG_MSG( 1, ("add supported group(%04x)",*group_list)); +#if defined(MBEDTLS_ECP_C) if( group_type == SSL_GROUP_IS_ECDHE ) { const mbedtls_ecp_curve_info *curve_info; @@ -7320,6 +7326,8 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, return( 0 ); } -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED || + MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C || + MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #endif /* MBEDTLS_SSL_TLS_C */ From 136320ba0b581970b99f2d73e8e5d248514de3f7 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 21 Dec 2021 17:09:00 +0800 Subject: [PATCH 07/16] fix ci fail Signed-off-by: Jerry Yu --- library/ssl_misc.h | 2 +- tests/ssl-opt.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index cea8ab53af..f854ef1dfc 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1735,7 +1735,7 @@ static inline int mbedtls_ssl_named_group_is_ecdhe( uint16_t named_group ) * RFC 4492 section 5.1.1 */ return( named_group >= MBEDTLS_SSL_IANA_TLS_GROUP_SECT163K1 && - named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 ); + named_group < MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE2048 ); } static inline int mbedtls_ssl_tls13_named_group_is_ecdhe( uint16_t named_group ) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 691c0e7d5b..b060e7b462 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6585,7 +6585,7 @@ run_test "Force a non ECC ciphersuite in the client side" \ "$P_SRV debug_level=3" \ "$P_CLI debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA256" \ 0 \ - -C "client hello, adding supported_elliptic_curves extension" \ + -C "client hello, adding supported_groups extension" \ -C "client hello, adding supported_point_formats extension" \ -S "found supported elliptic curves extension" \ -S "found supported point formats extension" @@ -6609,7 +6609,7 @@ run_test "Force an ECC ciphersuite in the client side" \ "$P_SRV debug_level=3" \ "$P_CLI debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ 0 \ - -c "client hello, adding supported_elliptic_curves extension" \ + -c "client hello, adding supported_groups extension" \ -c "client hello, adding supported_point_formats extension" \ -s "found supported elliptic curves extension" \ -s "found supported point formats extension" From ffef9c52d462458fd206854ab6af0073572f5505 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 24 Dec 2021 22:31:08 +0800 Subject: [PATCH 08/16] fix alignment issue Signed-off-by: Jerry Yu --- library/ssl_srv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 977ecc8f33..3794dada4e 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -318,8 +318,8 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) static int ssl_parse_supported_groups_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t len ) + const unsigned char *buf, + size_t len ) { size_t list_size, our_size; const unsigned char *p; From 7f029d8a9494283f8df9bb7013e0ebf51af204dc Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 11 Jan 2022 11:08:53 +0800 Subject: [PATCH 09/16] fix coding style issues Signed-off-by: Jerry Yu --- library/ssl_tls.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e2ad47b8cd..bb533c957a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7294,7 +7294,7 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, continue; } - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); p += 2; @@ -7315,7 +7315,8 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, /* Write length of named_group_list */ MBEDTLS_PUT_UINT16_BE( named_group_list_len, buf, 4 ); - MBEDTLS_SSL_DEBUG_BUF( 3, "Supported groups extension", buf + 4, named_group_list_len + 2 ); + MBEDTLS_SSL_DEBUG_BUF( 3, "Supported groups extension", + buf + 4, named_group_list_len + 2 ); *out_len = p - buf; From 63282b43210291931dc14341c94e198a54079c4d Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 11 Jan 2022 15:43:53 +0800 Subject: [PATCH 10/16] Refactor write supported group Signed-off-by: Jerry Yu --- library/ssl_tls.c | 76 +++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 48 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bb533c957a..6ca63ccb08 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7208,36 +7208,26 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, * struct { * NamedGroup named_group_list<2..2^16-1>; * } NamedGroupList; + * From RFC8422: + * enum { + * deprecated(1..22), + * secp256r1 (23), secp384r1 (24), secp521r1 (25), + * x25519(29), x448(30), + * reserved (0xFE00..0xFEFF), + * deprecated(0xFF01..0xFF02), + * (0xFFFF) + * } NamedCurve; + * struct { + * NamedCurve named_curve_list<2..2^16-1> + * } NamedCurveList; + * + * RFC8422 and RFC8446 share simillar structure and same extension id. The function + * only check if the curve/group is supported by library. It does not need check + * the IANA values. ECP module should check if the group/curve is deprecated or + * supported. + * + * DHE groups hasn't been supported yet. */ - -#define SSL_GROUP_IS_UNSUPPORTED 0 -#define SSL_GROUP_IS_ECDHE 1 -#define SSL_GROUP_IS_DHE 2 -static int ssl_check_group_type( const mbedtls_ssl_config *conf, - const uint16_t group ) -{ -#if defined(MBEDTLS_ECP_C) -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( mbedtls_ssl_conf_is_tls12_only( conf ) - && mbedtls_ssl_named_group_is_ecdhe( group ) ) - return( SSL_GROUP_IS_ECDHE ); -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( mbedtls_ssl_conf_is_tls13_only( conf ) - && mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) - return( SSL_GROUP_IS_ECDHE ); -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ -#else - ((void) conf); -#endif /* MBEDTLS_ECDH_C */ - - if( mbedtls_ssl_tls13_named_group_is_dhe( group ) ) - return( SSL_GROUP_IS_DHE ); - - return( SSL_GROUP_IS_UNSUPPORTED ); -} - int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -7272,31 +7262,21 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, for ( ; *group_list != 0; group_list++ ) { MBEDTLS_SSL_DEBUG_MSG( 1, ("got supported group(%04x)",*group_list)); - int group_type = ssl_check_group_type( ssl->conf, *group_list ); - if( group_type == SSL_GROUP_IS_UNSUPPORTED ) - continue; - MBEDTLS_SSL_DEBUG_MSG( 1, ("add supported group(%04x)",*group_list)); + #if defined(MBEDTLS_ECP_C) - if( group_type == SSL_GROUP_IS_ECDHE ) + const mbedtls_ecp_curve_info *curve_info; + curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); + if( curve_info != NULL ) { - const mbedtls_ecp_curve_info *curve_info; - curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); - if( curve_info == NULL ) - continue; + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); + p += 2; MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", curve_info->name, *group_list ) ); - } -#endif /* MBEDTLS_ECDH_C */ - - if( group_type == SSL_GROUP_IS_DHE ) - { - /* Implement DHE group here. */ continue; } - - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); - p += 2; +#endif /* MBEDTLS_ECP_C */ + /* Add DHE groups here */ } From f46b016058939d4cb131f40c7ef804ac4c0825f4 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 11 Jan 2022 16:28:00 +0800 Subject: [PATCH 11/16] skip some extensions if ephemeral not enabled Signed-off-by: Jerry Yu --- library/ssl_tls.c | 6 +--- library/ssl_tls13_client.c | 63 +++++++++++++++++++------------------ library/ssl_tls13_generic.c | 10 ------ 3 files changed, 33 insertions(+), 46 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6ca63ccb08..69fa39b466 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7239,11 +7239,7 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, const uint16_t *group_list = mbedtls_ssl_get_groups( ssl ); *out_len = 0; -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( mbedtls_ssl_conf_is_tls13_only( ssl->conf ) - && !mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - return( 0 ); -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_groups extension" ) ); /* Check if we have space for header and length fields: diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5b6aee1f83..9541fc33b5 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -219,9 +219,6 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, *out_len = 0; - if( !mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - return( 0 ); - /* Check if we have space for header and length fields: * - extension_type (2 bytes) * - extension_data_length (2 bytes) @@ -620,36 +617,40 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, * * It is REQUIRED for ECDHE cipher_suites. */ - ret = mbedtls_ssl_write_supported_groups_ext( ssl, p, end, &output_len ); - if( ret != 0 ) - return( ret ); - p += output_len; + /* Skip the extensions on the client if all allowed key exchanges + * are PSK-based. */ + if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + { + ret = mbedtls_ssl_write_supported_groups_ext( ssl, p, end, &output_len ); + if( ret != 0 ) + return( ret ); + p += output_len; - /* Write key_share extension - * - * We need to send the key shares under three conditions: - * 1) A certificate-based ciphersuite is being offered. In this case - * supported_groups and supported_signature extensions have been - * successfully added. - * 2) A PSK-based ciphersuite with ECDHE is offered. In this case the - * psk_key_exchange_modes has been added as the last extension. - * 3) Or, in case all ciphers are supported ( which includes #1 and #2 - * from above ) - */ - ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); - if( ret != 0 ) - return( ret ); - p += output_len; - - /* Write signature_algorithms extension - * - * It is REQUIRED for certificate authenticated cipher_suites. - */ - ret = mbedtls_ssl_tls13_write_sig_alg_ext( ssl, p, end, &output_len ); - if( ret != 0 ) - return( ret ); - p += output_len; + /* Write key_share extension + * + * We need to send the key shares under three conditions: + * 1) A certificate-based ciphersuite is being offered. In this case + * supported_groups and supported_signature extensions have been + * successfully added. + * 2) A PSK-based ciphersuite with ECDHE is offered. In this case the + * psk_key_exchange_modes has been added as the last extension. + * 3) Or, in case all ciphers are supported ( which includes #1 and #2 + * from above ) + */ + ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); + if( ret != 0 ) + return( ret ); + p += output_len; + /* Write signature_algorithms extension + * + * It is REQUIRED for certificate authenticated cipher_suites. + */ + ret = mbedtls_ssl_tls13_write_sig_alg_ext( ssl, p, end, &output_len ); + if( ret != 0 ) + return( ret ); + p += output_len; + } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 1260740e5d..c789ed41c7 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -165,16 +165,6 @@ int mbedtls_ssl_tls13_write_sig_alg_ext( mbedtls_ssl_context *ssl, *out_len = 0; - /* Skip the extension on the client if all allowed key exchanges - * are PSK-based. */ -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT && - !mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - { - return( 0 ); - } -#endif /* MBEDTLS_SSL_CLI_C */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding signature_algorithms extension" ) ); /* Check if we have space for header and length field: From 3ad14ac9e948382d5d947c40bc93ef7b5995da12 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 11 Jan 2022 17:13:16 +0800 Subject: [PATCH 12/16] Add named group IANA value check Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 1 - library/ssl_misc.h | 55 +++++++++++++++++++++++++++++++++++++++---- library/ssl_tls.c | 29 +++++++++++++---------- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 45b991b4ca..072ebbe460 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -191,7 +191,6 @@ /* Elliptic Curve Groups (ECDHE) */ #define MBEDTLS_SSL_IANA_TLS_GROUP_NONE 0 -#define MBEDTLS_SSL_IANA_TLS_GROUP_SECT163K1 0x0001 /* RFC 4492 */ #define MBEDTLS_SSL_IANA_TLS_GROUP_SECP192K1 0x0012 #define MBEDTLS_SSL_IANA_TLS_GROUP_SECP192R1 0x0013 #define MBEDTLS_SSL_IANA_TLS_GROUP_SECP224K1 0x0014 diff --git a/library/ssl_misc.h b/library/ssl_misc.h index f854ef1dfc..eb828a942b 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1489,6 +1489,7 @@ static inline int mbedtls_ssl_conf_is_tls13_only( const mbedtls_ssl_config *conf } return( 0 ); } + #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) @@ -1503,8 +1504,43 @@ static inline int mbedtls_ssl_conf_is_tls12_only( const mbedtls_ssl_config *conf } return( 0 ); } + #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +static inline int mbedtls_ssl_conf_is_tls13_enabled( const mbedtls_ssl_config *conf ) +{ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( conf->min_major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && + conf->max_major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && + conf->min_minor_ver <= MBEDTLS_SSL_MINOR_VERSION_4 && + conf->max_minor_ver >= MBEDTLS_SSL_MINOR_VERSION_4 ) + { + return( 1 ); + } + return( 0 ); +#else + ((void) conf); + return( 0 ); +#endif +} + +static inline int mbedtls_ssl_conf_is_tls12_enabled( const mbedtls_ssl_config *conf ) +{ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( conf->min_major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && + conf->max_major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && + conf->min_minor_ver <= MBEDTLS_SSL_MINOR_VERSION_3 && + conf->max_minor_ver >= MBEDTLS_SSL_MINOR_VERSION_3 ) + { + return( 1 ); + } + return( 0 ); +#else + ((void) conf); + return( 0 ); +#endif +} + #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) static inline int mbedtls_ssl_conf_is_hybrid_tls12_tls13( const mbedtls_ssl_config *conf ) { @@ -1729,13 +1765,24 @@ static inline const void *mbedtls_ssl_get_groups( const mbedtls_ssl_context *ssl /* * Helper functions for NamedGroup. */ -static inline int mbedtls_ssl_named_group_is_ecdhe( uint16_t named_group ) +static inline int mbedtls_ssl_tls12_named_group_is_ecdhe( uint16_t named_group ) { /* - * RFC 4492 section 5.1.1 + * RFC 8422 section 5.1.1 */ - return( named_group >= MBEDTLS_SSL_IANA_TLS_GROUP_SECT163K1 && - named_group < MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE2048 ); + return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP192K1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP192R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP224K1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP224R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256K1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP384R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP256R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP384R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP512R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X448 ); } static inline int mbedtls_ssl_tls13_named_group_is_ecdhe( uint16_t named_group ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 69fa39b466..844d5c3d9b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7221,10 +7221,7 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, * NamedCurve named_curve_list<2..2^16-1> * } NamedCurveList; * - * RFC8422 and RFC8446 share simillar structure and same extension id. The function - * only check if the curve/group is supported by library. It does not need check - * the IANA values. ECP module should check if the group/curve is deprecated or - * supported. + * RFC8422 and RFC8446 share simillar structure and same extension id. * * DHE groups hasn't been supported yet. */ @@ -7260,16 +7257,22 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ("got supported group(%04x)",*group_list)); #if defined(MBEDTLS_ECP_C) - const mbedtls_ecp_curve_info *curve_info; - curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); - if( curve_info != NULL ) + if( ( mbedtls_ssl_conf_is_tls13_enabled(ssl->conf) && + mbedtls_ssl_tls13_named_group_is_ecdhe( *group_list ) ) || + ( mbedtls_ssl_conf_is_tls12_enabled(ssl->conf) && + mbedtls_ssl_tls12_named_group_is_ecdhe( *group_list ) ) ) { - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); - p += 2; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", - curve_info->name, *group_list ) ); - continue; + const mbedtls_ecp_curve_info *curve_info; + curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); + if( curve_info != NULL ) + { + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); + p += 2; + MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", + curve_info->name, *group_list ) ); + continue; + } } #endif /* MBEDTLS_ECP_C */ /* Add DHE groups here */ From 1510cea0f32f745c540fdbd10beaa9da563eec12 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 12 Jan 2022 10:56:49 +0800 Subject: [PATCH 13/16] fix coding style issues Signed-off-by: Jerry Yu --- library/ssl_tls.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 844d5c3d9b..55c5e14e18 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7252,14 +7252,14 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, if( group_list == NULL ) return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - for ( ; *group_list != 0; group_list++ ) + for( ; *group_list != 0; group_list++ ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ("got supported group(%04x)",*group_list)); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "got supported group(%04x)", *group_list ) ); #if defined(MBEDTLS_ECP_C) - if( ( mbedtls_ssl_conf_is_tls13_enabled(ssl->conf) && + if( ( mbedtls_ssl_conf_is_tls13_enabled( ssl->conf ) && mbedtls_ssl_tls13_named_group_is_ecdhe( *group_list ) ) || - ( mbedtls_ssl_conf_is_tls12_enabled(ssl->conf) && + ( mbedtls_ssl_conf_is_tls12_enabled( ssl->conf ) && mbedtls_ssl_tls12_named_group_is_ecdhe( *group_list ) ) ) { const mbedtls_ecp_curve_info *curve_info; @@ -7279,7 +7279,7 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, } - /* Length of named_group_list*/ + /* Length of named_group_list */ named_group_list_len = p - named_group_list; if( named_group_list_len == 0 ) { From f0fede56a6b994d577a1f3b93d6a068cd619fa80 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 12 Jan 2022 10:57:47 +0800 Subject: [PATCH 14/16] minor performance improvement Signed-off-by: Jerry Yu --- library/ssl_misc.h | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index eb828a942b..515f684eec 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1770,27 +1770,28 @@ static inline int mbedtls_ssl_tls12_named_group_is_ecdhe( uint16_t named_group ) /* * RFC 8422 section 5.1.1 */ - return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP192K1 || + return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP256R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP384R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP512R1 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X448 || + /* Below deprected curves should be removed with notice to users */ + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP192K1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP192R1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP224K1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP224R1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256K1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP384R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP256R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP384R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_BP512R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X448 ); + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 ); } static inline int mbedtls_ssl_tls13_named_group_is_ecdhe( uint16_t named_group ) { - return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1 || + return( named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || + named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP384R1 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_SECP521R1 || - named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X25519 || named_group == MBEDTLS_SSL_IANA_TLS_GROUP_X448 ); } From b925f2180641f8931b5e356cedd21987a96f4f6b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 12 Jan 2022 11:17:02 +0800 Subject: [PATCH 15/16] fix comment issues Signed-off-by: Jerry Yu --- library/ssl_srv.c | 5 +++++ library/ssl_tls.c | 30 +++++++++++++++++------------- library/ssl_tls13_client.c | 24 ++++-------------------- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 3794dada4e..f5af5c7c6f 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -317,6 +317,11 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +/* + * The TLS 1.3 supported groups extension was defined to be a compatible + * generalization of the TLS 1.2 supported elliptic curves extension. They both + * share the same extension identifier. + */ static int ssl_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 55c5e14e18..76449032f9 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7193,9 +7193,11 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) /* - * Functions for writing supported_groups extension. + * Function for writing a supported groups (TLS 1.3) or supported elliptic + * curves (TLS 1.2) extension. * - * Stucture of supported_groups from RFC8446: + * The "extension_data" field of a supported groups extension contains a + * "NamedGroupList" value (TLS 1.3 RFC8446): * enum { * secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019), * x25519(0x001D), x448(0x001E), @@ -7208,7 +7210,9 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, * struct { * NamedGroup named_group_list<2..2^16-1>; * } NamedGroupList; - * From RFC8422: + * + * The "extension_data" field of a supported elliptic curves extension contains + * a "NamedCurveList" value (TLS 1.2 RFC 8422): * enum { * deprecated(1..22), * secp256r1 (23), secp384r1 (24), secp521r1 (25), @@ -7221,9 +7225,11 @@ int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, * NamedCurve named_curve_list<2..2^16-1> * } NamedCurveList; * - * RFC8422 and RFC8446 share simillar structure and same extension id. + * The TLS 1.3 supported groups extension was defined to be a compatible + * generalization of the TLS 1.2 supported elliptic curves extension. They both + * share the same extension identifier. * - * DHE groups hasn't been supported yet. + * DHE groups are not supported yet. */ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -7264,15 +7270,13 @@ int mbedtls_ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, { const mbedtls_ecp_curve_info *curve_info; curve_info = mbedtls_ecp_curve_info_from_tls_id( *group_list ); - if( curve_info != NULL ) - { - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); - p += 2; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", - curve_info->name, *group_list ) ); + if( curve_info == NULL ) continue; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( *group_list, p, 0 ); + p += 2; + MBEDTLS_SSL_DEBUG_MSG( 3, ( "NamedGroup: %s ( %x )", + curve_info->name, *group_list ) ); } #endif /* MBEDTLS_ECP_C */ /* Add DHE groups here */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 9541fc33b5..c6f8ce3d86 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -613,12 +613,11 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p += output_len; #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - /* Write supported_groups extension - * - * It is REQUIRED for ECDHE cipher_suites. + + /* + * Add the extensions related to (EC)DHE ephemeral key establishment only if + * enabled as per the configuration. */ - /* Skip the extensions on the client if all allowed key exchanges - * are PSK-based. */ if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { ret = mbedtls_ssl_write_supported_groups_ext( ssl, p, end, &output_len ); @@ -626,26 +625,11 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, return( ret ); p += output_len; - /* Write key_share extension - * - * We need to send the key shares under three conditions: - * 1) A certificate-based ciphersuite is being offered. In this case - * supported_groups and supported_signature extensions have been - * successfully added. - * 2) A PSK-based ciphersuite with ECDHE is offered. In this case the - * psk_key_exchange_modes has been added as the last extension. - * 3) Or, in case all ciphers are supported ( which includes #1 and #2 - * from above ) - */ ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; - /* Write signature_algorithms extension - * - * It is REQUIRED for certificate authenticated cipher_suites. - */ ret = mbedtls_ssl_tls13_write_sig_alg_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); From d491ea4f1841bc45566355fe587a7c3ca7a9f2ea Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 13 Jan 2022 16:15:25 +0800 Subject: [PATCH 16/16] fix comment issue Signed-off-by: Jerry Yu --- library/ssl_srv.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index f5af5c7c6f..a8b1e7de6c 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -318,9 +318,43 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) /* + * Function for parsing a supported groups (TLS 1.3) or supported elliptic + * curves (TLS 1.2) extension. + * + * The "extension_data" field of a supported groups extension contains a + * "NamedGroupList" value (TLS 1.3 RFC8446): + * enum { + * secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019), + * x25519(0x001D), x448(0x001E), + * ffdhe2048(0x0100), ffdhe3072(0x0101), ffdhe4096(0x0102), + * ffdhe6144(0x0103), ffdhe8192(0x0104), + * ffdhe_private_use(0x01FC..0x01FF), + * ecdhe_private_use(0xFE00..0xFEFF), + * (0xFFFF) + * } NamedGroup; + * struct { + * NamedGroup named_group_list<2..2^16-1>; + * } NamedGroupList; + * + * The "extension_data" field of a supported elliptic curves extension contains + * a "NamedCurveList" value (TLS 1.2 RFC 8422): + * enum { + * deprecated(1..22), + * secp256r1 (23), secp384r1 (24), secp521r1 (25), + * x25519(29), x448(30), + * reserved (0xFE00..0xFEFF), + * deprecated(0xFF01..0xFF02), + * (0xFFFF) + * } NamedCurve; + * struct { + * NamedCurve named_curve_list<2..2^16-1> + * } NamedCurveList; + * * The TLS 1.3 supported groups extension was defined to be a compatible * generalization of the TLS 1.2 supported elliptic curves extension. They both * share the same extension identifier. + * + * DHE groups are not supported yet. */ static int ssl_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf,