Make TLS state changes explicit

This is to enable hardening the security when changing
states in state machine so that the state cannot be changed by bit flipping.
The later commit changes the enumerations so that the states have large
hamming distance in between them to prevent this kind of attack.
This commit is contained in:
Jarno Lamsa 2019-11-12 15:36:21 +02:00
parent 171f422109
commit 2b20516b60
3 changed files with 109 additions and 33 deletions

View file

@ -1116,7 +1116,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl )
ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE;
ssl->out_msg[0] = MBEDTLS_SSL_HS_CLIENT_HELLO;
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO;
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) )
@ -1839,7 +1839,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl )
else
{
/* Start a new session */
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_CERTIFICATE;
#if defined(MBEDTLS_HAVE_TIME)
ssl->session_negotiate->start = mbedtls_time( NULL );
#endif
@ -3143,7 +3143,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl )
if( ! mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO_DONE;
return( 0 );
}
@ -3165,7 +3165,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl )
if( ! mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO_DONE;
return( 0 );
}
@ -3183,7 +3183,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl )
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO_DONE;
ssl->client_auth = ( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST );
MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request",
@ -3340,7 +3340,7 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl )
return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO_DONE );
}
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CERTIFICATE;
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) )
@ -3827,7 +3827,7 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate verify" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
return( 0 );
}
@ -3866,14 +3866,14 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate verify" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
return( 0 );
}
if( ssl->client_auth == 0 || mbedtls_ssl_own_cert( ssl ) == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate verify" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
return( 0 );
}
@ -3997,7 +3997,7 @@ sign:
ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE;
ssl->out_msg[0] = MBEDTLS_SSL_HS_CERTIFICATE_VERIFY;
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 )
{

View file

@ -1360,7 +1360,7 @@ have_ciphersuite_v2:
}
ssl->in_left = 0;
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO;
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse client hello v2" ) );
@ -2298,7 +2298,7 @@ have_ciphersuite:
mbedtls_ssl_get_ciphersuite_name(
mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ) ) ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO;
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) )
@ -2858,7 +2858,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl )
* New session, create a new session id,
* unless we're about to issue a session ticket
*/
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_CERTIFICATE;
#if defined(MBEDTLS_HAVE_TIME)
ssl->session_negotiate->start = mbedtls_time( NULL );
@ -3008,7 +3008,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate request" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO_DONE;
return( 0 );
}
@ -3030,7 +3030,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl )
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate request" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_SERVER_HELLO_DONE;
#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION)
if( ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET )
@ -3693,7 +3693,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl )
/* Key exchanges not involving ephemeral keys don't use
* ServerKeyExchange, so end here. */
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write server key exchange" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CERTIFICATE_REQUEST;
return( 0 );
}
#endif /* MBEDTLS_KEY_EXCHANGE__SOME_NON_PFS__ENABLED */
@ -3751,7 +3751,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl )
ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE;
ssl->out_msg[0] = MBEDTLS_SSL_HS_SERVER_KEY_EXCHANGE;
ssl->state++;
ssl->state = MBEDTLS_SSL_CERTIFICATE_REQUEST;
if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 )
{
@ -3773,7 +3773,7 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl )
ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE;
ssl->out_msg[0] = MBEDTLS_SSL_HS_SERVER_HELLO_DONE;
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CERTIFICATE;
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) )
@ -4422,7 +4422,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
return( 0 );
}
@ -4450,7 +4450,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_cert_req_allowed( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
return( 0 );
}
@ -4478,7 +4478,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
if( peer_pk == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) );
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
return( 0 );
}
@ -4490,7 +4490,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
goto exit;
}
ssl->state++;
ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
/* Process the message contents */
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ||

View file

@ -6740,7 +6740,14 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_uses_srv_cert( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate" ) );
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_CLIENT_KEY_EXCHANGE;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE;
}
return( 0 );
}
@ -6758,7 +6765,14 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_uses_srv_cert( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate" ) );
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_CLIENT_KEY_EXCHANGE;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE;
}
return( 0 );
}
@ -6782,7 +6796,14 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl )
if( !mbedtls_ssl_ciphersuite_uses_srv_cert( ciphersuite_info ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate" ) );
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_CLIENT_KEY_EXCHANGE;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE;
}
return( 0 );
}
@ -6793,7 +6814,14 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl )
if( ssl->client_auth == 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate" ) );
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_CLIENT_KEY_EXCHANGE;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE;
}
return( 0 );
}
@ -6867,7 +6895,14 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl )
write_msg:
#endif
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_CLIENT_KEY_EXCHANGE;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE;
}
if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 )
{
@ -7523,7 +7558,16 @@ crt_verify:
exit:
if( ret == 0 )
ssl->state++;
{
if( ssl->state == MBEDTLS_SSL_CLIENT_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_CLIENT_KEY_EXCHANGE;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CERTIFICATE )
{
ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE;
}
}
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ret == MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS )
@ -7553,7 +7597,14 @@ int mbedtls_ssl_write_change_cipher_spec( mbedtls_ssl_context *ssl )
ssl->out_msglen = 1;
ssl->out_msg[0] = 1;
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC )
{
ssl->state = MBEDTLS_SSL_CLIENT_FINISHED;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC )
{
ssl->state = MBEDTLS_SSL_SERVER_FINISHED;
}
if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 )
{
@ -7636,7 +7687,14 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl )
}
#endif
ssl->state++;
if( ssl->state == MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC )
{
ssl->state = MBEDTLS_SSL_CLIENT_FINISHED;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC )
{
ssl->state = MBEDTLS_SSL_SERVER_FINISHED;
}
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse change cipher spec" ) );
@ -7742,7 +7800,7 @@ void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl )
#endif
ssl_handshake_wrapup_free_hs_transform( ssl );
ssl->state++;
ssl->state = MBEDTLS_SSL_HANDSHAKE_OVER;
MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= handshake wrapup" ) );
}
@ -7804,7 +7862,16 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl )
}
else
#endif /* !MBEDTLS_SSL_NO_SESSION_RESUMPTION */
ssl->state++;
{
if( ssl->state == MBEDTLS_SSL_CLIENT_FINISHED )
{
ssl->state = MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_FINISHED )
{
ssl->state = MBEDTLS_SSL_FLUSH_BUFFERS;
}
}
/*
* Switch to our negotiated transform and session parameters for outbound
@ -7964,7 +8031,16 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl )
}
else
#endif /* !MBEDTLS_SSL_NO_SESSION_RESUMPTION */
ssl->state++;
{
if( ssl->state == MBEDTLS_SSL_CLIENT_FINISHED )
{
ssl->state = MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC;
}
else if( ssl->state == MBEDTLS_SSL_SERVER_FINISHED )
{
ssl->state = MBEDTLS_SSL_FLUSH_BUFFERS;
}
}
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) )