diff --git a/ChangeLog b/ChangeLog index ded60d39f..0d323b1d9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,16 @@ mbed TLS ChangeLog (Sorted per branch, date) = 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 8467b1302..e19dfc903 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7078,7 +7078,9 @@ static int ssl_write_real( mbedtls_ssl_context *ssl, 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 @@ static int ssl_write_real( mbedtls_ssl_context *ssl, #endif len = max_len; } -#endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ if( ssl->out_left != 0 ) { @@ -7124,7 +7125,7 @@ static int ssl_write_real( mbedtls_ssl_context *ssl, * * 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 5032a9f3d..8e2feb1a1 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -63,6 +63,9 @@ int main( void ) #include #include +#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 @@ 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: 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 main( int argc, char *argv[] ) { 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 @@ int main( int argc, char *argv[] ) 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 @@ send_request: 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 @@ send_request: 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 @@ send_request: 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 d9c5bbfa4..258141dff 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -413,6 +413,16 @@ scripts/config.pl unset MBEDTLS_NET_C # getaddrinfo() undeclared, etc. 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 @@ rm -rf "$OUT_OF_SOURCE_DIR" msg "Done, cleaning up" cleanup - diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 64f26a0cf..5dc47a39d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1348,7 +1348,23 @@ run_test "Session resume using cache: openssl server" \ # 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 @@ run_test "Max fragment length: not used, reference" \ -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 @@ run_test "Max fragment length: used by client" \ -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 @@ run_test "Max fragment length: used by server" \ -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 @@ run_test "Max fragment length: gnutls server" \ -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 @@ run_test "Max fragment length: client, message just fits" \ -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 @@ run_test "Max fragment length: client, larger message" \ -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 @@ run_test "Large packet SSLv3 BlockCipher" \ "$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 @@ run_test "Large packet SSLv3 StreamCipher" \ "$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 @@ run_test "Large packet TLS 1.0 BlockCipher" \ "$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 @@ run_test "Large packet TLS 1.0 BlockCipher truncated MAC" \ 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 @@ run_test "Large packet TLS 1.0 StreamCipher truncated MAC" \ 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 @@ run_test "Large packet TLS 1.1 BlockCipher" \ "$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 @@ run_test "Large packet TLS 1.1 StreamCipher" \ "$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 @@ run_test "Large packet TLS 1.1 BlockCipher truncated MAC" \ 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 @@ run_test "Large packet TLS 1.1 StreamCipher truncated MAC" \ 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 @@ run_test "Large packet TLS 1.2 BlockCipher" \ "$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 @@ run_test "Large packet TLS 1.2 BlockCipher larger MAC" \ "$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 @@ run_test "Large packet TLS 1.2 BlockCipher truncated MAC" \ 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 @@ run_test "Large packet TLS 1.2 StreamCipher" \ "$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 @@ run_test "Large packet TLS 1.2 StreamCipher truncated MAC" \ 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 @@ run_test "Large packet TLS 1.2 AEAD" \ "$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 @@ run_test "Large packet TLS 1.2 AEAD shorter tag" \ "$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