diff --git a/docs/architecture/tls13-support.md b/docs/architecture/tls13-support.md index 2cf2a48522..6c39bc52ce 100644 --- a/docs/architecture/tls13-support.md +++ b/docs/architecture/tls13-support.md @@ -409,3 +409,101 @@ General coding rules: buf_len ); ``` even if it fits. + + +Overview of handshake code organization +--------------------------------------- + +The TLS 1.3 handshake protocol is implemented as a state machine. The +functions `mbedtls_ssl_tls13_handshake_{client,server}_step` are the top level +functions of that implementation. They are implemented as a switch over all the +possible states of the state machine. + +Most of the states are either dedicated to the processing or writing of an +handshake message. + +The implementation does not go systematically through all states as this would +result in too many checks of whether something needs to be done or not in a +given state to be duplicated across several state handlers. For example, on +client side, the states related to certificate parsing and validation are +bypassed if the handshake is based on a pre-shared key and thus does not +involve certificates. + +On the contrary, the implementation goes systematically though some states +even if they could be bypassed if it helps in minimizing when and where inbound +and outbound keys are updated. The `MBEDTLS_SSL_CLIENT_CERTIFICATE` state on +client side is a example of that. + +The names of the handlers processing/writing an handshake message are +prefixed with `(mbedtls_)ssl_tls13_{process,write}`. To ease the maintenance and +reduce the risk of bugs, the code of the message processing and writing +handlers is split into a sequence of stages. + +The sending of data to the peer only occurs in `mbedtls_ssl_handshake_step` +between the calls to the handlers and as a consequence handlers do not have to +care about the MBEDTLS_ERR_SSL_WANT_WRITE error code. Furthermore, all pending +data are flushed before to call the next handler. That way, handlers do not +have to worry about pending data when changing outbound keys. + +### Message processing handlers +For message processing handlers, the stages are: + +* coordination stage: check if the state should be bypassed. This stage is +optional. The check is either purely based on the reading of the value of some +fields of the SSL context or based on the reading of the type of the next +message. The latter occurs when it is not known what the next handshake message +will be, an example of that on client side being if we are going to receive a +CertificateRequest message or not. The intent is, apart from the next record +reading to not modify the SSL context as this stage may be repeated if the +next handshake message has not been received yet. + +* fetching stage: at this stage we are sure of the type of the handshake +message we must receive next and we try to fetch it. If we did not go through +a coordination stage involving the next record type reading, the next +handshake message may not have been received yet, the handler returns with +`MBEDTLS_ERR_SSL_WANT_READ` without changing the current state and it will be +called again later. + +* pre-processing stage: prepare the SSL context for the message parsing. This +stage is optional. Any processing that must be done before the parsing of the +message or that can be done to simplify the parsing code. Some simple and +partial parsing of the handshake message may append at that stage like in the +ServerHello message pre-processing. + +* parsing stage: parse the message and restrict as much as possible any +update of the SSL context. The idea of the pre-processing/parsing/post-processing +organization is to concentrate solely on the parsing in the parsing function to +reduce the size of its code and to simplify it. + +* post-processing stage: following the parsing, further update of the SSL +context to prepare for the next incoming and outgoing messages. This stage is +optional. For example, secret and key computations occur at this stage, as well +as handshake messages checksum update. + +* state change: the state change is done in the main state handler to ease the +navigation of the state machine transitions. + + +### Message writing handlers +For message writing handlers, the stages are: + +* coordination stage: check if the state should be bypassed. This stage is +optional. The check is based on the value of some fields of the SSL context. + +* preparation stage: prepare for the message writing. This stage is optional. +Any processing that must be done before the writing of the message or that can +be done to simplify the writing code. + +* writing stage: write the message and restrict as much as possible any update +of the SSL context. The idea of the preparation/writing/finalization +organization is to concentrate solely on the writing in the writing function to +reduce the size of its code and simplify it. + +* finalization stage: following the writing, further update of the SSL +context to prepare for the next incoming and outgoing messages. This stage is +optional. For example, handshake secret and key computation occur at that +stage (ServerHello writing finalization), switching to handshake keys for +outbound message on server side as well. + +* state change: the state change is done in the main state handler to ease +the navigation of the state machine transitions. diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index ead0db8355..416316b9aa 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -121,11 +121,12 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_ALPN) static int ssl_tls13_parse_alpn_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, size_t len ) + const unsigned char *buf, size_t len ) { - size_t list_len, name_len; const unsigned char *p = buf; const unsigned char *end = buf + len; + size_t protocol_name_list_len, protocol_name_len; + const unsigned char *protocol_name_list_end; /* If we didn't send it, the server shouldn't send it */ if( ssl->conf->alpn_list == NULL ) @@ -141,21 +142,22 @@ static int ssl_tls13_parse_alpn_ext( mbedtls_ssl_context *ssl, * the "ProtocolNameList" MUST contain exactly one "ProtocolName" */ - /* Min length is 2 ( list_len ) + 1 ( name_len ) + 1 ( name ) */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); - - list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + protocol_name_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_len ); - name_len = *p++; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_len - 1 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, protocol_name_list_len ); + protocol_name_list_end = p + protocol_name_list_len; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, protocol_name_list_end, 1 ); + protocol_name_len = *p++; /* Check that the server chosen protocol was in our list and save it */ - for ( const char **alpn = ssl->conf->alpn_list; *alpn != NULL; alpn++ ) + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, protocol_name_list_end, protocol_name_len ); + for( const char **alpn = ssl->conf->alpn_list; *alpn != NULL; alpn++ ) { - if( name_len == strlen( *alpn ) && - memcmp( buf + 3, *alpn, name_len ) == 0 ) + if( protocol_name_len == strlen( *alpn ) && + memcmp( p, *alpn, protocol_name_len ) == 0 ) { ssl->alpn_chosen = *alpn; return( 0 ); @@ -667,6 +669,7 @@ static int ssl_tls13_is_supported_versions_ext_present( * - cipher_suite 2 bytes * - legacy_compression_method 1 byte */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, legacy_session_id_echo_len + 4 ); p += legacy_session_id_echo_len + 4; /* Case of no extension */ @@ -740,12 +743,12 @@ static int ssl_tls13_is_downgrade_negotiation( mbedtls_ssl_context *ssl, } /* Returns a negative value on failure, and otherwise - * - SSL_SERVER_HELLO_COORDINATE_HELLO or - * - SSL_SERVER_HELLO_COORDINATE_HRR + * - SSL_SERVER_HELLO or + * - SSL_SERVER_HELLO_HRR * to indicate which message is expected and to be parsed next. */ -#define SSL_SERVER_HELLO_COORDINATE_HELLO 0 -#define SSL_SERVER_HELLO_COORDINATE_HRR 1 +#define SSL_SERVER_HELLO 0 +#define SSL_SERVER_HELLO_HRR 1 static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -772,37 +775,32 @@ static int ssl_server_hello_is_hrr( mbedtls_ssl_context *ssl, if( memcmp( buf + 2, mbedtls_ssl_tls13_hello_retry_request_magic, sizeof( mbedtls_ssl_tls13_hello_retry_request_magic ) ) == 0 ) { - return( SSL_SERVER_HELLO_COORDINATE_HRR ); + return( SSL_SERVER_HELLO_HRR ); } - return( SSL_SERVER_HELLO_COORDINATE_HELLO ); + return( SSL_SERVER_HELLO ); } -/* Fetch and preprocess +/* * Returns a negative value on failure, and otherwise - * - SSL_SERVER_HELLO_COORDINATE_HELLO or - * - SSL_SERVER_HELLO_COORDINATE_HRR or - * - SSL_SERVER_HELLO_COORDINATE_TLS1_2 + * - SSL_SERVER_HELLO or + * - SSL_SERVER_HELLO_HRR or + * - SSL_SERVER_HELLO_TLS1_2 */ -#define SSL_SERVER_HELLO_COORDINATE_TLS1_2 2 -static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, - unsigned char **buf, - size_t *buf_len ) +#define SSL_SERVER_HELLO_TLS1_2 2 +static int ssl_tls13_preprocess_server_hello( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - const unsigned char *end; - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, - MBEDTLS_SSL_HS_SERVER_HELLO, - buf, buf_len ) ); - end = *buf + *buf_len; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_is_supported_versions_ext_present( - ssl, *buf, end ) ); + ssl, buf, end ) ); if( ret == 0 ) { MBEDTLS_SSL_PROC_CHK_NEG( - ssl_tls13_is_downgrade_negotiation( ssl, *buf, end ) ); + ssl_tls13_is_downgrade_negotiation( ssl, buf, end ) ); /* If the server is negotiating TLS 1.2 or below and: * . we did not propose TLS 1.2 or @@ -810,7 +808,7 @@ static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, * version of the protocol and thus we are under downgrade attack * abort the handshake with an "illegal parameter" alert. */ - if( ssl->handshake->min_tls_version > MBEDTLS_SSL_VERSION_TLS1_2 || ret ) + if( handshake->min_tls_version > MBEDTLS_SSL_VERSION_TLS1_2 || ret ) { MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); @@ -820,7 +818,7 @@ static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, ssl->keep_current_message = 1; ssl->tls_version = MBEDTLS_SSL_VERSION_TLS1_2; mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, - *buf, *buf_len ); + buf, (size_t)(end - buf) ); if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { @@ -829,23 +827,25 @@ static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, return( ret ); } - return( SSL_SERVER_HELLO_COORDINATE_TLS1_2 ); + return( SSL_SERVER_HELLO_TLS1_2 ); } - ret = ssl_server_hello_is_hrr( ssl, *buf, end ); + handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; + + ret = ssl_server_hello_is_hrr( ssl, buf, end ); switch( ret ) { - case SSL_SERVER_HELLO_COORDINATE_HELLO: + case SSL_SERVER_HELLO: MBEDTLS_SSL_DEBUG_MSG( 2, ( "received ServerHello message" ) ); break; - case SSL_SERVER_HELLO_COORDINATE_HRR: + case SSL_SERVER_HELLO_HRR: MBEDTLS_SSL_DEBUG_MSG( 2, ( "received HelloRetryRequest message" ) ); /* If a client receives a second * HelloRetryRequest in the same connection (i.e., where the ClientHello * was itself in response to a HelloRetryRequest), it MUST abort the * handshake with an "unexpected_message" alert. */ - if( ssl->handshake->hello_retry_request_count > 0 ) + if( handshake->hello_retry_request_count > 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Multiple HRRs received" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, @@ -868,7 +868,7 @@ static int ssl_tls13_server_hello_coordinate( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - ssl->handshake->hello_retry_request_count++; + handshake->hello_retry_request_count++; break; } @@ -1247,11 +1247,6 @@ static int ssl_tls13_postprocess_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); ssl->session_in = ssl->session_negotiate; - /* - * State machine update - */ - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); - cleanup: if( ret != 0 ) { @@ -1267,17 +1262,6 @@ static int ssl_tls13_postprocess_hrr( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; -#if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) - /* If not offering early data, the client sends a dummy CCS record - * immediately before its second flight. This may either be before - * its second ClientHello or before its encrypted handshake flight. - */ - mbedtls_ssl_handshake_set_state( ssl, - MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO ); -#else - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); -#endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ - mbedtls_ssl_session_reset_msg_layer( ssl, 0 ); /* @@ -1306,20 +1290,17 @@ static int ssl_tls13_process_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> %s", __func__ ) ); - /* Coordination step - * - Fetch record - * - Make sure it's either a ServerHello or a HRR. - * - Switch processing routine in case of HRR - */ - ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, + MBEDTLS_SSL_HS_SERVER_HELLO, + &buf, &buf_len ) ); - ret = ssl_tls13_server_hello_coordinate( ssl, &buf, &buf_len ); + ret = ssl_tls13_preprocess_server_hello( ssl, buf, buf + buf_len ); if( ret < 0 ) goto cleanup; else - is_hrr = ( ret == SSL_SERVER_HELLO_COORDINATE_HRR ); + is_hrr = ( ret == SSL_SERVER_HELLO_HRR ); - if( ret == SSL_SERVER_HELLO_COORDINATE_TLS1_2 ) + if( ret == SSL_SERVER_HELLO_TLS1_2 ) { ret = 0; goto cleanup; @@ -1335,9 +1316,24 @@ static int ssl_tls13_process_server_hello( mbedtls_ssl_context *ssl ) buf, buf_len ); if( is_hrr ) + { MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_hrr( ssl ) ); +#if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) + /* If not offering early data, the client sends a dummy CCS record + * immediately before its second flight. This may either be before + * its second ClientHello or before its encrypted handshake flight. + */ + mbedtls_ssl_handshake_set_state( ssl, + MBEDTLS_SSL_CLIENT_CCS_BEFORE_2ND_CLIENT_HELLO ); +#else + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); +#endif /* MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE */ + } else + { MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_server_hello( ssl ) ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); + } cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= %s ( %s )", __func__, @@ -1347,56 +1343,13 @@ cleanup: /* * - * EncryptedExtensions message + * Handler for MBEDTLS_SSL_ENCRYPTED_EXTENSIONS * * The EncryptedExtensions message contains any extensions which * should be protected, i.e., any which are not needed to establish * the cryptographic context. */ -/* - * Overview - */ - -/* Main entry point; orchestrates the other functions */ -static int ssl_tls13_process_encrypted_extensions( mbedtls_ssl_context *ssl ); - -static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ); -static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl ); - -/* - * Handler for MBEDTLS_SSL_ENCRYPTED_EXTENSIONS - */ -static int ssl_tls13_process_encrypted_extensions( mbedtls_ssl_context *ssl ) -{ - int ret; - unsigned char *buf; - size_t buf_len; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse encrypted extensions" ) ); - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, - MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, - &buf, &buf_len ) ); - - /* Process the message contents */ - MBEDTLS_SSL_PROC_CHK( - ssl_tls13_parse_encrypted_extensions( ssl, buf, buf + buf_len ) ); - - mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, - buf, buf_len ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_encrypted_extensions( ssl ) ); - -cleanup: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse encrypted extensions" ) ); - return( ret ); - -} - /* Parse EncryptedExtensions message * struct { * Extension extensions<0..2^16-1>; @@ -1416,8 +1369,8 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, p += 2; MBEDTLS_SSL_DEBUG_BUF( 3, "encrypted extensions", p, extensions_len ); - extensions_end = p + extensions_len; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); + extensions_end = p + extensions_len; while( p < extensions_end ) { @@ -1483,8 +1436,25 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, return( ret ); } -static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl ) +static int ssl_tls13_process_encrypted_extensions( mbedtls_ssl_context *ssl ) { + int ret; + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse encrypted extensions" ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, + MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, + &buf, &buf_len ) ); + + /* Process the message contents */ + MBEDTLS_SSL_PROC_CHK( + ssl_tls13_parse_encrypted_extensions( ssl, buf, buf + buf_len ) ); + + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, + buf, buf_len ); + #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) if( mbedtls_ssl_tls13_some_psk_enabled( ssl ) ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); @@ -1494,12 +1464,16 @@ static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl ((void) ssl); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); #endif - return( 0 ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse encrypted extensions" ) ); + return( ret ); + } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* - * * STATE HANDLING: CertificateRequest * */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index acd227defd..893de43946 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -812,7 +812,7 @@ static int ssl_tls13_write_certificate_body( mbedtls_ssl_context *ssl, /* Currently, we don't have any certificate extensions defined. * Hence, we are sending an empty extension with length zero. */ - MBEDTLS_PUT_UINT24_BE( 0, p, 0 ); + MBEDTLS_PUT_UINT16_BE( 0, p, 0 ); p += 2; } @@ -1437,12 +1437,12 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, mbedtls_ssl_handshake_params *handshake = ssl->handshake; /* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; /* Check if key size is consistent with given buffer length. */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, peerkey_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, peerkey_len ); /* Store peer's ECDH public key. */ memcpy( handshake->ecdh_psa_peerkey, p, peerkey_len ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 719bf05225..bfb2204d70 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1095,7 +1095,7 @@ static int ssl_tls13_write_hrr_key_share_ext( mbedtls_ssl_context *ssl, * - extension_data_length (2 bytes) * - selected_group (2 bytes) */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 6 ); + MBEDTLS_SSL_CHK_BUF_PTR( buf, end, 6 ); MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, buf, 0 ); MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); @@ -1307,8 +1307,7 @@ cleanup: /* * Handler for MBEDTLS_SSL_HELLO_RETRY_REQUEST */ -static int ssl_tls13_write_hello_retry_request_coordinate( - mbedtls_ssl_context *ssl ) +static int ssl_tls13_prepare_hello_retry_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if( ssl->handshake->hello_retry_request_count > 0 ) @@ -1342,7 +1341,7 @@ static int ssl_tls13_write_hello_retry_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write hello retry request" ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_hello_retry_request_coordinate( ssl ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_hello_retry_request( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, @@ -1636,19 +1635,18 @@ static int ssl_tls13_write_server_finished( mbedtls_ssl_context *ssl ) return( ret ); } - if( ssl->handshake->certificate_request_sent ) - { - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); + mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to handshake keys for inbound traffic" ) ); - mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); - } + if( ssl->handshake->certificate_request_sent ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_CERTIFICATE ); else { MBEDTLS_SSL_DEBUG_MSG( 2, ( "skip parse certificate" ) ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "skip parse certificate verify" ) ); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_FINISHED ); } + return( 0 ); } @@ -1659,12 +1657,6 @@ static int ssl_tls13_process_client_finished( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if( ! ssl->handshake->certificate_request_sent ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "Switch to handshake traffic keys for inbound traffic" ) ); - mbedtls_ssl_set_inbound_transform( ssl, ssl->handshake->transform_handshake ); - } ret = mbedtls_ssl_tls13_process_finished_message( ssl ); if( ret != 0 ) return( ret ); diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 9be1bff82b..b8caca397d 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2101,14 +2101,9 @@ void perform_handshake( handshake_test_options* options ) TEST_ASSERT( mbedtls_ssl_is_handshake_over( &client.ssl ) == 1 ); /* Make sure server state is moved to HANDSHAKE_OVER also. */ - TEST_ASSERT( mbedtls_move_handshake_to_state( &(server.ssl), - &(client.ssl), - MBEDTLS_SSL_HANDSHAKE_OVER ) - == expected_handshake_result ); - if( expected_handshake_result != 0 ) - { - goto exit; - } + TEST_EQUAL( mbedtls_move_handshake_to_state( &(server.ssl), + &(client.ssl), + MBEDTLS_SSL_HANDSHAKE_OVER ), 0 ); TEST_ASSERT( mbedtls_ssl_is_handshake_over( &server.ssl ) == 1 );