From 74af2a827ed385235d9ccf6055017b6cad5adfcf Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 22 Sep 2021 07:40:30 +0000 Subject: [PATCH 01/12] TLS1.3: Add client finish processing in client side Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 8 +-- library/ssl_tls13_generic.c | 126 ++++++++++++++++++++++++++++++++++++ library/ssl_tls13_keys.c | 50 ++++++++++++++ 3 files changed, 179 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 6deab2a8c7..a0f0c9c986 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1618,11 +1618,9 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) /* * Handler for MBEDTLS_SSL_CLIENT_FINISHED */ -static int ssl_tls1_3_write_client_finished( mbedtls_ssl_context *ssl ) +static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_FLUSH_BUFFERS ); - return( 0 ); + return ( mbedtls_ssl_tls1_3_finished_out_process( ssl ) ); } /* @@ -1689,7 +1687,7 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_CLIENT_FINISHED: - ret = ssl_tls1_3_write_client_finished( ssl ); + ret = ssl_tls13_write_client_finished( ssl ); break; case MBEDTLS_SSL_FLUSH_BUFFERS: diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index b2a70f3cdd..a42ede1e39 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -29,6 +29,7 @@ #include "mbedtls/debug.h" #include "mbedtls/oid.h" #include "mbedtls/platform.h" +#include #include "ssl_misc.h" #include "ssl_tls13_keys.h" @@ -1014,6 +1015,131 @@ cleanup: return( ret ); } +/* + * + * STATE HANDLING: Outgoing Finished + * + */ + +/* + * Overview + */ + +/* Main entry point: orchestrates the other functions */ + +int mbedtls_ssl_finished_out_process( mbedtls_ssl_context *ssl ); + +static int ssl_finished_out_prepare( mbedtls_ssl_context *ssl ); +static int ssl_finished_out_write( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen, + size_t *olen ); +static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ); + + +int mbedtls_ssl_tls1_3_finished_out_process( mbedtls_ssl_context *ssl ) +{ + int ret; + unsigned char *buf; + size_t buf_len, msg_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished" ) ); + + if( !ssl->handshake->state_local.finished_out.preparation_done ) + { + MBEDTLS_SSL_PROC_CHK( ssl_finished_out_prepare( ssl ) ); + ssl->handshake->state_local.finished_out.preparation_done = 1; + } + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_finished_out_write( + ssl, buf, buf_len, &msg_len ) ); + + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, + buf, msg_len ); + + MBEDTLS_SSL_PROC_CHK( ssl_finished_out_postprocess( ssl ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, + buf_len, msg_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_flush_output( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished" ) ); + return( ret ); +} + +static int ssl_finished_out_prepare( mbedtls_ssl_context *ssl ) +{ + int ret; + + /* Compute transcript of handshake up to now. */ + ret = mbedtls_ssl_tls1_3_calc_finished( ssl, + ssl->handshake->state_local.finished_out.digest, + sizeof( ssl->handshake->state_local.finished_out.digest ), + &ssl->handshake->state_local.finished_out.digest_len, + ssl->conf->endpoint ); + + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "calc_finished failed", ret ); + return( ret ); + } + + return( 0 ); +} + +static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + +#if defined(MBEDTLS_SSL_CLI_C) + if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) + { + /* Compute resumption_master_secret */ + ret = mbedtls_ssl_tls1_3_generate_resumption_master_secret( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_tls1_3_generate_resumption_master_secret ", ret ); + return ( ret ); + } + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_FLUSH_BUFFERS ); + } + else +#endif /* MBEDTLS_SSL_CLI_C */ + { + /* Should never happen */ + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + return( 0 ); +} + +static int ssl_finished_out_write( mbedtls_ssl_context *ssl, + unsigned char *buf, + size_t buflen, + size_t *olen ) +{ + size_t finished_len = ssl->handshake->state_local.finished_out.digest_len; + + /* Note: Even if DTLS is used, the current message writing functions + * write TLS headers, and it is only at sending time that the actual + * DTLS header is generated. That's why we unconditionally shift by + * 4 bytes here as opposed to mbedtls_ssl_hs_hdr_len( ssl ). */ + + if( buflen < finished_len ) + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + + memcpy( buf, ssl->handshake->state_local.finished_out.digest, + ssl->handshake->state_local.finished_out.digest_len ); + + *olen = finished_len; + return( 0 ); +} #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 3ca28d56ec..6dc27a4514 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -593,6 +593,56 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) return( 0 ); } +#if defined(MBEDTLS_SSL_NEW_SESSION_TICKET) +int mbedtls_ssl_tls1_3_generate_resumption_master_secret( + mbedtls_ssl_context *ssl ) +{ + int ret = 0; + + mbedtls_md_type_t md_type; + mbedtls_md_info_t const *md_info; + size_t md_size; + + unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; + size_t transcript_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "=> mbedtls_ssl_tls1_3_generate_resumption_master_secret" ) ); + + md_type = ssl->handshake->ciphersuite_info->mac; + md_info = mbedtls_md_info_from_type( md_type ); + md_size = mbedtls_md_get_size( md_info ); + + ret = mbedtls_ssl_get_handshake_transcript( ssl, md_type, + transcript, sizeof( transcript ), + &transcript_len ); + if( ret != 0 ) + return( ret ); + + ret = mbedtls_ssl_tls1_3_derive_resumption_master_secret( md_type, + ssl->handshake->tls1_3_master_secrets.app, + transcript, transcript_len, + &ssl->session_negotiate->app_secrets ); + if( ret != 0 ) + return( ret ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "Resumption master secret", + ssl->session_negotiate->app_secrets.resumption_master_secret, + md_size ); + + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "<= mbedtls_ssl_tls1_3_generate_resumption_master_secret" ) ); + return( 0 ); +} +#else /* MBEDTLS_SSL_NEW_SESSION_TICKET */ +int mbedtls_ssl_tls1_3_generate_resumption_master_secret( + mbedtls_ssl_context *ssl ) +{ + ((void) ssl); + return( 0 ); +} +#endif /* MBEDTLS_SSL_NEW_SESSION_TICKET */ + static int ssl_tls1_3_calc_finished_core( mbedtls_md_type_t md_type, unsigned char const *base_key, unsigned char const *transcript, From eab1023dbf9ffe9ae986da7fc053b2ee51c5b8bd Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 25 Oct 2021 07:38:31 +0000 Subject: [PATCH 02/12] Fix some compiling errors for name mismatch Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_generic.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index a0f0c9c986..b64d3269d1 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1620,7 +1620,7 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) */ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { - return ( mbedtls_ssl_tls1_3_finished_out_process( ssl ) ); + return ( mbedtls_ssl_tls13_finished_out_process( ssl ) ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index a42ede1e39..d1a20dfd13 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1037,7 +1037,7 @@ static int ssl_finished_out_write( mbedtls_ssl_context *ssl, static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls1_3_finished_out_process( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_tls13_finished_out_process( mbedtls_ssl_context *ssl ) { int ret; unsigned char *buf; @@ -1076,7 +1076,7 @@ static int ssl_finished_out_prepare( mbedtls_ssl_context *ssl ) int ret; /* Compute transcript of handshake up to now. */ - ret = mbedtls_ssl_tls1_3_calc_finished( ssl, + ret = mbedtls_ssl_tls1_3_calculate_expected_finished( ssl, ssl->handshake->state_local.finished_out.digest, sizeof( ssl->handshake->state_local.finished_out.digest ), &ssl->handshake->state_local.finished_out.digest_len, From c00ba8131088b85d883cf0683c8fc2f9c9f7aecb Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 29 Oct 2021 02:42:35 +0000 Subject: [PATCH 03/12] Remove MBEDTLS_SSL_NEW_SESSION_TICKET in TLS1.3 MVP Signed-off-by: XiaokangQian --- library/ssl_tls13_keys.c | 43 ---------------------------------------- 1 file changed, 43 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 6dc27a4514..5eb52ab8c5 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -593,55 +593,12 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) return( 0 ); } -#if defined(MBEDTLS_SSL_NEW_SESSION_TICKET) -int mbedtls_ssl_tls1_3_generate_resumption_master_secret( - mbedtls_ssl_context *ssl ) -{ - int ret = 0; - - mbedtls_md_type_t md_type; - mbedtls_md_info_t const *md_info; - size_t md_size; - - unsigned char transcript[MBEDTLS_MD_MAX_SIZE]; - size_t transcript_len; - - MBEDTLS_SSL_DEBUG_MSG( 2, - ( "=> mbedtls_ssl_tls1_3_generate_resumption_master_secret" ) ); - - md_type = ssl->handshake->ciphersuite_info->mac; - md_info = mbedtls_md_info_from_type( md_type ); - md_size = mbedtls_md_get_size( md_info ); - - ret = mbedtls_ssl_get_handshake_transcript( ssl, md_type, - transcript, sizeof( transcript ), - &transcript_len ); - if( ret != 0 ) - return( ret ); - - ret = mbedtls_ssl_tls1_3_derive_resumption_master_secret( md_type, - ssl->handshake->tls1_3_master_secrets.app, - transcript, transcript_len, - &ssl->session_negotiate->app_secrets ); - if( ret != 0 ) - return( ret ); - - MBEDTLS_SSL_DEBUG_BUF( 4, "Resumption master secret", - ssl->session_negotiate->app_secrets.resumption_master_secret, - md_size ); - - MBEDTLS_SSL_DEBUG_MSG( 2, - ( "<= mbedtls_ssl_tls1_3_generate_resumption_master_secret" ) ); - return( 0 ); -} -#else /* MBEDTLS_SSL_NEW_SESSION_TICKET */ int mbedtls_ssl_tls1_3_generate_resumption_master_secret( mbedtls_ssl_context *ssl ) { ((void) ssl); return( 0 ); } -#endif /* MBEDTLS_SSL_NEW_SESSION_TICKET */ static int ssl_tls1_3_calc_finished_core( mbedtls_md_type_t md_type, unsigned char const *base_key, From e1655e4db8b4972648621eff854e91d0f40e8c94 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 3 Nov 2021 07:13:47 +0000 Subject: [PATCH 04/12] Change naming styles and fix ci failure Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_generic.c | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index b64d3269d1..6c009213bf 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1620,7 +1620,7 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) */ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { - return ( mbedtls_ssl_tls13_finished_out_process( ssl ) ); + return ( mbedtls_ssl_tls13_process_finished_out( ssl ) ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index d1a20dfd13..6fc141a5d4 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1027,17 +1027,15 @@ cleanup: /* Main entry point: orchestrates the other functions */ -int mbedtls_ssl_finished_out_process( mbedtls_ssl_context *ssl ); - -static int ssl_finished_out_prepare( mbedtls_ssl_context *ssl ); -static int ssl_finished_out_write( mbedtls_ssl_context *ssl, +static int ssl_prepare_finished_out( mbedtls_ssl_context *ssl ); +static int ssl_write_finished_out( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, size_t *olen ); -static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ); +static int ssl_postprocess_finished_out( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls13_finished_out_process( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_tls13_process_finished_out( mbedtls_ssl_context *ssl ) { int ret; unsigned char *buf; @@ -1047,20 +1045,20 @@ int mbedtls_ssl_tls13_finished_out_process( mbedtls_ssl_context *ssl ) if( !ssl->handshake->state_local.finished_out.preparation_done ) { - MBEDTLS_SSL_PROC_CHK( ssl_finished_out_prepare( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_prepare_finished_out( ssl ) ); ssl->handshake->state_local.finished_out.preparation_done = 1; } MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_finished_out_write( + MBEDTLS_SSL_PROC_CHK( ssl_write_finished_out( ssl, buf, buf_len, &msg_len ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( ssl_finished_out_postprocess( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_postprocess_finished_out( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, buf_len, msg_len ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_flush_output( ssl ) ); @@ -1071,7 +1069,7 @@ cleanup: return( ret ); } -static int ssl_finished_out_prepare( mbedtls_ssl_context *ssl ) +static int ssl_prepare_finished_out( mbedtls_ssl_context *ssl ) { int ret; @@ -1091,7 +1089,7 @@ static int ssl_finished_out_prepare( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ) +static int ssl_postprocess_finished_out( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -1112,6 +1110,7 @@ static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ) else #endif /* MBEDTLS_SSL_CLI_C */ { + ((void) ssl); /* Should never happen */ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } @@ -1119,7 +1118,7 @@ static int ssl_finished_out_postprocess( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_finished_out_write( mbedtls_ssl_context *ssl, +static int ssl_write_finished_out( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, size_t *olen ) From cc90c9441363143fb3693a3b26d40cf92991961b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 9 Nov 2021 12:30:09 +0000 Subject: [PATCH 05/12] Rebase and change code Solve conflicts. Rename functions Align coding style Signed-off-by: XiaokangQian --- library/ssl_misc.h | 1 + library/ssl_tls13_client.c | 2 +- library/ssl_tls13_generic.c | 37 +++++++++++++++---------------------- library/ssl_tls13_keys.c | 7 ------- 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 362117fd9a..2408fd1211 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1185,6 +1185,7 @@ 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_write_finished_message( 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 6c009213bf..df8dfdf963 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1620,7 +1620,7 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) */ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { - return ( mbedtls_ssl_tls13_process_finished_out( ssl ) ); + return ( mbedtls_ssl_tls13_write_finished_message( ssl ) ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 6fc141a5d4..39a04ac20e 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -975,7 +975,7 @@ cleanup: } #endif /* MBEDTLS_SSL_CLI_C */ -static int ssl_tls13_postprocess_finished_message( mbedtls_ssl_context* ssl ) +static int ssl_tls13_postprocess_finished_message( mbedtls_ssl_context *ssl ) { #if defined(MBEDTLS_SSL_CLI_C) @@ -1017,7 +1017,7 @@ cleanup: /* * - * STATE HANDLING: Outgoing Finished + * STATE HANDLING: Write and send Finished message. * */ @@ -1027,15 +1027,15 @@ cleanup: /* Main entry point: orchestrates the other functions */ -static int ssl_prepare_finished_out( mbedtls_ssl_context *ssl ); -static int ssl_write_finished_out( mbedtls_ssl_context *ssl, +static int ssl_prepare_finished_message( mbedtls_ssl_context *ssl ); +static int ssl_tls13_write_finished_message_bod( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, size_t *olen ); -static int ssl_postprocess_finished_out( mbedtls_ssl_context *ssl ); +static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ); -int mbedtls_ssl_tls13_process_finished_out( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) { int ret; unsigned char *buf; @@ -1045,20 +1045,20 @@ int mbedtls_ssl_tls13_process_finished_out( mbedtls_ssl_context *ssl ) if( !ssl->handshake->state_local.finished_out.preparation_done ) { - MBEDTLS_SSL_PROC_CHK( ssl_prepare_finished_out( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_prepare_finished_message( ssl ) ); ssl->handshake->state_local.finished_out.preparation_done = 1; } MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_write_finished_out( + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_finished_message_bod( ssl, buf, buf_len, &msg_len ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, - buf, msg_len ); + buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( ssl_postprocess_finished_out( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_message( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, buf_len, msg_len ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_flush_output( ssl ) ); @@ -1069,12 +1069,12 @@ cleanup: return( ret ); } -static int ssl_prepare_finished_out( mbedtls_ssl_context *ssl ) +static int ssl_prepare_finished_message( mbedtls_ssl_context *ssl ) { int ret; /* Compute transcript of handshake up to now. */ - ret = mbedtls_ssl_tls1_3_calculate_expected_finished( ssl, + ret = mbedtls_ssl_tls13_calculate_verify_data( ssl, ssl->handshake->state_local.finished_out.digest, sizeof( ssl->handshake->state_local.finished_out.digest ), &ssl->handshake->state_local.finished_out.digest_len, @@ -1089,21 +1089,14 @@ static int ssl_prepare_finished_out( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_postprocess_finished_out( mbedtls_ssl_context *ssl ) +static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ) { - int ret = 0; #if defined(MBEDTLS_SSL_CLI_C) if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { /* Compute resumption_master_secret */ - ret = mbedtls_ssl_tls1_3_generate_resumption_master_secret( ssl ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, - "mbedtls_ssl_tls1_3_generate_resumption_master_secret ", ret ); - return ( ret ); - } + ((void) ssl); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_FLUSH_BUFFERS ); } @@ -1118,7 +1111,7 @@ static int ssl_postprocess_finished_out( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_write_finished_out( mbedtls_ssl_context *ssl, +static int ssl_tls13_write_finished_message_bod( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen, size_t *olen ) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 5eb52ab8c5..3ca28d56ec 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -593,13 +593,6 @@ int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) return( 0 ); } -int mbedtls_ssl_tls1_3_generate_resumption_master_secret( - mbedtls_ssl_context *ssl ) -{ - ((void) ssl); - 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, From 8773aa0da95fbae3d83c7e63a72363338a8d2798 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 10 Nov 2021 07:33:09 +0000 Subject: [PATCH 06/12] Align coding styles in generic for client finish Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 39a04ac20e..d52ec2f799 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1027,10 +1027,10 @@ cleanup: /* Main entry point: orchestrates the other functions */ -static int ssl_prepare_finished_message( mbedtls_ssl_context *ssl ); -static int ssl_tls13_write_finished_message_bod( mbedtls_ssl_context *ssl, +static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ); +static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, unsigned char *buf, - size_t buflen, + unsigned char *end, size_t *olen ); static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ); @@ -1041,19 +1041,19 @@ int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) unsigned char *buf; size_t buf_len, msg_len; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished message" ) ); if( !ssl->handshake->state_local.finished_out.preparation_done ) { - MBEDTLS_SSL_PROC_CHK( ssl_prepare_finished_message( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_message( ssl ) ); ssl->handshake->state_local.finished_out.preparation_done = 1; } MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_finished_message_bod( - ssl, buf, buf_len, &msg_len ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_finished_message_body( + ssl, buf, buf + buf_len, &msg_len ) ); mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, buf, msg_len ); @@ -1065,11 +1065,11 @@ int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) cleanup: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished message" ) ); return( ret ); } -static int ssl_prepare_finished_message( mbedtls_ssl_context *ssl ) +static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ) { int ret; @@ -1111,25 +1111,19 @@ static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ) return( 0 ); } -static int ssl_tls13_write_finished_message_bod( mbedtls_ssl_context *ssl, +static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, unsigned char *buf, - size_t buflen, + unsigned char *end, size_t *olen ) { - size_t finished_len = ssl->handshake->state_local.finished_out.digest_len; + size_t verify_data_len = ssl->handshake->state_local.finished_out.digest_len; - /* Note: Even if DTLS is used, the current message writing functions - * write TLS headers, and it is only at sending time that the actual - * DTLS header is generated. That's why we unconditionally shift by - * 4 bytes here as opposed to mbedtls_ssl_hs_hdr_len( ssl ). */ - - if( buflen < finished_len ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + MBEDTLS_SSL_CHK_BUF_PTR( buf, end, verify_data_len ); memcpy( buf, ssl->handshake->state_local.finished_out.digest, - ssl->handshake->state_local.finished_out.digest_len ); + verify_data_len ); - *olen = finished_len; + *olen = verify_data_len; return( 0 ); } From 35dc625e37d7a96ea83ba35c62c0f92c16cb25f1 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 11 Nov 2021 08:16:19 +0000 Subject: [PATCH 07/12] Move the location of functions Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 89 ++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index d52ec2f799..064da54874 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1020,55 +1020,10 @@ cleanup: * STATE HANDLING: Write and send Finished message. * */ - /* - * Overview + * Implement */ -/* Main entry point: orchestrates the other functions */ - -static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ); -static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *olen ); -static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ); - - -int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) -{ - int ret; - unsigned char *buf; - size_t buf_len, msg_len; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished message" ) ); - - if( !ssl->handshake->state_local.finished_out.preparation_done ) - { - MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_message( ssl ) ); - ssl->handshake->state_local.finished_out.preparation_done = 1; - } - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, - MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_finished_message_body( - ssl, buf, buf + buf_len, &msg_len ) ); - - mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, - buf, msg_len ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_message( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, - buf_len, msg_len ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_flush_output( ssl ) ); - -cleanup: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished message" ) ); - return( ret ); -} - static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ) { int ret; @@ -1095,7 +1050,6 @@ static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_CLI_C) if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - /* Compute resumption_master_secret */ ((void) ssl); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_FLUSH_BUFFERS ); @@ -1112,9 +1066,9 @@ static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ) } static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *olen ) + unsigned char *buf, + unsigned char *end, + size_t *olen ) { size_t verify_data_len = ssl->handshake->state_local.finished_out.digest_len; @@ -1127,6 +1081,41 @@ static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, return( 0 ); } +/* Main entry point: orchestrates the other functions */ +int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *buf; + size_t buf_len, msg_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished message" ) ); + + if( !ssl->handshake->state_local.finished_out.preparation_done ) + { + MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_message( ssl ) ); + ssl->handshake->state_local.finished_out.preparation_done = 1; + } + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_finished_message_body( + ssl, buf, buf + buf_len, &msg_len ) ); + + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, + buf, msg_len ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_message( ssl ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, + buf_len, msg_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_flush_output( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished message" ) ); + return( ret ); +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_SSL_TLS_C */ From 0fa6643eb5969a166587446f09cce4d1faceeb6a Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 15 Nov 2021 03:33:57 +0000 Subject: [PATCH 08/12] Align coding stles and remove useless code Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 9 ++++++++- library/ssl_tls13_generic.c | 29 +++++++---------------------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index df8dfdf963..69d9c665f9 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1620,7 +1620,14 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) */ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { - return ( mbedtls_ssl_tls13_write_finished_message( ssl ) ); + int ret; + + ret = mbedtls_ssl_tls13_write_finished_message( ssl ); + if( ret != 0 ) + return( ret ); + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_FLUSH_BUFFERS ); + return( 0 ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 064da54874..97ef33d631 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1046,21 +1046,8 @@ static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ) static int ssl_tls13_finalize_finished_message( mbedtls_ssl_context *ssl ) { - -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - ((void) ssl); - - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_FLUSH_BUFFERS ); - } - else -#endif /* MBEDTLS_SSL_CLI_C */ - { - ((void) ssl); - /* Should never happen */ - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } + // TODO: Add back resumption keys calculation after MVP. + ((void) ssl); return( 0 ); } @@ -1071,7 +1058,11 @@ static int ssl_tls13_write_finished_message_body( mbedtls_ssl_context *ssl, size_t *olen ) { size_t verify_data_len = ssl->handshake->state_local.finished_out.digest_len; - + /* + * struct { + * opaque verify_data[Hash.length]; + * } Finished; + */ MBEDTLS_SSL_CHK_BUF_PTR( buf, end, verify_data_len ); memcpy( buf, ssl->handshake->state_local.finished_out.digest, @@ -1090,12 +1081,6 @@ int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished message" ) ); - if( !ssl->handshake->state_local.finished_out.preparation_done ) - { - MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_message( ssl ) ); - ssl->handshake->state_local.finished_out.preparation_done = 1; - } - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); From dce82245acc80b69a1d28f4e66115018371a9870 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 15 Nov 2021 06:01:26 +0000 Subject: [PATCH 09/12] Fix the compile issue about prepare message Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 97ef33d631..3678e681a2 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1081,6 +1081,8 @@ int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write finished message" ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_message( ssl ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); From 9ec8fcfddd090f91154b99cb64b3735cff77f357 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 15 Nov 2021 08:24:08 +0000 Subject: [PATCH 10/12] Improve failure messag for calculating verify data Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 3678e681a2..f17bf994c2 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1037,7 +1037,7 @@ static int ssl_tls13_prepare_finished_message( mbedtls_ssl_context *ssl ) if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "calc_finished failed", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "calculate_verify_data failed", ret ); return( ret ); } From a3087e881e9d4c024ec43c9951d4425f3d87db31 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 16 Nov 2021 02:04:21 +0000 Subject: [PATCH 11/12] Fix finished message decryption fail issue Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 69d9c665f9..1516523e54 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1328,6 +1328,7 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) handshake->transform_handshake = transform_handshake; mbedtls_ssl_set_inbound_transform( ssl, transform_handshake ); + mbedtls_ssl_set_outbound_transform( ssl, ssl->handshake->transform_handshake ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); ssl->session_in = ssl->session_negotiate; From 3ce4d51c11602db443ffccd798af3d92c32d6e79 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 17 Nov 2021 02:11:36 +0000 Subject: [PATCH 12/12] Move set_outbound_transform to finalize server finished. Signed-off-by: XiaokangQian --- library/ssl_tls13_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 1516523e54..a2e5f33a0d 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1328,7 +1328,6 @@ static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) handshake->transform_handshake = transform_handshake; mbedtls_ssl_set_inbound_transform( ssl, transform_handshake ); - mbedtls_ssl_set_outbound_transform( ssl, ssl->handshake->transform_handshake ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); ssl->session_in = ssl->session_negotiate; @@ -1612,6 +1611,7 @@ static int ssl_tls1_3_process_server_finished( mbedtls_ssl_context *ssl ) if( ret != 0 ) return( ret ); + mbedtls_ssl_set_outbound_transform( ssl, ssl->handshake->transform_handshake ); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); return( 0 ); }