From 031d6335b7e762cb8ae8dd7fb44e04a22235f978 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 1 May 2019 17:09:11 +0100 Subject: [PATCH 1/7] Fix mpi_bigendian_to_host() on bigendian systems The previous implementation of mpi_bigendian_to_host() did a byte-swapping regardless of the endianness of the system. Fixes #2622. --- library/bignum.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 98ee12a71..b5e022ac7 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -742,10 +742,15 @@ cleanup: static mbedtls_mpi_uint mpi_uint_bigendian_to_host_c( mbedtls_mpi_uint x ) { uint8_t i; + unsigned char *x_ptr; mbedtls_mpi_uint tmp = 0; - /* This works regardless of the endianness. */ - for( i = 0; i < ciL; i++, x >>= 8 ) - tmp |= ( x & 0xFF ) << ( ( ciL - 1 - i ) << 3 ); + + for( i = 0, x_ptr = (unsigned char*) &x; i < ciL; i++, x_ptr++ ) + { + tmp <<= CHAR_BIT; + tmp |= (mbedtls_mpi_uint) *x_ptr; + } + return( tmp ); } From 5f9aa2be7d20cc8248b3d8e115213ffe2e6ea638 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 2 May 2019 09:33:56 +0100 Subject: [PATCH 2/7] Adapt ChangeLog --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 58ff14734..e429caf8e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,9 @@ Bugfix for the parameter. * Add a check for MBEDTLS_X509_CRL_PARSE_C in ssl_server2, guarding the crl sni entry parameter. Reported by inestlerode in #560. + * Fix bug in endianness conversion in bignum module. This lead to + functionally incorrect code on bigendian systems which don't have + __BYTE_ORDER__ defined. Reported by Brendan Shanks. Fixes #2622. Changes * Server's RSA certificate in certs.c was SHA-1 signed. In the default From d364f4c7dd085f2c49ce42c989a33e4f4244a879 Mon Sep 17 00:00:00 2001 From: Unknown Date: Mon, 2 Sep 2019 10:42:57 -0400 Subject: [PATCH 3/7] ssl-opt.sh: wait for proxy to start before running the script further --- tests/ssl-opt.sh | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 55a4fe1ef..47b6b80c9 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -426,9 +426,9 @@ has_mem_err() { fi } -# Wait for process $2 to be listening on port $1 +# Wait for process $2 named $3 to be listening on port $1. Print error to $4. if type lsof >/dev/null 2>/dev/null; then - wait_server_start() { + wait_app_start() { START_TIME=$(date +%s) if [ "$DTLS" -eq 1 ]; then proto=UDP @@ -438,8 +438,8 @@ if type lsof >/dev/null 2>/dev/null; then # Make a tight loop, server normally takes less than 1s to start. while ! lsof -a -n -b -i "$proto:$1" -p "$2" >/dev/null 2>/dev/null; do if [ $(( $(date +%s) - $START_TIME )) -gt $DOG_DELAY ]; then - echo "SERVERSTART TIMEOUT" - echo "SERVERSTART TIMEOUT" >> $SRV_OUT + echo "$3 START TIMEOUT" + echo "$3 START TIMEOUT" >> $4 break fi # Linux and *BSD support decimal arguments to sleep. On other @@ -448,12 +448,22 @@ if type lsof >/dev/null 2>/dev/null; then done } else - echo "Warning: lsof not available, wait_server_start = sleep" - wait_server_start() { + echo "Warning: lsof not available, wait_app_start = sleep" + wait_app_start() { sleep "$START_DELAY" } fi +# Wait for server process $2 to be listening on port $1. +wait_server_start() { + wait_app_start $1 $2 "SERVER" $SRV_OUT +} + +# Wait for proxy process $2 to be listening on port $1. +wait_proxy_start() { + wait_app_start $1 $2 "PROXY" $PXY_OUT +} + # Given the client or server debug output, parse the unix timestamp that is # included in the first 4 bytes of the random bytes and check that it's within # acceptable bounds @@ -610,7 +620,7 @@ run_test() { echo "$PXY_CMD" > $PXY_OUT $PXY_CMD >> $PXY_OUT 2>&1 & PXY_PID=$! - # assume proxy starts faster than server + wait_proxy_start "$PXY_PORT" "$PXY_PID" fi check_osrv_dtls From c6f1c84663d4d5bdef128a82c801580cb25769e2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 3 Sep 2019 14:07:31 +0200 Subject: [PATCH 4/7] Remove config.pl calls with no effect When MBEDTLS_MEMORY_BUFFER_ALLOC_C is disabled, other MBEDTLS_MEMORY_xxx options have no effect, so don't bother unsetting them explicitly. --- tests/scripts/all.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 22c81296c..aa5f0b77f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -867,7 +867,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_BACKTRACE # too slow for tests 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 @@ -1052,9 +1051,7 @@ 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_BACKTRACE scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C - scripts/config.pl unset MBEDTLS_MEMORY_DEBUG make CC=gcc CFLAGS='-O1 -Werror -Wall -Wextra -m32 -fsanitize=address' LDFLAGS='-m32 -fsanitize=address' msg "test: i386, make, gcc -O1 (ASan build)" From 751bb4c0e1ee34a29b3f35da63f26772db930d7d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 31 Jul 2019 18:12:10 +0200 Subject: [PATCH 5/7] Disable MEMORY_BUFFER_ALLOC with ASan MBEDTLS_MEMORY_BUFFER_ALLOC_C makes ASan mostly ineffective since it hides allocations. So disable it when testing with ASan. --- tests/scripts/all.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index aa5f0b77f..7103dbf1f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -824,7 +824,7 @@ 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 + MBEDTLS_USE_PSA_CRYPTO, ASan" scripts/config.pl full - scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE # too slow for tests + 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 set MBEDTLS_PSA_CRYPTO_C scripts/config.pl unset MBEDTLS_USE_PSA_CRYPTO @@ -1035,6 +1035,7 @@ 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)" @@ -1051,7 +1052,7 @@ 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 + 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 6ce30722d06943a1780368560fbffbdd234c18b7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 31 Jul 2019 18:13:20 +0200 Subject: [PATCH 6/7] When not using PSA crypto, disable it In the test with the full config without MBEDTLS_USE_PSA_CRYPTO, don't build MBEDTLS_PSA_CRYPTO_C, since it isn't supposed to be used. --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7103dbf1f..7a8719e78 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -826,7 +826,7 @@ component_test_no_use_psa_crypto_full_cmake_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 set MBEDTLS_PSA_CRYPTO_C + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_C scripts/config.pl unset MBEDTLS_USE_PSA_CRYPTO scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C From dc3a179995f0ae32c944c1d913e4b25f176dd72b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 3 Sep 2019 11:42:04 +0200 Subject: [PATCH 7/7] Fix copypasta in msg --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7a8719e78..2c67778ef 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -822,7 +822,7 @@ component_build_default_make_gcc_and_cxx () { 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 + MBEDTLS_USE_PSA_CRYPTO, ASan" + 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