Merge remote-tracking branch 'upstream-public/pr/1094' into development
diff --git a/ChangeLog b/ChangeLog
index ded60d3..0d323b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,16 @@
= mbed TLS x.x.x branch released xxxx-xx-xx
+Security
+ * Fix a potential heap buffer overflow in mbedtls_ssl_write. When the (by
+ default enabled) maximum fragment length extension is disabled in the
+ config and the application data buffer passed to mbedtls_ssl_write
+ is larger than the internal message buffer (16384 bytes by default), the
+ latter overflows. The exploitability of this issue depends on whether the
+ application layer can be forced into sending such large packets. The issue
+ was independently reported by Tim Nordell via e-mail and by Florin Petriuc
+ and sjorsdewit on GitHub. Fix proposed by Florin Petriuc in #1022. Fixes #707.
+
Features
* Allow comments in test data files.
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 8467b13..e19dfc9 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -7078,7 +7078,9 @@
int ret;
#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH)
size_t max_len = mbedtls_ssl_get_max_frag_len( ssl );
-
+#else
+ size_t max_len = MBEDTLS_SSL_MAX_CONTENT_LEN;
+#endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
if( len > max_len )
{
#if defined(MBEDTLS_SSL_PROTO_DTLS)
@@ -7093,7 +7095,6 @@
#endif
len = max_len;
}
-#endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
if( ssl->out_left != 0 )
{
@@ -7124,7 +7125,7 @@
*
* With non-blocking I/O, ssl_write_real() may return WANT_WRITE,
* then the caller will call us again with the same arguments, so
- * remember wether we already did the split or not.
+ * remember whether we already did the split or not.
*/
#if defined(MBEDTLS_SSL_CBC_RECORD_SPLITTING)
static int ssl_write_split( mbedtls_ssl_context *ssl,
diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c
index 5032a9f..8e2feb1 100644
--- a/programs/ssl/ssl_client2.c
+++ b/programs/ssl/ssl_client2.c
@@ -63,6 +63,9 @@
#include <stdlib.h>
#include <string.h>
+#define MAX_REQUEST_SIZE 20000
+#define MAX_REQUEST_SIZE_STR "20000"
+
#define DFL_SERVER_NAME "localhost"
#define DFL_SERVER_ADDR NULL
#define DFL_SERVER_PORT "4433"
@@ -242,8 +245,8 @@
" 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: 16384)\n" \
+ " request_size=%%d default: about 34 (basic request)\n" \
+ " (minimum: 0, max: " MAX_REQUEST_SIZE_STR " )\n" \
" debug_level=%%d default: 0 (disabled)\n" \
" nbio=%%d default: 0 (blocking I/O)\n" \
" options: 1 (non-blocking), 2 (added delays)\n" \
@@ -437,7 +440,9 @@
{
int ret = 0, len, tail_len, i, written, frags, retry_left;
mbedtls_net_context server_fd;
- unsigned char buf[MBEDTLS_SSL_MAX_CONTENT_LEN + 1];
+
+ unsigned char buf[MAX_REQUEST_SIZE + 1];
+
#if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED)
unsigned char psk[MBEDTLS_PSK_MAX_LEN];
size_t psk_len = 0;
@@ -602,7 +607,8 @@
else if( strcmp( p, "request_size" ) == 0 )
{
opt.request_size = atoi( q );
- if( opt.request_size < 0 || opt.request_size > MBEDTLS_SSL_MAX_CONTENT_LEN )
+ if( opt.request_size < 0 ||
+ opt.request_size > MAX_REQUEST_SIZE )
goto usage;
}
else if( strcmp( p, "ca_file" ) == 0 )
@@ -1494,8 +1500,8 @@
mbedtls_printf( " > Write to server:" );
fflush( stdout );
- len = mbedtls_snprintf( (char *) buf, sizeof(buf) - 1, GET_REQUEST,
- opt.request_page );
+ len = mbedtls_snprintf( (char *) buf, sizeof( buf ) - 1, GET_REQUEST,
+ opt.request_page );
tail_len = (int) strlen( GET_REQUEST_END );
/* Add padding to GET request to reach opt.request_size in length */
@@ -1506,7 +1512,7 @@
len += opt.request_size - len - tail_len;
}
- strncpy( (char *) buf + len, GET_REQUEST_END, sizeof(buf) - len - 1 );
+ strncpy( (char *) buf + len, GET_REQUEST_END, sizeof( buf ) - len - 1 );
len += tail_len;
/* Truncate if request size is smaller than the "natural" size */
@@ -1550,6 +1556,12 @@
frags = 1;
written = ret;
+
+ if( written < len )
+ {
+ mbedtls_printf( " warning\n ! request didn't fit into single datagram and "
+ "was truncated to size %u", (unsigned) written );
+ }
}
buf[written] = '\0';
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index d9c5bbf..258141d 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -413,6 +413,16 @@
scripts/config.pl set MBEDTLS_NO_PLATFORM_ENTROPY # uses syscall() on GNU/Linux
CC=gcc CFLAGS='-Werror -Wall -Wextra -O0 -std=c99 -pedantic' make lib
+msg "build: default config except MFL extension (ASan build)" # ~ 30s
+cleanup
+cp "$CONFIG_H" "$CONFIG_BAK"
+scripts/config.pl unset MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
+CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan .
+make
+
+msg "test: ssl-opt.sh, MFL-related tests"
+tests/ssl-opt.sh -f "Max fragment length"
+
msg "build: default config with MBEDTLS_TEST_NULL_ENTROPY (ASan build)"
cleanup
cp "$CONFIG_H" "$CONFIG_BAK"
@@ -628,4 +638,3 @@
msg "Done, cleaning up"
cleanup
-
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 64f26a0..5dc47a3 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -1348,7 +1348,23 @@
# Tests for Max Fragment Length extension
-run_test "Max fragment length: not used, reference" \
+MAX_CONTENT_LEN_EXPECT='16384'
+MAX_CONTENT_LEN_CONFIG=$( ../scripts/config.pl get MBEDTLS_SSL_MAX_CONTENT_LEN)
+
+if [ -n "$MAX_CONTENT_LEN_CONFIG" ] && [ "$MAX_CONTENT_LEN_CONFIG" -ne "$MAX_CONTENT_LEN_EXPECT" ]; then
+ printf "The ${CONFIG_H} file contains a value for the configuration of\n"
+ printf "MBEDTLS_SSL_MAX_CONTENT_LEN that is different from the script’s\n"
+ printf "test value of ${MAX_CONTENT_LEN_EXPECT}. \n"
+ printf "\n"
+ printf "The tests assume this value and if it changes, the tests in this\n"
+ printf "script should also be adjusted.\n"
+ printf "\n"
+
+ exit 1
+fi
+
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
+run_test "Max fragment length: enabled, default" \
"$P_SRV debug_level=3" \
"$P_CLI debug_level=3" \
0 \
@@ -1359,6 +1375,55 @@
-S "server hello, max_fragment_length extension" \
-C "found max_fragment_length extension"
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
+run_test "Max fragment length: enabled, default, larger message" \
+ "$P_SRV debug_level=3" \
+ "$P_CLI debug_level=3 request_size=16385" \
+ 0 \
+ -c "Maximum fragment length is 16384" \
+ -s "Maximum fragment length is 16384" \
+ -C "client hello, adding max_fragment_length extension" \
+ -S "found max fragment length extension" \
+ -S "server hello, max_fragment_length extension" \
+ -C "found max_fragment_length extension" \
+ -c "16385 bytes written in 2 fragments" \
+ -s "16384 bytes read" \
+ -s "1 bytes read"
+
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
+run_test "Max fragment length, DTLS: enabled, default, larger message" \
+ "$P_SRV debug_level=3 dtls=1" \
+ "$P_CLI debug_level=3 dtls=1 request_size=16385" \
+ 1 \
+ -c "Maximum fragment length is 16384" \
+ -s "Maximum fragment length is 16384" \
+ -C "client hello, adding max_fragment_length extension" \
+ -S "found max fragment length extension" \
+ -S "server hello, max_fragment_length extension" \
+ -C "found max_fragment_length extension" \
+ -c "fragment larger than.*maximum "
+
+requires_config_disabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
+run_test "Max fragment length: disabled, larger message" \
+ "$P_SRV debug_level=3" \
+ "$P_CLI debug_level=3 request_size=16385" \
+ 0 \
+ -C "Maximum fragment length is 16384" \
+ -S "Maximum fragment length is 16384" \
+ -c "16385 bytes written in 2 fragments" \
+ -s "16384 bytes read" \
+ -s "1 bytes read"
+
+requires_config_disabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
+run_test "Max fragment length DTLS: disabled, larger message" \
+ "$P_SRV debug_level=3 dtls=1" \
+ "$P_CLI debug_level=3 dtls=1 request_size=16385" \
+ 1 \
+ -C "Maximum fragment length is 16384" \
+ -S "Maximum fragment length is 16384" \
+ -c "fragment larger than.*maximum "
+
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
run_test "Max fragment length: used by client" \
"$P_SRV debug_level=3" \
"$P_CLI debug_level=3 max_frag_len=4096" \
@@ -1370,6 +1435,7 @@
-s "server hello, max_fragment_length extension" \
-c "found max_fragment_length extension"
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
run_test "Max fragment length: used by server" \
"$P_SRV debug_level=3 max_frag_len=4096" \
"$P_CLI debug_level=3" \
@@ -1381,6 +1447,7 @@
-S "server hello, max_fragment_length extension" \
-C "found max_fragment_length extension"
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
requires_gnutls
run_test "Max fragment length: gnutls server" \
"$G_SRV" \
@@ -1390,6 +1457,7 @@
-c "client hello, adding max_fragment_length extension" \
-c "found max_fragment_length extension"
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
run_test "Max fragment length: client, message just fits" \
"$P_SRV debug_level=3" \
"$P_CLI debug_level=3 max_frag_len=2048 request_size=2048" \
@@ -1403,6 +1471,7 @@
-c "2048 bytes written in 1 fragments" \
-s "2048 bytes read"
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
run_test "Max fragment length: client, larger message" \
"$P_SRV debug_level=3" \
"$P_CLI debug_level=3 max_frag_len=2048 request_size=2345" \
@@ -1417,6 +1486,7 @@
-s "2048 bytes read" \
-s "297 bytes read"
+requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
run_test "Max fragment length: DTLS client, larger message" \
"$P_SRV debug_level=3 dtls=1" \
"$P_CLI debug_level=3 dtls=1 max_frag_len=2048 request_size=2345" \
@@ -3417,6 +3487,7 @@
"$P_CLI request_size=16384 force_version=ssl3 recsplit=0 \
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
requires_config_enabled MBEDTLS_SSL_PROTO_SSL3
@@ -3425,6 +3496,7 @@
"$P_CLI request_size=16384 force_version=ssl3 \
force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.0 BlockCipher" \
@@ -3432,6 +3504,7 @@
"$P_CLI request_size=16384 force_version=tls1 recsplit=0 \
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.0 BlockCipher truncated MAC" \
@@ -3440,6 +3513,7 @@
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA \
trunc_hmac=1" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.0 StreamCipher truncated MAC" \
@@ -3448,6 +3522,7 @@
force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA \
trunc_hmac=1" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.1 BlockCipher" \
@@ -3455,6 +3530,7 @@
"$P_CLI request_size=16384 force_version=tls1_1 \
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.1 StreamCipher" \
@@ -3462,6 +3538,7 @@
"$P_CLI request_size=16384 force_version=tls1_1 \
force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.1 BlockCipher truncated MAC" \
@@ -3470,6 +3547,7 @@
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA \
trunc_hmac=1" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.1 StreamCipher truncated MAC" \
@@ -3478,6 +3556,7 @@
force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA \
trunc_hmac=1" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 BlockCipher" \
@@ -3485,6 +3564,7 @@
"$P_CLI request_size=16384 force_version=tls1_2 \
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 BlockCipher larger MAC" \
@@ -3492,6 +3572,7 @@
"$P_CLI request_size=16384 force_version=tls1_2 \
force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA384" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 BlockCipher truncated MAC" \
@@ -3500,6 +3581,7 @@
force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA \
trunc_hmac=1" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 StreamCipher" \
@@ -3507,6 +3589,7 @@
"$P_CLI request_size=16384 force_version=tls1_2 \
force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 StreamCipher truncated MAC" \
@@ -3515,6 +3598,7 @@
force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA \
trunc_hmac=1" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 AEAD" \
@@ -3522,6 +3606,7 @@
"$P_CLI request_size=16384 force_version=tls1_2 \
force_ciphersuite=TLS-RSA-WITH-AES-256-CCM" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
run_test "Large packet TLS 1.2 AEAD shorter tag" \
@@ -3529,6 +3614,7 @@
"$P_CLI request_size=16384 force_version=tls1_2 \
force_ciphersuite=TLS-RSA-WITH-AES-256-CCM-8" \
0 \
+ -c "16384 bytes written in 1 fragments" \
-s "Read from client: 16384 bytes read"
# Tests for DTLS HelloVerifyRequest