commit 2b20516b60dcd098b333d710b5ad462d8701b15d
author Jarno Lamsa <jarno.lamsa@arm.com> Tue Nov 12 15:36:21 2019 +0200
committer Jarno Lamsa <jarno.lamsa@arm.com> Thu Dec 19 07:56:10 2019 +0200
tree b4e29125f5bc8f75a7451a79ef90a99c674eb823
parent 171f422109514925d022f51567c4dc6a92dd2ded

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.

library/ssl_tls.c

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 ) )