Merge remote-tracking branch 'public/pr/1852' into development
diff --git a/ChangeLog b/ChangeLog
index ac40da8..e0e2ea9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -37,6 +37,16 @@
* 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 1c35f0d..f1856e2 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1979,28 +1979,28 @@
* 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 @@
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 @@
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 5d8969d..0dd9e3f 100644
--- a/programs/ssl/ssl_client2.c
+++ b/programs/ssl/ssl_client2.c
@@ -246,8 +246,12 @@
" 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 @@
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 @@
#endif
}
}
+
+ frags++;
+ written += ret;
}
+ while( written < len );
}
else /* Not stream, so datagram */
{
@@ -1730,6 +1741,13 @@
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 0f61535..a1c7d04 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -1170,6 +1170,38 @@
-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.