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.
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 7c03b41..2c209d3 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -1116,7 +1116,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 )
{
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 1ef8f94..fb64a2b 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -1360,7 +1360,7 @@
}
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 @@
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 @@
* 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 @@
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 @@
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 @@
/* 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
goto exit;
}
- ssl->state++;
+ ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC;
/* Process the message contents */
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ||
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index c12af96..fa132ea 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -6740,7 +6740,14 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
#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 @@
#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 @@
}
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 @@
}
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 ) )