From 2b20516b60dcd098b333d710b5ad462d8701b15d Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 12 Nov 2019 15:36:21 +0200 Subject: [PATCH 01/30] 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_cli.c | 20 +++++----- library/ssl_srv.c | 24 ++++++------ library/ssl_tls.c | 98 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 109 insertions(+), 33 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 7c03b41f9..2c209d3e9 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -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 ) { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 1ef8f9466..fb64a2be4 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -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 || diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c12af9684..fa132ea2d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -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 ) ) From 70abd7aadcb8221164c05d91d2c14c67dd542bad Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 12 Nov 2019 15:39:38 +0200 Subject: [PATCH 02/30] Add enumeration for invalid state The invalid state can be used when state-mismatch is noticed. The invalid state should report a FI-alert upwards. --- include/mbedtls/ssl.h | 1 + library/ssl_cli.c | 1 + library/ssl_srv.c | 1 + 3 files changed, 3 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8008b516b..f147069d3 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -583,6 +583,7 @@ typedef enum MBEDTLS_SSL_HANDSHAKE_OVER, MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, + MBEDTLS_SSL_INVALID } mbedtls_ssl_states; diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 2c209d3e9..d8b1ce0af 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -4254,6 +4254,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_wrapup( ssl ); break; + case MBEDTLS_SSL_INVALID: default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index fb64a2be4..1a341c48c 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4818,6 +4818,7 @@ int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_wrapup( ssl ); break; + case MBEDTLS_SSL_INVALID: default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); From b01800974f6fcbd8a4929bed6cb6116babb2a9b8 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 12 Nov 2019 15:46:46 +0200 Subject: [PATCH 03/30] Use invalid state If mismatch in the state has been noticed, use the invalid state. --- library/ssl_tls.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index fa132ea2d..ca5ca6d18 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6748,6 +6748,10 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } return( 0 ); } @@ -6773,6 +6777,10 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } return( 0 ); } @@ -6804,6 +6812,10 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } return( 0 ); } @@ -6822,6 +6834,10 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } return( 0 ); } @@ -6903,6 +6919,10 @@ write_msg: { ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { @@ -7567,6 +7587,10 @@ exit: { ssl->state = MBEDTLS_SSL_SERVER_KEY_EXCHANGE; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) @@ -7605,6 +7629,10 @@ int mbedtls_ssl_write_change_cipher_spec( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_SERVER_FINISHED; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { @@ -7695,6 +7723,10 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_SERVER_FINISHED; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse change cipher spec" ) ); @@ -7871,6 +7903,10 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_FLUSH_BUFFERS; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } } /* @@ -8040,6 +8076,10 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) { ssl->state = MBEDTLS_SSL_FLUSH_BUFFERS; } + else + { + ssl->state = MBEDTLS_SSL_INVALID; + } } #if defined(MBEDTLS_SSL_PROTO_DTLS) From 4708d66af5426a03da9e8e998d8ec4d01e63c7d1 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Wed, 13 Nov 2019 13:12:50 +0200 Subject: [PATCH 04/30] Change the mbedtls_ssl_states values The changed values have now the minimum hamming distance of 16 from each other. This is to prevent changing the state by just flipping one bit. --- include/mbedtls/ssl.h | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index f147069d3..4609e73bd 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -564,26 +564,26 @@ extern "C" { */ typedef enum { - MBEDTLS_SSL_HELLO_REQUEST, - MBEDTLS_SSL_CLIENT_HELLO, - MBEDTLS_SSL_SERVER_HELLO, - MBEDTLS_SSL_SERVER_CERTIFICATE, - MBEDTLS_SSL_SERVER_KEY_EXCHANGE, - MBEDTLS_SSL_CERTIFICATE_REQUEST, - MBEDTLS_SSL_SERVER_HELLO_DONE, - MBEDTLS_SSL_CLIENT_CERTIFICATE, - MBEDTLS_SSL_CLIENT_KEY_EXCHANGE, - MBEDTLS_SSL_CERTIFICATE_VERIFY, - MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC, - MBEDTLS_SSL_CLIENT_FINISHED, - MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC, - MBEDTLS_SSL_SERVER_FINISHED, - MBEDTLS_SSL_FLUSH_BUFFERS, - MBEDTLS_SSL_HANDSHAKE_WRAPUP, - MBEDTLS_SSL_HANDSHAKE_OVER, - MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, - MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, - MBEDTLS_SSL_INVALID + MBEDTLS_SSL_HELLO_REQUEST = 0x0, + MBEDTLS_SSL_CLIENT_HELLO = 0x0000FFFF, + MBEDTLS_SSL_SERVER_HELLO = 0x00FF00FF, + MBEDTLS_SSL_SERVER_CERTIFICATE = 0x00FFFF00, + MBEDTLS_SSL_SERVER_KEY_EXCHANGE = 0x0F0F0F0F, + MBEDTLS_SSL_CERTIFICATE_REQUEST = 0x0F0FF0F0, + MBEDTLS_SSL_SERVER_HELLO_DONE = 0x0FF00FF0, + MBEDTLS_SSL_CLIENT_CERTIFICATE = 0x0FF0F00F, + MBEDTLS_SSL_CLIENT_KEY_EXCHANGE = 0x33333333, + MBEDTLS_SSL_CERTIFICATE_VERIFY = 0x3333CCCC, + MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC = 0x33CC33CC, + MBEDTLS_SSL_CLIENT_FINISHED = 0x33CCCC33, + MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC = 0x3C3C3C3C, + MBEDTLS_SSL_SERVER_FINISHED = 0x3C3CC3C3, + MBEDTLS_SSL_FLUSH_BUFFERS = 0x3CC33CC3, + MBEDTLS_SSL_HANDSHAKE_WRAPUP = 0x3CC3C33C, + MBEDTLS_SSL_HANDSHAKE_OVER = 0x55555555, + MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET = 0x5555AAAA, + MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT = 0x55AA55AA, + MBEDTLS_SSL_INVALID = 0x55AAAA55 } mbedtls_ssl_states; From 552e8f2d6a28bdd664f01da2564ea261dcf56477 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 14 Nov 2019 10:05:36 +0200 Subject: [PATCH 05/30] Add double check to entropy-loop To prevent glitching and going through without strong source --- library/entropy.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index 75421cfb2..fdb2e152f 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -258,7 +258,8 @@ int mbedtls_entropy_update_manual( mbedtls_entropy_context *ctx, */ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) { - int ret, i, have_one_strong = 0; + int ret, i; + volatile int have_one_strong_fi = 0; unsigned char buf[MBEDTLS_ENTROPY_MAX_GATHER]; size_t olen; @@ -271,7 +272,7 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) for( i = 0; i < ctx->source_count; i++ ) { if( ctx->source[i].strong == MBEDTLS_ENTROPY_SOURCE_STRONG ) - have_one_strong = 1; + have_one_strong_fi = 1; olen = 0; if( ( ret = ctx->source[i].f_source( ctx->source[i].p_source, @@ -292,8 +293,14 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) } } - if( have_one_strong == 0 ) - ret = MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE; + if( have_one_strong_fi == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( have_one_strong_fi == 0) + { + ret = MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE; + } + } cleanup: mbedtls_platform_zeroize( buf, sizeof( buf ) ); From d05da1fa45f9b40cc5a56f7d9ff1c55992590168 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 14 Nov 2019 10:12:36 +0200 Subject: [PATCH 06/30] Add double check for checking if source is strong To prevent glitching past a strong source. --- library/entropy.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/entropy.c b/library/entropy.c index fdb2e152f..6b0b47b3e 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -271,7 +271,13 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) */ for( i = 0; i < ctx->source_count; i++ ) { - if( ctx->source[i].strong == MBEDTLS_ENTROPY_SOURCE_STRONG ) + volatile int strong_fi = ctx->source[i].strong; + if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) + have_one_strong_fi = 1; + + mbedtls_platform_enforce_volatile_reads(); + + if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) have_one_strong_fi = 1; olen = 0; From acb5eb00caaace5ba62fc49fb7ff21c54dffa294 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 14 Nov 2019 14:13:10 +0200 Subject: [PATCH 07/30] Add a double check to protect from glitch Check that the encryption has been done for the outbut buffer. This is to ensure that glitching out the encryption doesn't result as a unecrypted buffer to be sent. --- library/ssl_tls.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ca5ca6d18..d7ad696e5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4490,6 +4490,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) { unsigned i; size_t protected_record_size; + volatile int encrypted_fi = 0; /* Skip writing the record content type to after the encryption, * as it may change when using the CID extension. */ @@ -4544,6 +4545,13 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ ssl->out_msglen = len = rec.data_len; (void)mbedtls_platform_put_uint16_be( ssl->out_len, rec.data_len ); + encrypted_fi = 1; + } + + //Double check to ensure the encryption has been done + if( ssl->transform_out != NULL && encrypted_fi == 0 ) + { + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } protected_record_size = len + mbedtls_ssl_out_hdr_len( ssl ); From 9e8e8209931a7869e4e8a45a197c64603d61a9ad Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Wed, 11 Dec 2019 13:51:11 +0200 Subject: [PATCH 08/30] Increase hamming distance for some error codes The MBEDTLS_ERR_SSL_WANT_READ and MBEDTLS_ERR_SSL_WANT_WRITE are errors that can be ignored, so increase the hamming distance between them and the non-ignorable errors and keep still some distance from a success case. This mitigates an attack where single bit-flipping could change a non-ignorable error to being an ignorable one. --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4609e73bd..b99be9396 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -125,8 +125,8 @@ #define MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED -0x6A80 /**< DTLS client must retry for hello verification */ #define MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL -0x6A00 /**< A buffer is too small to receive or write a message */ #define MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE -0x6980 /**< None of the common ciphersuites is usable (eg, no suitable certificate, see debug messages). */ -#define MBEDTLS_ERR_SSL_WANT_READ -0x6900 /**< No data of requested type currently available on underlying transport. */ -#define MBEDTLS_ERR_SSL_WANT_WRITE -0x6880 /**< Connection requires a write call. */ +#define MBEDTLS_ERR_SSL_WANT_READ -0xFF6900 /**< No data of requested type currently available on underlying transport. */ +#define MBEDTLS_ERR_SSL_WANT_WRITE -0xFF6880 /**< Connection requires a write call. */ #define MBEDTLS_ERR_SSL_TIMEOUT -0x6800 /**< The operation timed out. */ #define MBEDTLS_ERR_SSL_CLIENT_RECONNECT -0x6780 /**< The client initiated a reconnect from the same port. */ #define MBEDTLS_ERR_SSL_UNEXPECTED_RECORD -0x6700 /**< Record header looks valid but is not expected. */ From 83a56a630a7abaec20ae79adc83aba68c703bcd1 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Wed, 11 Dec 2019 15:00:27 +0200 Subject: [PATCH 09/30] Double check mbedtls_pk_verify The verification could be skipped in server, changed the default flow so that the handshake status is ever updated if the verify succeeds, and that is checked twice. --- library/ssl_srv.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 1a341c48c..38ff1ab47 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4432,7 +4432,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) #else /* !MBEDTLS_KEY_EXCHANGE__CERT_REQ_ALLOWED__ENABLED */ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { - int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; + volatile int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; size_t i, sig_len; unsigned char hash[48]; unsigned char *hash_start = hash; @@ -4618,17 +4618,25 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) md_alg, ssl, hash, &dummy_hlen ); } - if( ( ret = mbedtls_pk_verify( peer_pk, - md_alg, hash_start, hashlen, - ssl->in_msg + i, sig_len ) ) != 0 ) + ret = mbedtls_pk_verify( peer_pk, + md_alg, hash_start, hashlen, + ssl->in_msg + i, sig_len ); + + if( ret == 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret ); - goto exit; + mbedtls_platform_enforce_volatile_reads(); + + if( ret == 0 ) + { + mbedtls_ssl_update_handshake_status( ssl ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate verify" ) ); + goto exit; + } + } - mbedtls_ssl_update_handshake_status( ssl ); - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate verify" ) ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret ); exit: From 47aab8da8a76914048f15bbf466c09a439b3f982 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Fri, 13 Dec 2019 14:26:55 +0200 Subject: [PATCH 10/30] Protect return value from mbedtls_pk_verify Use double checks and default flow assumes failure. --- library/ssl_cli.c | 50 +++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index d8b1ce0af..0a9f4a526 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2731,7 +2731,7 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) { - int ret; + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; unsigned char *p; unsigned char *end; @@ -3031,39 +3031,47 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, rs_ctx = &ssl->handshake->ecrs_ctx.pk; #endif - if( ( ret = mbedtls_pk_verify_restartable( peer_pk, - md_alg, hash, hashlen, p, sig_len, rs_ctx ) ) != 0 ) + ret = mbedtls_pk_verify_restartable( peer_pk, + md_alg, hash, hashlen, p, sig_len, rs_ctx ); + + if( ret == 0 ) { -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ) -#endif + mbedtls_platform_enforce_volatile_reads(); + + if( ret == 0 ) { - mbedtls_ssl_pend_fatal_alert( ssl, - MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR ); +#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) + /* We don't need the peer's public key anymore. Free it, + * so that more RAM is available for upcoming expensive + * operations like ECDHE. */ + mbedtls_pk_free( peer_pk ); +#else + mbedtls_x509_crt_pk_release( + ssl->session_negotiate->peer_cert ); +#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ + return( ret ); } + } +#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ) +#endif + { + mbedtls_ssl_pend_fatal_alert( ssl, + MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR ); + } MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) - ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; + if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) + ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; #endif #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) mbedtls_x509_crt_pk_release( ssl->session_negotiate->peer_cert ); #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ - return( ret ); - } -#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) - /* We don't need the peer's public key anymore. Free it, - * so that more RAM is available for upcoming expensive - * operations like ECDHE. */ - mbedtls_pk_free( peer_pk ); -#else - mbedtls_x509_crt_pk_release( ssl->session_negotiate->peer_cert ); -#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ } #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ - return( 0 ); + return( ret ); } static int ssl_in_server_key_exchange_postprocess( mbedtls_ssl_context *ssl ) From b83a2136d6b2a399c4269e0196f6757ad778297f Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Fri, 13 Dec 2019 14:40:06 +0200 Subject: [PATCH 11/30] Protect the return value from mbedtls_pk_verify Add double checks to the return value and default flow assumes failure. --- library/pk.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/library/pk.c b/library/pk.c index dfdd4d143..0c5b166e2 100644 --- a/library/pk.c +++ b/library/pk.c @@ -1515,6 +1515,7 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, const unsigned char *sig, size_t sig_len, mbedtls_pk_restart_ctx *rs_ctx ) { + volatile int verify_ret = MBEDTLS_ERR_PK_HW_ACCEL_FAILED; PK_VALIDATE_RET( ctx != NULL ); PK_VALIDATE_RET( ( md_alg == MBEDTLS_MD_NONE && hash_len == 0 ) || hash != NULL ); @@ -1547,8 +1548,19 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, (void) rs_ctx; #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ - return( pk_info_verify_func( MBEDTLS_PK_CTX_INFO( ctx ), - ctx->pk_ctx, md_alg, hash, hash_len, sig, sig_len ) ); + verify_ret = pk_info_verify_func( MBEDTLS_PK_CTX_INFO( ctx ), + ctx->pk_ctx, md_alg, hash, hash_len, sig, sig_len ); + + if( verify_ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( verify_ret == 0 ) + { + return( verify_ret ); + } + } + + return( MBEDTLS_ERR_ECP_HW_ACCEL_FAILED ); } /* From 91dbb79ae4d8fb6b572ab40c64686d1143e860a9 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Mon, 16 Dec 2019 12:20:27 +0200 Subject: [PATCH 12/30] Fix error return code --- library/pk.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/pk.c b/library/pk.c index 0c5b166e2..27276a829 100644 --- a/library/pk.c +++ b/library/pk.c @@ -1550,7 +1550,7 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, verify_ret = pk_info_verify_func( MBEDTLS_PK_CTX_INFO( ctx ), ctx->pk_ctx, md_alg, hash, hash_len, sig, sig_len ); - + if( verify_ret == 0 ) { mbedtls_platform_enforce_volatile_reads(); @@ -1558,9 +1558,13 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, { return( verify_ret ); } + else + { + verify_ret = MBEDTLS_ERR_PK_HW_ACCEL_FAILED; + } } - return( MBEDTLS_ERR_ECP_HW_ACCEL_FAILED ); + return( verify_ret ); } /* From 46afd5d8fad4307fcdc23b74491216fb04ff5163 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 17 Dec 2019 08:50:53 +0200 Subject: [PATCH 13/30] Fix CI issues Default flow assumes failure causes multiple issues with compatibility tests when the return value is initialised with error value in ssl_in_server_key_exchange_parse. The function would need a significant change in structure for this. --- library/ssl_cli.c | 2 +- library/ssl_srv.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 0a9f4a526..e0c104e01 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2731,7 +2731,7 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) { - volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + volatile int ret = 0; unsigned char *p; unsigned char *end; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 38ff1ab47..8d14374c4 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4632,8 +4632,8 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate verify" ) ); goto exit; - } - + } + } MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret ); From 6122b590427fc0945c0f3dbbabd2cf92ad7c074e Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 17 Dec 2019 10:06:46 +0200 Subject: [PATCH 14/30] Address review comments --- library/entropy.c | 23 ++++++++++++----------- library/ssl_cli.c | 4 ++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index 6b0b47b3e..78ea6d411 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -258,7 +258,8 @@ int mbedtls_entropy_update_manual( mbedtls_entropy_context *ctx, */ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) { - int ret, i; + int i; + volatile int ret = MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE; volatile int have_one_strong_fi = 0; unsigned char buf[MBEDTLS_ENTROPY_MAX_GATHER]; size_t olen; @@ -299,19 +300,19 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) } } - if( have_one_strong_fi == 0 ) - { - mbedtls_platform_enforce_volatile_reads(); - if( have_one_strong_fi == 0) - { - ret = MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE; - } - } - cleanup: mbedtls_platform_zeroize( buf, sizeof( buf ) ); - return( ret ); + if( have_one_strong_fi == 1 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( have_one_strong_fi == 1 ) + { + return( ret ); + } + } + + return( MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE ); } /* diff --git a/library/ssl_cli.c b/library/ssl_cli.c index e0c104e01..a1d2e192d 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3051,6 +3051,10 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ return( ret ); } + else + { + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ) From b57d7fd568da2bd241ce0d08b8a88e82e4d81d94 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 17 Dec 2019 15:46:48 +0200 Subject: [PATCH 15/30] Add flags for protecting TLS state machine Flags are there to prevent skipping vital parts of the TLS handshake. --- include/mbedtls/ssl_internal.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 4872f6fb5..5f0bbdc2d 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -226,6 +226,9 @@ : ( MBEDTLS_SSL_IN_CONTENT_LEN ) \ ) +#define MBEDTLS_SSL_FI_FLAG_UNSET 0x0 +#define MBEDTLS_SSL_FI_FLAG_SET 0x7F + /* * Check that we obey the standard's message size bounds */ @@ -385,6 +388,11 @@ struct mbedtls_ssl_handshake_params #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) uint8_t got_peer_pubkey; /*!< Did we store the peer's public key from its certificate? */ #endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ + volatile uint8_t peer_authenticated; /*!< Is the peer authenticated? */ + volatile uint8_t hello_random_set; /*!< Has the hello random been set? */ + volatile uint8_t key_derivation_done; /*!< Has the key derivation been done? */ + volatile uint8_t premaster_generated; /*!< Has the PMS been generated? */ + volatile uint8_t got_proper_keys; /*!< Has the proper keys been set? */ #if defined(MBEDTLS_SSL_PROTO_DTLS) unsigned char verify_cookie_len; /*!< Cli: cookie length Srv: flag for sending a cookie */ From 98801af26b9bf8e543e93bd279439ba58977c1c1 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Tue, 17 Dec 2019 15:57:41 +0200 Subject: [PATCH 16/30] Protect setting of hello_random flag The handshake flag tells when the handshake hello.random is set and can be used later to decide if we have the correct keys. --- library/ssl_cli.c | 23 ++++++++++++++++++----- library/ssl_srv.c | 18 ++++++++++++++++-- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index a1d2e192d..6b95cfa47 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -687,7 +687,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t t; #endif - + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_UNSET; /* * When responding to a verify request, MUST reuse random (RFC 6347 4.2.1) */ @@ -713,13 +713,19 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) p += 4; #endif /* MBEDTLS_HAVE_TIME */ - if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 28 ) ) != 0 ) + ret = mbedtls_ssl_conf_get_frng( ssl->conf ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), p, 28 ); + if( ret == 0 ) { - return( ret ); + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; + return( 0 ); + } } - return( 0 ); + return( ret ); } /** @@ -1719,8 +1725,15 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, current time: %lu", (unsigned long)mbedtls_platform_get_uint32_be( &buf[2] ) ) ); + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_UNSET; + mbedtls_platform_memcpy( ssl->handshake->randbytes + 32, buf + 2, 32 ); + if( mbedtls_platform_memcmp( ssl->handshake->randbytes + 32, buf + 2, 32 ) == 0 ) + { + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; + } + n = buf[34]; MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", buf + 2, 32 ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 8d14374c4..e349ed8dc 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1223,8 +1223,14 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) mbedtls_platform_memcpy( ssl->session_negotiate->id, p, ssl->session_negotiate->id_len ); p += sess_len; + + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_UNSET; memset( ssl->handshake->randbytes, 0, 64 ); mbedtls_platform_memcpy( ssl->handshake->randbytes + 32 - chal_len, p, chal_len ); + if( mbedtls_platform_memcmp( ssl->handshake->randbytes + 32 - chal_len, p, chal_len ) == 0 ) + { + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; + } /* * Check for TLS_EMPTY_RENEGOTIATION_INFO_SCSV @@ -1717,10 +1723,14 @@ read_record_header: /* * Save client random (inc. Unix time) */ + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_UNSET; MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", buf + 2, 32 ); mbedtls_platform_memcpy( ssl->handshake->randbytes, buf + 2, 32 ); - + if( mbedtls_platform_memcmp( ssl->handshake->randbytes, buf + 2, 32 ) == 0 ) + { + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; + } /* * Check the session ID length and save session ID */ @@ -2814,8 +2824,12 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) } p += 28; - + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_UNSET; mbedtls_platform_memcpy( ssl->handshake->randbytes + 32, buf + 6, 32 ); + if( mbedtls_platform_memcmp( ssl->handshake->randbytes + 32, buf + 6, 32 ) == 0 ) + { + ssl->handshake->hello_random_set = MBEDTLS_SSL_FI_FLAG_SET; + } MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", buf + 6, 32 ); From 67f0a1e833b63797b9446f0fb0d5fb7f2bc64dbd Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Wed, 18 Dec 2019 16:28:51 +0200 Subject: [PATCH 17/30] Protect setting of premaster_generated flag The flag is used for tracking if the premaster has been succesfully generated. Note that when resuming a session, the flag should not be used when trying to notice if all the key generation/derivation has been done. --- library/ssl_cli.c | 38 +++++++++++++++++------ library/ssl_srv.c | 14 +++++++++ library/ssl_tls.c | 79 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 112 insertions(+), 19 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 6b95cfa47..08c65d2a7 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2342,8 +2342,9 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, unsigned char* out, unsigned add_length_tag ) { - int ret; + volatile int ret; + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; /* * Generate (part of) the pre-master secret as * struct { @@ -2364,14 +2365,21 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ), ssl->conf->transport, out ); - if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) - ( mbedtls_ssl_conf_get_prng( ssl->conf ), out + 2, 46 ) ) != 0 ) + ret = mbedtls_ssl_conf_get_frng( ssl->conf ) + ( mbedtls_ssl_conf_get_prng( ssl->conf ), out + 2, 46 ); + + if( ret == 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); - return( ret ); + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + return( 0 ); + } } - return( 0 ); + MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); + return( ret ); } /* @@ -2383,11 +2391,12 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, unsigned char *out, size_t buflen, size_t *olen ) { - int ret; + volatile int ret; size_t len_bytes = mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; mbedtls_pk_context *peer_pk = NULL; + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; if( buflen < len_bytes ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for encrypted pms" ) ); @@ -2427,16 +2436,27 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, goto cleanup; } - if( ( ret = mbedtls_pk_encrypt( peer_pk, + ret = mbedtls_pk_encrypt( peer_pk, ppms, 48, out + len_bytes, olen, buflen - len_bytes, mbedtls_ssl_conf_get_frng( ssl->conf ), - mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ); + + if( ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + } + } + else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_rsa_pkcs1_encrypt", ret ); goto cleanup; } + #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) if( len_bytes == 2 ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index e349ed8dc..3053818e2 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3978,7 +3978,9 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, unsigned char mask; size_t i, peer_pmslen; unsigned int diff; + volatile unsigned int pmscounter = 0; + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; /* In case of a failure in decryption, the decryption may write less than * 2 bytes of output, but we always read the first two bytes. It doesn't * matter in the end because diff will be nonzero in that case due to @@ -4056,7 +4058,19 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, /* Set pms to either the true or the fake PMS, without * data-dependent branches. */ for( i = 0; i < ssl->handshake->pmslen; i++ ) + { pms[i] = ( mask & fake_pms[i] ) | ( (~mask) & peer_pms[i] ); + pmscounter++; + } + + if( pmscounter == ssl->handshake->pmslen ) + { + mbedtls_platform_enforce_volatile_reads(); + if( pmscounter == ssl->handshake->pmslen ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + } + } return( 0 ); } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d7ad696e5..66772f479 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1881,7 +1881,7 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) { - int ret; + volatile int ret; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> derive keys" ) ); @@ -1958,11 +1958,11 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) { - int ret; + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); - + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; #if defined(MBEDTLS_USE_TINYCRYPT) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || @@ -1980,6 +1980,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; ssl->handshake->pmslen = NUM_ECC_BYTES; } @@ -1989,12 +1990,26 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_DHE_RSA ) { - if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, + ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, ssl->handshake->premaster, MBEDTLS_PREMASTER_SIZE, &ssl->handshake->pmslen, mbedtls_ssl_conf_get_frng( ssl->conf ), - mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ); + if( ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); + return( ret ); + } + } + else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); return( ret ); @@ -2018,12 +2033,26 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, + ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, &ssl->handshake->pmslen, ssl->handshake->premaster, MBEDTLS_MPI_MAX_SIZE, mbedtls_ssl_conf_get_frng( ssl->conf ), - mbedtls_ssl_conf_get_prng( ssl->conf ) ) ) != 0 ) + mbedtls_ssl_conf_get_prng( ssl->conf ) ); + if( ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); + return( ret ); + } + } + else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); return( ret ); @@ -2039,8 +2068,22 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) if( mbedtls_ssl_ciphersuite_uses_psk( ciphersuite_info ) ) { - if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, - mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ) ) != 0 ) + ret = mbedtls_ssl_psk_derive_premaster( ssl, + mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) ); + if( ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + return( ret ); + } + } + else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); return( ret ); @@ -2056,7 +2099,20 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) ssl->handshake->premaster, 32, &ssl->handshake->pmslen, mbedtls_ssl_conf_get_frng( ssl->conf ), mbedtls_ssl_conf_get_prng( ssl->conf ) ); - if( ret != 0 ) + if( ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); + return( ret ); + } + } + else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); return( ret ); @@ -2091,6 +2147,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch unsigned char *end = p + sizeof( ssl->handshake->premaster ); const unsigned char *psk = ssl->conf->psk; size_t psk_len = ssl->conf->psk_len; + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; /* If the psk callback was called, use its result */ if( ssl->handshake->psk != NULL ) @@ -2214,6 +2271,8 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch ssl->handshake->pmslen = p - ssl->handshake->premaster; + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; + return( 0 ); } #endif /* MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED */ From 4031a450197339383c1835304bf488a8b0f481c8 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 08:11:12 +0200 Subject: [PATCH 18/30] Protect key_derivation_done flag The flag is used to track that the key derivation has been done. --- library/ssl_tls.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 66772f479..851a65922 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1884,7 +1884,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) volatile int ret; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> derive keys" ) ); - + ssl->handshake->key_derivation_done = MBEDTLS_SSL_FI_FLAG_UNSET; /* Compute master secret if needed */ ret = ssl_compute_master( ssl->handshake, ssl->session_negotiate->master, @@ -1925,7 +1925,19 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) mbedtls_ssl_get_minor_ver( ssl ), mbedtls_ssl_conf_get_endpoint( ssl->conf ), ssl ); - if( ret != 0 ) + if( ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ret == 0 ) + { + ssl->handshake->key_derivation_done = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } + } + else { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_populate_transform", ret ); return( ret ); From ba4730fe4c80e98d462b0cbe2df790a09a6c587b Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 08:42:03 +0200 Subject: [PATCH 19/30] Protect setting of peer_authenticated flag Use flow counting and double checks when setting the flag. Also protect the flow to prevent causing a glitch. --- library/ssl_tls.c | 75 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 851a65922..1b7d4f7f7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7297,16 +7297,19 @@ static int ssl_parse_certificate_coordinate( mbedtls_ssl_context *ssl, } static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, - int authmode, + volatile int authmode, mbedtls_x509_crt *chain, void *rs_ctx ) { - int verify_ret; + volatile int verify_ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE; + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + volatile int flow_counter = 0; mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; + ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_UNSET; if( authmode == MBEDTLS_SSL_VERIFY_NONE ) return( 0 ); @@ -7339,9 +7342,22 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_X509_REMOVE_VERIFY_CALLBACK */ rs_ctx ); + if( verify_ret == 0 ) + { + mbedtls_platform_enforce_volatile_reads(); + if( verify_ret == 0 ) + { + flow_counter++; + } + else + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } + } if( verify_ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "x509_verify_cert", verify_ret ); + flow_counter++; } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) @@ -7355,7 +7371,6 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECP_C) || defined(MBEDTLS_USE_TINYCRYPT) { - int ret; #if defined(MBEDTLS_USE_TINYCRYPT) ret = mbedtls_ssl_check_curve_uecc( ssl, MBEDTLS_UECC_DP_SECP256R1 ); #else /* MBEDTLS_USE_TINYCRYPT */ @@ -7383,16 +7398,27 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) ); if( verify_ret == 0 ) verify_ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE; + flow_counter++; + } + if( ret == 0 ) + { + flow_counter++; } } #endif /* MBEDTLS_ECP_C || MEDTLS_USE_TINYCRYPT */ - if( mbedtls_ssl_check_cert_usage( chain, + ret = mbedtls_ssl_check_cert_usage( chain, ciphersuite_info, ( mbedtls_ssl_conf_get_endpoint( ssl->conf ) == MBEDTLS_SSL_IS_CLIENT ), - &ssl->session_negotiate->verify_result ) != 0 ) + &ssl->session_negotiate->verify_result ); + if( ret == 0 ) { + flow_counter++; + } + else + { + flow_counter++; MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate (usage extensions)" ) ); if( verify_ret == 0 ) verify_ret = MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE; @@ -7408,13 +7434,27 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, ( verify_ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || verify_ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) ) { - verify_ret = 0; + mbedtls_platform_enforce_volatile_reads(); + if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && + ( verify_ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || + verify_ret == MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE ) ) + { + verify_ret = 0; + flow_counter++; + } + else + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } + } else { + flow_counter++; } if( ca_chain == NULL && authmode == MBEDTLS_SSL_VERIFY_REQUIRED ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no CA chain" ) ); verify_ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; + flow_counter++; } if( verify_ret != 0 ) @@ -7449,6 +7489,29 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, mbedtls_ssl_pend_fatal_alert( ssl, alert ); } + if( verify_ret == 0 && +#if defined(MBEDTLS_ECP_C) || defined(MBEDTLS_USE_TINYCRYPT) + flow_counter == 5 ) +#else + flow_counter == 4 ) +#endif + { + mbedtls_platform_enforce_volatile_reads(); + if( verify_ret == 0 && +#if defined(MBEDTLS_ECP_C) || defined(MBEDTLS_USE_TINYCRYPT) + flow_counter == 5 ) +#else + flow_counter == 4 ) +#endif + { + ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } + } + #if defined(MBEDTLS_DEBUG_C) if( ssl->session_negotiate->verify_result != 0 ) { From e1621d47005f8aff6422678b0f003209168c5332 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 08:58:56 +0200 Subject: [PATCH 20/30] Check that the peer_authenticated flag Check that the peer has been authenticated in the end of the handshake. --- include/mbedtls/ssl_internal.h | 4 +-- library/ssl_cli.c | 2 +- library/ssl_srv.c | 5 +++- library/ssl_tls.c | 50 ++++++++++++++++++++++++++++++++-- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 5f0bbdc2d..bc8af84b3 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -514,7 +514,7 @@ struct mbedtls_ssl_handshake_params #endif /* !MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE */ #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) - int resume; /*!< session resume indicator*/ + volatile int resume; /*!< session resume indicator*/ #endif /* !MBEDTLS_SSL_NO_SESSION_RESUMPTION */ #if defined(MBEDTLS_SSL_SRV_C) && \ @@ -913,7 +913,7 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ); int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ); int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ); -void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ); int mbedtls_ssl_send_fatal_handshake_failure( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 08c65d2a7..4871885d4 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -4296,7 +4296,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_HANDSHAKE_WRAPUP: - mbedtls_ssl_handshake_wrapup( ssl ); + ret = mbedtls_ssl_handshake_wrapup( ssl ); break; case MBEDTLS_SSL_INVALID: diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 3053818e2..7d0079794 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4450,6 +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->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC; return( 0 ); } @@ -4478,6 +4479,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->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC; return( 0 ); } @@ -4506,6 +4508,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->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC; return( 0 ); } @@ -4851,7 +4854,7 @@ int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ) break; case MBEDTLS_SSL_HANDSHAKE_WRAPUP: - mbedtls_ssl_handshake_wrapup( ssl ); + ret = mbedtls_ssl_handshake_wrapup( ssl ); break; case MBEDTLS_SSL_INVALID: diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1b7d4f7f7..4fc6aa832 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7309,10 +7309,13 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; - ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_UNSET; if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + { + ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; return( 0 ); + } + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate verify" ) ); #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_ca_chain != NULL ) { @@ -7456,6 +7459,10 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, verify_ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; flow_counter++; } + else + { + flow_counter++; + } if( verify_ret != 0 ) { @@ -7504,12 +7511,15 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, flow_counter == 4 ) #endif { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> PEER AUTHENTICATED" ) ); ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; } else { return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } + } else { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> PEER NOT AUTHENTICATED, %d", flow_counter)); } #if defined(MBEDTLS_DEBUG_C) @@ -7602,6 +7612,7 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ) if( crt_expected == SSL_CERTIFICATE_SKIP ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate" ) ); + ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; goto exit; } @@ -7917,8 +7928,9 @@ static void ssl_handshake_wrapup_free_hs_transform( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= handshake wrapup: final free" ) ); } -void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) { + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; MBEDTLS_SSL_DEBUG_MSG( 3, ( "=> handshake wrapup" ) ); #if defined(MBEDTLS_SSL_RENEGOTIATION) @@ -7959,6 +7971,39 @@ void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_SRV_C && !MBEDTLS_SSL_NO_SESSION_CACHE */ +#if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) + if( ssl->handshake->resume ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ssl->handshake->resume ) + { + ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + return( ret ); + } + } +#endif + + if( ssl->handshake->peer_authenticated == MBEDTLS_SSL_FI_FLAG_SET ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ssl->handshake->peer_authenticated == MBEDTLS_SSL_FI_FLAG_SET ) + { + ret = 0; + } + else + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + } + } + else + { + ret = MBEDTLS_ERR_SSL_PEER_VERIFY_FAILED; + } + #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->flight != NULL ) @@ -7977,6 +8022,7 @@ void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) ssl->state = MBEDTLS_SSL_HANDSHAKE_OVER; MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= handshake wrapup" ) ); + return ret; } int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) From 06164057b3c3130c42cb3496663264415f1333bd Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 14:40:36 +0200 Subject: [PATCH 21/30] Check that we have all the proper keys The proper keys should be set at the end of the handshake, if not, fail the handshake. --- library/ssl_cli.c | 2 -- library/ssl_srv.c | 1 - library/ssl_tls.c | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 4871885d4..08d4fd357 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2344,7 +2344,6 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, { volatile int ret; - ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; /* * Generate (part of) the pre-master secret as * struct { @@ -2396,7 +2395,6 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; mbedtls_pk_context *peer_pk = NULL; - ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; if( buflen < len_bytes ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for encrypted pms" ) ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 7d0079794..e70dd12b9 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3980,7 +3980,6 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, unsigned int diff; volatile unsigned int pmscounter = 0; - ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; /* In case of a failure in decryption, the decryption may write less than * 2 bytes of output, but we always read the first two bytes. It doesn't * matter in the end because diff will be nonzero in that case due to diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4fc6aa832..20683366c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1974,7 +1974,6 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_ciphersuite_handle_t ciphersuite_info = mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ); - ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; #if defined(MBEDTLS_USE_TINYCRYPT) if( mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA || @@ -2159,7 +2158,6 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch unsigned char *end = p + sizeof( ssl->handshake->premaster ); const unsigned char *psk = ssl->conf->psk; size_t psk_len = ssl->conf->psk_len; - ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_UNSET; /* If the psk callback was called, use its result */ if( ssl->handshake->psk != NULL ) @@ -7977,7 +7975,9 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) mbedtls_platform_enforce_volatile_reads(); if( ssl->handshake->resume ) { + /* When doing session resume, no premaster or peer authentication */ ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; + ssl->handshake->premaster_generated = MBEDTLS_SSL_FI_FLAG_SET; } else { @@ -7997,13 +7997,41 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) else { ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + goto cleanup; } } else { ret = MBEDTLS_ERR_SSL_PEER_VERIFY_FAILED; + goto cleanup; } + if( ssl->handshake->hello_random_set == MBEDTLS_SSL_FI_FLAG_SET && + ssl->handshake->key_derivation_done == MBEDTLS_SSL_FI_FLAG_SET && + ssl->handshake->premaster_generated == MBEDTLS_SSL_FI_FLAG_SET ) + { + mbedtls_platform_enforce_volatile_reads(); + if( ssl->handshake->hello_random_set == MBEDTLS_SSL_FI_FLAG_SET && + ssl->handshake->key_derivation_done == MBEDTLS_SSL_FI_FLAG_SET && + ssl->handshake->premaster_generated == MBEDTLS_SSL_FI_FLAG_SET ) + { + ret = 0; + } + else + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + goto cleanup; + } + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "hello random %d", ssl->handshake->hello_random_set ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "key_derivation_done %d", ssl->handshake->key_derivation_done ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "premaster_generated %d", ssl->handshake->premaster_generated ) ); + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + } + +cleanup: #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->flight != NULL ) From afff4d0679bbf2ffaad3bc35e0e7a0798ccc1908 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 14:41:56 +0200 Subject: [PATCH 22/30] Remove unused flag --- include/mbedtls/ssl_internal.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index bc8af84b3..19328d88f 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -392,7 +392,6 @@ struct mbedtls_ssl_handshake_params volatile uint8_t hello_random_set; /*!< Has the hello random been set? */ volatile uint8_t key_derivation_done; /*!< Has the key derivation been done? */ volatile uint8_t premaster_generated; /*!< Has the PMS been generated? */ - volatile uint8_t got_proper_keys; /*!< Has the proper keys been set? */ #if defined(MBEDTLS_SSL_PROTO_DTLS) unsigned char verify_cookie_len; /*!< Cli: cookie length Srv: flag for sending a cookie */ From f5b6af01d33996d0b381a8f64f346451697f5bec Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 14:46:40 +0200 Subject: [PATCH 23/30] Fix double check in entropy_gather_internal The double check was wrong way, glitching either check could have compromised the flow there. --- library/entropy.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index 78ea6d411..d1bde6a0d 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -43,9 +43,7 @@ #include #endif -#if defined(MBEDTLS_ENTROPY_NV_SEED) #include "mbedtls/platform.h" -#endif #if defined(MBEDTLS_SELF_TEST) #if defined(MBEDTLS_PLATFORM_C) @@ -274,12 +272,14 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) { volatile int strong_fi = ctx->source[i].strong; if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) - have_one_strong_fi = 1; + { + mbedtls_platform_enforce_volatile_reads(); - mbedtls_platform_enforce_volatile_reads(); - - if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) - have_one_strong_fi = 1; + if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) + have_one_strong_fi = 1; + else + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } olen = 0; if( ( ret = ctx->source[i].f_source( ctx->source[i].p_source, @@ -310,6 +310,10 @@ cleanup: { return( ret ); } + else + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } } return( MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE ); From 88db2ae9a0e3651c711e7a7b312d91ef8f9eddad Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 14:51:34 +0200 Subject: [PATCH 24/30] Use Platform fault when double check fails --- 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 20683366c..d9f1c4a2e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4617,10 +4617,10 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) encrypted_fi = 1; } - //Double check to ensure the encryption has been done + /* Double check to ensure the encryption has been done */ if( ssl->transform_out != NULL && encrypted_fi == 0 ) { - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } protected_record_size = len + mbedtls_ssl_out_hdr_len( ssl ); From 489dccd15863b5bccdc5c8517fa1a91e737c80f4 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 15:11:16 +0200 Subject: [PATCH 25/30] Adress review comments --- library/ssl_tls.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d9f1c4a2e..ba6384832 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7309,7 +7309,6 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, if( authmode == MBEDTLS_SSL_VERIFY_NONE ) { - ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; return( 0 ); } @@ -7929,6 +7928,14 @@ static void ssl_handshake_wrapup_free_hs_transform( mbedtls_ssl_context *ssl ) int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) { volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + +#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + const int authmode = ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET + ? ssl->handshake->sni_authmode + : mbedtls_ssl_conf_get_authmode( ssl->conf ); +#else + const int authmode = mbedtls_ssl_conf_get_authmode( ssl->conf ); +#endif MBEDTLS_SSL_DEBUG_MSG( 3, ( "=> handshake wrapup" ) ); #if defined(MBEDTLS_SSL_RENEGOTIATION) @@ -7969,6 +7976,19 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_SRV_C && !MBEDTLS_SSL_NO_SESSION_CACHE */ + if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + { + if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + { + ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; + } + else + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + goto cleanup; + } + } + #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) if( ssl->handshake->resume ) { @@ -7982,7 +8002,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) else { ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; - return( ret ); + goto cleanup; } } #endif From 8d09e5744c9641e53f2e0bfec12e3b204676bbf5 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 15:20:19 +0200 Subject: [PATCH 26/30] Increase hamming distance for session resume flag This is to prevent glitching a single bit for the resume flag. --- library/ssl_cli.c | 6 +++--- library/ssl_srv.c | 8 ++++---- library/ssl_tls.c | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 08d4fd357..11c6f467c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -868,7 +868,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) * appropriate length. Otherwise make the length 0 (for now, see next code * block for behaviour with tickets). */ - if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == 0 || + if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == MBEDTLS_SSL_FI_FLAG_UNSET || mbedtls_ssl_get_renego_status( ssl ) != MBEDTLS_SSL_INITIAL_HANDSHAKE || ssl->session_negotiate->id_len < 16 || ssl->session_negotiate->id_len > 32 ) @@ -1832,11 +1832,11 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->session_negotiate->id_len != n || mbedtls_platform_memcmp( ssl->session_negotiate->id, buf + 35, n ) != 0 ) { - ssl->handshake->resume = 0; + ssl->handshake->resume = MBEDTLS_SSL_FI_FLAG_UNSET; } #endif /* !MBEDTLS_SSL_NO_SESSION_RESUMPTION */ - if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == 1 ) + if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == MBEDTLS_SSL_FI_FLAG_SET ) { /* Resume a session */ ssl->state = MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index e70dd12b9..cd3aaf737 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -627,7 +627,7 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "session successfully restored from ticket" ) ); - ssl->handshake->resume = 1; + ssl->handshake->resume = MBEDTLS_SSL_FI_FLAG_SET; /* Don't send a new ticket after all, this one is OK */ ssl->handshake->new_session_ticket = 0; @@ -2839,19 +2839,19 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) * It may be already set to 1 by ssl_parse_session_ticket_ext(). * If not, try looking up session ID in our cache. */ - if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == 0 && + if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == MBEDTLS_SSL_FI_FLAG_UNSET && mbedtls_ssl_get_renego_status( ssl ) == MBEDTLS_SSL_INITIAL_HANDSHAKE && ssl->session_negotiate->id_len != 0 && ssl->conf->f_get_cache != NULL && ssl->conf->f_get_cache( ssl->conf->p_cache, ssl->session_negotiate ) == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "session successfully restored from cache" ) ); - ssl->handshake->resume = 1; + ssl->handshake->resume = MBEDTLS_SSL_FI_FLAG_SET; } #endif /* !MBEDTLS_SSL_NO_SESSION_CACHE */ #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) - if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == 1 ) + if( mbedtls_ssl_handshake_get_resume( ssl->handshake ) == MBEDTLS_SSL_FI_FLAG_SET ) { /* * Resuming a session diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ba6384832..e8a230d3e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1825,7 +1825,7 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, mbedtls_ssl_handshake_get_ciphersuite( handshake ); #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) - if( handshake->resume != 0 ) + if( handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "no premaster (session resumed)" ) ); return( 0 ); @@ -7969,7 +7969,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) */ if( ssl->conf->f_set_cache != NULL && ssl->session->id_len != 0 && - ssl->handshake->resume == 0 ) + ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_UNSET ) { if( ssl->conf->f_set_cache( ssl->conf->p_cache, ssl->session ) != 0 ) MBEDTLS_SSL_DEBUG_MSG( 1, ( "cache did not store session" ) ); @@ -7990,10 +7990,10 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) } #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) - if( ssl->handshake->resume ) + if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { mbedtls_platform_enforce_volatile_reads(); - if( ssl->handshake->resume ) + if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { /* When doing session resume, no premaster or peer authentication */ ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; @@ -8111,7 +8111,7 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) * In case of session resuming, invert the client and server * ChangeCipherSpec messages order. */ - if( ssl->handshake->resume != 0 ) + if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { #if defined(MBEDTLS_SSL_CLI_C) if( mbedtls_ssl_conf_get_endpoint( ssl->conf ) == @@ -8290,7 +8290,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) #endif #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) - if( ssl->handshake->resume != 0 ) + if( ssl->handshake->resume == MBEDTLS_SSL_FI_FLAG_SET ) { #if defined(MBEDTLS_SSL_CLI_C) if( mbedtls_ssl_conf_get_endpoint( ssl->conf ) == MBEDTLS_SSL_IS_CLIENT ) @@ -9019,7 +9019,7 @@ int mbedtls_ssl_set_session( mbedtls_ssl_context *ssl, const mbedtls_ssl_session session ) ) != 0 ) return( ret ); - ssl->handshake->resume = 1; + ssl->handshake->resume = MBEDTLS_SSL_FI_FLAG_SET; return( 0 ); } From 616fbe177c7ef310ffaf01863644f21eb558d3b9 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 16:00:31 +0200 Subject: [PATCH 27/30] Increase hamming distance for authmode Prevent glitching mode by single bit flipping. --- include/mbedtls/ssl.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b99be9396..97069a7c3 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -191,10 +191,10 @@ #define MBEDTLS_SSL_COMPRESS_NULL 0 #define MBEDTLS_SSL_COMPRESS_DEFLATE 1 -#define MBEDTLS_SSL_VERIFY_NONE 0 -#define MBEDTLS_SSL_VERIFY_OPTIONAL 1 -#define MBEDTLS_SSL_VERIFY_REQUIRED 2 -#define MBEDTLS_SSL_VERIFY_UNSET 3 /* Used only for sni_authmode */ +#define MBEDTLS_SSL_VERIFY_NONE 0x0 +#define MBEDTLS_SSL_VERIFY_OPTIONAL 0xf +#define MBEDTLS_SSL_VERIFY_REQUIRED 0x33 +#define MBEDTLS_SSL_VERIFY_UNSET 0x3c /* Used only for sni_authmode */ #define MBEDTLS_SSL_LEGACY_RENEGOTIATION 0 #define MBEDTLS_SSL_SECURE_RENEGOTIATION 1 @@ -1197,7 +1197,7 @@ struct mbedtls_ssl_config #endif /* !MBEDTLS_SSL_CONF_ENDPOINT */ unsigned int transport : 1; /*!< stream (TLS) or datagram (DTLS) */ #if !defined(MBEDTLS_SSL_CONF_AUTHMODE) - unsigned int authmode : 2; /*!< MBEDTLS_SSL_VERIFY_XXX */ + unsigned int authmode : 6; /*!< MBEDTLS_SSL_VERIFY_XXX */ #endif /* !MBEDTLS_SSL_CONF_AUTHMODE */ #if !defined(MBEDTLS_SSL_CONF_ALLOW_LEGACY_RENEGOTIATION) /* needed even with renego disabled for LEGACY_BREAK_HANDSHAKE */ From af60cd769890259fb646c3a38ab2770d44ec8e1f Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Thu, 19 Dec 2019 16:45:23 +0200 Subject: [PATCH 28/30] Protect the peer_authenticated flag more Add more protection to the flag preventing attacker possibly to glitch using faulty certificate. --- library/entropy.c | 1 + library/ssl_srv.c | 3 --- library/ssl_tls.c | 25 +++++++++++++++++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index d1bde6a0d..9818a542d 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -314,6 +314,7 @@ cleanup: { return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } + } return( MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index cd3aaf737..92d1da016 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4449,7 +4449,6 @@ 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->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC; return( 0 ); } @@ -4478,7 +4477,6 @@ 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->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC; return( 0 ); } @@ -4507,7 +4505,6 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) if( peer_pk == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) ); - ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; ssl->state = MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC; return( 0 ); } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e8a230d3e..46b6679a5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -48,6 +48,8 @@ #include "mbedtls/ssl_internal.h" #include "mbedtls/platform_util.h" #include "mbedtls/version.h" +#include "mbedtls/platform.h" + #include @@ -7261,7 +7263,7 @@ static int ssl_srv_check_client_no_crt_notification( mbedtls_ssl_context *ssl ) * indicating whether a Certificate message is expected or not. */ #define SSL_CERTIFICATE_EXPECTED 0 -#define SSL_CERTIFICATE_SKIP 1 +#define SSL_CERTIFICATE_SKIP 0xff static int ssl_parse_certificate_coordinate( mbedtls_ssl_context *ssl, int authmode ) { @@ -7609,7 +7611,6 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ) if( crt_expected == SSL_CERTIFICATE_SKIP ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate" ) ); - ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; goto exit; } @@ -7935,6 +7936,10 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) : mbedtls_ssl_conf_get_authmode( ssl->conf ); #else const int authmode = mbedtls_ssl_conf_get_authmode( ssl->conf ); +#endif +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + volatile int crt_expected = SSL_CERTIFICATE_EXPECTED; + crt_expected = ssl_parse_certificate_coordinate( ssl, authmode ); #endif MBEDTLS_SSL_DEBUG_MSG( 3, ( "=> handshake wrapup" ) ); @@ -7976,9 +7981,21 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_SRV_C && !MBEDTLS_SSL_NO_SESSION_CACHE */ - if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + if( authmode == MBEDTLS_SSL_VERIFY_NONE || + authmode == MBEDTLS_SSL_VERIFY_OPTIONAL || +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + crt_expected == SSL_CERTIFICATE_SKIP ) +#else + 1 ) +#endif { - if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + if( authmode == MBEDTLS_SSL_VERIFY_NONE || + authmode == MBEDTLS_SSL_VERIFY_OPTIONAL || +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + crt_expected == SSL_CERTIFICATE_SKIP ) +#else + 1 ) +#endif { ssl->handshake->peer_authenticated = MBEDTLS_SSL_FI_FLAG_SET; } From 015aa44b93ae04ac50ccd763a5d152e2c44b0ab0 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Fri, 20 Dec 2019 12:09:37 +0200 Subject: [PATCH 29/30] Make authmode volatile This is to enforce reading it from memory for the double check to prevent compiler from optimising it away. --- library/ssl_tls.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 46b6679a5..611f26fc8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7931,11 +7931,11 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - const int authmode = ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET + volatile const int authmode = ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET ? ssl->handshake->sni_authmode : mbedtls_ssl_conf_get_authmode( ssl->conf ); #else - const int authmode = mbedtls_ssl_conf_get_authmode( ssl->conf ); + volatile const int authmode = mbedtls_ssl_conf_get_authmode( ssl->conf ); #endif #if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) volatile int crt_expected = SSL_CERTIFICATE_EXPECTED; @@ -7989,6 +7989,7 @@ int mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) 1 ) #endif { + mbedtls_platform_enforce_volatile_reads(); if( authmode == MBEDTLS_SSL_VERIFY_NONE || authmode == MBEDTLS_SSL_VERIFY_OPTIONAL || #if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) From 5aa4c07b85d85a28686cabbcfe256fa310f8475c Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Fri, 20 Dec 2019 12:42:49 +0200 Subject: [PATCH 30/30] Minor review fixes --- include/mbedtls/entropy.h | 4 ++-- include/mbedtls/ssl.h | 5 +++++ library/entropy.c | 8 ++++---- library/ssl_cli.c | 8 ++++++-- library/ssl_tls.c | 8 ++++---- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/entropy.h b/include/mbedtls/entropy.h index ca06dc3c5..52cb6a0e5 100644 --- a/include/mbedtls/entropy.h +++ b/include/mbedtls/entropy.h @@ -83,8 +83,8 @@ #define MBEDTLS_ENTROPY_MAX_SEED_SIZE 1024 /**< Maximum size of seed we read from seed file */ #define MBEDTLS_ENTROPY_SOURCE_MANUAL MBEDTLS_ENTROPY_MAX_SOURCES -#define MBEDTLS_ENTROPY_SOURCE_STRONG 1 /**< Entropy source is strong */ -#define MBEDTLS_ENTROPY_SOURCE_WEAK 0 /**< Entropy source is weak */ +#define MBEDTLS_ENTROPY_SOURCE_STRONG 0x7F /**< Entropy source is strong */ +#define MBEDTLS_ENTROPY_SOURCE_WEAK 0x0 /**< Entropy source is weak */ #ifdef __cplusplus extern "C" { diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 97069a7c3..e14f58f71 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -125,6 +125,11 @@ #define MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED -0x6A80 /**< DTLS client must retry for hello verification */ #define MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL -0x6A00 /**< A buffer is too small to receive or write a message */ #define MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE -0x6980 /**< None of the common ciphersuites is usable (eg, no suitable certificate, see debug messages). */ +/* + * MBEDTLS_ERR_SSL_WANT_READ and MBEDTLS_ERR_SSL_WANT_WRITE are dismissable errors, + * therefore the hamming distance to other non-dismissable errors should be + * large to prevent bit-flipping a non-dismissable error to dismissable. + */ #define MBEDTLS_ERR_SSL_WANT_READ -0xFF6900 /**< No data of requested type currently available on underlying transport. */ #define MBEDTLS_ERR_SSL_WANT_WRITE -0xFF6880 /**< Connection requires a write call. */ #define MBEDTLS_ERR_SSL_TIMEOUT -0x6800 /**< The operation timed out. */ diff --git a/library/entropy.c b/library/entropy.c index 9818a542d..b4d1f2921 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -258,7 +258,7 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) { int i; volatile int ret = MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE; - volatile int have_one_strong_fi = 0; + volatile int have_one_strong_fi = MBEDTLS_ENTROPY_SOURCE_WEAK; unsigned char buf[MBEDTLS_ENTROPY_MAX_GATHER]; size_t olen; @@ -276,7 +276,7 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) mbedtls_platform_enforce_volatile_reads(); if( strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) - have_one_strong_fi = 1; + have_one_strong_fi = MBEDTLS_ENTROPY_SOURCE_STRONG; else return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } @@ -303,10 +303,10 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) cleanup: mbedtls_platform_zeroize( buf, sizeof( buf ) ); - if( have_one_strong_fi == 1 ) + if( have_one_strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { mbedtls_platform_enforce_volatile_reads(); - if( have_one_strong_fi == 1 ) + if( have_one_strong_fi == MBEDTLS_ENTROPY_SOURCE_STRONG ) { return( ret ); } diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 11c6f467c..479554d78 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2342,7 +2342,7 @@ static int ssl_rsa_generate_partial_pms( mbedtls_ssl_context *ssl, unsigned char* out, unsigned add_length_tag ) { - volatile int ret; + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; /* * Generate (part of) the pre-master secret as @@ -2390,7 +2390,7 @@ static int ssl_rsa_encrypt_partial_pms( mbedtls_ssl_context *ssl, unsigned char *out, size_t buflen, size_t *olen ) { - volatile int ret; + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; size_t len_bytes = mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; mbedtls_pk_context *peer_pk = NULL; @@ -2762,6 +2762,10 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, unsigned char *buf, size_t buflen ) { + /* + * Initialising to an error value would need a significant + * structural change to provide default flow assumes failure + */ volatile int ret = 0; unsigned char *p; unsigned char *end; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 611f26fc8..19bdc9079 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1883,7 +1883,7 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) { - volatile int ret; + volatile int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> derive keys" ) ); ssl->handshake->key_derivation_done = MBEDTLS_SSL_FI_FLAG_UNSET; @@ -2062,7 +2062,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); - return( ret ); + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } } else @@ -2093,7 +2093,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); - return( ret ); + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } } else @@ -2122,7 +2122,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) else { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_derive_secret", ret ); - return( ret ); + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } } else