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" \