From 9df7c80c786c3d70550b96ad57efd33d864bf8e0 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 8 Mar 2022 18:38:54 +0100 Subject: [PATCH 1/9] TLS 1.3: Always go through the CLIENT_CERTIFICATE state Even if certificate authentication is disabled at build time, go through the MBEDTLS_SSL_CLIENT_CERTIFICATE state. It simplifies overall the code for a small code size cost when certificate authentication is disabled at build time. Furthermore that way we have only one point in the code where we switch to the handshake keys for record encryption. Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 20 ++++++-------------- library/ssl_tls13_generic.c | 8 +------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index eac3d4f04f..0af8ce7482 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1917,12 +1917,7 @@ static int ssl_tls13_process_server_finished( mbedtls_ssl_context *ssl ) ssl, MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED ); #else -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); -#else - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ return( 0 ); @@ -1944,7 +1939,6 @@ static int ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE */ @@ -1954,9 +1948,14 @@ static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) ( "Switch to handshake traffic keys for outbound traffic" ) ); mbedtls_ssl_set_outbound_transform( ssl, ssl->handshake->transform_handshake ); +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) return( mbedtls_ssl_tls13_write_certificate( ssl ) ); +#else + return( 0 ); +#endif } +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY */ @@ -1973,13 +1972,6 @@ static int ssl_tls13_write_client_finished( mbedtls_ssl_context *ssl ) { int ret; - if( !ssl->handshake->client_auth ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "Switch to handshake traffic keys for outbound traffic" ) ); - mbedtls_ssl_set_outbound_transform( ssl, - ssl->handshake->transform_handshake ); - } ret = mbedtls_ssl_tls13_write_finished_message( ssl ); if( ret != 0 ) return( ret ); @@ -2060,11 +2052,11 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) ret = ssl_tls13_process_server_finished( ssl ); break; -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) case MBEDTLS_SSL_CLIENT_CERTIFICATE: ret = ssl_tls13_write_client_certificate( ssl ); break; +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY: ret = ssl_tls13_write_client_certificate_verify( ssl ); break; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index f006438a8d..4fee3b0308 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1529,14 +1529,8 @@ static int ssl_tls13_finalize_change_cipher_spec( mbedtls_ssl_context* ssl ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); break; case MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED: -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) mbedtls_ssl_handshake_set_state( ssl, - MBEDTLS_SSL_CLIENT_CERTIFICATE ); -#else - mbedtls_ssl_handshake_set_state( ssl, - MBEDTLS_SSL_CLIENT_FINISHED ); -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - + MBEDTLS_SSL_CLIENT_CERTIFICATE ); break; default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); From 66dbf9118e8db8218e1ecd23cf41930dfbea5ee8 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 2 Feb 2022 15:33:46 +0100 Subject: [PATCH 2/9] TLS 1.3: Do not send handshake data in handshake step handlers Send data (call to mbedtls_ssl_flush_output()) only from the loop over the handshake steps. That way, we do not have to take care of the partial writings (MBEDTLS_ERR_SSL_WANT_WRITE error code) on the network in handshake step handlers. Signed-off-by: Ronald Cron --- library/ssl_misc.h | 5 +++-- library/ssl_msg.c | 5 +++-- library/ssl_tls.c | 15 +++++++++++++++ library/ssl_tls13_generic.c | 7 ++----- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9ed5dfa207..6735000e63 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1231,10 +1231,11 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ); int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, - int update_checksum ); + int update_checksum, + uint8_t force_flush ); static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) { - return( mbedtls_ssl_write_handshake_msg_ext( ssl, 1 /* update checksum */ ) ); + return( mbedtls_ssl_write_handshake_msg_ext( ssl, 1 /* update checksum */, 1 /* force flush */ ) ); } int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 5f80ed5118..5105625cc0 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -2368,7 +2368,8 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) * - ssl->out_msg: the record contents (handshake headers + content) */ int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, - int update_checksum ) + int update_checksum, + uint8_t force_flush ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const size_t hs_len = ssl->out_msglen - 4; @@ -2495,7 +2496,7 @@ int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, else #endif { - if( ( ret = mbedtls_ssl_write_record( ssl, SSL_FORCE_FLUSH ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_record( ssl, force_flush ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_record", ret ); return( ret ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index adb18ab6c2..f3e5a0af93 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2693,6 +2693,21 @@ static int ssl_prepare_handshake_step( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + /* + * We may have not been able to send to the peer all the handshake data + * that were written into the output buffer by the previous handshake step: + * the write to the network callback returned with the + * #MBEDTLS_ERR_SSL_WANT_WRITE error code. + * We proceed to the next handshake step only when all data from the + * previous one have been sent to the peer, thus we make sure that this is + * the case here by calling `mbedtls_ssl_flush_output()`. The function may + * return with the #MBEDTLS_ERR_SSL_WANT_WRITE error code in which case + * we have to wait before to go ahead. + * In the case of TLS 1.3, handshake step handlers do not send data to the + * peer. Data are only sent here and through + * `mbedtls_ssl_handle_pending_alert` in case an error that triggered an + * alert occured. + */ if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 ) return( ret ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 4fee3b0308..a54207c4f7 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -103,7 +103,7 @@ int mbedtls_ssl_tls13_finish_handshake_msg( mbedtls_ssl_context *ssl, /* Add reserved 4 bytes for handshake header */ msg_with_header_len = msg_len + 4; ssl->out_msglen = msg_with_header_len; - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_handshake_msg_ext( ssl, 0 ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_handshake_msg_ext( ssl, 0, 0 ) ); cleanup: return( ret ); @@ -1483,7 +1483,6 @@ int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *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 ) ); cleanup: @@ -1564,8 +1563,6 @@ int mbedtls_ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write change cipher spec" ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_flush_output( ssl ) ); - /* Write CCS message */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_change_cipher_spec_body( ssl, ssl->out_msg, @@ -1578,7 +1575,7 @@ int mbedtls_ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_change_cipher_spec( ssl ) ); /* Dispatch message */ - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_record( ssl, 1 ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_record( ssl, 0 ) ); cleanup: From 3addfa4964deefecccab0cb1bbbaa05f763759de Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 8 Feb 2022 16:10:25 +0100 Subject: [PATCH 3/9] Move state change from WRITE_CLIENT_HELLO to its main handler Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 0af8ce7482..c76d5d30e4 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -798,12 +798,6 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls13_finalize_client_hello( mbedtls_ssl_context *ssl ) -{ - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); - return( 0 ); -} - static int ssl_tls13_prepare_client_hello( mbedtls_ssl_context *ssl ) { int ret; @@ -870,11 +864,12 @@ static int ssl_tls13_write_client_hello( mbedtls_ssl_context *ssl ) msg_len ); ssl->handshake->update_checksum( ssl, buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_client_hello( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, buf_len, msg_len ) ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); + cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write client hello" ) ); From 9f55f6316e9dd798a2c88e06a10f510e4d0e4314 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 2 Feb 2022 16:02:47 +0100 Subject: [PATCH 4/9] Move state change from CSS states to their main handler Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 27 +++++++++------------------ library/ssl_tls13_generic.c | 31 ------------------------------- 2 files changed, 9 insertions(+), 49 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index c76d5d30e4..22e8aa4cd9 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1918,22 +1918,6 @@ static int ssl_tls13_process_server_finished( mbedtls_ssl_context *ssl ) return( 0 ); } -/* - * Handler for MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED - */ -#if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) -static int ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ) -{ - int ret; - - ret = mbedtls_ssl_tls13_write_change_cipher_spec( ssl ); - if( ret != 0 ) - return( ret ); - - return( 0 ); -} -#endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ - /* * Handler for MBEDTLS_SSL_CLIENT_CERTIFICATE */ @@ -2073,9 +2057,16 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) * Injection of dummy-CCS's for middlebox compatibility */ #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) - case MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED: case MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO: - ret = ssl_tls13_write_change_cipher_spec( ssl ); + ret = mbedtls_ssl_tls13_write_change_cipher_spec( ssl ); + if( ret == 0 ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); + break; + + case MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED: + ret = mbedtls_ssl_tls13_write_change_cipher_spec( ssl ); + if( ret == 0 ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); break; #endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index a54207c4f7..330d1ccb2c 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1515,34 +1515,6 @@ void mbedtls_ssl_tls13_handshake_wrapup( mbedtls_ssl_context *ssl ) * */ #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) - -static int ssl_tls13_finalize_change_cipher_spec( mbedtls_ssl_context* ssl ) -{ - -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - switch( ssl->state ) - { - case MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO: - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); - break; - case MBEDTLS_SSL_CLIENT_CCS_AFTER_SERVER_FINISHED: - mbedtls_ssl_handshake_set_state( ssl, - MBEDTLS_SSL_CLIENT_CERTIFICATE ); - break; - default: - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - } -#else - ((void) ssl); -#endif /* MBEDTLS_SSL_CLI_C */ - - return( 0 ); -} - static int ssl_tls13_write_change_cipher_spec_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, @@ -1571,9 +1543,6 @@ int mbedtls_ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ) ssl->out_msgtype = MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC; - /* Update state */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_change_cipher_spec( ssl ) ); - /* Dispatch message */ MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_record( ssl, 0 ) ); From 00d012f2be8a57766b2f253930d360277378380b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 8 Mar 2022 15:57:12 +0100 Subject: [PATCH 5/9] Fix type of force_flush parameter Signed-off-by: Ronald Cron --- library/ssl_misc.h | 4 ++-- library/ssl_msg.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 6735000e63..97a47e4f06 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1232,13 +1232,13 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ); int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, int update_checksum, - uint8_t force_flush ); + int force_flush ); static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) { return( mbedtls_ssl_write_handshake_msg_ext( ssl, 1 /* update checksum */, 1 /* force flush */ ) ); } -int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); +int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, int force_flush ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 5105625cc0..ffb1346d67 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -2157,7 +2157,7 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) ( cur->type == MBEDTLS_SSL_MSG_HANDSHAKE && cur->p[0] == MBEDTLS_SSL_HS_FINISHED ); - uint8_t const force_flush = ssl->disable_datagram_packing == 1 ? + int const force_flush = ssl->disable_datagram_packing == 1 ? SSL_FORCE_FLUSH : SSL_DONT_FORCE_FLUSH; /* Swap epochs before sending Finished: we can't do it after @@ -2369,7 +2369,7 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) */ int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, int update_checksum, - uint8_t force_flush ) + int force_flush ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const size_t hs_len = ssl->out_msglen - 4; @@ -2520,11 +2520,11 @@ int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, * - ssl->out_msglen: length of the record content (excl headers) * - ssl->out_msg: record content */ -int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) +int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, int force_flush ) { int ret, done = 0; size_t len = ssl->out_msglen; - uint8_t flush = force_flush; + int flush = force_flush; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write record" ) ); From 3f20b77517e49e33e941d31aceaddb700ad64d6f Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 8 Mar 2022 16:00:02 +0100 Subject: [PATCH 6/9] Improve comment Signed-off-by: Ronald Cron --- library/ssl_tls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f3e5a0af93..e2f6d7790a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2695,8 +2695,8 @@ static int ssl_prepare_handshake_step( mbedtls_ssl_context *ssl ) /* * We may have not been able to send to the peer all the handshake data - * that were written into the output buffer by the previous handshake step: - * the write to the network callback returned with the + * that were written into the output buffer by the previous handshake step, + * if the write to the network callback returned with the * #MBEDTLS_ERR_SSL_WANT_WRITE error code. * We proceed to the next handshake step only when all data from the * previous one have been sent to the peer, thus we make sure that this is From 5bb8fc830a69c213bc366201d4aa9192d998b4eb Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 07:00:13 +0100 Subject: [PATCH 7/9] Call Certificate writing generic handler only if necessary Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 11 +++-- library/ssl_tls13_generic.c | 92 +++++++------------------------------ 2 files changed, 24 insertions(+), 79 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 22e8aa4cd9..dbaa70c511 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1928,10 +1928,15 @@ static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) mbedtls_ssl_set_outbound_transform( ssl, ssl->handshake->transform_handshake ); #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - return( mbedtls_ssl_tls13_write_certificate( ssl ) ); -#else - return( 0 ); + if( ssl->handshake->client_auth ) + return( mbedtls_ssl_tls13_write_certificate( ssl ) ); + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "No certificate message to send." ) ); #endif + + return( 0 ); } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 330d1ccb2c..d054d4d75d 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -847,54 +847,6 @@ cleanup: return( ret ); } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - -/* - * STATE HANDLING: Output Certificate - */ -/* Check if a certificate should be written, and if yes, - * if it is available. - * Returns a negative error code on failure ( such as no certificate - * being available on the server ), and otherwise - * SSL_WRITE_CERTIFICATE_SEND or - * SSL_WRITE_CERTIFICATE_SKIP - * indicating that a Certificate message should be written based - * on the configured certificate, or whether it should be silently skipped. - */ -#define SSL_WRITE_CERTIFICATE_SEND 0 -#define SSL_WRITE_CERTIFICATE_SKIP 1 - -static int ssl_tls13_write_certificate_coordinate( mbedtls_ssl_context *ssl ) -{ - - /* For PSK and ECDHE-PSK ciphersuites there is no certificate to exchange. */ - if( mbedtls_ssl_tls13_some_psk_enabled( ssl ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate" ) ); - return( SSL_WRITE_CERTIFICATE_SKIP ); - } - -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - /* The client MUST send a Certificate message if and only - * if the server has requested client authentication via a - * CertificateRequest message. - * - * client_auth indicates whether the server had requested - * client authentication. - */ - if( ssl->handshake->client_auth == 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate" ) ); - return( SSL_WRITE_CERTIFICATE_SKIP ); - } - } -#endif /* MBEDTLS_SSL_CLI_C */ - - return( SSL_WRITE_CERTIFICATE_SEND ); - -} - /* * enum { * X509(0), @@ -1006,39 +958,27 @@ static int ssl_tls13_finalize_write_certificate( mbedtls_ssl_context *ssl ) int mbedtls_ssl_tls13_write_certificate( mbedtls_ssl_context *ssl ) { int ret; + unsigned char *buf; + size_t buf_len, msg_len; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate" ) ); - /* Coordination: Check if we need to send a certificate. */ - MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_write_certificate_coordinate( ssl ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_CERTIFICATE, &buf, &buf_len ) ); - if( ret == SSL_WRITE_CERTIFICATE_SEND ) - { - unsigned char *buf; - size_t buf_len, msg_len; + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_body( ssl, + buf, + buf + buf_len, + &msg_len ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, - MBEDTLS_SSL_HS_CERTIFICATE, &buf, &buf_len ) ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_body( ssl, - buf, - buf + buf_len, - &msg_len ) ); - - mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, - MBEDTLS_SSL_HS_CERTIFICATE, - buf, - msg_len ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_write_certificate( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( - ssl, buf_len, msg_len ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate" ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_write_certificate( ssl ) ); - } + mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, + MBEDTLS_SSL_HS_CERTIFICATE, + buf, + msg_len ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_write_certificate( ssl ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( + ssl, buf_len, msg_len ) ); cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write certificate" ) ); From 7a94aca81af23741534a90649b3d4a1eb9b34c61 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 07:44:27 +0100 Subject: [PATCH 8/9] Move state change from CLIENT_CERTIFICATE to its main handler Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 25 +++++++++++++++++++++---- library/ssl_tls13_generic.c | 22 ---------------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index dbaa70c511..6516523746 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1923,19 +1923,36 @@ static int ssl_tls13_process_server_finished( mbedtls_ssl_context *ssl ) */ static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) { + int non_empty_certificate_msg = 0; + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake traffic keys for outbound traffic" ) ); mbedtls_ssl_set_outbound_transform( ssl, ssl->handshake->transform_handshake ); #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) if( ssl->handshake->client_auth ) - return( mbedtls_ssl_tls13_write_certificate( ssl ) ); + { + int ret = mbedtls_ssl_tls13_write_certificate( ssl ); + if( ret != 0 ) + return( ret ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "No certificate message to send." ) ); + if( mbedtls_ssl_own_cert( ssl ) != NULL ) + non_empty_certificate_msg = 1; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "No certificate message to send." ) ); + } #endif + if( non_empty_certificate_msg ) + { + mbedtls_ssl_handshake_set_state( ssl, + MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY ); + } + else + 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 d054d4d75d..088b7bbada 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -934,27 +934,6 @@ static int ssl_tls13_write_certificate_body( mbedtls_ssl_context *ssl, return( 0 ); } -static int ssl_tls13_finalize_write_certificate( mbedtls_ssl_context *ssl ) -{ -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - const mbedtls_x509_crt *crt = mbedtls_ssl_own_cert( ssl ); - if( ssl->handshake->client_auth && crt != NULL ) - { - mbedtls_ssl_handshake_set_state( ssl, - MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY ); - } - else - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); - return( 0 ); - } - else -#endif /* MBEDTLS_SSL_CLI_C */ - ((void) ssl); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); -} - int mbedtls_ssl_tls13_write_certificate( mbedtls_ssl_context *ssl ) { int ret; @@ -976,7 +955,6 @@ int mbedtls_ssl_tls13_write_certificate( mbedtls_ssl_context *ssl ) buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_write_certificate( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, buf_len, msg_len ) ); cleanup: From a8b38879e1e7be5fbf1a020b72ed56ddacaa011c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 07:59:25 +0100 Subject: [PATCH 9/9] Move state change from CLIENT_CERTIFICATE_VERIFY to its main handler Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 7 ++++++- library/ssl_tls13_generic.c | 18 ------------------ 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 6516523746..e4c3af5b7c 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1962,7 +1962,12 @@ static int ssl_tls13_write_client_certificate( mbedtls_ssl_context *ssl ) */ static int ssl_tls13_write_client_certificate_verify( mbedtls_ssl_context *ssl ) { - return( mbedtls_ssl_tls13_write_certificate_verify( ssl ) ); + int ret = mbedtls_ssl_tls13_write_certificate_verify( ssl ); + + if( ret == 0 ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); + + return( ret ); } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 088b7bbada..24a3d9dc34 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1106,22 +1106,6 @@ static int ssl_tls13_write_certificate_verify_body( mbedtls_ssl_context *ssl, return( ret ); } -static int ssl_tls13_finalize_certificate_verify( mbedtls_ssl_context *ssl ) -{ -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); - } - else -#endif /* MBEDTLS_SSL_CLI_C */ - { - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); - } - - return( 0 ); -} - int mbedtls_ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -1138,8 +1122,6 @@ int mbedtls_ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ) mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, buf, msg_len ); - /* Update state */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_certificate_verify( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, buf_len, msg_len ) );