From 61e35e0047271167720935ecf458778c1bd9ffae Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 16 Sep 2021 18:59:08 +0800 Subject: [PATCH 01/23] tls13: add generate handshake keys Signed-off-by: Jerry Yu --- library/ssl_misc.h | 23 ++++++++ library/ssl_tls13_keys.c | 117 +++++++++++++++++++++++++++++++++++++++ library/ssl_tls13_keys.h | 36 +++++------- 3 files changed, 155 insertions(+), 21 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9041c51d2..b801499ca 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -508,6 +508,27 @@ struct mbedtls_ssl_key_set }; typedef struct mbedtls_ssl_key_set mbedtls_ssl_key_set; +typedef struct +{ + unsigned char binder_key [ MBEDTLS_MD_MAX_SIZE ]; + unsigned char client_early_traffic_secret [ MBEDTLS_MD_MAX_SIZE ]; + unsigned char early_exporter_master_secret[ MBEDTLS_MD_MAX_SIZE ]; +} mbedtls_ssl_tls1_3_early_secrets; + +typedef struct +{ + unsigned char client_handshake_traffic_secret[ MBEDTLS_MD_MAX_SIZE ]; + unsigned char server_handshake_traffic_secret[ MBEDTLS_MD_MAX_SIZE ]; +} mbedtls_ssl_tls1_3_handshake_secrets; + +typedef struct +{ + unsigned char client_application_traffic_secret_N[ MBEDTLS_MD_MAX_SIZE ]; + unsigned char server_application_traffic_secret_N[ MBEDTLS_MD_MAX_SIZE ]; + unsigned char exporter_master_secret [ MBEDTLS_MD_MAX_SIZE ]; + unsigned char resumption_master_secret [ MBEDTLS_MD_MAX_SIZE ]; +} mbedtls_ssl_tls1_3_application_secrets; + /* * This structure contains the parameters only needed during handshake. */ @@ -715,6 +736,8 @@ struct mbedtls_ssl_handshake_params unsigned char handshake[MBEDTLS_TLS1_3_MD_MAX_SIZE]; unsigned char app [MBEDTLS_TLS1_3_MD_MAX_SIZE]; } tls1_3_master_secrets; + + mbedtls_ssl_tls1_3_handshake_secrets tls1_3_hs_secrets; #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index b07c1c3b9..a20fa5150 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -846,4 +846,121 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ) return( 0 ); } +/* mbedtls_ssl_tls1_3_generate_handshake_keys() generates keys necessary for + * protecting the handshake messages, as described in Section 7 of TLS 1.3. */ +int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, + mbedtls_ssl_key_set *traffic_keys ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + mbedtls_md_type_t md_type; + mbedtls_md_info_t const *md_info; + size_t md_size; + + unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + size_t transcript_len; + + mbedtls_cipher_info_t const *cipher_info; + size_t keylen, ivlen; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls1_3_generate_handshake_keys" ) ); + + cipher_info = mbedtls_cipher_info_from_type( + ssl->handshake->ciphersuite_info->cipher ); + keylen = cipher_info->key_bitlen >> 3; + ivlen = cipher_info->iv_size; + + md_type = ssl->handshake->ciphersuite_info->mac; + md_info = mbedtls_md_info_from_type( md_type ); + md_size = mbedtls_md_get_size( md_info ); + + ret = mbedtls_ssl_get_handshake_transcript( ssl, md_type, + transcript, + sizeof( transcript ), + &transcript_len ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_get_handshake_transcript", + ret ); + return( ret ); + } + + ret = mbedtls_ssl_tls1_3_derive_handshake_secrets( md_type, + ssl->handshake->tls1_3_master_secrets.handshake, + transcript, transcript_len, + &ssl->handshake->tls1_3_hs_secrets ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_handshake_secrets", + ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_BUF( 4, "Client handshake traffic secret", + ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret, + md_size ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "Server handshake traffic secret", + ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret, + md_size ); + + /* + * Export client handshake traffic secret + */ +#if defined(MBEDTLS_SSL_EXPORT_KEYS) + if( ssl->f_export_keys != NULL ) + { + ssl->f_export_keys( ssl->p_export_keys, + MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_HANDSHAKE_TRAFFIC_SECRET, + ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret, + md_size, + ssl->handshake->randbytes + 32, + ssl->handshake->randbytes, + MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); + + ssl->f_export_keys( ssl->p_export_keys, + MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_HANDSHAKE_TRAFFIC_SECRET, + ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret, + md_size, + ssl->handshake->randbytes + 32, + ssl->handshake->randbytes, + MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); + } +#endif /* MBEDTLS_SSL_EXPORT_KEYS */ + + ret = mbedtls_ssl_tls1_3_make_traffic_keys( md_type, + ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret, + ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret, + md_size, + keylen, ivlen, traffic_keys ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_make_traffic_keys", ret ); + goto exit; + } + + MBEDTLS_SSL_DEBUG_BUF( 4, "client_handshake write_key", + traffic_keys->client_write_key, + traffic_keys->key_len); + + MBEDTLS_SSL_DEBUG_BUF( 4, "server_handshake write_key", + traffic_keys->server_write_key, + traffic_keys->key_len); + + MBEDTLS_SSL_DEBUG_BUF( 4, "client_handshake write_iv", + traffic_keys->client_write_iv, + traffic_keys->iv_len); + + MBEDTLS_SSL_DEBUG_BUF( 4, "server_handshake write_iv", + traffic_keys->server_write_iv, + traffic_keys->iv_len); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_generate_handshake_keys" ) ); + +exit: + + return( ret ); +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 866aae911..602d06def 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -70,27 +70,6 @@ extern const struct mbedtls_ssl_tls1_3_labels_struct mbedtls_ssl_tls1_3_labels; #define MBEDTLS_SSL_TLS1_3_KEY_SCHEDULE_MAX_CONTEXT_LEN \ MBEDTLS_MD_MAX_SIZE -typedef struct -{ - unsigned char binder_key [ MBEDTLS_MD_MAX_SIZE ]; - unsigned char client_early_traffic_secret [ MBEDTLS_MD_MAX_SIZE ]; - unsigned char early_exporter_master_secret[ MBEDTLS_MD_MAX_SIZE ]; -} mbedtls_ssl_tls1_3_early_secrets; - -typedef struct -{ - unsigned char client_handshake_traffic_secret[ MBEDTLS_MD_MAX_SIZE ]; - unsigned char server_handshake_traffic_secret[ MBEDTLS_MD_MAX_SIZE ]; -} mbedtls_ssl_tls1_3_handshake_secrets; - -typedef struct -{ - unsigned char client_application_traffic_secret_N[ MBEDTLS_MD_MAX_SIZE ]; - unsigned char server_application_traffic_secret_N[ MBEDTLS_MD_MAX_SIZE ]; - unsigned char exporter_master_secret [ MBEDTLS_MD_MAX_SIZE ]; - unsigned char resumption_master_secret [ MBEDTLS_MD_MAX_SIZE ]; -} mbedtls_ssl_tls1_3_application_secrets; - /* Maximum desired length for expanded key material generated * by HKDF-Expand-Label. * @@ -553,4 +532,19 @@ int mbedtls_ssl_tls13_populate_transform( mbedtls_ssl_transform *transform, */ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ); +/** + * \brief Compute TLS 1.3 handshake traffic keys. + * + * \param ssl The SSL context to operate on. This must be in + * key schedule stage \c Handshake, see + * mbedtls_ssl_tls13_key_schedule_stage_handshake(). + * \param traffic_keys The address at which to store the handshake traffic key + * keys. This must be writable but may be uninitialized. + * + * \returns \c 0 on success. + * \returns A negative error code on failure. + */ +int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, + mbedtls_ssl_key_set *traffic_keys ); + #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ From a0650ebb9d99204d1e14183ee2da0893ade4efab Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 9 Sep 2021 17:14:45 +0800 Subject: [PATCH 02/23] tls13: add handshake key schedule Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 92 ++++++++++++++++++++++++++++++++++++++++ library/ssl_tls13_keys.h | 20 ++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index a20fa5150..165cf4cbe 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -963,4 +963,96 @@ exit: return( ret ); } +static int ssl_tls1_3_complete_ephemeral_secret( mbedtls_ssl_context *ssl, + unsigned char *secret, + size_t secret_len, + unsigned char **actual_secret, + size_t *actual_len ) +{ + int ret = 0; + + *actual_secret = NULL; + *actual_len = 0; + /* + * Compute ECDHE secret for second stage of secret evolution. + */ +#if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) + if( mbedtls_ssl_tls1_3_some_ephemeral_enabled( ssl ) ) + { + if( mbedtls_ssl_tls13_named_group_is_ecdhe( + ssl->handshake->offered_group_id ) ) + { +#if defined(MBEDTLS_ECDH_C) + ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, + actual_len, secret, secret_len, + ssl->conf->f_rng, + ssl->conf->p_rng ); + + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( ret ); + } + + *actual_secret = secret; +#endif /* MBEDTLS_ECDH_C */ + } + else if( mbedtls_ssl_tls13_named_group_is_dhe( + ssl->handshake->offered_group_id ) ) + { + /* TODO: Not supported yet */ + } + } +#else + ((void) ssl); + ((void) secret); + ((void) secret_len); +#endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED */ + + return( ret ); +} + +int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + mbedtls_md_type_t const md_type = ssl->handshake->ciphersuite_info->mac; +#if defined(MBEDTLS_DEBUG_C) + mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); + size_t const md_size = mbedtls_md_get_size( md_info ); +#endif /* MBEDTLS_DEBUG_C */ + + unsigned char *ephemeral; + size_t ephemeral_len; + + unsigned char ecdhe[66]; /* TODO: Magic constant! */ + + /* Finalize calculation of ephemeral input to key schedule, if present. */ + ret = ssl_tls1_3_complete_ephemeral_secret( ssl, ecdhe, sizeof( ecdhe ), + &ephemeral, &ephemeral_len ); + if( ret != 0 ) + return( ret ); + + /* + * Compute HandshakeSecret + */ + + ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, + ssl->handshake->tls1_3_master_secrets.early, + ephemeral, ephemeral_len, + ssl->handshake->tls1_3_master_secrets.handshake ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_evolve_secret", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_BUF( 4, "Handshake secret", + ssl->handshake->tls1_3_master_secrets.handshake, md_size ); + +#if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) + mbedtls_platform_zeroize( ecdhe, sizeof( ecdhe ) ); +#endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED */ + return( 0 ); +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 602d06def..536d976cd 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -532,12 +532,30 @@ int mbedtls_ssl_tls13_populate_transform( mbedtls_ssl_transform *transform, */ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ); +/** + * \brief Transition into handshake stage of TLS 1.3 key schedule. + * + * The TLS 1.3 key schedule can be viewed as a simple state machine + * with states Initial -> Early -> Handshake -> Application, and + * this function represents the Early -> Handshake transition. + * + * In the handshake stage, mbedtls_ssl_tls1_3_generate_handshake_keys() + * can be used to derive the handshake traffic keys. + * + * \param ssl The SSL context to operate on. This must be in key schedule + * stage \c Early. + * + * \returns \c 0 on success. + * \returns A negative error code on failure. + */ +int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); + /** * \brief Compute TLS 1.3 handshake traffic keys. * * \param ssl The SSL context to operate on. This must be in * key schedule stage \c Handshake, see - * mbedtls_ssl_tls13_key_schedule_stage_handshake(). + * mbedtls_ssl_tls1_3_key_schedule_stage_handshake(). * \param traffic_keys The address at which to store the handshake traffic key * keys. This must be writable but may be uninitialized. * From 1efa815db78808b70c7e0856a8ebb944376d82a2 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 9 Sep 2021 15:42:32 +0800 Subject: [PATCH 03/23] tls13: add ecdh_read_public Signed-off-by: Jerry Yu --- library/ecdh.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ library/ecdh_misc.h | 6 +++++ 2 files changed, 64 insertions(+) diff --git a/library/ecdh.c b/library/ecdh.c index b72bd1fe0..0067e0bd4 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -806,6 +806,64 @@ int mbedtls_ecdh_setup_no_everest( mbedtls_ecdh_context *ctx, #endif } +static int ecdh_tls1_3_read_public_internal( mbedtls_ecdh_context_mbed *ctx, + const unsigned char *buf, + size_t blen ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const unsigned char *p = buf; + const unsigned char *end = buf + blen; + size_t data_len; + + if( end - p < 3 ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + + data_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + if( data_len < 1 || data_len != ( blen - 2 ) ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + + /* + * Save buffer start for read_binary and update buf + */ + if( ( ret = mbedtls_ecp_point_read_binary( &ctx->grp, + &ctx->Qp, p, data_len ) ) != 0) + { + return( ret ); + } + + return( 0 ); +} + +/* + * Parse and import the client's TLS 1.3 public value + */ +int mbedtls_ecdh_tls1_3_read_public( mbedtls_ecdh_context *ctx, + const unsigned char *buf, + size_t blen ) +{ + ECDH_VALIDATE_RET( ctx != NULL ); + ECDH_VALIDATE_RET( buf != NULL ); + +#if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) + return( ecdh_tls1_3_read_public_internal( ctx, buf, blen ) ); +#else + switch( ctx->var ) + { +#if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) + case MBEDTLS_ECDH_VARIANT_EVEREST: + return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); +#endif + case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0: + return( ecdh_tls1_3_read_public_internal( &ctx->ctx.mbed_ecdh, + buf, blen ) ); + default: + return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; + } +#endif +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_ECDH_C */ diff --git a/library/ecdh_misc.h b/library/ecdh_misc.h index d1342f8b9..94d31394b 100644 --- a/library/ecdh_misc.h +++ b/library/ecdh_misc.h @@ -43,6 +43,12 @@ int mbedtls_ecdh_tls13_make_params( mbedtls_ecdh_context *ctx, size_t *olen, int ( *f_rng )( void *, unsigned char *, size_t ), void *p_rng ); +/* + * TLS 1.3 version of mbedtls_ecdh_read_public in ecdh.h + */ +int mbedtls_ecdh_tls1_3_read_public( mbedtls_ecdh_context *ctx, + const unsigned char *buf, + size_t blen ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ From e1b9c297b9c9ce845a1dd325d9cfd69fa5f12465 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 10 Sep 2021 10:08:31 +0800 Subject: [PATCH 04/23] Add read_server_hello Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 584 ++++++++++++++++++++++++++++++++++++- 1 file changed, 581 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 633bb8da2..054a45d7f 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -29,11 +29,14 @@ #include "mbedtls/debug.h" #include "mbedtls/error.h" +#include "mbedtls/platform.h" #include "ssl_misc.h" #include "ecdh_misc.h" +#include "ssl_tls13_keys.h" #define CLIENT_HELLO_RANDOM_LEN 32 +#define SERVER_HELLO_RANDOM_LEN 32 /* Write extensions */ @@ -92,6 +95,36 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, return( 0 ); } +static int ssl_tls1_3_parse_supported_versions_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t len ) +{ + /* TODO: Implement full version and remove force version set in + * ssl_tls_parse_server_hello. + * + * From page 40,RFC 8446 + * If supported_versions extension is present, clients MUST ignore the + * ServerHello.legacy_version value and MUST use only the + * "supported_versions" extension to determine the selected version. If + * the "supported_versions" extension in the ServerHello contains a + * version not offered by the client or contains a version prior to + * TLS 1.3, the client MUST abort the handshake with an + * "illegal_parameter" alert. + */ + + ((void) ssl); + + if( len != 2 || + buf[0] != MBEDTLS_SSL_MAJOR_VERSION_3 || + buf[1] != MBEDTLS_SSL_MINOR_VERSION_4 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "unexpected version" ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + return( 0 ); +} + #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* @@ -452,6 +485,125 @@ cleanup: return( ret ); } +#if defined(MBEDTLS_ECDH_C) + +/* TODO: Code for MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED missing */ +static int ssl_tls1_3_check_ecdh_params( const mbedtls_ssl_context *ssl ) +{ + const mbedtls_ecp_curve_info *curve_info; + mbedtls_ecp_group_id grp_id; +#if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) + grp_id = ssl->handshake->ecdh_ctx.grp.id; +#else + grp_id = ssl->handshake->ecdh_ctx.grp_id; +#endif + + curve_info = mbedtls_ecp_curve_info_from_grp_id( grp_id ); + if( curve_info == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + 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, + MBEDTLS_DEBUG_ECDH_QP ); + + return( 0 ); +} + +/* The ssl_tls1_3_parse_key_share_ext() function is used + * by the client to parse a KeyShare extension in + * a ServerHello message. + * + * The server only provides a single KeyShareEntry. + */ +static int ssl_tls1_3_read_public_ecdhe_share( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + ret = mbedtls_ecdh_tls1_3_read_public( &ssl->handshake->ecdh_ctx, + buf, len ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ecdh_tls13_read_public" ), ret ); + return( ret ); + } + + if( ssl_tls1_3_check_ecdh_params( ssl ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_tls1_3_check_ecdh_params() failed!" ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + return( 0 ); +} +#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ + +/* + * Parse key_share extension in Server Hello + * struct { + * KeyShareEntry server_share; + * } KeyShareServerHello; + * struct { + * NamedGroup group; + * opaque key_exchange<1..2^16-1>; + * } KeyShareEntry; + */ +static int ssl_tls1_3_parse_key_share_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret = 0; + const unsigned char *p = buf; + uint16_t server_share_group, offered_group; + + /* server_share_group (2 bytes) */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); + server_share_group = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + /* Check that chosen group matches the one we offered. */ + offered_group = ssl->handshake->offered_group_id; + if( offered_group != server_share_group ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "Invalid server key share, our group %u, their group %u", + (unsigned) offered_group, (unsigned) server_share_group ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + +#if defined(MBEDTLS_ECDH_C) + if( mbedtls_ssl_tls13_named_group_is_ecdhe( server_share_group ) ) + { + /* Complete ECDHE key agreement */ + ret = ssl_tls1_3_read_public_ecdhe_share( ssl, p, end - p ); + if( ret != 0 ) + return( ret ); + } +#endif /* MBEDTLS_ECDH_C */ + else if( 0 /* other KEMs? */ ) + { + /* Do something */ + } + else + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_KEY_SHARE; + return( ret ); +} + #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* Write cipher_suites @@ -738,13 +890,439 @@ cleanup: } /* + * Functions for parsing and processing ServerHello + */ +static int ssl_server_hello_is_hrr( unsigned const char *buf ) +{ + const unsigned char magic_hrr_string[32] = + { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, + 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, + 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, + 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33 ,0x9C }; + + /* Check whether this message is a HelloRetryRequest ( HRR ) message. + * + * ServerHello and HRR are only distinguished by Random set to the + * special value of the SHA-256 of "HelloRetryRequest". + * + * struct { + * ProtocolVersion legacy_version = 0x0303; + * Random random; + * opaque legacy_session_id_echo<0..32>; + * CipherSuite cipher_suite; + * uint8 legacy_compression_method = 0; + * Extension extensions<6..2 ^ 16 - 1>; + * } ServerHello; + * + */ + if( memcmp( buf + 2, magic_hrr_string, + sizeof( magic_hrr_string ) ) == 0 ) + { + return( 1 ); + } + + return( 0 ); +} + +/* Fetch and preprocess + * Returns a negative value on failure, and otherwise + * - SSL_SERVER_HELLO_COORDINATE_HELLO or + * - SSL_SERVER_HELLO_COORDINATE_HRR + * to indicate which message is expected and to be parsed next. */ +#define SSL_SERVER_HELLO_COORDINATE_HELLO 0 +#define SSL_SERVER_HELLO_COORDINATE_HRR 1 +static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl, + unsigned char **buf, + size_t *buf_len ) +{ + int ret; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_read_record( ssl, 0 ) ); + + /* TBD: If we do an HRR, keep track of the number + * of ClientHello's we sent, and fail if it + * exceeds the configured threshold. */ + + if( ( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) || + ( ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_HELLO ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "unexpected message" ) ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, + MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + } + + *buf = ssl->in_msg + 4; + *buf_len = ssl->in_hslen - 4; + + if( ssl_server_hello_is_hrr( ssl->in_msg + 4 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); + ret = SSL_SERVER_HELLO_COORDINATE_HRR; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "received ServerHello message" ) ); + ret = SSL_SERVER_HELLO_COORDINATE_HELLO; + } + +cleanup: + + return( ret ); +} + +static int ssl_tls1_3_check_server_hello_session_id( mbedtls_ssl_context *ssl, + const unsigned char **buf, + const unsigned char *end ) +{ + const unsigned char *p = *buf; + size_t recv_id_len; + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + recv_id_len = *p++ ; + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, recv_id_len ); + + /* legacy_session_id_echo */ + if( ssl->session_negotiate->id_len != recv_id_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Mismatch of session id length:" + " id_len = %" MBEDTLS_PRINTF_SIZET + " , recv_id_len = %" MBEDTLS_PRINTF_SIZET, + ssl->session_negotiate->id_len, recv_id_len ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + + if( memcmp( ssl->session_negotiate->id, p , recv_id_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unexpected legacy_session_id_echo" ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "Expected Session ID", + ssl->session_negotiate->id, + ssl->session_negotiate->id_len ); + MBEDTLS_SSL_DEBUG_BUF( 3, "Received Session ID", p, + ssl->session_negotiate->id_len ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + + p += recv_id_len; + *buf = p; + + MBEDTLS_SSL_DEBUG_BUF( 3, "Session ID", ssl->session_negotiate->id, + recv_id_len ); + return( 0 ); +} + +static int ssl_tls1_3_cipher_suite_is_offered( mbedtls_ssl_context *ssl, + uint16_t cipher_suite ) +{ + /* Check whether we have offered this ciphersuite */ + for ( int i = 0; ssl->conf->ciphersuite_list[i] != 0; i++ ) + { + if( ssl->conf->ciphersuite_list[i] == cipher_suite ) + { + return( 1 ); + } + } + return( 0 ); +} + +/* Parse ServerHello message and configure context + * + * struct { + * ProtocolVersion legacy_version = 0x0303; // TLS 1.2 + * Random random; + * opaque legacy_session_id_echo<0..32>; + * CipherSuite cipher_suite; + * uint8 legacy_compression_method = 0; + * Extension extensions<6..2 ^ 16 - 1>; + * } ServerHello; + */ +static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + + int ret; + const unsigned char *p = buf; + size_t field_len; /* Length of field */ + const unsigned char *ext_end; /* Pointer to end of individual extension */ + uint16_t cipher_suite; + const mbedtls_ssl_ciphersuite_t* ciphersuite_info; + + /* + * Check there is space for minimal fields + * + * - legacy_version ( 2 bytes) + * - random (32 bytes) + * - legacy_session_id_echo ( 1 byte ), minimum size + * - cipher_suite ( 2 bytes) + * - legacy_compression_method ( 1 byte ) + */ + if( mbedtls_ssl_chk_buf_ptr( p, end, 38 ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server hello message - min size not reached" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + MBEDTLS_SSL_DEBUG_BUF( 4, "server hello", p, end - p ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", p, 2 ); + + /* legacy_version must be 0x0303 (TLS 1.2) */ + if( !( p[0] == MBEDTLS_SSL_MAJOR_VERSION_3 && + p[1] == MBEDTLS_SSL_MINOR_VERSION_3 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unsupported version of TLS." ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION, + MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); + return( MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); + } + p += 2; + /* Internally we use the correct 1.3 version + * TODO: Remove below lines after supported_versions extension + * finished. + */ + ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; + ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4; + + /* Store server-provided random values */ + memcpy( ssl->handshake->randbytes + CLIENT_HELLO_RANDOM_LEN, p, + SERVER_HELLO_RANDOM_LEN ); + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", + p, SERVER_HELLO_RANDOM_LEN ); + p += SERVER_HELLO_RANDOM_LEN; + + /* Read and store session id (legacy_session_id_echo) */ + if( ssl_tls1_3_check_server_hello_session_id( ssl, &p, end ) != 0 ) + { + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + + /* Read server-selected ciphersuite, + Check if there is space for cipher_suite. */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); + cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + /* Configure ciphersuites */ + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); + ssl->handshake->ciphersuite_info = ciphersuite_info; + if( ciphersuite_info == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite info for %04x not found", + cipher_suite ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + + mbedtls_ssl_optimize_checksum( ssl, ssl->handshake->ciphersuite_info ); + + ssl->session_negotiate->ciphersuite = cipher_suite; + + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: ( %04x ) - %s", + cipher_suite, ciphersuite_info->name ) ); + +#if defined(MBEDTLS_HAVE_TIME) + ssl->session_negotiate->start = time( NULL ); +#endif /* MBEDTLS_HAVE_TIME */ + + /* Check whether we have offered this ciphersuite */ + /* Via the force_ciphersuite version we may have instructed the client */ + /* to use a difference ciphersuite. */ + if( ssl_tls1_3_cipher_suite_is_offered( ssl, cipher_suite ) == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite(%04x) is not in offered list", + cipher_suite ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + /* Ensure that compression method is set to zero + * + * legacy_compression_method == 0 ( 1 byte) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + if( p[0] != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + p++; + + /* Check there is space fore extensions_length */ + if( mbedtls_ssl_chk_buf_ptr( p, end, 2 ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + /* Get length of extensions field (2 bytes)*/ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + field_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + /* Check there is space for extensions_data */ + if( mbedtls_ssl_chk_buf_ptr( p, end, field_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + /* Set end of extensions */ + ext_end = p + field_len; + + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "server hello, total extension length: %" MBEDTLS_PRINTF_SIZET , + field_len ) ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello extensions", p, field_len ); + + while ( p < ext_end ) + { + unsigned int extension_type; + size_t extension_data_len; + + /* + * .... + * Extension extensions<6..2 ^ 16 - 1>; + * .... + * struct { + * ExtensionType extension_type; + * opaque extension_data<0..2^16-1>; + * } Extension; + * extension_type (2 bytes) + * extension_data_length (2 bytes) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, ext_end, 4 ); + extension_type = MBEDTLS_GET_UINT16_BE( p, 0 ); + extension_data_len = MBEDTLS_GET_UINT16_BE( p, 2 ); + p += 4; + + if( mbedtls_ssl_chk_buf_ptr( p, ext_end, extension_data_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + switch( extension_type ) + { + case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS: + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "found supported_versions extension" ) ); + + ret = ssl_tls1_3_parse_supported_versions_ext( ssl, + p, extension_data_len ); + if( ret != 0 ) + return( ret ); + break; + + case MBEDTLS_TLS_EXT_PRE_SHARED_KEY: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found pre_shared_key extension." ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "pre_shared_key:Not supported yet" ) ); + break; + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + case MBEDTLS_TLS_EXT_KEY_SHARE: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key_shares extension" ) ); + if( ( ret = ssl_tls1_3_parse_key_share_ext( ssl, + p, p + extension_data_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "ssl_tls1_3_parse_key_share_ext", + ret ); + return( ret ); + } + break; +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + + default: + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "unknown extension found: %u ( ignoring )", + extension_type ) ); + } + + p += extension_data_len; + } + + return( 0 ); +} + +static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) +{ + ((void) ssl); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess hasn't been implemented" ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} + +/* + * Wait and Parse ServerHello handshake message. * Handler for MBEDTLS_SSL_SERVER_HELLO */ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); - return( 0 ); + int ret = 0; + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> %s", __func__ ) ); + + /* Coordination step + * - Fetch record + * - Make sure it's either a ServerHello or a HRR. + * - Switch processing routine in case of HRR + */ + + ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; + ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; + + + ret = ssl_server_hello_coordinate( ssl, &buf, &buf_len ); + /* Parsing step + * We know what message to expect by now and call + * the respective parsing function. + */ + if( ret == SSL_SERVER_HELLO_COORDINATE_HELLO ) + { + MBEDTLS_SSL_PROC_CHK( ssl_tls1_3_parse_server_hello( ssl, buf, + buf + buf_len ) ); + + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, + MBEDTLS_SSL_HS_SERVER_HELLO, + buf, buf_len ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) ); + } + else if( ret == SSL_SERVER_HELLO_COORDINATE_HRR ) + { + /* TODO: Implement HRR in future #4915 */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "HRR hasn't been implemented" ) ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, + MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; + } + +cleanup: + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= %s", __func__ ) ); + return( ret ); } /* From 0b17784932d4094e39f0ec090cb2663012ef2821 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sun, 5 Sep 2021 19:41:30 +0800 Subject: [PATCH 05/23] Add finalize function Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 98 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 054a45d7f..5502c885f 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1265,11 +1265,99 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) +static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) { - ((void) ssl); - MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess hasn't been implemented" ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + int ret; + mbedtls_ssl_key_set traffic_keys; + mbedtls_ssl_transform *transform_handshake; + + /* We need to set the key exchange algorithm based on the + * following rules: + * + * 1 ) IF PRE_SHARED_KEY extension was received + * THEN set MBEDTLS_KEY_EXCHANGE_PSK + * 2 ) IF PRE_SHARED_KEY extension && KEY_SHARE was received + * THEN set MBEDTLS_KEY_EXCHANGE_ECDHE_PSK + * 3 ) IF KEY_SHARES extension was received && SIG_ALG extension received + * THEN set MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA + * ELSE unknown key exchange mechanism. + */ + + if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PRE_SHARED_KEY ) + { + if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) + ssl->handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + else + ssl->handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; + } + else if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) + ssl->handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + /* Start the TLS 1.3 key schedule: Set the PSK and derive early secret. + * + * TODO: We don't have to do this in case we offered 0-RTT and the + * server accepted it. In this case, we could skip generating + * the early secret. */ + ret = mbedtls_ssl_tls1_3_key_schedule_stage_early( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_key_schedule_stage_early_data", + ret ); + return( ret ); + } + + /* Compute handshake secret */ + ret = mbedtls_ssl_tls1_3_key_schedule_stage_handshake( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_master_secret", ret ); + return( ret ); + } + + /* Next evolution in key schedule: Establish handshake secret and + * key material. */ + ret = mbedtls_ssl_tls1_3_generate_handshake_keys( ssl, &traffic_keys ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_tls1_3_generate_handshake_keys", ret ); + return( ret ); + } + + transform_handshake = + mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); + if( transform_handshake == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + + ret = mbedtls_ssl_tls13_populate_transform( transform_handshake, + ssl->conf->endpoint, + ssl->session_negotiate->ciphersuite, + &traffic_keys, + ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_populate_transform", ret ); + return( ret ); + } + + ssl->handshake->transform_handshake = transform_handshake; + mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); + ssl->session_in = ssl->session_negotiate; + + /* + * State machine update + */ + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); + + mbedtls_platform_zeroize( &traffic_keys, sizeof( traffic_keys ) ); + return( 0 ); } /* @@ -1308,7 +1396,7 @@ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_HS_SERVER_HELLO, buf, buf_len ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls1_3_finalize_server_hello( ssl ) ); } else if( ret == SSL_SERVER_HELLO_COORDINATE_HRR ) { From 4ae2d62ccee7083bf7a18d5c7db68f1dbf174ada Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 15 Sep 2021 15:34:56 +0800 Subject: [PATCH 06/23] Improve tls13 handshake test Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e90a35226..ec3722127 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8692,7 +8692,9 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "tls1_3 client state: 20" \ -c "tls1_3 client state: 11" \ -c "tls1_3 client state: 14" \ - -c "tls1_3 client state: 15" + -c "tls1_3 client state: 15" \ + -c "<= ssl_tls1_3_process_server_hello" \ + -c "=> ssl_tls1_3_process_server_hello" requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL @@ -8713,7 +8715,9 @@ run_test "TLS1.3: Test client hello msg work - gnutls" \ -c "tls1_3 client state: 20" \ -c "tls1_3 client state: 11" \ -c "tls1_3 client state: 14" \ - -c "tls1_3 client state: 15" + -c "tls1_3 client state: 15" \ + -c "<= ssl_tls1_3_process_server_hello" \ + -c "=> ssl_tls1_3_process_server_hello" # Test heap memory usage after handshake requires_config_enabled MBEDTLS_MEMORY_DEBUG From fd532e506b5ebc313e7d2ef9330d13d643bbb623 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sun, 19 Sep 2021 15:57:53 +0800 Subject: [PATCH 07/23] fix set key exchange mode issue Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5502c885f..463821b28 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1274,26 +1274,38 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) /* We need to set the key exchange algorithm based on the * following rules: * - * 1 ) IF PRE_SHARED_KEY extension was received - * THEN set MBEDTLS_KEY_EXCHANGE_PSK - * 2 ) IF PRE_SHARED_KEY extension && KEY_SHARE was received - * THEN set MBEDTLS_KEY_EXCHANGE_ECDHE_PSK - * 3 ) IF KEY_SHARES extension was received && SIG_ALG extension received - * THEN set MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA + * 1) IF PRE_SHARED_KEY extension was received + * THEN set KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + * 2) IF PRE_SHARED_KEY extension && KEY_SHARE was received + * THEN set KEY_EXCHANGE_MODE_PSK; + * 3) IF KEY_SHARES extension was received && SIG_ALG extension received + * THEN set KEY_EXCHANGE_MODE_EPHEMERAL * ELSE unknown key exchange mechanism. */ - if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PRE_SHARED_KEY ) { if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) - ssl->handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + { + /* Condition 2) */ + ssl->handshake->tls1_3_kex_modes = + MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + } else - ssl->handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; + { + /* Condition 1) */ + ssl->handshake->tls1_3_kex_modes = + MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; + } + } + else if( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) ) + { + /* Condition 3) */ + ssl->handshake->tls1_3_kex_modes = + MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; } - else if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) - ssl->handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; else { + /* ELSE case */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } From de4fb2cc346be9b977ead03a9e932abc575b3c57 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sun, 19 Sep 2021 18:05:08 +0800 Subject: [PATCH 08/23] Apply check read ptr macro Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 52 +++++++++----------------------------- 1 file changed, 12 insertions(+), 40 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 463821b28..57a0b2882 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -570,7 +570,7 @@ static int ssl_tls1_3_parse_key_share_ext( mbedtls_ssl_context *ssl, uint16_t server_share_group, offered_group; /* server_share_group (2 bytes) */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2); server_share_group = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; @@ -979,10 +979,10 @@ static int ssl_tls1_3_check_server_hello_session_id( mbedtls_ssl_context *ssl, const unsigned char *p = *buf; size_t recv_id_len; - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); recv_id_len = *p++ ; - MBEDTLS_SSL_CHK_BUF_PTR( p, end, recv_id_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, recv_id_len ); /* legacy_session_id_echo */ if( ssl->session_negotiate->id_len != recv_id_len ) @@ -1042,13 +1042,12 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - int ret; const unsigned char *p = buf; size_t field_len; /* Length of field */ const unsigned char *ext_end; /* Pointer to end of individual extension */ uint16_t cipher_suite; - const mbedtls_ssl_ciphersuite_t* ciphersuite_info; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info; /* * Check there is space for minimal fields @@ -1059,14 +1058,7 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, * - cipher_suite ( 2 bytes) * - legacy_compression_method ( 1 byte ) */ - if( mbedtls_ssl_chk_buf_ptr( p, end, 38 ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "bad server hello message - min size not reached" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 38 ); MBEDTLS_SSL_DEBUG_BUF( 4, "server hello", p, end - p ); @@ -1106,7 +1098,7 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, /* Read server-selected ciphersuite, Check if there is space for cipher_suite. */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2); cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; @@ -1153,7 +1145,7 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, * * legacy_compression_method == 0 ( 1 byte) */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); if( p[0] != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); @@ -1164,26 +1156,13 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, p++; /* Check there is space fore extensions_length */ - if( mbedtls_ssl_chk_buf_ptr( p, end, 2 ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); /* Get length of extensions field (2 bytes)*/ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); field_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; + /* Check there is space for extensions_data */ - if( mbedtls_ssl_chk_buf_ptr( p, end, field_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, field_len ); /* Set end of extensions */ ext_end = p + field_len; @@ -1209,18 +1188,12 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, * extension_type (2 bytes) * extension_data_length (2 bytes) */ - MBEDTLS_SSL_CHK_BUF_PTR( p, ext_end, 4 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, ext_end, 4 ); extension_type = MBEDTLS_GET_UINT16_BE( p, 0 ); extension_data_len = MBEDTLS_GET_UINT16_BE( p, 2 ); p += 4; - if( mbedtls_ssl_chk_buf_ptr( p, ext_end, extension_data_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, ext_end, extension_data_len ); switch( extension_type ) { @@ -1393,7 +1366,6 @@ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; - ret = ssl_server_hello_coordinate( ssl, &buf, &buf_len ); /* Parsing step * We know what message to expect by now and call From 42920ec5a59a4453ec55f824a64c10776bf61f71 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 28 Sep 2021 14:31:15 +0800 Subject: [PATCH 09/23] tls1_3:skip handshake msg test with PSA_CRYPTO tls1_3 hasn't implemented PSA version get transcript Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ec3722127..2b91025fd 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8675,6 +8675,7 @@ run_test "TLS1.3: handshake dispatch test: tls1_3 only" \ requires_openssl_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL +requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS1.3: Test client hello msg work - openssl" \ "$O_NEXT_SRV -tls1_3 -msg" \ "$P_CLI debug_level=2 min_version=tls1_3 max_version=tls1_3" \ @@ -8698,6 +8699,7 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ requires_gnutls_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL +requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS1.3: Test client hello msg work - gnutls" \ "$G_NEXT_SRV --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 --debug=4" \ "$P_CLI debug_level=2 min_version=tls1_3 max_version=tls1_3" \ From 5ccfcd4ca13577ee578ddef5b985a4d0087665e7 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 11 Oct 2021 16:39:29 +0800 Subject: [PATCH 10/23] Add local variable to represent handshake Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 165cf4cbe..5e6182f1c 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -826,17 +826,18 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_md_type_t md_type; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; - if( ssl->handshake->ciphersuite_info == NULL ) + if( handshake->ciphersuite_info == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "cipher suite info not found" ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } - md_type = ssl->handshake->ciphersuite_info->mac; + md_type = handshake->ciphersuite_info->mac; ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, NULL, NULL, 0, - ssl->handshake->tls1_3_master_secrets.early ); + handshake->tls1_3_master_secrets.early ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_evolve_secret", ret ); @@ -1015,7 +1016,8 @@ static int ssl_tls1_3_complete_ephemeral_secret( mbedtls_ssl_context *ssl, int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) { int ret = 0; - mbedtls_md_type_t const md_type = ssl->handshake->ciphersuite_info->mac; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + mbedtls_md_type_t const md_type = handshake->ciphersuite_info->mac; #if defined(MBEDTLS_DEBUG_C) mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); size_t const md_size = mbedtls_md_get_size( md_info ); @@ -1037,9 +1039,9 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) */ ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, - ssl->handshake->tls1_3_master_secrets.early, - ephemeral, ephemeral_len, - ssl->handshake->tls1_3_master_secrets.handshake ); + handshake->tls1_3_master_secrets.early, + ephemeral, ephemeral_len, + handshake->tls1_3_master_secrets.handshake ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_evolve_secret", ret ); @@ -1047,7 +1049,7 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) } MBEDTLS_SSL_DEBUG_BUF( 4, "Handshake secret", - ssl->handshake->tls1_3_master_secrets.handshake, md_size ); + handshake->tls1_3_master_secrets.handshake, md_size ); #if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) mbedtls_platform_zeroize( ecdhe, sizeof( ecdhe ) ); From f0ac2352d6b8cc3f63902fcb4cdd72cf1190df3c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 11 Oct 2021 17:47:07 +0800 Subject: [PATCH 11/23] Refactor key_schedule_stage_handshake Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_keys.c | 102 ++++++++++++++----------------------- library/ssl_tls13_keys.h | 4 +- 3 files changed, 41 insertions(+), 67 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 57a0b2882..8a167a51d 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1297,7 +1297,7 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) } /* Compute handshake secret */ - ret = mbedtls_ssl_tls1_3_key_schedule_stage_handshake( ssl ); + ret = mbedtls_ssl_tls13_key_schedule_stage_handshake( ssl ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_master_secret", ret ); diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 5e6182f1c..7ba5b5f8f 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -964,83 +964,57 @@ exit: return( ret ); } -static int ssl_tls1_3_complete_ephemeral_secret( mbedtls_ssl_context *ssl, - unsigned char *secret, - size_t secret_len, - unsigned char **actual_secret, - size_t *actual_len ) +int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) { - int ret = 0; - - *actual_secret = NULL; - *actual_len = 0; - /* - * Compute ECDHE secret for second stage of secret evolution. - */ -#if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) - if( mbedtls_ssl_tls1_3_some_ephemeral_enabled( ssl ) ) - { - if( mbedtls_ssl_tls13_named_group_is_ecdhe( - ssl->handshake->offered_group_id ) ) - { -#if defined(MBEDTLS_ECDH_C) - ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, - actual_len, secret, secret_len, - ssl->conf->f_rng, - ssl->conf->p_rng ); - - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); - return( ret ); - } - - *actual_secret = secret; -#endif /* MBEDTLS_ECDH_C */ - } - else if( mbedtls_ssl_tls13_named_group_is_dhe( - ssl->handshake->offered_group_id ) ) - { - /* TODO: Not supported yet */ - } - } -#else - ((void) ssl); - ((void) secret); - ((void) secret_len); -#endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED */ - - return( ret ); -} - -int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) -{ - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ssl_handshake_params *handshake = ssl->handshake; mbedtls_md_type_t const md_type = handshake->ciphersuite_info->mac; + size_t ephemeral_len = 0; + unsigned char ecdhe[MBEDTLS_ECP_MAX_BYTES]; #if defined(MBEDTLS_DEBUG_C) mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); size_t const md_size = mbedtls_md_get_size( md_info ); #endif /* MBEDTLS_DEBUG_C */ - unsigned char *ephemeral; - size_t ephemeral_len; - - unsigned char ecdhe[66]; /* TODO: Magic constant! */ - - /* Finalize calculation of ephemeral input to key schedule, if present. */ - ret = ssl_tls1_3_complete_ephemeral_secret( ssl, ecdhe, sizeof( ecdhe ), - &ephemeral, &ephemeral_len ); - if( ret != 0 ) - return( ret ); +#if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) + /* + * Compute ECDHE secret used to compute the handshake secret from which + * client_handshake_traffic_secret and server_handshake_traffic_secret + * are derived in the handshake secret derivation stage. + */ + if( mbedtls_ssl_tls1_3_ephemeral_enabled( ssl ) ) + { + if( mbedtls_ssl_tls13_named_group_is_ecdhe( handshake->offered_group_id ) ) + { +#if defined(MBEDTLS_ECDH_C) + ret = mbedtls_ecdh_calc_secret( &handshake->ecdh_ctx, + &ephemeral_len, ecdhe, sizeof( ecdhe ), + ssl->conf->f_rng, + ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( ret ); + } +#endif /* MBEDTLS_ECDH_C */ + } + else if( mbedtls_ssl_tls13_named_group_is_dhe( handshake->offered_group_id ) ) + { + /* TODO: Not supported yet */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "DHE not supported." ) ); + return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); + } + } +#else + return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); +#endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED */ /* - * Compute HandshakeSecret + * Compute the Handshake Secret */ - ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, handshake->tls1_3_master_secrets.early, - ephemeral, ephemeral_len, + ecdhe, ephemeral_len, handshake->tls1_3_master_secrets.handshake ); if( ret != 0 ) { diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 536d976cd..71bd90de2 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -548,14 +548,14 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ); * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls1_3_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); /** * \brief Compute TLS 1.3 handshake traffic keys. * * \param ssl The SSL context to operate on. This must be in * key schedule stage \c Handshake, see - * mbedtls_ssl_tls1_3_key_schedule_stage_handshake(). + * mbedtls_ssl_tls13_key_schedule_stage_handshake(). * \param traffic_keys The address at which to store the handshake traffic key * keys. This must be writable but may be uninitialized. * From 4a1733831e4c1d0fd66e3ee1fef413892335f274 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 11 Oct 2021 21:45:31 +0800 Subject: [PATCH 12/23] fix various issues Signed-off-by: Jerry Yu --- library/ecdh.c | 5 +- library/ssl_tls13_client.c | 332 +++++++++++++++++++------------------ 2 files changed, 173 insertions(+), 164 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 0067e0bd4..8884260e8 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -812,10 +812,9 @@ static int ecdh_tls1_3_read_public_internal( mbedtls_ecdh_context_mbed *ctx, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; - const unsigned char *end = buf + blen; size_t data_len; - if( end - p < 3 ) + if( blen < 3 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); data_len = MBEDTLS_GET_UINT16_BE( p, 0 ); @@ -828,7 +827,7 @@ static int ecdh_tls1_3_read_public_internal( mbedtls_ecdh_context_mbed *ctx, * Save buffer start for read_binary and update buf */ if( ( ret = mbedtls_ecp_point_read_binary( &ctx->grp, - &ctx->Qp, p, data_len ) ) != 0) + &ctx->Qp, p, data_len ) ) != 0) { return( ret ); } diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 8a167a51d..57195b329 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -97,29 +97,19 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, static int ssl_tls1_3_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, - size_t len ) + size_t buf_len ) { - /* TODO: Implement full version and remove force version set in - * ssl_tls_parse_server_hello. - * - * From page 40,RFC 8446 - * If supported_versions extension is present, clients MUST ignore the - * ServerHello.legacy_version value and MUST use only the - * "supported_versions" extension to determine the selected version. If - * the "supported_versions" extension in the ServerHello contains a - * version not offered by the client or contains a version prior to - * TLS 1.3, the client MUST abort the handshake with an - * "illegal_parameter" alert. - */ - ((void) ssl); - if( len != 2 || + if( buf_len != 2 || buf[0] != MBEDTLS_SSL_MAJOR_VERSION_3 || buf[1] != MBEDTLS_SSL_MINOR_VERSION_4 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "unexpected version" ) ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } return( 0 ); @@ -487,7 +477,6 @@ cleanup: #if defined(MBEDTLS_ECDH_C) -/* TODO: Code for MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED missing */ static int ssl_tls1_3_check_ecdh_params( const mbedtls_ssl_context *ssl ) { const mbedtls_ecp_curve_info *curve_info; @@ -507,12 +496,7 @@ static int ssl_tls1_3_check_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, @@ -521,20 +505,20 @@ static int ssl_tls1_3_check_ecdh_params( const mbedtls_ssl_context *ssl ) return( 0 ); } -/* The ssl_tls1_3_parse_key_share_ext() function is used +/* The ssl_tls13_parse_key_share_ext() function is used * by the client to parse a KeyShare extension in - * a ServerHello message. + * a Server Hello message. * * The server only provides a single KeyShareEntry. */ static int ssl_tls1_3_read_public_ecdhe_share( mbedtls_ssl_context *ssl, const unsigned char *buf, - size_t len ) + size_t buf_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; ret = mbedtls_ecdh_tls1_3_read_public( &ssl->handshake->ecdh_ctx, - buf, len ); + buf, buf_len ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ecdh_tls13_read_public" ), ret ); @@ -544,12 +528,14 @@ static int ssl_tls1_3_read_public_ecdhe_share( mbedtls_ssl_context *ssl, if( ssl_tls1_3_check_ecdh_params( ssl ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_tls1_3_check_ecdh_params() failed!" ) ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } return( 0 ); } -#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDH_C */ /* * Parse key_share extension in Server Hello @@ -561,31 +547,36 @@ static int ssl_tls1_3_read_public_ecdhe_share( mbedtls_ssl_context *ssl, * opaque key_exchange<1..2^16-1>; * } KeyShareEntry; */ -static int ssl_tls1_3_parse_key_share_ext( mbedtls_ssl_context *ssl, +static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { int ret = 0; const unsigned char *p = buf; - uint16_t server_share_group, offered_group; + uint16_t group, offered_group; - /* server_share_group (2 bytes) */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2); - server_share_group = MBEDTLS_GET_UINT16_BE( p, 0 ); + /* ... + * NamedGroup group; (2 bytes) + * ... + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + group = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - /* Check that chosen group matches the one we offered. */ + /* Check that the chosen group matches the one we offered. */ offered_group = ssl->handshake->offered_group_id; - if( offered_group != server_share_group ) + if( offered_group != group ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid server key share, our group %u, their group %u", - (unsigned) offered_group, (unsigned) server_share_group ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + (unsigned) offered_group, (unsigned) group ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } #if defined(MBEDTLS_ECDH_C) - if( mbedtls_ssl_tls13_named_group_is_ecdhe( server_share_group ) ) + if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) { /* Complete ECDHE key agreement */ ret = ssl_tls1_3_read_public_ecdhe_share( ssl, p, end - p ); @@ -890,11 +881,11 @@ cleanup: } /* - * Functions for parsing and processing ServerHello + * Functions for parsing and processing Server Hello */ -static int ssl_server_hello_is_hrr( unsigned const char *buf ) +static int ssl_server_hello_is_hrr( unsigned const char *buf, size_t blen ) { - const unsigned char magic_hrr_string[32] = + static const unsigned char magic_hrr_string[32] = { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, @@ -902,7 +893,7 @@ static int ssl_server_hello_is_hrr( unsigned const char *buf ) /* Check whether this message is a HelloRetryRequest ( HRR ) message. * - * ServerHello and HRR are only distinguished by Random set to the + * Server Hello and HRR are only distinguished by Random set to the * special value of the SHA-256 of "HelloRetryRequest". * * struct { @@ -915,8 +906,10 @@ static int ssl_server_hello_is_hrr( unsigned const char *buf ) * } ServerHello; * */ - if( memcmp( buf + 2, magic_hrr_string, - sizeof( magic_hrr_string ) ) == 0 ) + if( blen < 2 + sizeof( magic_hrr_string ) ) + return (MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + + if( memcmp( buf + 2, magic_hrr_string, sizeof( magic_hrr_string ) ) == 0 ) { return( 1 ); } @@ -929,20 +922,16 @@ static int ssl_server_hello_is_hrr( unsigned const char *buf ) * - SSL_SERVER_HELLO_COORDINATE_HELLO or * - SSL_SERVER_HELLO_COORDINATE_HRR * to indicate which message is expected and to be parsed next. */ -#define SSL_SERVER_HELLO_COORDINATE_HELLO 0 +#define SSL_SERVER_HELLO_COORDINATE_HELLO 0 #define SSL_SERVER_HELLO_COORDINATE_HRR 1 static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl, unsigned char **buf, size_t *buf_len ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_read_record( ssl, 0 ) ); - /* TBD: If we do an HRR, keep track of the number - * of ClientHello's we sent, and fail if it - * exceeds the configured threshold. */ - if( ( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) || ( ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_HELLO ) ) { @@ -956,7 +945,7 @@ static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl, *buf = ssl->in_msg + 4; *buf_len = ssl->in_hslen - 4; - if( ssl_server_hello_is_hrr( ssl->in_msg + 4 ) ) + if( ssl_server_hello_is_hrr( *buf, *buf_len ) ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); ret = SSL_SERVER_HELLO_COORDINATE_HRR; @@ -972,54 +961,55 @@ cleanup: return( ret ); } -static int ssl_tls1_3_check_server_hello_session_id( mbedtls_ssl_context *ssl, - const unsigned char **buf, - const unsigned char *end ) +static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ssl, + const unsigned char **buf, + const unsigned char *end ) { const unsigned char *p = *buf; - size_t recv_id_len; + size_t legacy_session_id_echo_len; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - recv_id_len = *p++ ; + legacy_session_id_echo_len = *p++ ; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, recv_id_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, legacy_session_id_echo_len ); /* legacy_session_id_echo */ - if( ssl->session_negotiate->id_len != recv_id_len ) + if( ssl->session_negotiate->id_len != legacy_session_id_echo_len || + memcmp( ssl->session_negotiate->id, p , legacy_session_id_echo_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Mismatch of session id length:" " id_len = %" MBEDTLS_PRINTF_SIZET - " , recv_id_len = %" MBEDTLS_PRINTF_SIZET, - ssl->session_negotiate->id_len, recv_id_len ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - } - - if( memcmp( ssl->session_negotiate->id, p , recv_id_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unexpected legacy_session_id_echo" ) ); + " , legacy_session_id_echo_len = %" MBEDTLS_PRINTF_SIZET, + ssl->session_negotiate->id_len, legacy_session_id_echo_len ) ); MBEDTLS_SSL_DEBUG_BUF( 3, "Expected Session ID", ssl->session_negotiate->id, ssl->session_negotiate->id_len ); MBEDTLS_SSL_DEBUG_BUF( 3, "Received Session ID", p, - ssl->session_negotiate->id_len ); + legacy_session_id_echo_len ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - p += recv_id_len; + p += legacy_session_id_echo_len; *buf = p; MBEDTLS_SSL_DEBUG_BUF( 3, "Session ID", ssl->session_negotiate->id, - recv_id_len ); + ssl->session_negotiate->id_len ); return( 0 ); } -static int ssl_tls1_3_cipher_suite_is_offered( mbedtls_ssl_context *ssl, - uint16_t cipher_suite ) +static int ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, + int cipher_suite ) { + const int *ciphersuite_list = ssl->conf->ciphersuite_list; + /* Check whether we have offered this ciphersuite */ - for ( int i = 0; ssl->conf->ciphersuite_list[i] != 0; i++ ) + for ( size_t i = 0; ciphersuite_list[i] != 0; i++ ) { - if( ssl->conf->ciphersuite_list[i] == cipher_suite ) + if( ciphersuite_list[i] == cipher_suite ) { return( 1 ); } @@ -1044,8 +1034,8 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, { int ret; const unsigned char *p = buf; - size_t field_len; /* Length of field */ - const unsigned char *ext_end; /* Pointer to end of individual extension */ + size_t extensions_len; /* Length of field */ + const unsigned char *extensions_end; /* Pointer to end of individual extension */ uint16_t cipher_suite; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; @@ -1053,18 +1043,22 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, * Check there is space for minimal fields * * - legacy_version ( 2 bytes) - * - random (32 bytes) + * - random (SERVER_HELLO_RANDOM_LEN bytes) * - legacy_session_id_echo ( 1 byte ), minimum size * - cipher_suite ( 2 bytes) * - legacy_compression_method ( 1 byte ) */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 38 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, SERVER_HELLO_RANDOM_LEN + 6 ); MBEDTLS_SSL_DEBUG_BUF( 4, "server hello", p, end - p ); - MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", p, 2 ); - /* legacy_version must be 0x0303 (TLS 1.2) */ + /* ... + * ProtocaolVersion legacy_version = 0x0303; // TLS 1.2 + * ... + * with ProtocolVersion defined as: + * uint16 ProtocolVersion; + */ if( !( p[0] == MBEDTLS_SSL_MAJOR_VERSION_3 && p[1] == MBEDTLS_SSL_MINOR_VERSION_3 ) ) { @@ -1074,54 +1068,66 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); } p += 2; - /* Internally we use the correct 1.3 version - * TODO: Remove below lines after supported_versions extension - * finished. - */ - ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; - ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4; - /* Store server-provided random values */ + /* ... + * Random random; + * ... + * with Random defined as: + * opaque Random[32]; + */ memcpy( ssl->handshake->randbytes + CLIENT_HELLO_RANDOM_LEN, p, SERVER_HELLO_RANDOM_LEN ); MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", p, SERVER_HELLO_RANDOM_LEN ); p += SERVER_HELLO_RANDOM_LEN; - /* Read and store session id (legacy_session_id_echo) */ - if( ssl_tls1_3_check_server_hello_session_id( ssl, &p, end ) != 0 ) + /* ... + * opaque legacy_session_id_echo<0..32>; + * ... + */ + if( ssl_tls13_check_server_hello_session_id_echo( ssl, &p, end ) != 0 ) { MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - /* Read server-selected ciphersuite, - Check if there is space for cipher_suite. */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2); + /* ... + * CipherSuite cipher_suite; + * ... + * with CipherSuite defined as: + * uint8 CipherSuite[2]; + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - /* Configure ciphersuites */ + + /* + * Check whether this ciphersuite is supported and offered. + * Via the force_ciphersuite version we may have instructed the client + * to use a different ciphersuite. + */ ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); - ssl->handshake->ciphersuite_info = ciphersuite_info; - if( ciphersuite_info == NULL ) + if( ciphersuite_info == NULL || + ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite info for %04x not found", cipher_suite ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, - MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - mbedtls_ssl_optimize_checksum( ssl, ssl->handshake->ciphersuite_info ); + /* Configure ciphersuites */ + mbedtls_ssl_optimize_checksum( ssl, ciphersuite_info ); + + ssl->handshake->ciphersuite_info = ciphersuite_info; ssl->session_negotiate->ciphersuite = cipher_suite; - ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: ( %04x ) - %s", cipher_suite, ciphersuite_info->name ) ); @@ -1129,21 +1135,9 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, ssl->session_negotiate->start = time( NULL ); #endif /* MBEDTLS_HAVE_TIME */ - /* Check whether we have offered this ciphersuite */ - /* Via the force_ciphersuite version we may have instructed the client */ - /* to use a difference ciphersuite. */ - if( ssl_tls1_3_cipher_suite_is_offered( ssl, cipher_suite ) == 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite(%04x) is not in offered list", - cipher_suite ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, - MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - } - - /* Ensure that compression method is set to zero - * - * legacy_compression_method == 0 ( 1 byte) + /* ... + * uint8 legacy_compression_method = 0; + * ... */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); if( p[0] != 0 ) @@ -1155,45 +1149,39 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, } p++; - /* Check there is space fore extensions_length */ + /* + * .... + * Extension extensions<6..2 ^ 16 - 1>; + * .... + * struct { + * ExtensionType extension_type; (2 bytes) + * opaque extension_data<0..2^16-1>; + * } Extension; + */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - /* Get length of extensions field (2 bytes)*/ - field_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - /* Check there is space for extensions_data */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, field_len ); - /* Set end of extensions */ - ext_end = p + field_len; + /* Check extensions do not go beyond the buffer of data. */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); + extensions_end = p + extensions_len; MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, total extension length: %" MBEDTLS_PRINTF_SIZET , - field_len ) ); + extensions_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello extensions", p, extensions_len ); - MBEDTLS_SSL_DEBUG_BUF( 3, "server hello extensions", p, field_len ); - - while ( p < ext_end ) + while( p < extensions_end ) { unsigned int extension_type; size_t extension_data_len; - /* - * .... - * Extension extensions<6..2 ^ 16 - 1>; - * .... - * struct { - * ExtensionType extension_type; - * opaque extension_data<0..2^16-1>; - * } Extension; - * extension_type (2 bytes) - * extension_data_length (2 bytes) - */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, ext_end, 4 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extensions_end, 4 ); extension_type = MBEDTLS_GET_UINT16_BE( p, 0 ); extension_data_len = MBEDTLS_GET_UINT16_BE( p, 2 ); p += 4; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, ext_end, extension_data_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extensions_end, extension_data_len ); switch( extension_type ) { @@ -1210,16 +1198,20 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, case MBEDTLS_TLS_EXT_PRE_SHARED_KEY: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found pre_shared_key extension." ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "pre_shared_key:Not supported yet" ) ); - break; + + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT, + MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); + return( MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) case MBEDTLS_TLS_EXT_KEY_SHARE: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key_shares extension" ) ); - if( ( ret = ssl_tls1_3_parse_key_share_ext( ssl, + if( ( ret = ssl_tls13_parse_key_share_ext( ssl, p, p + extension_data_len ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, - "ssl_tls1_3_parse_key_share_ext", + "ssl_tls13_parse_key_share_ext", ret ); return( ret ); } @@ -1227,9 +1219,15 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ default: - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "unknown extension found: %u ( ignoring )", - extension_type ) ); + MBEDTLS_SSL_DEBUG_MSG( + 3, + ( "unknown extension found: %u ( ignoring )", + extension_type ) ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT, + MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); + return( MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); } p += extension_data_len; @@ -1242,7 +1240,7 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) { int ret; mbedtls_ssl_key_set traffic_keys; - mbedtls_ssl_transform *transform_handshake; + mbedtls_ssl_transform *transform_handshake = NULL; /* We need to set the key exchange algorithm based on the * following rules: @@ -1280,7 +1278,8 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) { /* ELSE case */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + goto cleanup; } /* Start the TLS 1.3 key schedule: Set the PSK and derive early secret. @@ -1293,7 +1292,7 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_key_schedule_stage_early_data", ret ); - return( ret ); + goto cleanup; } /* Compute handshake secret */ @@ -1301,7 +1300,7 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_master_secret", ret ); - return( ret ); + goto cleanup; } /* Next evolution in key schedule: Establish handshake secret and @@ -1311,13 +1310,16 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_generate_handshake_keys", ret ); - return( ret ); + goto cleanup; } transform_handshake = mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); if( transform_handshake == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + { + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto cleanup; + } ret = mbedtls_ssl_tls13_populate_transform( transform_handshake, ssl->conf->endpoint, @@ -1327,7 +1329,7 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_populate_transform", ret ); - return( ret ); + goto cleanup; } ssl->handshake->transform_handshake = transform_handshake; @@ -1341,17 +1343,27 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) */ mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); +cleanup: + mbedtls_platform_zeroize( &traffic_keys, sizeof( traffic_keys ) ); - return( 0 ); + if( ret != 0 ) + { + if( transform_handshake != NULL ) + mbedtls_free( transform_handshake ); + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + return( ret ); } /* - * Wait and Parse ServerHello handshake message. + * Wait and parse ServerHello handshake message. * Handler for MBEDTLS_SSL_SERVER_HELLO */ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *buf; size_t buf_len; @@ -1384,12 +1396,10 @@ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) } else if( ret == SSL_SERVER_HELLO_COORDINATE_HRR ) { - /* TODO: Implement HRR in future #4915 */ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "HRR hasn't been implemented" ) ); - - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, - MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; + MBEDTLS_SSL_DEBUG_MSG( 1, ( "HRR not supported" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE , + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; } cleanup: From c068b6671e2e2748f968d8b4dd4da583f2c25926 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 11 Oct 2021 22:30:19 +0800 Subject: [PATCH 13/23] Rename tls13 prefix to fix coding issues Signed-off-by: Jerry Yu --- library/ecdh.c | 18 +++++++-------- library/ecdh_misc.h | 6 ++--- library/ssl_misc.h | 2 +- library/ssl_tls13_client.c | 45 +++++++++++++++++++------------------- library/ssl_tls13_keys.c | 24 ++++++++++---------- library/ssl_tls13_keys.h | 6 ++--- 6 files changed, 51 insertions(+), 50 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 8884260e8..27e5d739c 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -806,9 +806,9 @@ int mbedtls_ecdh_setup_no_everest( mbedtls_ecdh_context *ctx, #endif } -static int ecdh_tls1_3_read_public_internal( mbedtls_ecdh_context_mbed *ctx, - const unsigned char *buf, - size_t blen ) +static int ecdh_tls13_read_public_internal( mbedtls_ecdh_context_mbed *ctx, + const unsigned char *buf, + size_t blen ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; @@ -838,15 +838,15 @@ static int ecdh_tls1_3_read_public_internal( mbedtls_ecdh_context_mbed *ctx, /* * Parse and import the client's TLS 1.3 public value */ -int mbedtls_ecdh_tls1_3_read_public( mbedtls_ecdh_context *ctx, - const unsigned char *buf, - size_t blen ) +int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx, + const unsigned char *buf, + size_t blen ) { ECDH_VALIDATE_RET( ctx != NULL ); ECDH_VALIDATE_RET( buf != NULL ); #if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) - return( ecdh_tls1_3_read_public_internal( ctx, buf, blen ) ); + return( ecdh_tls13_read_public_internal( ctx, buf, blen ) ); #else switch( ctx->var ) { @@ -855,8 +855,8 @@ int mbedtls_ecdh_tls1_3_read_public( mbedtls_ecdh_context *ctx, return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); #endif case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0: - return( ecdh_tls1_3_read_public_internal( &ctx->ctx.mbed_ecdh, - buf, blen ) ); + return( ecdh_tls13_read_public_internal( &ctx->ctx.mbed_ecdh, + buf, blen ) ); default: return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; } diff --git a/library/ecdh_misc.h b/library/ecdh_misc.h index 94d31394b..228f54a31 100644 --- a/library/ecdh_misc.h +++ b/library/ecdh_misc.h @@ -46,9 +46,9 @@ int mbedtls_ecdh_tls13_make_params( mbedtls_ecdh_context *ctx, size_t *olen, /* * TLS 1.3 version of mbedtls_ecdh_read_public in ecdh.h */ -int mbedtls_ecdh_tls1_3_read_public( mbedtls_ecdh_context *ctx, - const unsigned char *buf, - size_t blen ); +int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx, + const unsigned char *buf, + size_t blen ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index b801499ca..216035933 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -737,7 +737,7 @@ struct mbedtls_ssl_handshake_params unsigned char app [MBEDTLS_TLS1_3_MD_MAX_SIZE]; } tls1_3_master_secrets; - mbedtls_ssl_tls1_3_handshake_secrets tls1_3_hs_secrets; + mbedtls_ssl_tls1_3_handshake_secrets tls13_hs_secrets; #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 57195b329..768caed96 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -95,9 +95,9 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls1_3_parse_supported_versions_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t buf_len ) +static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t buf_len ) { ((void) ssl); @@ -477,7 +477,7 @@ cleanup: #if defined(MBEDTLS_ECDH_C) -static int ssl_tls1_3_check_ecdh_params( const mbedtls_ssl_context *ssl ) +static int ssl_tls13_check_ecdh_params( const mbedtls_ssl_context *ssl ) { const mbedtls_ecp_curve_info *curve_info; mbedtls_ecp_group_id grp_id; @@ -511,13 +511,13 @@ static int ssl_tls1_3_check_ecdh_params( const mbedtls_ssl_context *ssl ) * * The server only provides a single KeyShareEntry. */ -static int ssl_tls1_3_read_public_ecdhe_share( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t buf_len ) +static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t buf_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - ret = mbedtls_ecdh_tls1_3_read_public( &ssl->handshake->ecdh_ctx, + ret = mbedtls_ecdh_tls13_read_public( &ssl->handshake->ecdh_ctx, buf, buf_len ); if( ret != 0 ) { @@ -525,9 +525,9 @@ static int ssl_tls1_3_read_public_ecdhe_share( mbedtls_ssl_context *ssl, return( ret ); } - if( ssl_tls1_3_check_ecdh_params( ssl ) != 0 ) + if( ssl_tls13_check_ecdh_params( ssl ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_tls1_3_check_ecdh_params() failed!" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_tls13_check_ecdh_params() failed!" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); @@ -579,7 +579,7 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) { /* Complete ECDHE key agreement */ - ret = ssl_tls1_3_read_public_ecdhe_share( ssl, p, end - p ); + ret = ssl_tls13_read_public_ecdhe_share( ssl, p, end - p ); if( ret != 0 ) return( ret ); } @@ -1028,9 +1028,9 @@ static int ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, * Extension extensions<6..2 ^ 16 - 1>; * } ServerHello; */ -static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { int ret; const unsigned char *p = buf; @@ -1189,7 +1189,7 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported_versions extension" ) ); - ret = ssl_tls1_3_parse_supported_versions_ext( ssl, + ret = ssl_tls13_parse_supported_versions_ext( ssl, p, extension_data_len ); if( ret != 0 ) return( ret ); @@ -1236,7 +1236,7 @@ static int ssl_tls1_3_parse_server_hello( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) +static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) { int ret; mbedtls_ssl_key_set traffic_keys; @@ -1305,11 +1305,11 @@ static int ssl_tls1_3_finalize_server_hello( mbedtls_ssl_context *ssl ) /* Next evolution in key schedule: Establish handshake secret and * key material. */ - ret = mbedtls_ssl_tls1_3_generate_handshake_keys( ssl, &traffic_keys ); + ret = mbedtls_ssl_tls13_generate_handshake_keys( ssl, &traffic_keys ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, - "mbedtls_ssl_tls1_3_generate_handshake_keys", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_generate_handshake_keys", + ret ); goto cleanup; } @@ -1350,6 +1350,7 @@ cleanup: { if( transform_handshake != NULL ) mbedtls_free( transform_handshake ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); @@ -1385,14 +1386,14 @@ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) */ if( ret == SSL_SERVER_HELLO_COORDINATE_HELLO ) { - MBEDTLS_SSL_PROC_CHK( ssl_tls1_3_parse_server_hello( ssl, buf, - buf + buf_len ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_server_hello( ssl, buf, + buf + buf_len ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, buf_len ); - MBEDTLS_SSL_PROC_CHK( ssl_tls1_3_finalize_server_hello( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) ); } else if( ret == SSL_SERVER_HELLO_COORDINATE_HRR ) { diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 7ba5b5f8f..b568f3fd8 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -847,10 +847,10 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ) return( 0 ); } -/* mbedtls_ssl_tls1_3_generate_handshake_keys() generates keys necessary for +/* mbedtls_ssl_tls13_generate_handshake_keys() generates keys necessary for * protecting the handshake messages, as described in Section 7 of TLS 1.3. */ -int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, - mbedtls_ssl_key_set *traffic_keys ) +int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, + mbedtls_ssl_key_set *traffic_keys ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -864,7 +864,7 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, mbedtls_cipher_info_t const *cipher_info; size_t keylen, ivlen; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls1_3_generate_handshake_keys" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls13_generate_handshake_keys" ) ); cipher_info = mbedtls_cipher_info_from_type( ssl->handshake->ciphersuite_info->cipher ); @@ -890,7 +890,7 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, ret = mbedtls_ssl_tls1_3_derive_handshake_secrets( md_type, ssl->handshake->tls1_3_master_secrets.handshake, transcript, transcript_len, - &ssl->handshake->tls1_3_hs_secrets ); + &ssl->handshake->tls13_hs_secrets ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_handshake_secrets", @@ -899,11 +899,11 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, } MBEDTLS_SSL_DEBUG_BUF( 4, "Client handshake traffic secret", - ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret, + ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret, md_size ); MBEDTLS_SSL_DEBUG_BUF( 4, "Server handshake traffic secret", - ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret, + ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret, md_size ); /* @@ -914,7 +914,7 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, { ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_HANDSHAKE_TRAFFIC_SECRET, - ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret, + ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret, md_size, ssl->handshake->randbytes + 32, ssl->handshake->randbytes, @@ -922,7 +922,7 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_HANDSHAKE_TRAFFIC_SECRET, - ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret, + ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret, md_size, ssl->handshake->randbytes + 32, ssl->handshake->randbytes, @@ -931,8 +931,8 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXPORT_KEYS */ ret = mbedtls_ssl_tls1_3_make_traffic_keys( md_type, - ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret, - ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret, + ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret, + ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret, md_size, keylen, ivlen, traffic_keys ); if( ret != 0 ) @@ -957,7 +957,7 @@ int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, traffic_keys->server_write_iv, traffic_keys->iv_len); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_generate_handshake_keys" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls13_generate_handshake_keys" ) ); exit: diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 71bd90de2..384f433b5 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -539,7 +539,7 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_early( mbedtls_ssl_context *ssl ); * with states Initial -> Early -> Handshake -> Application, and * this function represents the Early -> Handshake transition. * - * In the handshake stage, mbedtls_ssl_tls1_3_generate_handshake_keys() + * In the handshake stage, mbedtls_ssl_tls13_generate_handshake_keys() * can be used to derive the handshake traffic keys. * * \param ssl The SSL context to operate on. This must be in key schedule @@ -562,7 +562,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls1_3_generate_handshake_keys( mbedtls_ssl_context *ssl, - mbedtls_ssl_key_set *traffic_keys ); +int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, + mbedtls_ssl_key_set *traffic_keys ); #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ From f532bb2577180650174bfd227ca946a410328f59 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Oct 2021 10:34:03 +0800 Subject: [PATCH 14/23] Change MD size for tls13 keys Signed-off-by: Jerry Yu --- library/ssl_misc.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 216035933..fb611662c 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -510,15 +510,15 @@ typedef struct mbedtls_ssl_key_set mbedtls_ssl_key_set; typedef struct { - unsigned char binder_key [ MBEDTLS_MD_MAX_SIZE ]; - unsigned char client_early_traffic_secret [ MBEDTLS_MD_MAX_SIZE ]; - unsigned char early_exporter_master_secret[ MBEDTLS_MD_MAX_SIZE ]; + unsigned char binder_key [ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; + unsigned char client_early_traffic_secret [ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; + unsigned char early_exporter_master_secret[ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; } mbedtls_ssl_tls1_3_early_secrets; typedef struct { - unsigned char client_handshake_traffic_secret[ MBEDTLS_MD_MAX_SIZE ]; - unsigned char server_handshake_traffic_secret[ MBEDTLS_MD_MAX_SIZE ]; + unsigned char client_handshake_traffic_secret[ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; + unsigned char server_handshake_traffic_secret[ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; } mbedtls_ssl_tls1_3_handshake_secrets; typedef struct From 435208a9490ebe24764b008592360a0b958c4d9c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Oct 2021 11:22:16 +0800 Subject: [PATCH 15/23] Improve generate_handshake_keys Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index b568f3fd8..4fe1d5c65 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -858,20 +858,23 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, mbedtls_md_info_t const *md_info; size_t md_size; - unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + unsigned char transcript[MBEDTLS_TLS1_3_MD_MAX_SIZE]; size_t transcript_len; mbedtls_cipher_info_t const *cipher_info; size_t keylen, ivlen; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + const mbedtls_ssl_ciphersuite_t *ciphersuite_info = handshake->ciphersuite_info; + mbedtls_ssl_tls1_3_handshake_secrets *tls13_hs_secrets = &handshake->tls13_hs_secrets; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls13_generate_handshake_keys" ) ); - cipher_info = mbedtls_cipher_info_from_type( - ssl->handshake->ciphersuite_info->cipher ); + cipher_info = mbedtls_cipher_info_from_type( ciphersuite_info->cipher ); keylen = cipher_info->key_bitlen >> 3; ivlen = cipher_info->iv_size; - md_type = ssl->handshake->ciphersuite_info->mac; + md_type = ciphersuite_info->mac; md_info = mbedtls_md_info_from_type( md_type ); md_size = mbedtls_md_get_size( md_info ); @@ -888,9 +891,8 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, } ret = mbedtls_ssl_tls1_3_derive_handshake_secrets( md_type, - ssl->handshake->tls1_3_master_secrets.handshake, - transcript, transcript_len, - &ssl->handshake->tls13_hs_secrets ); + handshake->tls1_3_master_secrets.handshake, + transcript, transcript_len, tls13_hs_secrets ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_handshake_secrets", @@ -899,11 +901,10 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, } MBEDTLS_SSL_DEBUG_BUF( 4, "Client handshake traffic secret", - ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret, + tls13_hs_secrets->client_handshake_traffic_secret, md_size ); - MBEDTLS_SSL_DEBUG_BUF( 4, "Server handshake traffic secret", - ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret, + tls13_hs_secrets->server_handshake_traffic_secret, md_size ); /* @@ -914,27 +915,26 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, { ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_HANDSHAKE_TRAFFIC_SECRET, - ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret, + tls13_hs_secrets->client_handshake_traffic_secret, md_size, - ssl->handshake->randbytes + 32, - ssl->handshake->randbytes, + handshake->randbytes + 32, + handshake->randbytes, MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_HANDSHAKE_TRAFFIC_SECRET, - ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret, + tls13_hs_secrets->server_handshake_traffic_secret, md_size, - ssl->handshake->randbytes + 32, - ssl->handshake->randbytes, + handshake->randbytes + 32, + handshake->randbytes, MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); } #endif /* MBEDTLS_SSL_EXPORT_KEYS */ ret = mbedtls_ssl_tls1_3_make_traffic_keys( md_type, - ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret, - ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret, - md_size, - keylen, ivlen, traffic_keys ); + tls13_hs_secrets->client_handshake_traffic_secret, + tls13_hs_secrets->server_handshake_traffic_secret, + md_size, keylen, ivlen, traffic_keys ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_make_traffic_keys", ret ); From b85277e3af889e21bd779e2ba365f1af5253573a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Oct 2021 13:36:05 +0800 Subject: [PATCH 16/23] Address various issues Signed-off-by: Jerry Yu --- library/ecdh.c | 15 ++- library/ecdh_misc.h | 8 +- library/ssl_tls13_client.c | 185 +++++++++++++++++-------------------- library/ssl_tls13_keys.c | 1 - 4 files changed, 95 insertions(+), 114 deletions(-) diff --git a/library/ecdh.c b/library/ecdh.c index 27e5d739c..ddd4ef545 100644 --- a/library/ecdh.c +++ b/library/ecdh.c @@ -808,24 +808,21 @@ int mbedtls_ecdh_setup_no_everest( mbedtls_ecdh_context *ctx, static int ecdh_tls13_read_public_internal( mbedtls_ecdh_context_mbed *ctx, const unsigned char *buf, - size_t blen ) + size_t buf_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; size_t data_len; - if( blen < 3 ) + if( buf_len < 3 ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); data_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - if( data_len < 1 || data_len != ( blen - 2 ) ) + if( data_len < 1 || data_len != ( buf_len - 2 ) ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); - /* - * Save buffer start for read_binary and update buf - */ if( ( ret = mbedtls_ecp_point_read_binary( &ctx->grp, &ctx->Qp, p, data_len ) ) != 0) { @@ -840,13 +837,13 @@ static int ecdh_tls13_read_public_internal( mbedtls_ecdh_context_mbed *ctx, */ int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx, const unsigned char *buf, - size_t blen ) + size_t buf_len ) { ECDH_VALIDATE_RET( ctx != NULL ); ECDH_VALIDATE_RET( buf != NULL ); #if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) - return( ecdh_tls13_read_public_internal( ctx, buf, blen ) ); + return( ecdh_tls13_read_public_internal( ctx, buf, buf_len ) ); #else switch( ctx->var ) { @@ -856,7 +853,7 @@ int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx, #endif case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0: return( ecdh_tls13_read_public_internal( &ctx->ctx.mbed_ecdh, - buf, blen ) ); + buf, buf_len ) ); default: return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; } diff --git a/library/ecdh_misc.h b/library/ecdh_misc.h index 228f54a31..d0f338a83 100644 --- a/library/ecdh_misc.h +++ b/library/ecdh_misc.h @@ -36,19 +36,19 @@ int mbedtls_ecdh_setup_no_everest( mbedtls_ecdh_context *ctx, mbedtls_ecp_group_id grp_id ); /* - * TLS 1.3 version of mbedtls_ecdh_make_params in ecdh.h + * TLS 1.3 version of mbedtls_ecdh_make_params */ int mbedtls_ecdh_tls13_make_params( mbedtls_ecdh_context *ctx, size_t *olen, - unsigned char *buf, size_t blen, + unsigned char *buf, size_t buf_len, int ( *f_rng )( void *, unsigned char *, size_t ), void *p_rng ); /* - * TLS 1.3 version of mbedtls_ecdh_read_public in ecdh.h + * TLS 1.3 version of mbedtls_ecdh_read_public */ int mbedtls_ecdh_tls13_read_public( mbedtls_ecdh_context *ctx, const unsigned char *buf, - size_t blen ); + size_t buf_len ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 768caed96..15bf43bb1 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -97,12 +97,12 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, - size_t buf_len ) + const unsigned char *end ) { ((void) ssl); - if( buf_len != 2 || - buf[0] != MBEDTLS_SSL_MAJOR_VERSION_3 || + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2); + if( buf[0] != MBEDTLS_SSL_MAJOR_VERSION_3 || buf[1] != MBEDTLS_SSL_MINOR_VERSION_4 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "unexpected version" ) ); @@ -497,7 +497,7 @@ static int ssl_tls13_check_ecdh_params( const mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); if( mbedtls_ssl_check_curve( ssl, grp_id ) != 0 ) - return( -1 ); + return( -1 ); MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_QP ); @@ -505,12 +505,6 @@ static int ssl_tls13_check_ecdh_params( const mbedtls_ssl_context *ssl ) return( 0 ); } -/* The ssl_tls13_parse_key_share_ext() function is used - * by the client to parse a KeyShare extension in - * a Server Hello message. - * - * The server only provides a single KeyShareEntry. - */ static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t buf_len ) @@ -522,12 +516,16 @@ static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ecdh_tls13_read_public" ), ret ); - return( ret ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } if( ssl_tls13_check_ecdh_params( ssl ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "ssl_tls13_check_ecdh_params() failed!" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); @@ -538,7 +536,9 @@ static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_ECDH_C */ /* - * Parse key_share extension in Server Hello + * ssl_tls13_parse_key_share_ext() + * Parse key_share extension in Server Hello + * * struct { * KeyShareEntry server_share; * } KeyShareServerHello; @@ -551,7 +551,7 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; uint16_t group, offered_group; @@ -583,8 +583,9 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, if( ret != 0 ) return( ret ); } + else #endif /* MBEDTLS_ECDH_C */ - else if( 0 /* other KEMs? */ ) + if( 0 /* other KEMs? */ ) { /* Do something */ } @@ -883,9 +884,18 @@ cleanup: /* * Functions for parsing and processing Server Hello */ -static int ssl_server_hello_is_hrr( unsigned const char *buf, size_t blen ) +/* Fetch and preprocess + * Returns a negative value on failure, and otherwise + * - SSL_SERVER_HELLO_COORDINATE_HELLO or + * - SSL_SERVER_HELLO_COORDINATE_HRR + * to indicate which message is expected and to be parsed next. */ +#define SSL_SERVER_HELLO_COORDINATE_HELLO 0 +#define SSL_SERVER_HELLO_COORDINATE_HRR 1 +static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { - static const unsigned char magic_hrr_string[32] = + static const unsigned char magic_hrr_string[SERVER_HELLO_RANDOM_LEN] = { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, @@ -902,31 +912,23 @@ static int ssl_server_hello_is_hrr( unsigned const char *buf, size_t blen ) * opaque legacy_session_id_echo<0..32>; * CipherSuite cipher_suite; * uint8 legacy_compression_method = 0; - * Extension extensions<6..2 ^ 16 - 1>; + * Extension extensions<6..2^16-1>; * } ServerHello; * */ - if( blen < 2 + sizeof( magic_hrr_string ) ) - return (MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 + sizeof( magic_hrr_string ) ); if( memcmp( buf + 2, magic_hrr_string, sizeof( magic_hrr_string ) ) == 0 ) { - return( 1 ); + return( SSL_SERVER_HELLO_COORDINATE_HRR ); } - return( 0 ); + return( SSL_SERVER_HELLO_COORDINATE_HELLO ); } -/* Fetch and preprocess - * Returns a negative value on failure, and otherwise - * - SSL_SERVER_HELLO_COORDINATE_HELLO or - * - SSL_SERVER_HELLO_COORDINATE_HRR - * to indicate which message is expected and to be parsed next. */ -#define SSL_SERVER_HELLO_COORDINATE_HELLO 0 -#define SSL_SERVER_HELLO_COORDINATE_HRR 1 -static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl, - unsigned char **buf, - size_t *buf_len ) +static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, + unsigned char **buf, + size_t *buf_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -945,15 +947,15 @@ static int ssl_server_hello_coordinate( mbedtls_ssl_context *ssl, *buf = ssl->in_msg + 4; *buf_len = ssl->in_hslen - 4; - if( ssl_server_hello_is_hrr( *buf, *buf_len ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); - ret = SSL_SERVER_HELLO_COORDINATE_HRR; - } - else + ret = ssl_server_hello_is_hrr( ssl, *buf, *buf + *buf_len ); + switch( ret ) { + case SSL_SERVER_HELLO_COORDINATE_HELLO: MBEDTLS_SSL_DEBUG_MSG( 2, ( "received ServerHello message" ) ); - ret = SSL_SERVER_HELLO_COORDINATE_HELLO; + break; + case SSL_SERVER_HELLO_COORDINATE_HRR: + MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); + break; } cleanup: @@ -977,10 +979,6 @@ static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ss if( ssl->session_negotiate->id_len != legacy_session_id_echo_len || memcmp( ssl->session_negotiate->id, p , legacy_session_id_echo_len ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Mismatch of session id length:" - " id_len = %" MBEDTLS_PRINTF_SIZET - " , legacy_session_id_echo_len = %" MBEDTLS_PRINTF_SIZET, - ssl->session_negotiate->id_len, legacy_session_id_echo_len ) ); MBEDTLS_SSL_DEBUG_BUF( 3, "Expected Session ID", ssl->session_negotiate->id, ssl->session_negotiate->id_len ); @@ -1025,17 +1023,17 @@ static int ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, * opaque legacy_session_id_echo<0..32>; * CipherSuite cipher_suite; * uint8 legacy_compression_method = 0; - * Extension extensions<6..2 ^ 16 - 1>; + * Extension extensions<6..2^16-1>; * } ServerHello; */ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; - size_t extensions_len; /* Length of field */ - const unsigned char *extensions_end; /* Pointer to end of individual extension */ + size_t extensions_len; + const unsigned char *extensions_end; uint16_t cipher_suite; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; @@ -1054,7 +1052,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", p, 2 ); /* ... - * ProtocaolVersion legacy_version = 0x0303; // TLS 1.2 + * ProtocolVersion legacy_version = 0x0303; // TLS 1.2 * ... * with ProtocolVersion defined as: * uint16 ProtocolVersion; @@ -1112,9 +1110,8 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, if( ciphersuite_info == NULL || ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) == 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite info for %04x not found", + MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite(%04x) not found or not offered", cipher_suite ) ); - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); @@ -1142,17 +1139,16 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); if( p[0] != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad legacy compression method" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } p++; - /* - * .... - * Extension extensions<6..2 ^ 16 - 1>; - * .... + /* ... + * Extension extensions<6..2^16-1>; + * ... * struct { * ExtensionType extension_type; (2 bytes) * opaque extension_data<0..2^16-1>; @@ -1166,9 +1162,6 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); extensions_end = p + extensions_len; - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "server hello, total extension length: %" MBEDTLS_PRINTF_SIZET , - extensions_len ) ); MBEDTLS_SSL_DEBUG_BUF( 3, "server hello extensions", p, extensions_len ); while( p < extensions_end ) @@ -1190,7 +1183,8 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, ( "found supported_versions extension" ) ); ret = ssl_tls13_parse_supported_versions_ext( ssl, - p, extension_data_len ); + p, + p + extension_data_len ); if( ret != 0 ) return( ret ); break; @@ -1238,45 +1232,39 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ssl_key_set traffic_keys; mbedtls_ssl_transform *transform_handshake = NULL; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; - /* We need to set the key exchange algorithm based on the - * following rules: - * - * 1) IF PRE_SHARED_KEY extension was received - * THEN set KEY_EXCHANGE_MODE_PSK_EPHEMERAL; - * 2) IF PRE_SHARED_KEY extension && KEY_SHARE was received - * THEN set KEY_EXCHANGE_MODE_PSK; - * 3) IF KEY_SHARES extension was received && SIG_ALG extension received - * THEN set KEY_EXCHANGE_MODE_EPHEMERAL - * ELSE unknown key exchange mechanism. + /* Determine the key exchange mode: + * 1) If both the pre_shared_key and key_share extensions were received + * then the key exchange mode is PSK with EPHEMERAL. + * 2) If only the pre_shared_key extension was received then the key + * exchange mode is PSK-only. + * 3) If only the key_share extension was received then the key + * exchange mode is EPHEMERAL-only. */ - if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PRE_SHARED_KEY ) + switch( handshake->extensions_present & + ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ) ) { - if( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) - { - /* Condition 2) */ - ssl->handshake->tls1_3_kex_modes = - MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; - } - else - { - /* Condition 1) */ - ssl->handshake->tls1_3_kex_modes = - MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; - } - } - else if( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) ) - { - /* Condition 3) */ - ssl->handshake->tls1_3_kex_modes = - MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; - } - else - { - /* ELSE case */ + /* Only the pre_shared_key extension was received */ + case MBEDTLS_SSL_EXT_PRE_SHARED_KEY: + handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; + break; + + /* Only the key_share extension was received */ + case MBEDTLS_SSL_EXT_KEY_SHARE: + handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; + break; + + /* Both the pre_shared_key and key_share extensions were received */ + case ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ): + handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + break; + + /* Neither pre_shared_key nor key_share extension was received */ + default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) ); ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; goto cleanup; @@ -1313,8 +1301,7 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) goto cleanup; } - transform_handshake = - mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); + transform_handshake = mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); if( transform_handshake == NULL ) { ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; @@ -1332,8 +1319,8 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) goto cleanup; } - ssl->handshake->transform_handshake = transform_handshake; - mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); + handshake->transform_handshake = transform_handshake; + mbedtls_ssl_set_inbound_transform( ssl, transform_handshake ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); ssl->session_in = ssl->session_negotiate; @@ -1348,8 +1335,7 @@ cleanup: mbedtls_platform_zeroize( &traffic_keys, sizeof( traffic_keys ) ); if( ret != 0 ) { - if( transform_handshake != NULL ) - mbedtls_free( transform_handshake ); + mbedtls_free( transform_handshake ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, @@ -1375,11 +1361,10 @@ static int ssl_tls1_3_process_server_hello( mbedtls_ssl_context *ssl ) * - Make sure it's either a ServerHello or a HRR. * - Switch processing routine in case of HRR */ - ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; - ret = ssl_server_hello_coordinate( ssl, &buf, &buf_len ); + ret = ssl_tls13_server_hello_coordinate( ssl, &buf, &buf_len ); /* Parsing step * We know what message to expect by now and call * the respective parsing function. diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 4fe1d5c65..35829f232 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1000,7 +1000,6 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) } else if( mbedtls_ssl_tls13_named_group_is_dhe( handshake->offered_group_id ) ) { - /* TODO: Not supported yet */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "DHE not supported." ) ); return( MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ); } From 193f0e74497fc97e56a31a075fb395cca8e85494 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Oct 2021 18:33:13 +0800 Subject: [PATCH 17/23] fix build fail on tls1_3_md_max_size Signed-off-by: Jerry Yu --- library/ssl_misc.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index fb611662c..ceaf0588f 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -307,9 +307,7 @@ + ( MBEDTLS_SSL_CID_OUT_LEN_MAX ) ) #endif -#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) #define MBEDTLS_TLS1_3_MD_MAX_SIZE MBEDTLS_MD_MAX_SIZE -#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) /** From 745bb616a47d48378f4e4f2ce572f0f1678c4e96 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 13 Oct 2021 22:01:04 +0800 Subject: [PATCH 18/23] Fix format issue and enhance test Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 51 +++++++++++++++++++++----------------- tests/ssl-opt.sh | 9 +++++-- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 15bf43bb1..2924fd8b6 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -926,6 +926,11 @@ static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, return( SSL_SERVER_HELLO_COORDINATE_HELLO ); } +/* Fetch and preprocess + * Returns a negative value on failure, and otherwise + * - SSL_SERVER_HELLO_COORDINATE_HELLO or + * - SSL_SERVER_HELLO_COORDINATE_HRR + */ static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, unsigned char **buf, size_t *buf_len ) @@ -950,12 +955,12 @@ static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, ret = ssl_server_hello_is_hrr( ssl, *buf, *buf + *buf_len ); switch( ret ) { - case SSL_SERVER_HELLO_COORDINATE_HELLO: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "received ServerHello message" ) ); - break; - case SSL_SERVER_HELLO_COORDINATE_HRR: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); - break; + case SSL_SERVER_HELLO_COORDINATE_HELLO: + MBEDTLS_SSL_DEBUG_MSG( 2, ( "received ServerHello message" ) ); + break; + case SSL_SERVER_HELLO_COORDINATE_HRR: + MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); + break; } cleanup: @@ -1248,26 +1253,26 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) switch( handshake->extensions_present & ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ) ) { - /* Only the pre_shared_key extension was received */ - case MBEDTLS_SSL_EXT_PRE_SHARED_KEY: - handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; - break; + /* Only the pre_shared_key extension was received */ + case MBEDTLS_SSL_EXT_PRE_SHARED_KEY: + handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK; + break; - /* Only the key_share extension was received */ - case MBEDTLS_SSL_EXT_KEY_SHARE: - handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; - break; + /* Only the key_share extension was received */ + case MBEDTLS_SSL_EXT_KEY_SHARE: + handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_EPHEMERAL; + break; - /* Both the pre_shared_key and key_share extensions were received */ - case ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ): - handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; - break; + /* Both the pre_shared_key and key_share extensions were received */ + case ( MBEDTLS_SSL_EXT_PRE_SHARED_KEY | MBEDTLS_SSL_EXT_KEY_SHARE ): + handshake->tls1_3_kex_modes = MBEDTLS_SSL_TLS13_KEY_EXCHANGE_MODE_PSK_EPHEMERAL; + break; - /* Neither pre_shared_key nor key_share extension was received */ - default: - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) ); - ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; - goto cleanup; + /* Neither pre_shared_key nor key_share extension was received */ + default: + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unknown key exchange." ) ); + ret = MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; + goto cleanup; } /* Start the TLS 1.3 key schedule: Set the PSK and derive early secret. diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2b91025fd..ad7abbba0 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8678,7 +8678,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS1.3: Test client hello msg work - openssl" \ "$O_NEXT_SRV -tls1_3 -msg" \ - "$P_CLI debug_level=2 min_version=tls1_3 max_version=tls1_3" \ + "$P_CLI debug_level=3 min_version=tls1_3 max_version=tls1_3" \ 1 \ -c "SSL - The requested feature is not available" \ -s "ServerHello" \ @@ -8695,6 +8695,8 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "tls1_3 client state: 14" \ -c "tls1_3 client state: 15" \ -c "<= ssl_tls1_3_process_server_hello" \ + -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ + -c "ECDH curve: x25519" \ -c "=> ssl_tls1_3_process_server_hello" requires_gnutls_tls1_3 @@ -8702,7 +8704,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS1.3: Test client hello msg work - gnutls" \ "$G_NEXT_SRV --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3 --debug=4" \ - "$P_CLI debug_level=2 min_version=tls1_3 max_version=tls1_3" \ + "$P_CLI debug_level=3 min_version=tls1_3 max_version=tls1_3" \ 1 \ -c "SSL - The requested feature is not available" \ -s "SERVER HELLO was queued" \ @@ -8719,8 +8721,11 @@ run_test "TLS1.3: Test client hello msg work - gnutls" \ -c "tls1_3 client state: 14" \ -c "tls1_3 client state: 15" \ -c "<= ssl_tls1_3_process_server_hello" \ + -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ + -c "ECDH curve: x25519" \ -c "=> ssl_tls1_3_process_server_hello" + # Test heap memory usage after handshake requires_config_enabled MBEDTLS_MEMORY_DEBUG requires_config_enabled MBEDTLS_MEMORY_BUFFER_ALLOC_C From 337d5318aed405853849be13a16a2a4e4dc05592 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 15 Oct 2021 10:09:05 +0800 Subject: [PATCH 19/23] replace md_max_size with tls13_md_max_size Signed-off-by: Jerry Yu --- library/ssl_misc.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index ceaf0588f..904d8c77d 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -521,10 +521,10 @@ typedef struct typedef struct { - unsigned char client_application_traffic_secret_N[ MBEDTLS_MD_MAX_SIZE ]; - unsigned char server_application_traffic_secret_N[ MBEDTLS_MD_MAX_SIZE ]; - unsigned char exporter_master_secret [ MBEDTLS_MD_MAX_SIZE ]; - unsigned char resumption_master_secret [ MBEDTLS_MD_MAX_SIZE ]; + unsigned char client_application_traffic_secret_N[ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; + unsigned char server_application_traffic_secret_N[ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; + unsigned char exporter_master_secret [ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; + unsigned char resumption_master_secret [ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; } mbedtls_ssl_tls1_3_application_secrets; /* From 7a186a0cbfc9e8b1b6c7184f5df050830a3cb4d1 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 15 Oct 2021 18:46:14 +0800 Subject: [PATCH 20/23] fix comment issue Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 2924fd8b6..ca82fdcc5 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -884,8 +884,7 @@ cleanup: /* * Functions for parsing and processing Server Hello */ -/* Fetch and preprocess - * Returns a negative value on failure, and otherwise +/* Returns a negative value on failure, and otherwise * - SSL_SERVER_HELLO_COORDINATE_HELLO or * - SSL_SERVER_HELLO_COORDINATE_HRR * to indicate which message is expected and to be parsed next. */ From ad3a113fc6aa6dd8bf2255228813e8218729d53a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 25 Oct 2021 10:46:43 +0800 Subject: [PATCH 21/23] Remove MBEDTLS_SSL_EXPORT_KEYS It is always on now in `development` Signed-off-by: Jerry Yu --- library/ssl_tls13_keys.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 35829f232..96f531079 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -910,7 +910,6 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, /* * Export client handshake traffic secret */ -#if defined(MBEDTLS_SSL_EXPORT_KEYS) if( ssl->f_export_keys != NULL ) { ssl->f_export_keys( ssl->p_export_keys, @@ -929,7 +928,6 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, handshake->randbytes, MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); } -#endif /* MBEDTLS_SSL_EXPORT_KEYS */ ret = mbedtls_ssl_tls1_3_make_traffic_keys( md_type, tls13_hs_secrets->client_handshake_traffic_secret, From 188468b5f441ebab28289f8fb271ded078eab78a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Mon, 25 Oct 2021 10:48:24 +0800 Subject: [PATCH 22/23] Add reference link for Random definition Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index ca82fdcc5..989bdc0ab 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1071,7 +1071,8 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, } p += 2; - /* ... + /* From RFC8446, page 27. + * ... * Random random; * ... * with Random defined as: From e6d7e5cef6340a70055efa10378704c9c619ab61 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 26 Oct 2021 10:44:32 +0800 Subject: [PATCH 23/23] move CLIENT/SERVER_HELLO_RANDOM_LEN to `ssl_misc.h` Signed-off-by: Jerry Yu --- library/ssl_misc.h | 11 +++++++++-- library/ssl_tls13_client.c | 32 ++++++++++++++------------------ 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 904d8c77d..66fb26c62 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -309,6 +309,9 @@ #define MBEDTLS_TLS1_3_MD_MAX_SIZE MBEDTLS_MD_MAX_SIZE +#define MBEDTLS_CLIENT_HELLO_RANDOM_LEN 32 +#define MBEDTLS_SERVER_HELLO_RANDOM_LEN 32 + #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) /** * \brief Return the maximum fragment length (payload, in bytes) for @@ -715,7 +718,9 @@ struct mbedtls_ssl_handshake_params size_t pmslen; /*!< premaster length */ - unsigned char randbytes[64]; /*!< random bytes */ + unsigned char randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN + + MBEDTLS_SERVER_HELLO_RANDOM_LEN]; + /*!< random bytes */ unsigned char premaster[MBEDTLS_PREMASTER_SIZE]; /*!< premaster secret */ @@ -880,7 +885,9 @@ struct mbedtls_ssl_transform /* We need the Hello random bytes in order to re-derive keys from the * Master Secret and other session info, * see ssl_tls12_populate_transform() */ - unsigned char randbytes[64]; /*!< ServerHello.random+ClientHello.random */ + unsigned char randbytes[MBEDTLS_SERVER_HELLO_RANDOM_LEN + + MBEDTLS_CLIENT_HELLO_RANDOM_LEN]; + /*!< ServerHello.random+ClientHello.random */ #endif /* MBEDTLS_SSL_CONTEXT_SERIALIZATION */ }; diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 989bdc0ab..979db3144 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -35,9 +35,6 @@ #include "ecdh_misc.h" #include "ssl_tls13_keys.h" -#define CLIENT_HELLO_RANDOM_LEN 32 -#define SERVER_HELLO_RANDOM_LEN 32 - /* Write extensions */ /* @@ -709,11 +706,11 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p += 2; /* Write the random bytes ( random ).*/ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, CLIENT_HELLO_RANDOM_LEN ); - memcpy( p, ssl->handshake->randbytes, CLIENT_HELLO_RANDOM_LEN ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_CLIENT_HELLO_RANDOM_LEN ); + memcpy( p, ssl->handshake->randbytes, MBEDTLS_CLIENT_HELLO_RANDOM_LEN ); MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", - p, CLIENT_HELLO_RANDOM_LEN ); - p += CLIENT_HELLO_RANDOM_LEN; + p, MBEDTLS_CLIENT_HELLO_RANDOM_LEN ); + p += MBEDTLS_CLIENT_HELLO_RANDOM_LEN; /* * Write legacy_session_id @@ -834,7 +831,7 @@ static int ssl_tls13_prepare_client_hello( mbedtls_ssl_context *ssl ) if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, ssl->handshake->randbytes, - CLIENT_HELLO_RANDOM_LEN ) ) != 0 ) + MBEDTLS_CLIENT_HELLO_RANDOM_LEN ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); return( ret ); @@ -894,7 +891,7 @@ static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - static const unsigned char magic_hrr_string[SERVER_HELLO_RANDOM_LEN] = + static const unsigned char magic_hrr_string[MBEDTLS_SERVER_HELLO_RANDOM_LEN] = { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, 0x8C, 0x5E, @@ -1045,12 +1042,12 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, * Check there is space for minimal fields * * - legacy_version ( 2 bytes) - * - random (SERVER_HELLO_RANDOM_LEN bytes) + * - random (MBEDTLS_SERVER_HELLO_RANDOM_LEN bytes) * - legacy_session_id_echo ( 1 byte ), minimum size * - cipher_suite ( 2 bytes) * - legacy_compression_method ( 1 byte ) */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, SERVER_HELLO_RANDOM_LEN + 6 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN + 6 ); MBEDTLS_SSL_DEBUG_BUF( 4, "server hello", p, end - p ); MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", p, 2 ); @@ -1071,18 +1068,17 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, } p += 2; - /* From RFC8446, page 27. - * ... + /* ... * Random random; * ... * with Random defined as: - * opaque Random[32]; + * opaque Random[MBEDTLS_SERVER_HELLO_RANDOM_LEN]; */ - memcpy( ssl->handshake->randbytes + CLIENT_HELLO_RANDOM_LEN, p, - SERVER_HELLO_RANDOM_LEN ); + memcpy( &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], p, + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", - p, SERVER_HELLO_RANDOM_LEN ); - p += SERVER_HELLO_RANDOM_LEN; + p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; /* ... * opaque legacy_session_id_echo<0..32>;