Don't send empty fragments of nonempty handshake messages
This for example lead to the following corner case bug:
The code attempted to piggy-back a Finished message at
the end of a datagram where precisely 12 bytes of payload
were still available. This lead to an empty Finished fragment
being sent, and when mbedtls_ssl_flight_transmit() was called
again, it believed that it was just starting to send the
Finished message, thereby calling ssl_swap_epochs() which
had already happened in the call sending the empty fragment.
Therefore, the second call would send the 'rest' of the
Finished message with wrong epoch.
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 9b8f7fe..cc47058 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -2923,15 +2923,17 @@
size_t max_frag_len;
const mbedtls_ssl_flight_item * const cur = ssl->handshake->cur_msg;
+ int const is_finished =
+ ( cur->type == MBEDTLS_SSL_MSG_HANDSHAKE &&
+ cur->p[0] == MBEDTLS_SSL_HS_FINISHED );
+
uint8_t const force_flush = ssl->disable_datagram_packing == 1 ?
SSL_FORCE_FLUSH : SSL_DONT_FORCE_FLUSH;
/* Swap epochs before sending Finished: we can't do it after
* sending ChangeCipherSpec, in case write returns WANT_READ.
* Must be done before copying, may change out_msg pointer */
- if( cur->type == MBEDTLS_SSL_MSG_HANDSHAKE &&
- cur->p[0] == MBEDTLS_SSL_HS_FINISHED &&
- ssl->handshake->cur_msg_p == ( cur->p + 12 ) )
+ if( is_finished && ssl->handshake->cur_msg_p == ( cur->p + 12 ) )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "swap epochs to send finished message" ) );
ssl_swap_epochs( ssl );
@@ -2968,13 +2970,10 @@
const size_t rem_len = hs_len - frag_off;
size_t cur_hs_frag_len, max_hs_frag_len;
- if( max_frag_len < 12 )
+ if( ( max_frag_len < 12 ) || ( max_frag_len == 12 && hs_len != 0 ) )
{
- if( cur->type == MBEDTLS_SSL_MSG_HANDSHAKE &&
- cur->p[0] == MBEDTLS_SSL_HS_FINISHED )
- {
+ if( is_finished )
ssl_swap_epochs( ssl );
- }
if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 )
return( ret );