diff --git a/ChangeLog b/ChangeLog index ac40da85e..e0e2ea952 100644 --- a/ChangeLog +++ b/ChangeLog @@ -37,6 +37,16 @@ Bugfix * Fix compiler warning of 'use before initialisation' in mbedtls_pk_parse_key(). Found by Martin Boye Petersen and fixed by Dawid Drozd. #1098 + * Fix decryption of zero length messages (all padding) in some circumstances: + DTLS 1.0 and 1.2, and CBC ciphersuites using encrypt-then-MAC. Most often + seen when communicating with OpenSSL using TLS 1.0. Reported by @kFYatek + (#1632) and by Conor Murphy on the forum. Fix contributed by Espressif + Systems. + * Fail when receiving a TLS alert message with an invalid length, or invalid + zero-length messages when using TLS 1.2. Contributed by Espressif Systems. + * Fix ssl_client2 example to send application data with 0-length content + when the request_size argument is set to 0 as stated in the documentation. + Fixes #1833. Changes * Change the shebang line in Perl scripts to look up perl in the PATH. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1c35f0dec..f1856e278 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1979,28 +1979,28 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) * and fake check up to 256 bytes of padding */ size_t pad_count = 0, real_count = 1; - size_t padding_idx = ssl->in_msglen - padlen - 1; + size_t padding_idx = ssl->in_msglen - padlen; size_t i; /* * Padding is guaranteed to be incorrect if: - * 1. padlen >= ssl->in_msglen + * 1. padlen > ssl->in_msglen * - * 2. padding_idx >= MBEDTLS_SSL_IN_CONTENT_LEN + + * 2. padding_idx > MBEDTLS_SSL_IN_CONTENT_LEN + * ssl->transform_in->maclen * * In both cases we reset padding_idx to a safe value (0) to * prevent out-of-buffer reads. */ - correct &= ( ssl->in_msglen >= padlen + 1 ); - correct &= ( padding_idx < MBEDTLS_SSL_IN_CONTENT_LEN + + correct &= ( padlen <= ssl->in_msglen ); + correct &= ( padding_idx <= MBEDTLS_SSL_IN_CONTENT_LEN + ssl->transform_in->maclen ); padding_idx *= correct; - for( i = 1; i <= 256; i++ ) + for( i = 0; i < 256; i++ ) { - real_count &= ( i <= padlen ); + real_count &= ( i < padlen ); pad_count += real_count * ( ssl->in_msg[padding_idx + i] == padlen - 1 ); } @@ -2133,6 +2133,16 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) if( ssl->in_msglen == 0 ) { +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 + && ssl->in_msgtype != MBEDTLS_SSL_MSG_APPLICATION_DATA ) + { + /* TLS v1.2 explicitly disallows zero-length messages which are not application data */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid zero-length message type: %d", ssl->in_msgtype ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ + ssl->nb_zero++; /* @@ -4177,6 +4187,16 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ) if( ssl->in_msgtype == MBEDTLS_SSL_MSG_ALERT ) { + if( ssl->in_msglen != 2 ) + { + /* Note: Standard allows for more than one 2 byte alert + to be packed in a single message, but Mbed TLS doesn't + currently support this. */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid alert message, len: %d", + ssl->in_msglen ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } + MBEDTLS_SSL_DEBUG_MSG( 2, ( "got an alert message, type: [%d:%d]", ssl->in_msg[0], ssl->in_msg[1] ) ); diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 5d8969dbc..0dd9e3f7b 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -246,8 +246,12 @@ int main( void ) " server_addr=%%s default: given by name\n" \ " server_port=%%d default: 4433\n" \ " request_page=%%s default: \".\"\n" \ - " request_size=%%d default: about 34 (basic request)\n" \ - " (minimum: 0, max: " MAX_REQUEST_SIZE_STR " )\n" \ + " request_size=%%d default: about 34 (basic request)\n" \ + " (minimum: 0, max: " MAX_REQUEST_SIZE_STR ")\n" \ + " If 0, in the first exchange only an empty\n" \ + " application data message is sent followed by\n" \ + " a second non-empty message before attempting\n" \ + " to read a response from the server\n" \ " debug_level=%%d default: 0 (disabled)\n" \ " nbio=%%d default: 0 (blocking I/O)\n" \ " options: 1 (non-blocking), 2 (added delays)\n" \ @@ -1663,10 +1667,13 @@ send_request: if( opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM ) { - for( written = 0, frags = 0; written < len; written += ret, frags++ ) + written = 0; + frags = 0; + + do { while( ( ret = mbedtls_ssl_write( &ssl, buf + written, - len - written ) ) <= 0 ) + len - written ) ) < 0 ) { if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) @@ -1686,7 +1693,11 @@ send_request: #endif } } + + frags++; + written += ret; } + while( written < len ); } else /* Not stream, so datagram */ { @@ -1730,6 +1741,13 @@ send_request: mbedtls_printf( " %d bytes written in %d fragments\n\n%s\n", written, frags, (char *) buf ); + /* Send a non-empty request if request_size == 0 */ + if ( len == 0 ) + { + opt.request_size = DFL_REQUEST_SIZE; + goto send_request; + } + /* * 7. Read the HTTP response */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0f6153565..a1c7d0490 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1170,6 +1170,38 @@ run_test "Fallback SCSV: enabled, max version, openssl client" \ -s "received FALLBACK_SCSV" \ -S "inapropriate fallback" +# Test sending and receiving empty application data records + +run_test "Encrypt then MAC: empty application data record" \ + "$P_SRV auth_mode=none debug_level=4 etm=1" \ + "$P_CLI auth_mode=none etm=1 request_size=0 force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA" \ + 0 \ + -S "0000: 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f" \ + -s "dumping 'input payload after decrypt' (0 bytes)" \ + -c "0 bytes written in 1 fragments" + +run_test "Default, no Encrypt then MAC: empty application data record" \ + "$P_SRV auth_mode=none debug_level=4 etm=0" \ + "$P_CLI auth_mode=none etm=0 request_size=0" \ + 0 \ + -s "dumping 'input payload after decrypt' (0 bytes)" \ + -c "0 bytes written in 1 fragments" + +run_test "Encrypt then MAC, DTLS: empty application data record" \ + "$P_SRV auth_mode=none debug_level=4 etm=1 dtls=1" \ + "$P_CLI auth_mode=none etm=1 request_size=0 force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA dtls=1" \ + 0 \ + -S "0000: 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f" \ + -s "dumping 'input payload after decrypt' (0 bytes)" \ + -c "0 bytes written in 1 fragments" + +run_test "Default, no Encrypt then MAC, DTLS: empty application data record" \ + "$P_SRV auth_mode=none debug_level=4 etm=0 dtls=1" \ + "$P_CLI auth_mode=none etm=0 request_size=0 dtls=1" \ + 0 \ + -s "dumping 'input payload after decrypt' (0 bytes)" \ + -c "0 bytes written in 1 fragments" + ## ClientHello generated with ## "openssl s_client -CAfile tests/data_files/test-ca.crt -tls1_1 -connect localhost:4433 -cipher ..." ## then manually twiddling the ciphersuite list.