From aa5f5c1f5d1256955a690f04613e03683467a875 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sat, 18 Sep 2021 06:20:25 +0000 Subject: [PATCH 01/21] TLS1.3: Add server finish processing in client side Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 12 ++- library/ssl_misc.h | 103 +++++++++++++++++++ library/ssl_tls13_generic.c | 153 ++++++++++++++++++++++++++++ library/ssl_tls13_keys.c | 194 ++++++++++++++++++++++++++++++++++++ library/ssl_tls13_keys.h | 61 ++++++++++++ 5 files changed, 522 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 5d04a115f..3e1018125 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -39,7 +39,6 @@ #if defined(MBEDTLS_DHM_C) #include "mbedtls/dhm.h" #endif - /* Adding guard for MBEDTLS_ECDSA_C to ensure no compile errors due * to guards also being in ssl_srv.c and ssl_cli.c. There is a gap * in functionality that access to ecdh_ctx structure is needed for @@ -637,6 +636,7 @@ typedef enum MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + MBEDTLS_SSL_END_OF_EARLY_DATA, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY, #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ @@ -1050,6 +1050,14 @@ typedef void mbedtls_ssl_async_cancel_t( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED && !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +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; + #if defined(MBEDTLS_SSL_DTLS_SRTP) #define MBEDTLS_TLS_SRTP_MAX_MKI_LENGTH 255 @@ -1114,6 +1122,8 @@ struct mbedtls_ssl_session * to be studied whether one of them can be removed. */ unsigned char MBEDTLS_PRIVATE(minor_ver); /*!< The TLS version used in the session. */ + mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets); + #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) mbedtls_x509_crt *MBEDTLS_PRIVATE(peer_cert); /*!< peer X.509 cert chain */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 23d5970d9..ae6cbfab5 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -719,6 +719,104 @@ struct mbedtls_ssl_handshake_params * but can be overwritten by the HRR. */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ + /* + * State-local variables used during the processing + * of a specific handshake state. + */ + union + { + /* Outgoing Finished message */ + struct + { + uint8_t preparation_done; + + /* Buffer holding digest of the handshake up to + * but excluding the outgoing finished message. */ + unsigned char digest[MBEDTLS_MD_MAX_SIZE]; + size_t digest_len; + } finished_out; + + /* Incoming Finished message */ + struct + { + /* Buffer holding digest of the handshake up to but + * excluding the peer's incoming finished message. */ + unsigned char digest[MBEDTLS_MD_MAX_SIZE]; + size_t digest_len; + } finished_in; + +#if defined(MBEDTLS_SSL_CLI_C) + + /* Client, incoming ServerKeyExchange */ + struct + { + uint8_t preparation_done; + } srv_key_exchange; + + /* Client, incoming ServerHello */ + struct + { +#if defined(MBEDTLS_SSL_RENEGOTIATION) + int renego_info_seen; +#else + int dummy; +#endif + } srv_hello_in; + + /* Client, outgoing ClientKeyExchange */ + struct + { + uint8_t preparation_done; + } cli_key_exch_out; + + /* Client, outgoing Certificate Verify */ + struct + { + uint8_t preparation_done; + } crt_vrfy_out; + + /* Client, outgoing ClientHello */ + struct + { + uint8_t preparation_done; + } cli_hello_out; + +#endif /* MBEDTLS_SSL_CLI_C */ + +#if defined(MBEDTLS_SSL_SRV_C) + + /* Server, outgoing ClientKeyExchange */ + struct + { + uint8_t preparation_done; + } cli_key_exch_in; + + /* Server, outgoing ClientKeyExchange */ + struct + { + uint8_t preparation_done; + } encrypted_extensions_out; + +#endif /* MBEDTLS_SSL_SRV_C */ + + /* Incoming CertificateVerify */ + struct + { + unsigned char verify_buffer[ 64 + 33 + 1 + MBEDTLS_MD_MAX_SIZE ]; + size_t verify_buffer_len; + } certificate_verify_in; + + /* Outgoing CertificateVerify */ + struct + { + unsigned char handshake_hash[ MBEDTLS_MD_MAX_SIZE ]; + size_t handshake_hash_len; + } certificate_verify_out; + + } state_local; + + /* End of state-local variables. */ + mbedtls_ssl_ciphersuite_t const *ciphersuite_info; size_t pmslen; /*!< premaster length */ @@ -1162,6 +1260,11 @@ static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_read_certificate_process(mbedtls_ssl_context *ssl); +int mbedtls_ssl_write_certificate_process(mbedtls_ssl_context *ssl); +int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls1_3_finished_out_process( mbedtls_ssl_context *ssl ); + int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 75b11c93a..c9bf78e6a 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -845,6 +845,159 @@ cleanup: return( ret ); } +/* + * + * STATE HANDLING: Incoming Finished + * Overview + */ + +/* Main entry point: orchestrates the other functions */ +int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context* ssl ); + +static int ssl_finished_in_preprocess( mbedtls_ssl_context* ssl ); +static int ssl_finished_in_postprocess( mbedtls_ssl_context* ssl ); +static int ssl_finished_in_parse( mbedtls_ssl_context* ssl, + const unsigned char* buf, + size_t buflen ); + +/* + * Implementation + */ + +int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context* ssl ) +{ + int ret = 0; + unsigned char *buf; + size_t buflen; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished" ) ); + + /* Preprocessing step: Compute handshake digest */ + MBEDTLS_SSL_PROC_CHK( ssl_finished_in_preprocess( ssl ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( ssl, + MBEDTLS_SSL_HS_FINISHED, + &buf, &buflen ) ); + MBEDTLS_SSL_PROC_CHK( ssl_finished_in_parse( ssl, buf, buflen ) ); + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); + MBEDTLS_SSL_PROC_CHK( ssl_finished_in_postprocess( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished" ) ); + return( ret ); +} + +static int ssl_finished_in_preprocess( mbedtls_ssl_context* ssl ) +{ + int ret; + + ret = mbedtls_ssl_tls1_3_calc_finished( ssl, + ssl->handshake->state_local.finished_in.digest, + sizeof( ssl->handshake->state_local.finished_in.digest ), + &ssl->handshake->state_local.finished_in.digest_len, + ssl->conf->endpoint ^ 1 ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_calc_finished", ret ); + return( ret ); + } + + return( 0 ); +} + +static int ssl_finished_in_parse( mbedtls_ssl_context* ssl, + const unsigned char* buf, + size_t buflen ) +{ + /* Structural validation */ + if( buflen != ssl->handshake->state_local.finished_in.digest_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished 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_DEBUG_BUF( 4, "Hash (self-computed):", + ssl->handshake->state_local.finished_in.digest, + ssl->handshake->state_local.finished_in.digest_len ); + MBEDTLS_SSL_DEBUG_BUF( 4, "Hash (received message):", buf, + ssl->handshake->state_local.finished_in.digest_len ); + + /* Semantic validation */ + if( mbedtls_ssl_safer_memcmp( buf, + ssl->handshake->state_local.finished_in.digest, + ssl->handshake->state_local.finished_in.digest_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + return( 0 ); +} + +static int ssl_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + mbedtls_ssl_key_set traffic_keys; + mbedtls_ssl_transform *transform_application; + + ret = mbedtls_ssl_tls1_3_key_schedule_stage_application( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_tls1_3_key_schedule_stage_application", ret ); + return( ret ); + } + + ret = mbedtls_ssl_tls1_3_generate_application_keys( + ssl, &traffic_keys ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_tls1_3_generate_application_keys", ret ); + return( ret ); + } + + transform_application = + mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); + if( transform_application == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + + ret = mbedtls_ssl_tls13_populate_transform( + transform_application, + 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->transform_application = transform_application; + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_END_OF_EARLY_DATA ); + return( 0 ); +} + +static int ssl_finished_in_postprocess( mbedtls_ssl_context* ssl ) +{ + + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) + { + return( ssl_finished_in_postprocess_cli( ssl ) ); + } + + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 96f531079..010d6352d 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -564,6 +564,36 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( return( 0 ); } +int mbedtls_ssl_tls1_3_key_schedule_stage_application( + 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 */ + + /* + * Compute MasterSecret + */ + + ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, + ssl->handshake->tls1_3_master_secrets.handshake, + NULL, 0, + ssl->handshake->tls1_3_master_secrets.app ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_evolve_secret", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_BUF( 4, "Master secret", + ssl->handshake->tls1_3_master_secrets.app, md_size ); + + return( 0 ); +} + static int ssl_tls1_3_calc_finished_core( mbedtls_md_type_t md_type, unsigned char const *base_key, unsigned char const *transcript, @@ -614,6 +644,54 @@ exit: return( ret ); } +int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context* ssl, + unsigned char* dst, + size_t dst_len, + size_t *actual_len, + int from ) +{ + int ret; + + unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + size_t transcript_len; + + unsigned char const *base_key = NULL; + + mbedtls_md_type_t const md_type = ssl->handshake->ciphersuite_info->mac; + const mbedtls_md_info_t* const md = mbedtls_md_info_from_type( md_type ); + size_t const md_size = mbedtls_md_get_size( md ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls1_3_calc_finished" ) ); + + if( dst_len < md_size ) + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + + 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 ); + } + MBEDTLS_SSL_DEBUG_BUF( 4, "handshake hash", transcript, transcript_len ); + + if( from == MBEDTLS_SSL_IS_CLIENT ) + base_key = ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret; + else + base_key = ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret; + + ret = ssl_tls1_3_calc_finished_core( md_type, base_key, transcript, dst ); + if( ret != 0 ) + return( ret ); + *actual_len = md_size; + + MBEDTLS_SSL_DEBUG_BUF( 3, "verify_data for finished message", dst, md_size ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calc_finished" ) ); + return( 0 ); +} + int mbedtls_ssl_tls1_3_create_psk_binder( mbedtls_ssl_context *ssl, const mbedtls_md_type_t md_type, unsigned char const *psk, size_t psk_len, @@ -1028,4 +1106,120 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) return( 0 ); } +/* Generate application traffic keys since any records following a 1-RTT Finished message + * MUST be encrypted under the application traffic key. + */ +int mbedtls_ssl_tls1_3_generate_application_keys( + mbedtls_ssl_context *ssl, + mbedtls_ssl_key_set *traffic_keys ) +{ + int ret = 0; + + /* Address at which to store the application secrets */ + mbedtls_ssl_tls1_3_application_secrets * const app_secrets = + &ssl->session_negotiate->app_secrets; + + /* Holding the transcript up to and including the ServerFinished */ + unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + size_t transcript_len; + + /* Variables relating to the hash for the chosen ciphersuite. */ + mbedtls_md_type_t md_type; + mbedtls_md_info_t const *md_info; + size_t md_size; + + /* Variables relating to the cipher for the chosen ciphersuite. */ + mbedtls_cipher_info_t const *cipher_info; + size_t keylen, ivlen; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> derive application traffic keys" ) ); + + /* Extract basic information about hash and ciphersuite */ + + cipher_info = mbedtls_cipher_info_from_type( + ssl->handshake->ciphersuite_info->cipher ); + keylen = cipher_info->key_bitlen / 8; + 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 ); + + /* Compute current handshake transcript. It's the caller's responsiblity + * to call this at the right time, that is, after the ServerFinished. */ + + ret = mbedtls_ssl_get_handshake_transcript( ssl, md_type, + transcript, sizeof( transcript ), + &transcript_len ); + if( ret != 0 ) + return( ret ); + + /* Compute application secrets from master secret and transcript hash. */ + + ret = mbedtls_ssl_tls1_3_derive_application_secrets( md_type, + ssl->handshake->tls1_3_master_secrets.app, + transcript, transcript_len, + app_secrets ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_tls1_3_derive_application_secrets", ret ); + return( ret ); + } + + /* Derive first epoch of IV + Key for application traffic. */ + + ret = mbedtls_ssl_tls1_3_make_traffic_keys( md_type, + app_secrets->client_application_traffic_secret_N, + app_secrets->server_application_traffic_secret_N, + md_size, keylen, ivlen, traffic_keys ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_make_traffic_keys", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_BUF( 4, "Client application traffic secret", + app_secrets->client_application_traffic_secret_N, + md_size ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "Server application traffic secret", + app_secrets->server_application_traffic_secret_N, + md_size ); + + /* + * Export client/server application traffic secret 0 + */ +#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_APPLICATION_TRAFFIC_SECRET, + app_secrets->client_application_traffic_secret_N, 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_APPLICATION_TRAFFIC_SECRET, + app_secrets->server_application_traffic_secret_N, md_size, + ssl->handshake->randbytes + 32, + ssl->handshake->randbytes, + MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); + } +#endif /* MBEDTLS_SSL_EXPORT_KEYS */ + + MBEDTLS_SSL_DEBUG_BUF( 4, "client application_write_key:", + traffic_keys->client_write_key, keylen ); + MBEDTLS_SSL_DEBUG_BUF( 4, "server application write key", + traffic_keys->server_write_key, keylen ); + MBEDTLS_SSL_DEBUG_BUF( 4, "client application write IV", + traffic_keys->client_write_iv, ivlen ); + MBEDTLS_SSL_DEBUG_BUF( 4, "server application write IV", + traffic_keys->server_write_iv, ivlen ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= derive application traffic keys" ) ); + return( 0 ); +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 165b58a2d..78bfc2a3d 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -570,4 +570,65 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ); int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, mbedtls_ssl_key_set *traffic_keys ); +/** + * \brief Transition into application 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 Handshake -> Application transition. + * + * In the handshake stage, mbedtls_ssl_tls1_3_generate_application_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 Handshake. + * + * \returns \c 0 on success. + * \returns A negative error code on failure. + */ +int mbedtls_ssl_tls1_3_key_schedule_stage_application( + mbedtls_ssl_context *ssl ); + +/** + * \brief Compute TLS 1.3 application traffic keys. + * + * \param ssl The SSL context to operate on. This must be in + * key schedule stage \c Application, see + * mbedtls_ssl_tls1_3_key_schedule_stage_application(). + * \param traffic_keys The address at which to store the application 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_application_keys( + mbedtls_ssl_context* ssl, mbedtls_ssl_key_set *traffic_keys ); + +/** + * \brief Calculate content of TLS 1.3 Finished message. + * + * \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_application(). + * \param dst The address at which to write the Finished content. + * \param dst_len The size of \p dst in bytes. + * \param actual_len The address at which to store the amount of data + * actually written to \p dst upon success. + * \param from The endpoint the Finished message originates from: + * - #MBEDTLS_SSL_IS_CLIENT for the Client's Finished message + * - #MBEDTLS_SSL_IS_SERVER for the Server's Finished message + * + * \note Both client and server call this function twice, once to + * generate their own Finished message, and once to verify the + * peer's Finished message. + + * \returns \c 0 on success. + * \returns A negative error code on failure. + */ +int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context *ssl, + unsigned char *dst, + size_t dst_len, + size_t *actual_len, + int from ); + #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ From 4cab0240c77b97e94840f07fd1b904073ef6a695 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 12 Oct 2021 08:43:37 +0000 Subject: [PATCH 02/21] Change coding style Signed-off-by: XiaokangQian --- library/ssl_misc.h | 21 +++------------------ library/ssl_tls13_client.c | 4 +--- library/ssl_tls13_generic.c | 30 +++++++++++++++--------------- library/ssl_tls13_keys.c | 24 +++++++++++++++++------- library/ssl_tls13_keys.h | 6 +++--- 5 files changed, 39 insertions(+), 46 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index ae6cbfab5..3b0d61b35 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -783,22 +783,6 @@ struct mbedtls_ssl_handshake_params #endif /* MBEDTLS_SSL_CLI_C */ -#if defined(MBEDTLS_SSL_SRV_C) - - /* Server, outgoing ClientKeyExchange */ - struct - { - uint8_t preparation_done; - } cli_key_exch_in; - - /* Server, outgoing ClientKeyExchange */ - struct - { - uint8_t preparation_done; - } encrypted_extensions_out; - -#endif /* MBEDTLS_SSL_SRV_C */ - /* Incoming CertificateVerify */ struct { @@ -1262,8 +1246,9 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); int mbedtls_ssl_read_certificate_process(mbedtls_ssl_context *ssl); int mbedtls_ssl_write_certificate_process(mbedtls_ssl_context *ssl); -int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls1_3_finished_out_process( mbedtls_ssl_context *ssl ); + +int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_finished_out_process( mbedtls_ssl_context *ssl ); int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index e36e28d9d..2bde4a825 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1605,9 +1605,7 @@ static int ssl_tls1_3_process_certificate_verify( mbedtls_ssl_context *ssl ) */ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); - return( 0 ); + return ( mbedtls_ssl_tls13_finished_in_process( ssl ) ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index c9bf78e6a..87bc12ce2 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -852,11 +852,11 @@ cleanup: */ /* Main entry point: orchestrates the other functions */ -int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context* ssl ); +int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ); -static int ssl_finished_in_preprocess( mbedtls_ssl_context* ssl ); -static int ssl_finished_in_postprocess( mbedtls_ssl_context* ssl ); -static int ssl_finished_in_parse( mbedtls_ssl_context* ssl, +static int ssl_tls13_finished_in_preprocess( mbedtls_ssl_context* ssl ); +static int ssl_tls13_finished_in_postprocess( mbedtls_ssl_context* ssl ); +static int ssl_tls13_finished_in_parse( mbedtls_ssl_context* ssl, const unsigned char* buf, size_t buflen ); @@ -864,7 +864,7 @@ static int ssl_finished_in_parse( mbedtls_ssl_context* ssl, * Implementation */ -int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context* ssl ) +int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ) { int ret = 0; unsigned char *buf; @@ -873,15 +873,15 @@ int mbedtls_ssl_tls1_3_finished_in_process( mbedtls_ssl_context* ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished" ) ); /* Preprocessing step: Compute handshake digest */ - MBEDTLS_SSL_PROC_CHK( ssl_finished_in_preprocess( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_preprocess( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buflen ) ); - MBEDTLS_SSL_PROC_CHK( ssl_finished_in_parse( ssl, buf, buflen ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_parse( ssl, buf, buflen ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); - MBEDTLS_SSL_PROC_CHK( ssl_finished_in_postprocess( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_postprocess( ssl ) ); cleanup: @@ -889,7 +889,7 @@ cleanup: return( ret ); } -static int ssl_finished_in_preprocess( mbedtls_ssl_context* ssl ) +static int ssl_tls13_finished_in_preprocess( mbedtls_ssl_context* ssl ) { int ret; @@ -907,7 +907,7 @@ static int ssl_finished_in_preprocess( mbedtls_ssl_context* ssl ) return( 0 ); } -static int ssl_finished_in_parse( mbedtls_ssl_context* ssl, +static int ssl_tls13_finished_in_parse( mbedtls_ssl_context* ssl, const unsigned char* buf, size_t buflen ) { @@ -941,17 +941,17 @@ static int ssl_finished_in_parse( mbedtls_ssl_context* ssl, return( 0 ); } -static int ssl_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) +static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_key_set traffic_keys; mbedtls_ssl_transform *transform_application; - ret = mbedtls_ssl_tls1_3_key_schedule_stage_application( ssl ); + ret = mbedtls_ssl_tls13_key_schedule_stage_application( ssl ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, - "mbedtls_ssl_tls1_3_key_schedule_stage_application", ret ); + "mbedtls_ssl_tls13_key_schedule_stage_application", ret ); return( ret ); } @@ -987,12 +987,12 @@ static int ssl_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_finished_in_postprocess( mbedtls_ssl_context* ssl ) +static int ssl_tls13_finished_in_postprocess( mbedtls_ssl_context* ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - return( ssl_finished_in_postprocess_cli( ssl ) ); + return( ssl_tls13_finished_in_postprocess_cli( ssl ) ); } return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 010d6352d..ddbeb626e 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -564,7 +564,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( return( 0 ); } -int mbedtls_ssl_tls1_3_key_schedule_stage_application( +int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -577,7 +577,6 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_application( /* * Compute MasterSecret */ - ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, ssl->handshake->tls1_3_master_secrets.handshake, NULL, 0, @@ -687,7 +686,6 @@ int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context* ssl, *actual_len = md_size; MBEDTLS_SSL_DEBUG_BUF( 3, "verify_data for finished message", dst, md_size ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calc_finished" ) ); return( 0 ); } @@ -1152,7 +1150,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( transcript, sizeof( transcript ), &transcript_len ); if( ret != 0 ) - return( ret ); + goto cleanup; /* Compute application secrets from master secret and transcript hash. */ @@ -1164,7 +1162,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_derive_application_secrets", ret ); - return( ret ); + goto cleanup; } /* Derive first epoch of IV + Key for application traffic. */ @@ -1176,7 +1174,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_make_traffic_keys", ret ); - return( ret ); + goto cleanup; } MBEDTLS_SSL_DEBUG_BUF( 4, "Client application traffic secret", @@ -1219,7 +1217,19 @@ int mbedtls_ssl_tls1_3_generate_application_keys( traffic_keys->server_write_iv, ivlen ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= derive application traffic keys" ) ); - return( 0 ); + + cleanup: + + mbedtls_platform_zeroize( transcript, sizeof(transcript) ); + mbedtls_platform_zeroize( traffic_keys->client_write_key, + sizeof(traffic_keys->client_write_key) ); + mbedtls_platform_zeroize( traffic_keys->server_write_key, + sizeof(traffic_keys->server_write_key) ); + mbedtls_platform_zeroize( traffic_keys->client_write_iv, + sizeof(traffic_keys->client_write_iv) ); + mbedtls_platform_zeroize( traffic_keys->server_write_iv, + sizeof(traffic_keys->server_write_iv) ); + return( ret ); } #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 78bfc2a3d..31a5029b4 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -586,7 +586,7 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls1_3_key_schedule_stage_application( +int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ); /** @@ -594,7 +594,7 @@ int mbedtls_ssl_tls1_3_key_schedule_stage_application( * * \param ssl The SSL context to operate on. This must be in * key schedule stage \c Application, see - * mbedtls_ssl_tls1_3_key_schedule_stage_application(). + * mbedtls_ssl_tls13_key_schedule_stage_application(). * \param traffic_keys The address at which to store the application traffic key * keys. This must be writable but may be uninitialized. * @@ -609,7 +609,7 @@ int mbedtls_ssl_tls1_3_generate_application_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_application(). + * mbedtls_ssl_tls13_key_schedule_stage_application(). * \param dst The address at which to write the Finished content. * \param dst_len The size of \p dst in bytes. * \param actual_len The address at which to store the amount of data From a763498490601288b651f3f3843d06cd17a1693a Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 22 Oct 2021 06:32:32 +0000 Subject: [PATCH 03/21] Change code based on commetns Focus on the code style, naming rule,etc. Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 9 +++++++-- library/ssl_tls13_generic.c | 8 ++++---- library/ssl_tls13_keys.c | 16 ++++++++-------- library/ssl_tls13_keys.h | 6 +++--- tests/suites/test_suite_ssl.function | 4 ++-- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3e1018125..b88351f90 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -39,6 +39,7 @@ #if defined(MBEDTLS_DHM_C) #include "mbedtls/dhm.h" #endif + /* Adding guard for MBEDTLS_ECDSA_C to ensure no compile errors due * to guards also being in ssl_srv.c and ssl_cli.c. There is a gap * in functionality that access to ecdh_ctx structure is needed for @@ -1050,13 +1051,15 @@ typedef void mbedtls_ssl_async_cancel_t( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED && !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) 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; +} mbedtls_ssl_tls13_application_secrets; +#endif #if defined(MBEDTLS_SSL_DTLS_SRTP) @@ -1122,7 +1125,9 @@ struct mbedtls_ssl_session * to be studied whether one of them can be removed. */ unsigned char MBEDTLS_PRIVATE(minor_ver); /*!< The TLS version used in the session. */ - mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets); +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + mbedtls_ssl_tls13_application_secrets MBEDTLS_PRIVATE(app_secrets); +#endif #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 87bc12ce2..1d41cd3d3 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -870,7 +870,7 @@ int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ) unsigned char *buf; size_t buflen; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server finished_in_process" ) ); /* Preprocessing step: Compute handshake digest */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_preprocess( ssl ) ); @@ -885,7 +885,7 @@ int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ) cleanup: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse server finished_in_process" ) ); return( ret ); } @@ -893,14 +893,14 @@ static int ssl_tls13_finished_in_preprocess( mbedtls_ssl_context* ssl ) { int ret; - ret = mbedtls_ssl_tls1_3_calc_finished( ssl, + ret = mbedtls_ssl_tls1_3_calculate_expected_finished( ssl, ssl->handshake->state_local.finished_in.digest, sizeof( ssl->handshake->state_local.finished_in.digest ), &ssl->handshake->state_local.finished_in.digest_len, ssl->conf->endpoint ^ 1 ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_calc_finished", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_calculate_expected_finished", ret ); return( ret ); } diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index ddbeb626e..85026f538 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -469,7 +469,7 @@ int mbedtls_ssl_tls1_3_derive_application_secrets( mbedtls_md_type_t md_type, unsigned char const *application_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls1_3_application_secrets *derived ) + mbedtls_ssl_tls13_application_secrets *derived ) { int ret; mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); @@ -539,7 +539,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( mbedtls_md_type_t md_type, unsigned char const *application_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls1_3_application_secrets *derived ) + mbedtls_ssl_tls13_application_secrets *derived ) { int ret; mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); @@ -643,13 +643,13 @@ exit: return( ret ); } -int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context* ssl, +int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, unsigned char* dst, size_t dst_len, size_t *actual_len, int from ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; size_t transcript_len; @@ -660,7 +660,7 @@ int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context* ssl, const mbedtls_md_info_t* const md = mbedtls_md_info_from_type( md_type ); size_t const md_size = mbedtls_md_get_size( md ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls1_3_calc_finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls1_3_calculate_expected_finished" ) ); if( dst_len < md_size ) return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); @@ -686,7 +686,7 @@ int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context* ssl, *actual_len = md_size; MBEDTLS_SSL_DEBUG_BUF( 3, "verify_data for finished message", dst, md_size ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calc_finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calculate_expected_finished" ) ); return( 0 ); } @@ -1111,10 +1111,10 @@ int mbedtls_ssl_tls1_3_generate_application_keys( mbedtls_ssl_context *ssl, mbedtls_ssl_key_set *traffic_keys ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; /* Address at which to store the application secrets */ - mbedtls_ssl_tls1_3_application_secrets * const app_secrets = + mbedtls_ssl_tls13_application_secrets * const app_secrets = &ssl->session_negotiate->app_secrets; /* Holding the transcript up to and including the ServerFinished */ diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 31a5029b4..2509cffd5 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -344,7 +344,7 @@ int mbedtls_ssl_tls1_3_derive_application_secrets( mbedtls_md_type_t md_type, unsigned char const *master_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls1_3_application_secrets *derived ); + mbedtls_ssl_tls13_application_secrets *derived ); /** * \brief Derive TLS 1.3 resumption master secret from the master secret. @@ -374,7 +374,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( mbedtls_md_type_t md_type, unsigned char const *application_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls1_3_application_secrets *derived ); + mbedtls_ssl_tls13_application_secrets *derived ); /** * \brief Compute the next secret in the TLS 1.3 key schedule @@ -625,7 +625,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls1_3_calc_finished( mbedtls_ssl_context *ssl, +int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context *ssl, unsigned char *dst, size_t dst_len, size_t *actual_len, diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 75eda1dcd..6d262cee2 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3862,7 +3862,7 @@ void ssl_tls1_3_derive_application_secrets( int hash_alg, data_t *server_expected, data_t *exporter_expected ) { - mbedtls_ssl_tls1_3_application_secrets secrets; + mbedtls_ssl_tls13_application_secrets secrets; /* Double-check that we've passed sane parameters. */ mbedtls_md_type_t md_type = (mbedtls_md_type_t) hash_alg; @@ -3894,7 +3894,7 @@ void ssl_tls1_3_derive_resumption_secrets( int hash_alg, data_t *transcript, data_t *resumption_expected ) { - mbedtls_ssl_tls1_3_application_secrets secrets; + mbedtls_ssl_tls13_application_secrets secrets; /* Double-check that we've passed sane parameters. */ mbedtls_md_type_t md_type = (mbedtls_md_type_t) hash_alg; From 46c6fc74f19b5d742d4704013509a40bb2d7e01d Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 22 Oct 2021 10:20:28 +0000 Subject: [PATCH 04/21] Fix compile issue about MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL Signed-off-by: XiaokangQian --- library/ssl_tls13_keys.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 2509cffd5..1e6fff58b 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -19,6 +19,7 @@ #if !defined(MBEDTLS_SSL_TLS1_3_KEYS_H) #define MBEDTLS_SSL_TLS1_3_KEYS_H +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) /* This requires MBEDTLS_SSL_TLS1_3_LABEL( idx, name, string ) to be defined at * the point of use. See e.g. the definition of mbedtls_ssl_tls1_3_labels_union * below. */ @@ -631,4 +632,5 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context *ssl, size_t *actual_len, int from ); +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ From 7c91705e21b9a39116c7bf0c4ef6e1a357cd7751 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 25 Oct 2021 06:17:12 +0000 Subject: [PATCH 05/21] Remove support for MBEDTLS_SSL_EXPORT_KEYS Signed-off-by: XiaokangQian --- library/ssl_tls13_keys.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 85026f538..54a927528 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1185,28 +1185,6 @@ int mbedtls_ssl_tls1_3_generate_application_keys( app_secrets->server_application_traffic_secret_N, md_size ); - /* - * Export client/server application traffic secret 0 - */ -#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_APPLICATION_TRAFFIC_SECRET, - app_secrets->client_application_traffic_secret_N, 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_APPLICATION_TRAFFIC_SECRET, - app_secrets->server_application_traffic_secret_N, md_size, - ssl->handshake->randbytes + 32, - ssl->handshake->randbytes, - MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); - } -#endif /* MBEDTLS_SSL_EXPORT_KEYS */ - MBEDTLS_SSL_DEBUG_BUF( 4, "client application_write_key:", traffic_keys->client_write_key, keylen ); MBEDTLS_SSL_DEBUG_BUF( 4, "server application write key", From f13c56032f32f7a7e1a68de0d271e500a338bfac Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 26 Oct 2021 10:22:25 +0000 Subject: [PATCH 06/21] Revert some changes about tls13 and macros There is one PR #4988 to change it in the future Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 8 ++------ library/ssl_tls13_keys.c | 6 +++--- library/ssl_tls13_keys.h | 6 ++---- tests/suites/test_suite_ssl.function | 4 ++-- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b88351f90..ed6a445c2 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1051,15 +1051,13 @@ typedef void mbedtls_ssl_async_cancel_t( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED && !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) 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_tls13_application_secrets; -#endif +} mbedtls_ssl_tls1_3_application_secrets; #if defined(MBEDTLS_SSL_DTLS_SRTP) @@ -1125,9 +1123,7 @@ struct mbedtls_ssl_session * to be studied whether one of them can be removed. */ unsigned char MBEDTLS_PRIVATE(minor_ver); /*!< The TLS version used in the session. */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) - mbedtls_ssl_tls13_application_secrets MBEDTLS_PRIVATE(app_secrets); -#endif + mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets); #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 54a927528..2127393fc 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -469,7 +469,7 @@ int mbedtls_ssl_tls1_3_derive_application_secrets( mbedtls_md_type_t md_type, unsigned char const *application_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls13_application_secrets *derived ) + mbedtls_ssl_tls1_3_application_secrets *derived ) { int ret; mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); @@ -539,7 +539,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( mbedtls_md_type_t md_type, unsigned char const *application_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls13_application_secrets *derived ) + mbedtls_ssl_tls1_3_application_secrets *derived ) { int ret; mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); @@ -1114,7 +1114,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; /* Address at which to store the application secrets */ - mbedtls_ssl_tls13_application_secrets * const app_secrets = + mbedtls_ssl_tls1_3_application_secrets * const app_secrets = &ssl->session_negotiate->app_secrets; /* Holding the transcript up to and including the ServerFinished */ diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 1e6fff58b..e96cfc10c 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -19,7 +19,6 @@ #if !defined(MBEDTLS_SSL_TLS1_3_KEYS_H) #define MBEDTLS_SSL_TLS1_3_KEYS_H -#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) /* This requires MBEDTLS_SSL_TLS1_3_LABEL( idx, name, string ) to be defined at * the point of use. See e.g. the definition of mbedtls_ssl_tls1_3_labels_union * below. */ @@ -345,7 +344,7 @@ int mbedtls_ssl_tls1_3_derive_application_secrets( mbedtls_md_type_t md_type, unsigned char const *master_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls13_application_secrets *derived ); + mbedtls_ssl_tls1_3_application_secrets *derived ); /** * \brief Derive TLS 1.3 resumption master secret from the master secret. @@ -375,7 +374,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( mbedtls_md_type_t md_type, unsigned char const *application_secret, unsigned char const *transcript, size_t transcript_len, - mbedtls_ssl_tls13_application_secrets *derived ); + mbedtls_ssl_tls1_3_application_secrets *derived ); /** * \brief Compute the next secret in the TLS 1.3 key schedule @@ -632,5 +631,4 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context *ssl, size_t *actual_len, int from ); -#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 6d262cee2..75eda1dcd 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3862,7 +3862,7 @@ void ssl_tls1_3_derive_application_secrets( int hash_alg, data_t *server_expected, data_t *exporter_expected ) { - mbedtls_ssl_tls13_application_secrets secrets; + mbedtls_ssl_tls1_3_application_secrets secrets; /* Double-check that we've passed sane parameters. */ mbedtls_md_type_t md_type = (mbedtls_md_type_t) hash_alg; @@ -3894,7 +3894,7 @@ void ssl_tls1_3_derive_resumption_secrets( int hash_alg, data_t *transcript, data_t *resumption_expected ) { - mbedtls_ssl_tls13_application_secrets secrets; + mbedtls_ssl_tls1_3_application_secrets secrets; /* Double-check that we've passed sane parameters. */ mbedtls_md_type_t md_type = (mbedtls_md_type_t) hash_alg; From f26f6ade0c650dded604bd3350460593cb2ae77f Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 28 Oct 2021 05:50:31 +0000 Subject: [PATCH 07/21] Rebase and solve conflicts Remove the double definition and change name Signed-off-by: XiaokangQian --- library/ssl_misc.h | 8 -------- library/ssl_tls13_keys.c | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 3b0d61b35..7044769a2 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -522,14 +522,6 @@ typedef struct unsigned char server_handshake_traffic_secret[ MBEDTLS_TLS1_3_MD_MAX_SIZE ]; } mbedtls_ssl_tls1_3_handshake_secrets; -typedef struct -{ - 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; - /* * This structure contains the parameters only needed during handshake. */ diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 2127393fc..5bd3e4719 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -676,9 +676,9 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, MBEDTLS_SSL_DEBUG_BUF( 4, "handshake hash", transcript, transcript_len ); if( from == MBEDTLS_SSL_IS_CLIENT ) - base_key = ssl->handshake->tls1_3_hs_secrets.client_handshake_traffic_secret; + base_key = ssl->handshake->tls13_hs_secrets.client_handshake_traffic_secret; else - base_key = ssl->handshake->tls1_3_hs_secrets.server_handshake_traffic_secret; + base_key = ssl->handshake->tls13_hs_secrets.server_handshake_traffic_secret; ret = ssl_tls1_3_calc_finished_core( md_type, base_key, transcript, dst ); if( ret != 0 ) From 61bdbbc18b4da3cd20cdf7bc8b80031d4240513b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 28 Oct 2021 08:03:38 +0000 Subject: [PATCH 08/21] Add cleanup in functions for secure reason Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 24 +++++++++++++++++++----- library/ssl_tls13_keys.c | 20 ++++++++------------ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 1d41cd3d3..dd550f772 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -952,7 +952,7 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_key_schedule_stage_application", ret ); - return( ret ); + goto cleanup; } ret = mbedtls_ssl_tls1_3_generate_application_keys( @@ -961,13 +961,16 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_generate_application_keys", ret ); - return( ret ); + goto cleanup; } transform_application = mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); if( transform_application == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + { + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto cleanup; + } ret = mbedtls_ssl_tls13_populate_transform( transform_application, @@ -978,13 +981,24 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_populate_transform", ret ); - return( ret ); + goto cleanup; } ssl->transform_application = transform_application; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_END_OF_EARLY_DATA ); - return( 0 ); + +cleanup: + + mbedtls_platform_zeroize( &traffic_keys, sizeof(mbedtls_ssl_key_set) ); + if( ret != 0) + { + mbedtls_free( transform_application ); + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + return( ret ); } static int ssl_tls13_finished_in_postprocess( mbedtls_ssl_context* ssl ) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 5bd3e4719..34d8a19df 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -567,7 +567,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; 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 ); @@ -671,7 +671,7 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_get_handshake_transcript", ret ); - return( ret ); + goto exit; } MBEDTLS_SSL_DEBUG_BUF( 4, "handshake hash", transcript, transcript_len ); @@ -682,12 +682,16 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, ret = ssl_tls1_3_calc_finished_core( md_type, base_key, transcript, dst ); if( ret != 0 ) - return( ret ); + goto exit; *actual_len = md_size; MBEDTLS_SSL_DEBUG_BUF( 3, "verify_data for finished message", dst, md_size ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calculate_expected_finished" ) ); - return( 0 ); + +exit: + + mbedtls_platform_zeroize( transcript, sizeof( transcript) ); + return( ret ); } int mbedtls_ssl_tls1_3_create_psk_binder( mbedtls_ssl_context *ssl, @@ -1199,14 +1203,6 @@ int mbedtls_ssl_tls1_3_generate_application_keys( cleanup: mbedtls_platform_zeroize( transcript, sizeof(transcript) ); - mbedtls_platform_zeroize( traffic_keys->client_write_key, - sizeof(traffic_keys->client_write_key) ); - mbedtls_platform_zeroize( traffic_keys->server_write_key, - sizeof(traffic_keys->server_write_key) ); - mbedtls_platform_zeroize( traffic_keys->client_write_iv, - sizeof(traffic_keys->client_write_iv) ); - mbedtls_platform_zeroize( traffic_keys->server_write_iv, - sizeof(traffic_keys->server_write_iv) ); return( ret ); } From 1aef02ee20a1271aae8cad561193060211f41269 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 28 Oct 2021 09:54:34 +0000 Subject: [PATCH 09/21] Fix initialized issues and remove useless code Fix the variable not inialized issue, remove the client certificate related code, remove early data related code. Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 1 - library/ssl_tls13_client.c | 28 ---------------------------- library/ssl_tls13_generic.c | 18 +++++++++--------- 3 files changed, 9 insertions(+), 38 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ed6a445c2..508a5e34a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -637,7 +637,6 @@ typedef enum MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, #if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) - MBEDTLS_SSL_END_OF_EARLY_DATA, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY, #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 2bde4a825..5dba0f0c4 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1608,26 +1608,6 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) return ( mbedtls_ssl_tls13_finished_in_process( ssl ) ); } -/* - * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE - */ -static int ssl_tls1_3_write_client_certificate( mbedtls_ssl_context *ssl ) -{ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY ); - return( 0 ); -} - -/* - * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY - */ -static int ssl_tls1_3_write_client_certificate_verify( mbedtls_ssl_context *ssl ) -{ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); - return( 0 ); -} - /* * Handler for MBEDTLS_SSL_CLIENT_FINISHED */ @@ -1701,14 +1681,6 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) ret = ssl_tls1_3_process_server_finished( ssl ); break; - case MBEDTLS_SSL_CLIENT_CERTIFICATE: - ret = ssl_tls1_3_write_client_certificate( ssl ); - break; - - case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY: - ret = ssl_tls1_3_write_client_certificate_verify( ssl ); - break; - case MBEDTLS_SSL_CLIENT_FINISHED: ret = ssl_tls1_3_write_client_finished( ssl ); break; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index dd550f772..f37948585 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -945,7 +945,7 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_key_set traffic_keys; - mbedtls_ssl_transform *transform_application; + mbedtls_ssl_transform *transform_application = NULL; ret = mbedtls_ssl_tls13_key_schedule_stage_application( ssl ); if( ret != 0 ) @@ -986,18 +986,18 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) ssl->transform_application = transform_application; - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_END_OF_EARLY_DATA ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); cleanup: mbedtls_platform_zeroize( &traffic_keys, sizeof(mbedtls_ssl_key_set) ); - if( ret != 0) - { - mbedtls_free( transform_application ); - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, - MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - } + if( ret != 0) + { + mbedtls_free( transform_application ); + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } return( ret ); } From 8903bd97b0eea085f09670f227946eef07420176 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 3 Nov 2021 05:56:49 +0000 Subject: [PATCH 10/21] Change some naming style issues and remove useless code Signed-off-by: XiaokangQian --- library/ssl_misc.h | 21 ++------------------- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_generic.c | 35 ++++++++++++++++------------------- library/ssl_tls13_keys.h | 8 ++++---- 4 files changed, 23 insertions(+), 43 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 7044769a2..6b9bc599d 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -775,20 +775,6 @@ struct mbedtls_ssl_handshake_params #endif /* MBEDTLS_SSL_CLI_C */ - /* Incoming CertificateVerify */ - struct - { - unsigned char verify_buffer[ 64 + 33 + 1 + MBEDTLS_MD_MAX_SIZE ]; - size_t verify_buffer_len; - } certificate_verify_in; - - /* Outgoing CertificateVerify */ - struct - { - unsigned char handshake_hash[ MBEDTLS_MD_MAX_SIZE ]; - size_t handshake_hash_len; - } certificate_verify_out; - } state_local; /* End of state-local variables. */ @@ -1236,11 +1222,8 @@ static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_read_certificate_process(mbedtls_ssl_context *ssl); -int mbedtls_ssl_write_certificate_process(mbedtls_ssl_context *ssl); - -int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls13_finished_out_process( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_process_finished_in( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_process_finished_out( mbedtls_ssl_context *ssl ); int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5dba0f0c4..8644db958 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1605,7 +1605,7 @@ static int ssl_tls1_3_process_certificate_verify( mbedtls_ssl_context *ssl ) */ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) { - return ( mbedtls_ssl_tls13_finished_in_process( ssl ) ); + return ( mbedtls_ssl_tls13_process_finished_in( ssl ) ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index f37948585..b2e5ad061 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -851,20 +851,17 @@ cleanup: * Overview */ -/* Main entry point: orchestrates the other functions */ -int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ); - -static int ssl_tls13_finished_in_preprocess( mbedtls_ssl_context* ssl ); -static int ssl_tls13_finished_in_postprocess( mbedtls_ssl_context* ssl ); -static int ssl_tls13_finished_in_parse( mbedtls_ssl_context* ssl, - const unsigned char* buf, - size_t buflen ); +static int ssl_tls13_preprocess_finished_in( mbedtls_ssl_context *ssl ); +static int ssl_tls13_postprocess_finished_in( mbedtls_ssl_context *ssl ); +static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t buflen ); /* * Implementation */ -int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ) +int mbedtls_ssl_tls13_process_finished_in( mbedtls_ssl_context *ssl ) { int ret = 0; unsigned char *buf; @@ -873,15 +870,15 @@ int mbedtls_ssl_tls13_finished_in_process( mbedtls_ssl_context* ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server finished_in_process" ) ); /* Preprocessing step: Compute handshake digest */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_preprocess( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_in( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buflen ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_parse( ssl, buf, buflen ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_in( ssl, buf, buflen ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finished_in_postprocess( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_finished_in( ssl ) ); cleanup: @@ -889,7 +886,7 @@ cleanup: return( ret ); } -static int ssl_tls13_finished_in_preprocess( mbedtls_ssl_context* ssl ) +static int ssl_tls13_preprocess_finished_in( mbedtls_ssl_context *ssl ) { int ret; @@ -907,9 +904,9 @@ static int ssl_tls13_finished_in_preprocess( mbedtls_ssl_context* ssl ) return( 0 ); } -static int ssl_tls13_finished_in_parse( mbedtls_ssl_context* ssl, - const unsigned char* buf, - size_t buflen ) +static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t buflen ) { /* Structural validation */ if( buflen != ssl->handshake->state_local.finished_in.digest_len ) @@ -941,7 +938,7 @@ static int ssl_tls13_finished_in_parse( mbedtls_ssl_context* ssl, return( 0 ); } -static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) +static int ssl_tls13_postprocess_finished_in_cli( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_key_set traffic_keys; @@ -1001,12 +998,12 @@ cleanup: return( ret ); } -static int ssl_tls13_finished_in_postprocess( mbedtls_ssl_context* ssl ) +static int ssl_tls13_postprocess_finished_in( mbedtls_ssl_context* ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - return( ssl_tls13_finished_in_postprocess_cli( ssl ) ); + return( ssl_tls13_postprocess_finished_in_cli( ssl ) ); } return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index e96cfc10c..fbc6e83c8 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -626,9 +626,9 @@ int mbedtls_ssl_tls1_3_generate_application_keys( * \returns A negative error code on failure. */ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context *ssl, - unsigned char *dst, - size_t dst_len, - size_t *actual_len, - int from ); + unsigned char *dst, + size_t dst_len, + size_t *actual_len, + int from ); #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ From ac0385c08f76108c733b8677af8473e51a2f8648 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 3 Nov 2021 06:40:11 +0000 Subject: [PATCH 11/21] Change code based on comments Move set_state function into client Add back export_key callback function in generate application keys Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 9 ++++++++- library/ssl_tls13_generic.c | 2 -- library/ssl_tls13_keys.c | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 8644db958..c30d5627d 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1605,7 +1605,14 @@ static int ssl_tls1_3_process_certificate_verify( mbedtls_ssl_context *ssl ) */ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) { - return ( mbedtls_ssl_tls13_process_finished_in( ssl ) ); + int ret; + + ret = mbedtls_ssl_tls13_process_finished_in( ssl ); + if( ret != 0 ) + return( ret ); + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); + return( 0 ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index b2e5ad061..9754395f7 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -983,8 +983,6 @@ static int ssl_tls13_postprocess_finished_in_cli( mbedtls_ssl_context *ssl ) ssl->transform_application = transform_application; - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); - cleanup: mbedtls_platform_zeroize( &traffic_keys, sizeof(mbedtls_ssl_key_set) ); diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 34d8a19df..b97a70f29 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1189,6 +1189,26 @@ int mbedtls_ssl_tls1_3_generate_application_keys( app_secrets->server_application_traffic_secret_N, md_size ); + /* + * Export client/server application traffic secret 0 + */ + if( ssl->f_export_keys != NULL ) + { + ssl->f_export_keys( ssl->p_export_keys, + MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_APPLICATION_TRAFFIC_SECRET, + app_secrets->client_application_traffic_secret_N, 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_APPLICATION_TRAFFIC_SECRET, + app_secrets->server_application_traffic_secret_N, md_size, + ssl->handshake->randbytes + 32, + ssl->handshake->randbytes, + MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); + } + MBEDTLS_SSL_DEBUG_BUF( 4, "client application_write_key:", traffic_keys->client_write_key, keylen ); MBEDTLS_SSL_DEBUG_BUF( 4, "server application write key", From b51f8841c42864e39b7eba4f7e68c52c751ed26a Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 4 Nov 2021 03:02:47 +0000 Subject: [PATCH 12/21] Change comments for export_keys callback Signed-off-by: XiaokangQian --- library/ssl_tls13_keys.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index b97a70f29..1f0dd8a84 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1199,14 +1199,16 @@ int mbedtls_ssl_tls1_3_generate_application_keys( app_secrets->client_application_traffic_secret_N, md_size, ssl->handshake->randbytes + 32, ssl->handshake->randbytes, - MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); + MBEDTLS_SSL_TLS_PRF_NONE /* TODO: this should be replaced by + a new constant for TLS 1.3! */ ); ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_APPLICATION_TRAFFIC_SECRET, app_secrets->server_application_traffic_secret_N, md_size, ssl->handshake->randbytes + 32, ssl->handshake->randbytes, - MBEDTLS_SSL_TLS_PRF_NONE /* TODO: FIX! */ ); + MBEDTLS_SSL_TLS_PRF_NONE /* TODO: this should be replaced by + a new constant for TLS 1.3! */ ); } MBEDTLS_SSL_DEBUG_BUF( 4, "client application_write_key:", From 44c38f7e3669694d3fc8785a6442f8664a477039 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 5 Nov 2021 06:49:27 +0000 Subject: [PATCH 13/21] Chande debug message in finished and rename finalize functions Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 9754395f7..27cef7287 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -852,7 +852,7 @@ cleanup: */ static int ssl_tls13_preprocess_finished_in( mbedtls_ssl_context *ssl ); -static int ssl_tls13_postprocess_finished_in( mbedtls_ssl_context *ssl ); +static int ssl_tls13_finalize_finished_in( mbedtls_ssl_context *ssl ); static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t buflen ); @@ -867,7 +867,7 @@ int mbedtls_ssl_tls13_process_finished_in( mbedtls_ssl_context *ssl ) unsigned char *buf; size_t buflen; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server finished_in_process" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished_in" ) ); /* Preprocessing step: Compute handshake digest */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_in( ssl ) ); @@ -878,11 +878,11 @@ int mbedtls_ssl_tls13_process_finished_in( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_in( ssl, buf, buflen ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_finished_in( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_in( ssl ) ); cleanup: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse server finished_in_process" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished_in" ) ); return( ret ); } @@ -938,7 +938,7 @@ static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls13_postprocess_finished_in_cli( mbedtls_ssl_context *ssl ) +static int ssl_tls13_finalize_finished_in_cli( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_key_set traffic_keys; @@ -996,12 +996,12 @@ cleanup: return( ret ); } -static int ssl_tls13_postprocess_finished_in( mbedtls_ssl_context* ssl ) +static int ssl_tls13_finalize_finished_in( mbedtls_ssl_context* ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - return( ssl_tls13_postprocess_finished_in_cli( ssl ) ); + return( ssl_tls13_finalize_finished_in_cli( ssl ) ); } return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); From c5c39d5800d38add9e5eee30e44c41b338edce31 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 9 Nov 2021 11:55:10 +0000 Subject: [PATCH 14/21] Change code for styles and comments .etc Remove useless code in union. Rename functions and parameters. Move definitions into othe files. Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 10 ++-- library/ssl_misc.h | 46 ++----------------- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_generic.c | 92 +++++++++++++++++-------------------- library/ssl_tls13_keys.c | 6 +-- library/ssl_tls13_keys.h | 9 ++-- 6 files changed, 61 insertions(+), 104 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 508a5e34a..fba8f8f84 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -605,6 +605,8 @@ union mbedtls_ssl_premaster_secret #define MBEDTLS_PREMASTER_SIZE sizeof( union mbedtls_ssl_premaster_secret ) +#define MBEDTLS_TLS1_3_MD_MAX_SIZE MBEDTLS_MD_MAX_SIZE + /* Length in number of bytes of the TLS sequence number */ #define MBEDTLS_SSL_SEQUENCE_NUMBER_LEN 8 @@ -1052,10 +1054,10 @@ typedef void mbedtls_ssl_async_cancel_t( mbedtls_ssl_context *ssl ); 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; #if defined(MBEDTLS_SSL_DTLS_SRTP) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 6b9bc599d..89a5d4313 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -307,8 +307,6 @@ + ( MBEDTLS_SSL_CID_OUT_LEN_MAX ) ) #endif -#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 @@ -724,7 +722,7 @@ struct mbedtls_ssl_handshake_params /* Buffer holding digest of the handshake up to * but excluding the outgoing finished message. */ - unsigned char digest[MBEDTLS_MD_MAX_SIZE]; + unsigned char digest[MBEDTLS_TLS1_3_MD_MAX_SIZE]; size_t digest_len; } finished_out; @@ -733,48 +731,10 @@ struct mbedtls_ssl_handshake_params { /* Buffer holding digest of the handshake up to but * excluding the peer's incoming finished message. */ - unsigned char digest[MBEDTLS_MD_MAX_SIZE]; + unsigned char digest[MBEDTLS_TLS1_3_MD_MAX_SIZE]; size_t digest_len; } finished_in; -#if defined(MBEDTLS_SSL_CLI_C) - - /* Client, incoming ServerKeyExchange */ - struct - { - uint8_t preparation_done; - } srv_key_exchange; - - /* Client, incoming ServerHello */ - struct - { -#if defined(MBEDTLS_SSL_RENEGOTIATION) - int renego_info_seen; -#else - int dummy; -#endif - } srv_hello_in; - - /* Client, outgoing ClientKeyExchange */ - struct - { - uint8_t preparation_done; - } cli_key_exch_out; - - /* Client, outgoing Certificate Verify */ - struct - { - uint8_t preparation_done; - } crt_vrfy_out; - - /* Client, outgoing ClientHello */ - struct - { - uint8_t preparation_done; - } cli_hello_out; - -#endif /* MBEDTLS_SSL_CLI_C */ - } state_local; /* End of state-local variables. */ @@ -1222,7 +1182,7 @@ static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls13_process_finished_in( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ); int mbedtls_ssl_tls13_process_finished_out( mbedtls_ssl_context *ssl ); int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index c30d5627d..6deab2a8c 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1607,7 +1607,7 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) { int ret; - ret = mbedtls_ssl_tls13_process_finished_in( ssl ); + ret = mbedtls_ssl_tls13_process_finished_message( ssl ); if( ret != 0 ) return( ret ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 27cef7287..83f720224 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -847,69 +847,37 @@ cleanup: /* * - * STATE HANDLING: Incoming Finished - * Overview + * STATE HANDLING: Incoming Finished message. */ - -static int ssl_tls13_preprocess_finished_in( mbedtls_ssl_context *ssl ); -static int ssl_tls13_finalize_finished_in( mbedtls_ssl_context *ssl ); -static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t buflen ); - /* * Implementation */ -int mbedtls_ssl_tls13_process_finished_in( mbedtls_ssl_context *ssl ) -{ - int ret = 0; - unsigned char *buf; - size_t buflen; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished_in" ) ); - - /* Preprocessing step: Compute handshake digest */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_in( ssl ) ); - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( ssl, - MBEDTLS_SSL_HS_FINISHED, - &buf, &buflen ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_in( ssl, buf, buflen ) ); - mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_in( ssl ) ); - -cleanup: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished_in" ) ); - return( ret ); -} - -static int ssl_tls13_preprocess_finished_in( mbedtls_ssl_context *ssl ) +static int ssl_tls13_prepare_finished_in( mbedtls_ssl_context *ssl ) { int ret; - ret = mbedtls_ssl_tls1_3_calculate_expected_finished( ssl, + ret = mbedtls_ssl_tls13_calculate_verify_data( ssl, ssl->handshake->state_local.finished_in.digest, sizeof( ssl->handshake->state_local.finished_in.digest ), &ssl->handshake->state_local.finished_in.digest_len, - ssl->conf->endpoint ^ 1 ); + ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ? + MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_calculate_expected_finished", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_calculate_verify_data", ret ); return( ret ); } return( 0 ); } -static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t buflen ) +static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { /* Structural validation */ - if( buflen != ssl->handshake->state_local.finished_in.digest_len ) + if( (size_t)( end - buf ) != ssl->handshake->state_local.finished_in.digest_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); @@ -918,10 +886,10 @@ static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - MBEDTLS_SSL_DEBUG_BUF( 4, "Hash (self-computed):", + MBEDTLS_SSL_DEBUG_BUF( 4, "verify_data (self-computed):", ssl->handshake->state_local.finished_in.digest, ssl->handshake->state_local.finished_in.digest_len ); - MBEDTLS_SSL_DEBUG_BUF( 4, "Hash (received message):", buf, + MBEDTLS_SSL_DEBUG_BUF( 4, "verify_data (received message):", buf, ssl->handshake->state_local.finished_in.digest_len ); /* Semantic validation */ @@ -938,7 +906,7 @@ static int ssl_tls13_parse_finished_in( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls13_finalize_finished_in_cli( mbedtls_ssl_context *ssl ) +static int ssl_tls13_finalize_server_finished_message( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_key_set traffic_keys; @@ -985,8 +953,8 @@ static int ssl_tls13_finalize_finished_in_cli( mbedtls_ssl_context *ssl ) cleanup: - mbedtls_platform_zeroize( &traffic_keys, sizeof(mbedtls_ssl_key_set) ); - if( ret != 0) + mbedtls_platform_zeroize( &traffic_keys, sizeof( mbedtls_ssl_key_set ) ); + if( ret != 0 ) { mbedtls_free( transform_application ); MBEDTLS_SSL_PEND_FATAL_ALERT( @@ -996,17 +964,43 @@ cleanup: return( ret ); } -static int ssl_tls13_finalize_finished_in( mbedtls_ssl_context* ssl ) +static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context* ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - return( ssl_tls13_finalize_finished_in_cli( ssl ) ); + return( ssl_tls13_finalize_server_finished_message( ssl ) ); } return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } +int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + unsigned char *buf; + size_t buflen; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished_in" ) ); + + /* Preprocessing step: Compute handshake digest */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_in( ssl ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( ssl, + MBEDTLS_SSL_HS_FINISHED, + &buf, &buflen ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_message( ssl, buf, buf + buflen ) ); + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_message( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished_in" ) ); + return( ret ); +} + + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 1f0dd8a84..8f089f580 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -643,7 +643,7 @@ exit: return( ret ); } -int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, +int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context* ssl, unsigned char* dst, size_t dst_len, size_t *actual_len, @@ -660,7 +660,7 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, const mbedtls_md_info_t* const md = mbedtls_md_info_from_type( md_type ); size_t const md_size = mbedtls_md_get_size( md ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls1_3_calculate_expected_finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_tls13_calculate_verify_data" ) ); if( dst_len < md_size ) return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); @@ -686,7 +686,7 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, *actual_len = md_size; MBEDTLS_SSL_DEBUG_BUF( 3, "verify_data for finished message", dst, md_size ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calculate_expected_finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls13_calculate_verify_data" ) ); exit: diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index fbc6e83c8..c5c3a3416 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -605,16 +605,17 @@ int mbedtls_ssl_tls1_3_generate_application_keys( mbedtls_ssl_context* ssl, mbedtls_ssl_key_set *traffic_keys ); /** - * \brief Calculate content of TLS 1.3 Finished message. + * \brief Calculate the verify_data value for the client or server TLS 1.3 + * Finished message. * * \param ssl The SSL context to operate on. This must be in * key schedule stage \c Handshake, see * mbedtls_ssl_tls13_key_schedule_stage_application(). - * \param dst The address at which to write the Finished content. + * \param dst The address at which to write the verify_data value. * \param dst_len The size of \p dst in bytes. * \param actual_len The address at which to store the amount of data * actually written to \p dst upon success. - * \param from The endpoint the Finished message originates from: + * \param from The message to calculate the `verify_data` for: * - #MBEDTLS_SSL_IS_CLIENT for the Client's Finished message * - #MBEDTLS_SSL_IS_SERVER for the Server's Finished message * @@ -625,7 +626,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context *ssl, +int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context *ssl, unsigned char *dst, size_t dst_len, size_t *actual_len, From aaa0e197a81ea8fe8b8a9364987019603b81f519 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 10 Nov 2021 03:07:04 +0000 Subject: [PATCH 15/21] Change the alignment and names of functions and a macro Signed-off-by: XiaokangQian --- library/ssl_misc.h | 1 - library/ssl_tls13_generic.c | 12 ++++++------ library/ssl_tls13_keys.c | 10 +++++----- library/ssl_tls13_keys.h | 10 +++++----- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 89a5d4313..c0a370e06 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1183,7 +1183,6 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls13_process_finished_out( mbedtls_ssl_context *ssl ); int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 83f720224..9be6948cb 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -853,7 +853,7 @@ cleanup: * Implementation */ -static int ssl_tls13_prepare_finished_in( mbedtls_ssl_context *ssl ) +static int ssl_tls13_preprocess_finished_message( mbedtls_ssl_context *ssl ) { int ret; @@ -906,7 +906,7 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls13_finalize_server_finished_message( mbedtls_ssl_context *ssl ) +static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *ssl ) { int ret = 0; mbedtls_ssl_key_set traffic_keys; @@ -964,12 +964,12 @@ cleanup: return( ret ); } -static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context* ssl ) +static int ssl_tls13_postprocess_finished_message( mbedtls_ssl_context* ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - return( ssl_tls13_finalize_server_finished_message( ssl ) ); + return( ssl_tls13_postprocess_server_finished_message( ssl ) ); } return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); @@ -984,7 +984,7 @@ int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished_in" ) ); /* Preprocessing step: Compute handshake digest */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_in( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_message( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, @@ -992,7 +992,7 @@ int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_message( ssl, buf, buf + buflen ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, buflen ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_message( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_finished_message( ssl ) ); cleanup: diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 8f089f580..fbbf096c9 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -644,14 +644,14 @@ exit: } int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context* ssl, - unsigned char* dst, - size_t dst_len, - size_t *actual_len, - int from ) + unsigned char* dst, + size_t dst_len, + size_t *actual_len, + int from ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + unsigned char transcript[MBEDTLS_TLS1_3_MD_MAX_SIZE]; size_t transcript_len; unsigned char const *base_key = NULL; diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index c5c3a3416..53dbe732e 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -615,7 +615,7 @@ int mbedtls_ssl_tls1_3_generate_application_keys( * \param dst_len The size of \p dst in bytes. * \param actual_len The address at which to store the amount of data * actually written to \p dst upon success. - * \param from The message to calculate the `verify_data` for: + * \param which The message to calculate the `verify_data` for: * - #MBEDTLS_SSL_IS_CLIENT for the Client's Finished message * - #MBEDTLS_SSL_IS_SERVER for the Server's Finished message * @@ -627,9 +627,9 @@ int mbedtls_ssl_tls1_3_generate_application_keys( * \returns A negative error code on failure. */ int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context *ssl, - unsigned char *dst, - size_t dst_len, - size_t *actual_len, - int from ); + unsigned char *dst, + size_t dst_len, + size_t *actual_len, + int which ); #endif /* MBEDTLS_SSL_TLS1_3_KEYS_H */ From 57b2aff8a8bc32a5bf81087321f95f2c1e235e3d Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 10 Nov 2021 03:12:11 +0000 Subject: [PATCH 16/21] Align the union size Signed-off-by: XiaokangQian --- library/ssl_misc.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index c0a370e06..362117fd9 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -729,6 +729,8 @@ struct mbedtls_ssl_handshake_params /* Incoming Finished message */ struct { + uint8_t preparation_done; + /* Buffer holding digest of the handshake up to but * excluding the peer's incoming finished message. */ unsigned char digest[MBEDTLS_TLS1_3_MD_MAX_SIZE]; From d0aa3e930764be11c659729de019cef51ee152eb Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 10 Nov 2021 06:17:40 +0000 Subject: [PATCH 17/21] Inprove code base on review comments Change debug messag for server finished. Change name of generate_application_keys. Remove the client vertificate tests from ssl-opt.sh. Add test strings for server finished in ssl-opt.sh. Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 8 ++++---- library/ssl_tls13_keys.c | 2 +- library/ssl_tls13_keys.h | 4 ++-- tests/ssl-opt.sh | 11 +++++------ 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 9be6948cb..91b4bdfec 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -920,12 +920,12 @@ static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *s goto cleanup; } - ret = mbedtls_ssl_tls1_3_generate_application_keys( + ret = mbedtls_ssl_tls13_generate_application_keys( ssl, &traffic_keys ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, - "mbedtls_ssl_tls1_3_generate_application_keys", ret ); + "mbedtls_ssl_tls13_generate_application_keys", ret ); goto cleanup; } @@ -981,7 +981,7 @@ int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) unsigned char *buf; size_t buflen; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished_in" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse finished message" ) ); /* Preprocessing step: Compute handshake digest */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_preprocess_finished_message( ssl ) ); @@ -996,7 +996,7 @@ int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) cleanup: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished_in" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse finished message" ) ); return( ret ); } diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index fbbf096c9..c035504bf 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1111,7 +1111,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) /* Generate application traffic keys since any records following a 1-RTT Finished message * MUST be encrypted under the application traffic key. */ -int mbedtls_ssl_tls1_3_generate_application_keys( +int mbedtls_ssl_tls13_generate_application_keys( mbedtls_ssl_context *ssl, mbedtls_ssl_key_set *traffic_keys ) { diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 53dbe732e..7ea018339 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -577,7 +577,7 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, * with states Initial -> Early -> Handshake -> Application, and * this function represents the Handshake -> Application transition. * - * In the handshake stage, mbedtls_ssl_tls1_3_generate_application_keys() + * In the handshake stage, mbedtls_ssl_tls13_generate_application_keys() * can be used to derive the handshake traffic keys. * * \param ssl The SSL context to operate on. This must be in key schedule @@ -601,7 +601,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls1_3_generate_application_keys( +int mbedtls_ssl_tls13_generate_application_keys( mbedtls_ssl_context* ssl, mbedtls_ssl_key_set *traffic_keys ); /** diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0e78356bc..997bdee63 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8820,9 +8820,7 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "tls1_3 client state: 19" \ -c "tls1_3 client state: 5" \ -c "tls1_3 client state: 3" \ - -c "tls1_3 client state: 9" \ -c "tls1_3 client state: 13" \ - -c "tls1_3 client state: 7" \ -c "tls1_3 client state: 20" \ -c "tls1_3 client state: 11" \ -c "tls1_3 client state: 14" \ @@ -8835,7 +8833,8 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "Certificate verification flags clear" \ -c "=> parse certificate verify" \ -c "<= parse certificate verify" \ - -c "mbedtls_ssl_tls13_process_certificate_verify() returned 0" + -c "mbedtls_ssl_tls13_process_certificate_verify() returned 0" \ + -c "<= parse finished message" requires_gnutls_tls1_3 requires_gnutls_next_no_ticket @@ -8853,9 +8852,7 @@ run_test "TLS1.3: Test client hello msg work - gnutls" \ -c "tls1_3 client state: 19" \ -c "tls1_3 client state: 5" \ -c "tls1_3 client state: 3" \ - -c "tls1_3 client state: 9" \ -c "tls1_3 client state: 13" \ - -c "tls1_3 client state: 7" \ -c "tls1_3 client state: 20" \ -c "tls1_3 client state: 11" \ -c "tls1_3 client state: 14" \ @@ -8868,7 +8865,9 @@ run_test "TLS1.3: Test client hello msg work - gnutls" \ -c "Certificate verification flags clear" \ -c "=> parse certificate verify" \ -c "<= parse certificate verify" \ - -c "mbedtls_ssl_tls13_process_certificate_verify() returned 0" + -c "mbedtls_ssl_tls13_process_certificate_verify() returned 0" \ + -c "<= parse finished message" + # Test heap memory usage after handshake requires_config_enabled MBEDTLS_MEMORY_DEBUG From d6d234f69879d0038214724b8b14c7b52a38c551 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 11 Nov 2021 02:22:12 +0000 Subject: [PATCH 18/21] Solve the ABI_API check issue for mbedtls_ssl_session Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index fba8f8f84..89cc0513e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1124,7 +1124,9 @@ struct mbedtls_ssl_session * to be studied whether one of them can be removed. */ unsigned char MBEDTLS_PRIVATE(minor_ver); /*!< The TLS version used in the session. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets); +#endif #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) From 33062847764d9dd4c13cc7990fc3ba0b207cd097 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 11 Nov 2021 03:37:45 +0000 Subject: [PATCH 19/21] Change code base on comments Remove client certificate verify in tests. Change the layout of structure to fix abi_api check issues. Add comments of Finished. Align with the coding styles. Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 8 ++++---- library/ssl_tls13_generic.c | 32 ++++++++++++++++++++------------ library/ssl_tls13_keys.c | 30 ++++++++++++++++-------------- tests/ssl-opt.sh | 4 ++-- 4 files changed, 42 insertions(+), 32 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 89cc0513e..45e05447b 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1124,10 +1124,6 @@ struct mbedtls_ssl_session * to be studied whether one of them can be removed. */ unsigned char MBEDTLS_PRIVATE(minor_ver); /*!< The TLS version used in the session. */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) - mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets); -#endif - #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) mbedtls_x509_crt *MBEDTLS_PRIVATE(peer_cert); /*!< peer X.509 cert chain */ @@ -1154,6 +1150,10 @@ struct mbedtls_ssl_session #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) int MBEDTLS_PRIVATE(encrypt_then_mac); /*!< flag for EtM activation */ #endif + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + mbedtls_ssl_tls1_3_application_secrets MBEDTLS_PRIVATE(app_secrets); +#endif }; /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 91b4bdfec..e989d7177 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -876,8 +876,17 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { + /* + * struct { + * opaque verify_data[Hash.length]; + * } Finished; + */ + const unsigned char *expected_verify_data = + ssl->handshake->state_local.finished_in.digest; + size_t expected_verify_data_len = + ssl->handshake->state_local.finished_in.digest_len; /* Structural validation */ - if( (size_t)( end - buf ) != ssl->handshake->state_local.finished_in.digest_len ) + if( (size_t)( end - buf ) != expected_verify_data_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); @@ -887,19 +896,19 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, } MBEDTLS_SSL_DEBUG_BUF( 4, "verify_data (self-computed):", - ssl->handshake->state_local.finished_in.digest, - ssl->handshake->state_local.finished_in.digest_len ); + expected_verify_data, + expected_verify_data_len ); MBEDTLS_SSL_DEBUG_BUF( 4, "verify_data (received message):", buf, - ssl->handshake->state_local.finished_in.digest_len ); + expected_verify_data_len ); /* Semantic validation */ if( mbedtls_ssl_safer_memcmp( buf, - ssl->handshake->state_local.finished_in.digest, - ssl->handshake->state_local.finished_in.digest_len ) != 0 ) + expected_verify_data, + expected_verify_data_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } @@ -908,7 +917,7 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ssl_key_set traffic_keys; mbedtls_ssl_transform *transform_application = NULL; @@ -920,8 +929,7 @@ static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *s goto cleanup; } - ret = mbedtls_ssl_tls13_generate_application_keys( - ssl, &traffic_keys ); + ret = mbedtls_ssl_tls13_generate_application_keys( ssl, &traffic_keys ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, @@ -953,7 +961,7 @@ static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *s cleanup: - mbedtls_platform_zeroize( &traffic_keys, sizeof( mbedtls_ssl_key_set ) ); + mbedtls_platform_zeroize( &traffic_keys, sizeof( traffic_keys ) ); if( ret != 0 ) { mbedtls_free( transform_application ); @@ -977,7 +985,7 @@ static int ssl_tls13_postprocess_finished_message( mbedtls_ssl_context* ssl ) int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *buf; size_t buflen; diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index c035504bf..a030b65d3 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -568,7 +568,8 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - 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 ); @@ -578,9 +579,9 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( * Compute MasterSecret */ ret = mbedtls_ssl_tls1_3_evolve_secret( md_type, - ssl->handshake->tls1_3_master_secrets.handshake, + handshake->tls1_3_master_secrets.handshake, NULL, 0, - ssl->handshake->tls1_3_master_secrets.app ); + handshake->tls1_3_master_secrets.app ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_evolve_secret", ret ); @@ -588,7 +589,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( } MBEDTLS_SSL_DEBUG_BUF( 4, "Master secret", - ssl->handshake->tls1_3_master_secrets.app, md_size ); + handshake->tls1_3_master_secrets.app, md_size ); return( 0 ); } @@ -690,7 +691,7 @@ int mbedtls_ssl_tls13_calculate_verify_data( mbedtls_ssl_context* ssl, exit: - mbedtls_platform_zeroize( transcript, sizeof( transcript) ); + mbedtls_platform_zeroize( transcript, sizeof( transcript ) ); return( ret ); } @@ -1116,13 +1117,14 @@ int mbedtls_ssl_tls13_generate_application_keys( mbedtls_ssl_key_set *traffic_keys ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; /* Address at which to store the application secrets */ mbedtls_ssl_tls1_3_application_secrets * const app_secrets = &ssl->session_negotiate->app_secrets; /* Holding the transcript up to and including the ServerFinished */ - unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + unsigned char transcript[MBEDTLS_TLS1_3_MD_MAX_SIZE]; size_t transcript_len; /* Variables relating to the hash for the chosen ciphersuite. */ @@ -1139,11 +1141,11 @@ int mbedtls_ssl_tls13_generate_application_keys( /* Extract basic information about hash and ciphersuite */ cipher_info = mbedtls_cipher_info_from_type( - ssl->handshake->ciphersuite_info->cipher ); + handshake->ciphersuite_info->cipher ); keylen = cipher_info->key_bitlen / 8; ivlen = cipher_info->iv_size; - md_type = ssl->handshake->ciphersuite_info->mac; + md_type = handshake->ciphersuite_info->mac; md_info = mbedtls_md_info_from_type( md_type ); md_size = mbedtls_md_get_size( md_info ); @@ -1159,7 +1161,7 @@ int mbedtls_ssl_tls13_generate_application_keys( /* Compute application secrets from master secret and transcript hash. */ ret = mbedtls_ssl_tls1_3_derive_application_secrets( md_type, - ssl->handshake->tls1_3_master_secrets.app, + handshake->tls1_3_master_secrets.app, transcript, transcript_len, app_secrets ); if( ret != 0 ) @@ -1197,16 +1199,16 @@ int mbedtls_ssl_tls13_generate_application_keys( ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_APPLICATION_TRAFFIC_SECRET, app_secrets->client_application_traffic_secret_N, md_size, - ssl->handshake->randbytes + 32, - ssl->handshake->randbytes, + handshake->randbytes + 32, + handshake->randbytes, MBEDTLS_SSL_TLS_PRF_NONE /* TODO: this should be replaced by a new constant for TLS 1.3! */ ); ssl->f_export_keys( ssl->p_export_keys, MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_APPLICATION_TRAFFIC_SECRET, app_secrets->server_application_traffic_secret_N, md_size, - ssl->handshake->randbytes + 32, - ssl->handshake->randbytes, + handshake->randbytes + 32, + handshake->randbytes, MBEDTLS_SSL_TLS_PRF_NONE /* TODO: this should be replaced by a new constant for TLS 1.3! */ ); } @@ -1224,7 +1226,7 @@ int mbedtls_ssl_tls13_generate_application_keys( cleanup: - mbedtls_platform_zeroize( transcript, sizeof(transcript) ); + mbedtls_platform_zeroize( transcript, sizeof( transcript ) ); return( ret ); } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 997bdee63..43759c59e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8820,8 +8820,8 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "tls1_3 client state: 19" \ -c "tls1_3 client state: 5" \ -c "tls1_3 client state: 3" \ + -c "tls1_3 client state: 9" \ -c "tls1_3 client state: 13" \ - -c "tls1_3 client state: 20" \ -c "tls1_3 client state: 11" \ -c "tls1_3 client state: 14" \ -c "tls1_3 client state: 15" \ @@ -8852,8 +8852,8 @@ run_test "TLS1.3: Test client hello msg work - gnutls" \ -c "tls1_3 client state: 19" \ -c "tls1_3 client state: 5" \ -c "tls1_3 client state: 3" \ + -c "tls1_3 client state: 9" \ -c "tls1_3 client state: 13" \ - -c "tls1_3 client state: 20" \ -c "tls1_3 client state: 11" \ -c "tls1_3 client state: 14" \ -c "tls1_3 client state: 15" \ From c13f935c05df20d6cb80380d3b0fb40a754dfda9 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 11 Nov 2021 06:13:22 +0000 Subject: [PATCH 20/21] Align code styles of indent and so on Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index e989d7177..42c786aee 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -862,7 +862,7 @@ static int ssl_tls13_preprocess_finished_message( mbedtls_ssl_context *ssl ) sizeof( ssl->handshake->state_local.finished_in.digest ), &ssl->handshake->state_local.finished_in.digest_len, ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ? - MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT ); + MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_calculate_verify_data", ret ); @@ -878,7 +878,7 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, { /* * struct { - * opaque verify_data[Hash.length]; + * opaque verify_data[Hash.length]; * } Finished; */ const unsigned char *expected_verify_data = @@ -891,7 +891,7 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); + MBEDTLS_ERR_SSL_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } @@ -909,12 +909,14 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad finished message" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR, - MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } return( 0 ); } + +#if defined(MBEDTLS_SSL_CLI_C) static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -971,14 +973,19 @@ cleanup: } return( ret ); } +#endif /* MBEDTLS_SSL_CLI_C */ static int ssl_tls13_postprocess_finished_message( mbedtls_ssl_context* ssl ) { +#if defined(MBEDTLS_SSL_CLI_C) if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { return( ssl_tls13_postprocess_server_finished_message( ssl ) ); } +#else + ((void) ssl); +#endif /* MBEDTLS_SSL_CLI_C */ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } From a4c99f2c2d85595cb248fe04638796adb89e3612 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 11 Nov 2021 06:46:35 +0000 Subject: [PATCH 21/21] Remove useless blank line Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 1 - library/ssl_tls13_keys.c | 3 +-- library/ssl_tls13_keys.h | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 42c786aee..b2a70f3cd 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -915,7 +915,6 @@ static int ssl_tls13_parse_finished_message( mbedtls_ssl_context *ssl, return( 0 ); } - #if defined(MBEDTLS_SSL_CLI_C) static int ssl_tls13_postprocess_server_finished_message( mbedtls_ssl_context *ssl ) { diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index a030b65d3..3ca28d56e 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -564,8 +564,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( return( 0 ); } -int mbedtls_ssl_tls13_key_schedule_stage_application( - mbedtls_ssl_context *ssl ) +int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ssl_handshake_params *handshake = ssl->handshake; diff --git a/library/ssl_tls13_keys.h b/library/ssl_tls13_keys.h index 7ea018339..d598448b5 100644 --- a/library/ssl_tls13_keys.h +++ b/library/ssl_tls13_keys.h @@ -586,8 +586,7 @@ int mbedtls_ssl_tls13_generate_handshake_keys( mbedtls_ssl_context *ssl, * \returns \c 0 on success. * \returns A negative error code on failure. */ -int mbedtls_ssl_tls13_key_schedule_stage_application( - mbedtls_ssl_context *ssl ); +int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ); /** * \brief Compute TLS 1.3 application traffic keys.