From af46c5f9eb88ab0f3076c400db2f45b17bc02a40 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 26 Feb 2019 13:50:21 +0000 Subject: [PATCH 01/15] Check dependencies of MBEDTLS_MEMORY_BACKTRACE in check_config.h --- include/mbedtls/check_config.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 1bf4229f0..68dc4740a 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -303,6 +303,10 @@ #error "MBEDTLS_MEMORY_BUFFER_ALLOC_C defined, but not all prerequisites" #endif +#if defined(MBEDTLS_MEMORY_BACKTRACE) && !defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) +#error "MBEDTLS_MEMORY_BACKTRACE defined, but not all prerequesites" +#endif + #if defined(MBEDTLS_PADLOCK_C) && !defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_PADLOCK_C defined, but not all prerequisites" #endif From 909e68d45a85c961c0b4058a4a5bebc657f8b913 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 26 Feb 2019 13:51:00 +0000 Subject: [PATCH 02/15] Disable memory buffer allocator in full config This commit modifies `config.pl` to not set MBEDTLS_MEMORY_BUFFER_ALLOC with the `full` option. --- scripts/config.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/config.pl b/scripts/config.pl index 394258465..40e3959e4 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -26,6 +26,8 @@ # MBEDTLS_ECP_DP_M221_ENABLED # MBEDTLS_ECP_DP_M383_ENABLED # MBEDTLS_ECP_DP_M511_ENABLED +# MBEDTLS_MEMORY_BACKTRACE +# MBEDTLS_MEMORY_BUFFER_ALLOC_C # MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES # MBEDTLS_NO_PLATFORM_ENTROPY # MBEDTLS_REMOVE_ARC4_CIPHERSUITES @@ -89,6 +91,8 @@ MBEDTLS_PLATFORM_NO_STD_FUNCTIONS MBEDTLS_ECP_DP_M221_ENABLED MBEDTLS_ECP_DP_M383_ENABLED MBEDTLS_ECP_DP_M511_ENABLED +MBEDTLS_MEMORY_BACKTRACE +MBEDTLS_MEMORY_BUFFER_ALLOC_C MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES MBEDTLS_NO_PLATFORM_ENTROPY MBEDTLS_RSA_NO_CRT From 790c281f516a3b0f2dd40995593db55cad3dbb30 Mon Sep 17 00:00:00 2001 From: Unknown Date: Wed, 4 Sep 2019 06:46:44 -0400 Subject: [PATCH 03/15] Adapt all.sh to removal of buffer allocator from full config Previously, numerous all.sh tests manually disabled the buffer allocator or memory backtracting after setting a full config as the starting point. With the removal of MBEDTLS_MEMORY_BACKTRACE and MBEDTLS_MEMORY_BUFFER_ALLOC_C from full configs, this is no longer necessary. --- tests/scripts/all.sh | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 13c5c2d79..6a1050042 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -772,7 +772,6 @@ component_test_small_mbedtls_ssl_dtls_max_buffering () { component_test_full_cmake_clang () { msg "build: cmake, full config, clang" # ~ 50s scripts/config.pl full - scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE # too slow for tests CC=clang cmake -D CMAKE_BUILD_TYPE:String=Check -D ENABLE_TESTING=On . make @@ -842,7 +841,6 @@ component_test_no_use_psa_crypto_full_cmake_asan() { # full minus MBEDTLS_USE_PSA_CRYPTO: run the same set of tests as basic-build-test.sh msg "build: cmake, full config minus MBEDTLS_USE_PSA_CRYPTO, ASan" scripts/config.pl full - scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C # slow and makes ASan mostly ineffective scripts/config.pl set MBEDTLS_ECP_RESTARTABLE # not using PSA, so enable restartable ECC scripts/config.pl unset MBEDTLS_PSA_CRYPTO_C scripts/config.pl unset MBEDTLS_USE_PSA_CRYPTO @@ -885,7 +883,6 @@ component_test_check_params_without_platform () { msg "build+test: MBEDTLS_CHECK_PARAMS without MBEDTLS_PLATFORM_C" scripts/config.pl full # includes CHECK_PARAMS # Keep MBEDTLS_PARAM_FAILED as assert. - scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl unset MBEDTLS_PLATFORM_EXIT_ALT scripts/config.pl unset MBEDTLS_PLATFORM_TIME_ALT scripts/config.pl unset MBEDTLS_PLATFORM_FPRINTF_ALT @@ -900,7 +897,6 @@ component_test_check_params_without_platform () { component_test_check_params_silent () { msg "build+test: MBEDTLS_CHECK_PARAMS with alternative MBEDTLS_PARAM_FAILED()" scripts/config.pl full # includes CHECK_PARAMS - scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE # too slow for tests # Set MBEDTLS_PARAM_FAILED to nothing. sed -i 's/.*\(#define MBEDTLS_PARAM_FAILED( cond )\).*/\1/' "$CONFIG_H" make CC=gcc CFLAGS='-Werror -O1' all test @@ -921,7 +917,6 @@ component_test_no_platform () { scripts/config.pl unset MBEDTLS_PLATFORM_TIME_ALT scripts/config.pl unset MBEDTLS_PLATFORM_EXIT_ALT scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED - scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C @@ -1090,7 +1085,6 @@ component_test_m32_o1 () { # Build again with -O1, to compile in the i386 specific inline assembly msg "build: i386, make, gcc -O1 (ASan build)" # ~ 30s scripts/config.pl full - scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C # slow and makes ASan mostly ineffective make CC=gcc CFLAGS='-O1 -Werror -Wall -Wextra -m32 -fsanitize=address' LDFLAGS='-m32 -fsanitize=address' msg "test: i386, make, gcc -O1 (ASan build)" From 2ea2f053c56ba2e50f6ace8047f42fb718e0829e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 26 Feb 2019 14:33:57 +0000 Subject: [PATCH 04/15] Update documentation of exceptions for `config.pl full` --- scripts/config.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/config.pl b/scripts/config.pl index 40e3959e4..0d10e4948 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -39,6 +39,8 @@ # - this could be enabled if the respective tests were adapted # MBEDTLS_ZLIB_SUPPORT # MBEDTLS_PKCS11_C +# MBEDTLS_NO_UDBL_DIVISION +# MBEDTLS_NO_64BIT_MULTIPLICATION # MBEDTLS_PSA_CRYPTO_SPM # MBEDTLS_PSA_INJECT_ENTROPY # MBEDTLS_ECP_RESTARTABLE From 0fb9ba2760f6fbe63db54a31f35b3d536ca799d7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 26 Feb 2019 14:34:13 +0000 Subject: [PATCH 05/15] Add all.sh run with MBEDTLS_MEMORY_BUFFER_ALLOC_C enabled With the removal of MBEDTLS_MEMORY_BUFFER_ALLOC_C from the full config, there are no tests for it remaining in all.sh. This commit adds a build as well as runs of `make test` and `ssl-opt.sh` with MBEDTLS_MEMORY_BUFFER_ALLOC_C enabled to all.sh. --- tests/scripts/all.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6a1050042..a0c3ef400 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -959,6 +959,20 @@ component_build_no_sockets () { make CC=gcc CFLAGS='-Werror -Wall -Wextra -O0 -std=c99 -pedantic' lib } +component_test_memory_buffer_allocator () { + msg "build: default config with memory buffer allocator enabled" + scripts/config.pl set MBEDTLS_MEMORY_BUFFER_ALLOC_C + scripts/config.pl set MBEDTLS_MEMORY_BACKTRACE + CC=gcc cmake . + make + + msg "test: MBEDTLS_MEMORY_BUFFER_ALLOC_C" + make test + + msg "test: ssl-opt.sh, MBEDTLS_MEMORY_BUFFER_ALLOC_C" + if_build_succeeded tests/ssl-opt.sh +} + component_test_no_max_fragment_length () { # Run max fragment length tests with MFL disabled msg "build: default config except MFL extension (ASan build)" # ~ 30s From 0163551aa029c1f8616c19235108edeef97b4144 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 26 Feb 2019 14:27:09 +0000 Subject: [PATCH 06/15] Add all.sh run with full config and ASan enabled --- tests/scripts/all.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index a0c3ef400..c35fe1819 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -643,6 +643,22 @@ component_test_default_cmake_gcc_asan () { if_build_succeeded tests/compat.sh } +component_test_full_cmake_gcc_asan () { + msg "build: full config, cmake, gcc, ASan" + scripts/config.pl full + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: main suites (inc. selftests) (full config, ASan build)" + make test + + msg "test: ssl-opt.sh (full config, ASan build)" + if_build_succeeded tests/ssl-opt.sh + + msg "test: compat.sh (full config, ASan build)" + if_build_succeeded tests/compat.sh +} + component_test_ref_configs () { msg "test/build: ref-configs (ASan build)" # ~ 6 min 20s CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . From 2fcdd7446e5c83cdb5170a5d4b4470d6f6c9050a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 26 Feb 2019 13:59:48 +0000 Subject: [PATCH 07/15] Fix a memory leak in x509write test suite This leak wasn't discovered by the CI because the only test in all.sh exercising the respective path enabled the custom memory buffer allocator implementations of calloc() and free(), hence bypassing ASan. --- tests/suites/test_suite_x509write.function | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index e15802ff1..7a359b1c6 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -57,6 +57,7 @@ static int x509_crt_verifycsr( const unsigned char *buf, size_t buflen ) return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ); } + mbedtls_x509_csr_free( &csr ); return( 0 ); } #endif /* MBEDTLS_USE_PSA_CRYPTO */ From bf2dacb8fed20e4f72d584b9805ade53ad1f6dc2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 3 Jun 2019 16:28:24 +0100 Subject: [PATCH 08/15] Fix memory leak in CSR test suite on failure --- tests/suites/test_suite_x509write.function | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 7a359b1c6..7b369bb87 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -39,26 +39,36 @@ static int x509_crt_verifycsr( const unsigned char *buf, size_t buflen ) unsigned char hash[MBEDTLS_MD_MAX_SIZE]; const mbedtls_md_info_t *md_info; mbedtls_x509_csr csr; + int ret = 0; + + mbedtls_x509_csr_init( &csr ); if( mbedtls_x509_csr_parse( &csr, buf, buflen ) != 0 ) - return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); + { + ret = MBEDTLS_ERR_X509_BAD_INPUT_DATA; + goto cleanup; + } md_info = mbedtls_md_info_from_type( csr.sig_md ); if( mbedtls_md( md_info, csr.cri.p, csr.cri.len, hash ) != 0 ) { /* Note: this can't happen except after an internal error */ - return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); + ret = MBEDTLS_ERR_X509_BAD_INPUT_DATA; + goto cleanup; } if( mbedtls_pk_verify_ext( csr.sig_pk, csr.sig_opts, &csr.pk, csr.sig_md, hash, mbedtls_md_get_size( md_info ), csr.sig.p, csr.sig.len ) != 0 ) { - return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ); + ret = MBEDTLS_ERR_X509_CERT_VERIFY_FAILED; + goto cleanup; } +cleanup: + mbedtls_x509_csr_free( &csr ); - return( 0 ); + return( ret ); } #endif /* MBEDTLS_USE_PSA_CRYPTO */ From bfaa718e9021cf36d2c1b6dcf8f0f9ef519d62dd Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 3 Jun 2019 16:31:32 +0100 Subject: [PATCH 09/15] Add cfg dep MBEDTLS_MEMORY_DEBUG->MBEDTLS_MEMORY_BUFFER_ALLOC_C --- include/mbedtls/check_config.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 68dc4740a..fb3b6e154 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -307,6 +307,10 @@ #error "MBEDTLS_MEMORY_BACKTRACE defined, but not all prerequesites" #endif +#if defined(MBEDTLS_MEMORY_DEBUG) && !defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) +#error "MBEDTLS_MEMORY_DEBUG defined, but not all prerequesites" +#endif + #if defined(MBEDTLS_PADLOCK_C) && !defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_PADLOCK_C defined, but not all prerequisites" #endif From dc54953229ebfa5826875ddd44c01fb6ee553578 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 3 Jun 2019 16:33:18 +0100 Subject: [PATCH 10/15] Don't set MBEDTLS_MEMORY_DEBUG through `scripts/config.pl full` --- scripts/config.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/config.pl b/scripts/config.pl index 0d10e4948..b4b00581e 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -93,6 +93,7 @@ MBEDTLS_PLATFORM_NO_STD_FUNCTIONS MBEDTLS_ECP_DP_M221_ENABLED MBEDTLS_ECP_DP_M383_ENABLED MBEDTLS_ECP_DP_M511_ENABLED +MBEDTLS_MEMORY_DEBUG MBEDTLS_MEMORY_BACKTRACE MBEDTLS_MEMORY_BUFFER_ALLOC_C MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES From d7064202eaa75b42cdf4372cba8fe3383c3aa6ca Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 3 Jun 2019 16:35:02 +0100 Subject: [PATCH 11/15] Add missing dependency in memory buffer alloc set in all.sh --- tests/scripts/all.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c35fe1819..7d53ba8e4 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -979,6 +979,7 @@ component_test_memory_buffer_allocator () { msg "build: default config with memory buffer allocator enabled" scripts/config.pl set MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl set MBEDTLS_MEMORY_BACKTRACE + scripts/config.pl set MBEDTLS_PLATFORM_MEMORY CC=gcc cmake . make From 69f20aae77be91057d33f0a02d3401aeaf54e230 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 5 Sep 2019 09:27:47 -0400 Subject: [PATCH 12/15] all.sh: restructure memory allocator tests Run basic tests and ssl-opt with memory backtrace disabled, then run basic tests only with it enabled. --- tests/scripts/all.sh | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7d53ba8e4..d6c6bdcf2 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -975,10 +975,21 @@ component_build_no_sockets () { make CC=gcc CFLAGS='-Werror -Wall -Wextra -O0 -std=c99 -pedantic' lib } -component_test_memory_buffer_allocator () { - msg "build: default config with memory buffer allocator enabled" +component_test_memory_buffer_allocator_backtrace () { + msg "build: default config with memory buffer allocator and backtrace enabled" scripts/config.pl set MBEDTLS_MEMORY_BUFFER_ALLOC_C + scripts/config.pl set MBEDTLS_PLATFORM_MEMORY scripts/config.pl set MBEDTLS_MEMORY_BACKTRACE + CC=gcc cmake . + make + + msg "test: MBEDTLS_MEMORY_BUFFER_ALLOC_C and MBEDTLS_MEMORY_BACKTRACE" + make test +} + +component_test_memory_buffer_allocator () { + msg "build: default config with memory buffer allocator" + scripts/config.pl set MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl set MBEDTLS_PLATFORM_MEMORY CC=gcc cmake . make From 1e56d2c3dea3f779bc43a60b057ffbb58518ba5b Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 5 Sep 2019 10:09:37 -0400 Subject: [PATCH 13/15] Disable DTLS proxy tests for MEMORY_BUFFER_ALLOC test --- tests/scripts/all.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d6c6bdcf2..397a758af 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -998,7 +998,8 @@ component_test_memory_buffer_allocator () { make test msg "test: ssl-opt.sh, MBEDTLS_MEMORY_BUFFER_ALLOC_C" - if_build_succeeded tests/ssl-opt.sh + # MBEDTLS_MEMORY_BUFFER_ALLOC is slow. Skip tests that tend to time out. + if_build_succeeded tests/ssl-opt.sh -e '^DTLS proxy' } component_test_no_max_fragment_length () { From 4b3a45e190cc36ec69d0d805f3c4c7799fac05f7 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 6 Sep 2019 07:47:30 -0400 Subject: [PATCH 14/15] Remove unnecessary memory buffer alloc unsets This define is turned off by default --- tests/scripts/all.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 397a758af..c458bb35f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -889,7 +889,6 @@ component_test_check_params_functionality () { scripts/config.pl full # includes CHECK_PARAMS # Make MBEDTLS_PARAM_FAILED call mbedtls_param_failed(). scripts/config.pl unset MBEDTLS_CHECK_PARAMS_ASSERT - scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C # Only build and run tests. Do not build sample programs, because # they don't have a mbedtls_param_failed() function. make CC=gcc CFLAGS='-Werror -O1' lib test @@ -1111,7 +1110,6 @@ component_test_m32_o0 () { # Build once with -O0, to compile out the i386 specific inline assembly msg "build: i386, make, gcc -O0 (ASan build)" # ~ 30s scripts/config.pl full - scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C # slow and makes ASan mostly ineffective make CC=gcc CFLAGS='-O0 -Werror -Wall -Wextra -m32 -fsanitize=address' LDFLAGS='-m32 -fsanitize=address' msg "test: i386, make, gcc -O0 (ASan build)" From 9f409f6aecc9d584cee09945f5cc79aa3690d257 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 10 Sep 2019 02:58:34 -0400 Subject: [PATCH 15/15] Enable MBEDTLS_MEMORY_DEBUG in memory buffer alloc test in all.sh --- tests/scripts/all.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c458bb35f..c361b8303 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -979,6 +979,7 @@ component_test_memory_buffer_allocator_backtrace () { scripts/config.pl set MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl set MBEDTLS_PLATFORM_MEMORY scripts/config.pl set MBEDTLS_MEMORY_BACKTRACE + scripts/config.pl set MBEDTLS_MEMORY_DEBUG CC=gcc cmake . make