From a708dae94beadeb93d770439b18be367f53fe5d7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:06:03 +0200 Subject: [PATCH 01/26] Add comment to help syntax highlighting in editors --- tests/scripts/test-ref-configs.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index 80d5f3875..a434b46b2 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -18,7 +18,7 @@ use strict; my %configs = ( 'config-mini-tls1_1.h' => { - 'compat' => '-m tls1_1 -f \'^DES-CBC3-SHA$\|^TLS-RSA-WITH-3DES-EDE-CBC-SHA$\'', + 'compat' => '-m tls1_1 -f \'^DES-CBC3-SHA$\|^TLS-RSA-WITH-3DES-EDE-CBC-SHA$\'', #' }, 'config-suite-b.h' => { 'compat' => "-m tls1_2 -f 'ECDHE-ECDSA.*AES.*GCM' -p mbedTLS", From 3c1c8ea3e771c9469180bc2ba355e160a471f54b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:10:47 +0200 Subject: [PATCH 02/26] Prefer unsigned types for non-negative numbers Use size_t for some variables that are array indices. Use unsigned for some variables that are counts of "small" things. --- tests/suites/host_test.function | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index 0f98d23aa..f5dfc2b78 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -385,15 +385,16 @@ int execute_tests( int argc , const char ** argv ) const char *default_filename = "DATA_FILE"; const char *test_filename = NULL; const char **test_files = NULL; - int testfile_count = 0; + size_t testfile_count = 0; int option_verbose = 0; int function_id = 0; /* Other Local variables */ int arg_index = 1; const char *next_arg; - int testfile_index, ret, i, cnt; - int total_errors = 0, total_tests = 0, total_skipped = 0; + size_t testfile_index, i, cnt; + int ret; + unsigned total_errors = 0, total_tests = 0, total_skipped = 0; FILE *file; char buf[5000]; char *params[50]; @@ -473,7 +474,7 @@ int execute_tests( int argc , const char ** argv ) testfile_index < testfile_count; testfile_index++ ) { - int unmet_dep_count = 0; + size_t unmet_dep_count = 0; char *unmet_dependencies[20]; test_filename = test_files[ testfile_index ]; @@ -658,8 +659,8 @@ int execute_tests( int argc , const char ** argv ) else mbedtls_fprintf( stdout, "FAILED" ); - mbedtls_fprintf( stdout, " (%d / %d tests (%d skipped))\n", - total_tests - total_errors, total_tests, total_skipped ); + mbedtls_fprintf( stdout, " (%u / %u tests (%u skipped))\n", + total_tests - total_errors, total_tests, total_skipped ); #if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) && \ !defined(TEST_SUITE_MEMORY_BUFFER_ALLOC) From 31fccc80a54a6942b2a66f9ebbaad8cf1cdd1506 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:12:37 +0200 Subject: [PATCH 03/26] Fix typo in message --- tests/suites/host_test.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index f5dfc2b78..d74af05bf 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -639,7 +639,7 @@ int execute_tests( int argc , const char ** argv ) } else if( ret == DISPATCH_TEST_FN_NOT_FOUND ) { - mbedtls_fprintf( stderr, "FAILED: FATAL TEST FUNCTION NOT FUND\n" ); + mbedtls_fprintf( stderr, "FAILED: FATAL TEST FUNCTION NOT FOUND\n" ); fclose( file ); mbedtls_exit( 2 ); } From 47b7540fec350b77e76c9fc47a19878e5866750b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:12:51 +0200 Subject: [PATCH 04/26] Give a type name to test_info Make it possible to pass test_info around rather than always refer to the global variable. --- tests/suites/helpers.function | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index b4b6d7286..5d0a3906d 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -271,7 +271,7 @@ typedef enum TEST_RESULT_SKIPPED } test_result_t; -static struct +typedef struct { paramfail_test_state_t paramfail_test_state; test_result_t result; @@ -279,7 +279,8 @@ static struct const char *filename; int line_no; } -test_info; +test_info_t; +static test_info_t test_info; #if defined(MBEDTLS_PLATFORM_C) mbedtls_platform_context platform_ctx; From 51dcc24998298bffdfd58a42eaf8ff2c252222af Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:13:48 +0200 Subject: [PATCH 05/26] Test outcome file support: test suites If the environment variable MBEDTLS_TEST_OUTCOME_FILE is set, then for each test case, write a line to the file with the given name, of the form PLATFORM;CONFIGURATION;TEST SUITE;TEST CASE DESCRIPTION;PASS/FAIL/SKIP;CAUSE PLATFORM and CONFIGURATION come from the environment variables MBEDTLS_TEST_PLATFORM and MBEDTLS_TEST_CONFIGURATION. Errors while writing the test outcome file are not considered fatal, and are not reported except for an error initially opening the file. This is in line with other write errors that are not checked. --- tests/suites/host_test.function | 130 ++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index d74af05bf..9e56ca3ed 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -368,6 +368,118 @@ static int run_test_snprintf( void ) test_snprintf( 5, "123", 3 ) != 0 ); } +/** \brief Write the description of the test case to the outcome CSV file. + * + * \param outcome_file The file to write to. + * If this is \c NULL, this function does nothing. + * \param argv0 The test suite name. + * \param test_case The test case description. + */ +static void write_outcome_entry( FILE *outcome_file, + const char *argv0, + const char *test_case ) +{ + /* The non-varying fields are initialized on first use. */ + static const char *platform = NULL; + static const char *configuration = NULL; + static const char *test_suite = NULL; + + if( outcome_file == NULL ) + return; + + if( platform == NULL ) + { + platform = getenv( "MBEDTLS_TEST_PLATFORM" ); + if( platform == NULL ) + platform = "unknown"; + } + if( configuration == NULL ) + { + configuration = getenv( "MBEDTLS_TEST_CONFIGURATION" ); + if( configuration == NULL ) + configuration = "unknown"; + } + if( test_suite == NULL ) + { + test_suite = strrchr( argv0, '/' ); + if( test_suite != NULL ) + test_suite += 1; // skip the '/' + else + test_suite = argv0; + } + + /* Write the beginning of the outcome line. + * Ignore errors: writing the outcome file is on a best-effort basis. */ + mbedtls_fprintf( outcome_file, "%s;%s;%s;%s;", + platform, configuration, test_suite, test_case ); +} + +/** \brief Write the result of the test case to the outcome CSV file. + * + * \param outcome_file The file to write to. + * If this is \c NULL, this function does nothing. + * \param unmet_dep_count The number of unmet dependencies. + * \param unmet_dependencies The array of unmet dependencies. + * \param ret The test dispatch status (DISPATCH_xxx). + * \param test_info A pointer to the test info structure. + */ +static void write_outcome_result( FILE *outcome_file, + size_t unmet_dep_count, + char *unmet_dependencies[], + int ret, + const test_info_t *info ) +{ + if( outcome_file == NULL ) + return; + + /* Write the end of the outcome line. + * Ignore errors: writing the outcome file is on a best-effort basis. */ + switch( ret ) + { + case DISPATCH_TEST_SUCCESS: + if( unmet_dep_count > 0 ) + { + size_t i; + mbedtls_fprintf( outcome_file, "SKIP" ); + for( i = 0; i < unmet_dep_count; i++ ) + { + mbedtls_fprintf( outcome_file, "%c%s", + i == 0 ? ';' : ':', + unmet_dependencies[i] ); + } + break; + } + switch( info->result ) + { + case TEST_RESULT_SUCCESS: + mbedtls_fprintf( outcome_file, "PASS;" ); + break; + case TEST_RESULT_SKIPPED: + mbedtls_fprintf( outcome_file, "SKIP;Runtime skip" ); + break; + default: + mbedtls_fprintf( outcome_file, "FAIL;%s:%d:%s", + info->filename, info->line_no, + info->test ); + break; + } + break; + case DISPATCH_TEST_FN_NOT_FOUND: + mbedtls_fprintf( outcome_file, "FAIL;Test function not found" ); + break; + case DISPATCH_INVALID_TEST_DATA: + mbedtls_fprintf( outcome_file, "FAIL;Invalid test data" ); + break; + case DISPATCH_UNSUPPORTED_SUITE: + mbedtls_fprintf( outcome_file, "SKIP;Unsupported suite" ); + break; + default: + mbedtls_fprintf( outcome_file, "FAIL;Unknown cause" ); + break; + } + mbedtls_fprintf( outcome_file, "\n" ); + fflush( outcome_file ); +} /** * \brief Desktop implementation of execute_tests(). @@ -404,6 +516,8 @@ int execute_tests( int argc , const char ** argv ) #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) int stdout_fd = -1; #endif /* __unix__ || __APPLE__ __MACH__ */ + const char *outcome_file_name = getenv( "MBEDTLS_TEST_OUTCOME_FILE" ); + FILE *outcome_file = NULL; #if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) && \ !defined(TEST_SUITE_MEMORY_BUFFER_ALLOC) @@ -411,6 +525,15 @@ int execute_tests( int argc , const char ** argv ) mbedtls_memory_buffer_alloc_init( alloc_buf, sizeof( alloc_buf ) ); #endif + if( outcome_file_name != NULL ) + { + outcome_file = fopen( outcome_file_name, "a" ); + if( outcome_file == NULL ) + { + mbedtls_fprintf( stderr, "Unable to open outcome file. Continuing anyway.\n" ); + } + } + /* * The C standard doesn't guarantee that all-bits-0 is the representation * of a NULL pointer. We do however use that in our code for initializing @@ -506,6 +629,7 @@ int execute_tests( int argc , const char ** argv ) mbedtls_fprintf( stdout, "." ); mbedtls_fprintf( stdout, " " ); fflush( stdout ); + write_outcome_entry( outcome_file, argv[0], buf ); total_tests++; @@ -585,6 +709,9 @@ int execute_tests( int argc , const char ** argv ) } + write_outcome_result( outcome_file, + unmet_dep_count, unmet_dependencies, + ret, &test_info ); if( unmet_dep_count > 0 || ret == DISPATCH_UNSUPPORTED_SUITE ) { total_skipped++; @@ -653,6 +780,9 @@ int execute_tests( int argc , const char ** argv ) free( unmet_dependencies[i] ); } + if( outcome_file != NULL ) + fclose( outcome_file ); + mbedtls_fprintf( stdout, "\n----------------------------------------------------------------------------\n\n"); if( total_errors == 0 ) mbedtls_fprintf( stdout, "PASSED" ); From 560280b17df74422d48727ffc36b0ed9f43c28f7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:17:38 +0200 Subject: [PATCH 06/26] Test outcome file support: ssl-opt.sh If the environment variable MBEDTLS_TEST_OUTCOME_FILE is set, then for each test case, write a line to the file with the given name, of the form PLATFORM;CONFIGURATION;ssl-opt;TEST CASE DESCRIPTION;PASS/FAIL/SKIP;CAUSE PLATFORM and CONFIGURATION come from the environment variables MBEDTLS_TEST_PLATFORM and MBEDTLS_TEST_CONFIGURATION. If these variables are unset, the script uses some easily-calculated values. --- tests/ssl-opt.sh | 51 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 47b6b80c9..914f4e1e3 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -25,9 +25,9 @@ set -u # where it may output seemingly unlimited length error logs. ulimit -f 20971520 -if cd $( dirname $0 ); then :; else - echo "cd $( dirname $0 ) failed" >&2 - exit 1 +ORIGINAL_PWD=$PWD +if ! cd "$(dirname "$0")"; then + exit 125 fi # default values, can be overridden by the environment @@ -39,6 +39,17 @@ fi : ${GNUTLS_SERV:=gnutls-serv} : ${PERL:=perl} +guess_config_name() { + if git diff --quiet ../include/mbedtls/config.h 2>/dev/null; then + echo "default" + else + echo "unknown" + fi +} +: ${MBEDTLS_TEST_OUTCOME_FILE=} +: ${MBEDTLS_TEST_CONFIGURATION:="$(guess_config_name)"} +: ${MBEDTLS_TEST_PLATFORM:="$(uname -s | tr -c \\n0-9A-Za-z _)-$(uname -m | tr -c \\n0-9A-Za-z _)"} + O_SRV="$OPENSSL_CMD s_server -www -cert data_files/server5.crt -key data_files/server5.key" O_CLI="echo 'GET / HTTP/1.0' | $OPENSSL_CMD s_client" G_SRV="$GNUTLS_SERV --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key" @@ -97,9 +108,11 @@ print_usage() { printf " -n|--number\tExecute only numbered test (comma-separated, e.g. '245,256')\n" printf " -s|--show-numbers\tShow test numbers in front of test names\n" printf " -p|--preserve-logs\tPreserve logs of successful tests as well\n" - printf " --port\tTCP/UDP port (default: randomish 1xxxx)\n" + printf " --outcome-file\tFile where test outcomes are written\n" + printf " \t(default: \$MBEDTLS_TEST_OUTCOME_FILE, none if empty)\n" + printf " --port \tTCP/UDP port (default: randomish 1xxxx)\n" printf " --proxy-port\tTCP/UDP proxy port (default: randomish 2xxxx)\n" - printf " --seed\tInteger seed value to use for this test run\n" + printf " --seed \tInteger seed value to use for this test run\n" } get_options() { @@ -146,6 +159,14 @@ get_options() { done } +# Make the outcome file path relative to the original directory, not +# to .../tests +case "$MBEDTLS_TEST_OUTCOME_FILE" in + [!/]*) + MBEDTLS_TEST_OUTCOME_FILE="$ORIGINAL_PWD/$MBEDTLS_TEST_OUTCOME_FILE" + ;; +esac + # Skip next test; use this macro to skip tests which are legitimate # in theory and expected to be re-introduced at some point, but # aren't expected to succeed at the moment due to problems outside @@ -359,9 +380,22 @@ print_name() { } +# record_outcome [] +# The test name must be in $NAME. +record_outcome() { + echo "$1" + if [ -n "$MBEDTLS_TEST_OUTCOME_FILE" ]; then + printf '%s;%s;%s;%s;%s;%s\n' \ + "$MBEDTLS_TEST_PLATFORM" "$MBEDTLS_TEST_CONFIGURATION" \ + "ssl-opt" "$NAME" \ + "$1" "${2-}" \ + >>"$MBEDTLS_TEST_OUTCOME_FILE" + fi +} + # fail fail() { - echo "FAIL" + record_outcome "FAIL" "$1" echo " ! $1" mv $SRV_OUT o-srv-${TESTS}.log @@ -539,6 +573,7 @@ run_test() { if echo "$NAME" | grep "$FILTER" | grep -v "$EXCLUDE" >/dev/null; then : else SKIP_NEXT="NO" + # There was no request to run the test, so don't record its outcome. return fi @@ -586,7 +621,7 @@ run_test() { # should we skip? if [ "X$SKIP_NEXT" = "XYES" ]; then SKIP_NEXT="NO" - echo "SKIP" + record_outcome "SKIP" SKIPS=$(( $SKIPS + 1 )) return fi @@ -772,7 +807,7 @@ run_test() { fi # if we're here, everything is ok - echo "PASS" + record_outcome "PASS" if [ "$PRESERVE_LOGS" -gt 0 ]; then mv $SRV_OUT o-srv-${TESTS}.log mv $CLI_OUT o-cli-${TESTS}.log From 654bab7635e1b58399c004110fdfc36d29ba20d8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:19:20 +0200 Subject: [PATCH 07/26] ssl-opt: remove semicolons from test case descriptions Don't use semicolons in test case descriptions. The test outcome file is a semicolon-separated CSV file without quotes to keep things simple, so fields in that file may not contain semicolons. --- tests/ssl-opt.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 914f4e1e3..625fa349d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7027,7 +7027,7 @@ run_test "SSL async private: sign, error in resume then fall back to transpar requires_config_enabled MBEDTLS_SSL_ASYNC_PRIVATE requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -run_test "SSL async private: renegotiation: client-initiated; sign" \ +run_test "SSL async private: renegotiation: client-initiated, sign" \ "$P_SRV \ async_operations=s async_private_delay1=1 async_private_delay2=1 \ exchanges=2 renegotiation=1" \ @@ -7038,7 +7038,7 @@ run_test "SSL async private: renegotiation: client-initiated; sign" \ requires_config_enabled MBEDTLS_SSL_ASYNC_PRIVATE requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -run_test "SSL async private: renegotiation: server-initiated; sign" \ +run_test "SSL async private: renegotiation: server-initiated, sign" \ "$P_SRV \ async_operations=s async_private_delay1=1 async_private_delay2=1 \ exchanges=2 renegotiation=1 renegotiate=1" \ @@ -7049,7 +7049,7 @@ run_test "SSL async private: renegotiation: server-initiated; sign" \ requires_config_enabled MBEDTLS_SSL_ASYNC_PRIVATE requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -run_test "SSL async private: renegotiation: client-initiated; decrypt" \ +run_test "SSL async private: renegotiation: client-initiated, decrypt" \ "$P_SRV \ async_operations=d async_private_delay1=1 async_private_delay2=1 \ exchanges=2 renegotiation=1" \ @@ -7061,7 +7061,7 @@ run_test "SSL async private: renegotiation: client-initiated; decrypt" \ requires_config_enabled MBEDTLS_SSL_ASYNC_PRIVATE requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -run_test "SSL async private: renegotiation: server-initiated; decrypt" \ +run_test "SSL async private: renegotiation: server-initiated, decrypt" \ "$P_SRV \ async_operations=d async_private_delay1=1 async_private_delay2=1 \ exchanges=2 renegotiation=1 renegotiate=1" \ From 9004a1768b1656dce1fe5871633a4c04b03be1ff Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:20:36 +0200 Subject: [PATCH 08/26] Set meaningful test configuration names when running tests Set MBEDTLS_TEST_PLATFORM and MBEDTLS_TEST_CONFIGURATION to meaningful values in all.sh. These environment variables are used when writing an outcome file, which happens if MBEDTLS_TEST_OUTCOME_FILE is also set. When running one of the try-multiple-configuration scripts, set MBEDTLS_TEST_CONFIGURATION to a value that uniquely describes the configuration. --- tests/scripts/all.sh | 4 ++++ tests/scripts/curves.pl | 1 + tests/scripts/depends-hashes.pl | 1 + tests/scripts/depends-pkalgs.pl | 1 + tests/scripts/key-exchanges.pl | 1 + tests/scripts/test-ref-configs.pl | 1 + 6 files changed, 9 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c7bf4281d..a97189a90 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -121,6 +121,9 @@ pre_initialize_variables () { FORCE=0 KEEP_GOING=0 + : ${MBEDTLS_TEST_PLATFORM="$(uname -s | tr -c \\n0-9A-Za-z _)-$(uname -m | tr -c \\n0-9A-Za-z _)"} + export MBEDTLS_TEST_PLATFORM + # Default commands, can be overridden by the environment : ${OPENSSL:="openssl"} : ${OPENSSL_LEGACY:="$OPENSSL"} @@ -1424,6 +1427,7 @@ run_component () { # The cleanup function will restore it. cp -p "$CONFIG_H" "$CONFIG_BAK" current_component="$1" + export MBEDTLS_TEST_CONFIGURATION="$current_component" "$@" cleanup } diff --git a/tests/scripts/curves.pl b/tests/scripts/curves.pl index 4791d5521..ebe49c51b 100755 --- a/tests/scripts/curves.pl +++ b/tests/scripts/curves.pl @@ -51,6 +51,7 @@ for my $curve (@curves) { print "\n******************************************\n"; print "* Testing without curve: $curve\n"; print "******************************************\n"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = "-$curve"; system( "scripts/config.pl unset $curve" ) and abort "Failed to disable $curve\n"; diff --git a/tests/scripts/depends-hashes.pl b/tests/scripts/depends-hashes.pl index f57e7ed88..5dfc255ce 100755 --- a/tests/scripts/depends-hashes.pl +++ b/tests/scripts/depends-hashes.pl @@ -57,6 +57,7 @@ for my $hash (@hashes) { print "\n******************************************\n"; print "* Testing without hash: $hash\n"; print "******************************************\n"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = "-$hash"; system( "scripts/config.pl unset $hash" ) and abort "Failed to disable $hash\n"; diff --git a/tests/scripts/depends-pkalgs.pl b/tests/scripts/depends-pkalgs.pl index 97a43e881..ce6f3d5e6 100755 --- a/tests/scripts/depends-pkalgs.pl +++ b/tests/scripts/depends-pkalgs.pl @@ -72,6 +72,7 @@ while( my ($alg, $extras) = each %algs ) { print "\n******************************************\n"; print "* Testing without alg: $alg\n"; print "******************************************\n"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = "-$alg"; system( "scripts/config.pl unset $alg" ) and abort "Failed to disable $alg\n"; diff --git a/tests/scripts/key-exchanges.pl b/tests/scripts/key-exchanges.pl index 3bf7ae34f..bfde0693a 100755 --- a/tests/scripts/key-exchanges.pl +++ b/tests/scripts/key-exchanges.pl @@ -45,6 +45,7 @@ for my $kex (@kexes) { print "\n******************************************\n"; print "* Testing with key exchange: $kex\n"; print "******************************************\n"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = "-$kex"; # full config with all key exchanges disabled except one system( "scripts/config.pl full" ) and abort "Failed config full\n"; diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index a434b46b2..5e110113b 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -65,6 +65,7 @@ while( my ($conf, $data) = each %configs ) { print "\n******************************************\n"; print "* Testing configuration: $conf\n"; print "******************************************\n"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = "$conf"; system( "cp configs/$conf $config_h" ) and abort "Failed to activate $conf\n"; From 67ffdafde6e17f345e1a8560971bd6d7ce018c24 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 15:55:46 +0200 Subject: [PATCH 09/26] all.sh --outcome-file creates an outcome file By default, remove the outcome file before starting. With --append-outcome, append to the existing outcome file if there is one. --- tests/scripts/all.sh | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index a97189a90..4d31b0ecd 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -117,11 +117,14 @@ pre_initialize_variables () { CONFIG_H='include/mbedtls/config.h' CONFIG_BAK="$CONFIG_H.bak" + append_outcome=0 MEMORY=0 FORCE=0 KEEP_GOING=0 + : ${MBEDTLS_TEST_OUTCOME_FILE=} : ${MBEDTLS_TEST_PLATFORM="$(uname -s | tr -c \\n0-9A-Za-z _)-$(uname -m | tr -c \\n0-9A-Za-z _)"} + export MBEDTLS_TEST_OUTCOME_FILE export MBEDTLS_TEST_PLATFORM # Default commands, can be overridden by the environment @@ -193,14 +196,18 @@ General options: -f|--force Force the tests to overwrite any modified files. -k|--keep-going Run all tests and report errors at the end. -m|--memory Additional optional memory tests. + --append-outcome Append to the outcome file (if used). --armcc Run ARM Compiler builds (on by default). --except Exclude the COMPONENTs listed on the command line, instead of running only those. + --no-append-outcome Write a new outcome file and analyze it (default). --no-armcc Skip ARM Compiler builds. --no-force Refuse to overwrite modified files (default). --no-keep-going Stop at the first error (default). --no-memory No additional memory tests (default). --out-of-source-dir= Directory used for CMake out-of-source build tests. + --outcome-file= File where test outcomes are written (not done if + empty; default: \$MBEDTLS_TEST_OUTCOME_FILE). --random-seed Use a random seed value for randomized tests (default). -r|--release-test Run this script in release mode. This fixes the seed value to 1. -s|--seed Integer seed value to use for this test run. @@ -326,6 +333,7 @@ pre_parse_command_line () { while [ $# -gt 0 ]; do case "$1" in + --append-outcome) append_outcome=1;; --armcc) no_armcc=;; --armc5-bin-dir) shift; ARMC5_BIN_DIR="$1";; --armc6-bin-dir) shift; ARMC6_BIN_DIR="$1";; @@ -340,6 +348,7 @@ pre_parse_command_line () { --list-all-components) printf '%s\n' $ALL_COMPONENTS; exit;; --list-components) printf '%s\n' $SUPPORTED_COMPONENTS; exit;; --memory|-m) MEMORY=1;; + --no-append-outcome) append_outcome=0;; --no-armcc) no_armcc=1;; --no-force) FORCE=0;; --no-keep-going) KEEP_GOING=0;; @@ -347,6 +356,7 @@ pre_parse_command_line () { --openssl) shift; OPENSSL="$1";; --openssl-legacy) shift; OPENSSL_LEGACY="$1";; --openssl-next) shift; OPENSSL_NEXT="$1";; + --outcome-file) shift; MBEDTLS_TEST_OUTCOME_FILE="$1";; --out-of-source-dir) shift; OUT_OF_SOURCE_DIR="$1";; --random-seed) unset SEED;; --release-test|-r) SEED=1;; @@ -488,11 +498,22 @@ not() { ! "$@" } +pre_prepare_outcome_file () { + case "$MBEDTLS_TEST_OUTCOME_FILE" in + [!/]*) MBEDTLS_TEST_OUTCOME_FILE="$PWD/$MBEDTLS_TEST_OUTCOME_FILE";; + esac + if [ -n "$MBEDTLS_TEST_OUTCOME_FILE" ] && [ "$append_outcome" -eq 0 ]; then + rm -f "$MBEDTLS_TEST_OUTCOME_FILE" + fi +} + pre_print_configuration () { msg "info: $0 configuration" echo "MEMORY: $MEMORY" echo "FORCE: $FORCE" + echo "MBEDTLS_TEST_OUTCOME_FILE: ${MBEDTLS_TEST_OUTCOME_FILE:-(none)}" echo "SEED: ${SEED-"UNSET"}" + echo echo "OPENSSL: $OPENSSL" echo "OPENSSL_LEGACY: $OPENSSL_LEGACY" echo "OPENSSL_NEXT: $OPENSSL_NEXT" @@ -640,6 +661,8 @@ component_test_large_ecdsa_key_signature () { component_test_default_out_of_box () { msg "build: make, default config (out-of-box)" # ~1min make + # Disable fancy stuff + unset MBEDTLS_TEST_OUTCOME_FILE msg "test: main suites make, default config (out-of-box)" # ~10s make test @@ -1448,6 +1471,7 @@ else "$@" } fi +pre_prepare_outcome_file pre_print_configuration pre_check_tools cleanup From d46b0869f4ab2e1428eed347a652b969e3111b03 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 16:06:06 +0200 Subject: [PATCH 10/26] Create infrastructure for architecture documents in Markdown --- docs/architecture/.gitignore | 2 ++ docs/architecture/Makefile | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 docs/architecture/.gitignore create mode 100644 docs/architecture/Makefile diff --git a/docs/architecture/.gitignore b/docs/architecture/.gitignore new file mode 100644 index 000000000..23f832b73 --- /dev/null +++ b/docs/architecture/.gitignore @@ -0,0 +1,2 @@ +*.html +*.pdf diff --git a/docs/architecture/Makefile b/docs/architecture/Makefile new file mode 100644 index 000000000..4873daeb8 --- /dev/null +++ b/docs/architecture/Makefile @@ -0,0 +1,18 @@ +PANDOC = pandoc + +default: all + +all_markdown = \ + # This line is intentionally left blank + +html: $(all_markdown:.md=.html) +pdf: $(all_markdown:.md=.pdf) +all: html pdf + +.SUFFIXES: +.SUFFIXES: .md .html .pdf + +.md.html: + $(PANDOC) -o $@ $< +.md.pdf: + $(PANDOC) -o $@ $< From 508caf528ac3504f4757271c1d12dfb2d4a3967d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 16:29:15 +0200 Subject: [PATCH 11/26] Document the test outcome file --- docs/architecture/Makefile | 1 + docs/architecture/testing/test-framework.md | 28 +++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 docs/architecture/testing/test-framework.md diff --git a/docs/architecture/Makefile b/docs/architecture/Makefile index 4873daeb8..2b2a11b68 100644 --- a/docs/architecture/Makefile +++ b/docs/architecture/Makefile @@ -3,6 +3,7 @@ PANDOC = pandoc default: all all_markdown = \ + testing/test-framework.md \ # This line is intentionally left blank html: $(all_markdown:.md=.html) diff --git a/docs/architecture/testing/test-framework.md b/docs/architecture/testing/test-framework.md new file mode 100644 index 000000000..270a3f9be --- /dev/null +++ b/docs/architecture/testing/test-framework.md @@ -0,0 +1,28 @@ +# Mbed TLS test framework + +This document is an overview of the Mbed TLS test framework and test tools. + +This document is incomplete. You can help by expanding it. + +## Running tests + +### Outcome file + +#### Generating an outcome file + +Unit tests and `ssl-opt.sh` record the outcome of each test case in a **test outcome file**. This feature is enabled if the environment variable `MBEDTLS_TEST_OUTCOME_FILE` is set. Set it to the path of the desired file. + +If you run `all.sh --outcome-file test-outcome.csv`, this collects the outcome of all the test cases in `test-outcome.csv`. + +#### Outcome file format + +The outcome file is in a CSV format using `;` (semicolon) as the delimiter and no quoting. This means that fields may not contain newlines or semicolons. There is no title line. + +The outcome file has 6 fields: + +* **Platform**: a description of the platform, e.g. `Linux-x86_64` or `Linux-x86_64-gcc7-msan`. +* **Configuration**: a unique description of the configuration (`config.h`). +* **Test suite**: `test_suite_xxx` or `ssl-opt`. +* **Test case**: the description of the test case. +* **Result**: one of `PASS`, `SKIP` or `FAIL`. +* **Cause**: more information explaining the result. From ba94b5812792c79e31f7e9311a35a98eb5d8189c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 19:18:40 +0200 Subject: [PATCH 12/26] New test script check-test-cases.py This script checks test case descriptions in test_suite_*.data and ssl-opt.sh. It reports the following issues: * Error: forbidden character in a test case description. * Error: Duplicate test description. * Warning: Test description is too long. --- tests/scripts/check-test-cases.py | 128 ++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100755 tests/scripts/check-test-cases.py diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py new file mode 100755 index 000000000..688ad25c9 --- /dev/null +++ b/tests/scripts/check-test-cases.py @@ -0,0 +1,128 @@ +#!/usr/bin/env python3 + +"""Sanity checks for test data. +""" + +# Copyright (C) 2019, Arm Limited, All Rights Reserved +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# This file is part of Mbed TLS (https://tls.mbed.org) + +import glob +import os +import re +import sys + +class Results: + def __init__(self): + self.errors = 0 + self.warnings = 0 + + def error(self, file_name, line_number, fmt, *args): + sys.stderr.write(('{}:{}:ERROR:' + fmt + '\n'). + format(file_name, line_number, *args)) + self.errors += 1 + + def warning(self, file_name, line_number, fmt, *args): + sys.stderr.write(('{}:{}:Warning:' + fmt + '\n') + .format(file_name, line_number, args)) + self.warnings += 1 + +def collect_test_directories(): + if os.path.isdir('tests'): + tests_dir = 'tests' + elif os.path.isdir('suites'): + tests_dir = '.' + elif os.path.isdir('../suites'): + tests_dir = '..' + directories = [tests_dir] + crypto_tests_dir = os.path.normpath(os.path.join(tests_dir, + '../crypto/tests')) + if os.path.isdir(crypto_tests_dir): + directories.append(crypto_tests_dir) + return directories + +def check_test_suite(results, data_file_name): + in_paragraph = False + descriptions = {} + line_number = 0 + with open(data_file_name) as data_file: + for line in data_file: + line_number += 1 + line = line.rstrip('\r\n') + if not line: + in_paragraph = False + continue + if line.startswith('#'): + continue + if not in_paragraph: + # This is a test case description line. + if line in descriptions: + results.error(data_file_name, line_number, + 'Duplicate description (also line {}): {}', + descriptions[line], line) + else: + if re.search(r'[\t;]', line): + results.error(data_file_name, line_number, + 'Forbidden character in description') + if len(line) > 66: + results.warning(data_file_name, line_number, + 'Test description will be truncated') + descriptions[line] = line_number + in_paragraph = True + +def check_ssl_opt_sh(results, file_name): + descriptions = {} + line_number = 0 + with open(file_name) as file_contents: + for line in file_contents: + line_number += 1 + # Assume that all run_test calls have the same simple form + # with the test description entirely on the same line as the + # function name. + m = re.match(r'\s+run_test\s+"([^"])"', line) + if not m: + continue + description = m.group(1) + if description in descriptions: + results.error(data_file_name, line_number, + 'Duplicate description (also line {}): {}', + descriptions[line], line) + else: + if re.search(r'[\t;]', line): + results.error(data_file_name, line_number, + 'Forbidden character in description') + if len(line) > 66: + results.warning(data_file_name, line_number, + 'Test description will break visual alignment') + descriptions[line] = line_number + +def main(): + test_directories = collect_test_directories() + results = Results() + for directory in test_directories: + for data_file_name in glob.glob(os.path.join(directory, 'suites', + '*.data')): + check_test_suite(results, data_file_name) + ssl_opt_sh = os.path.join(directory, 'ssl-opt.sh') + if os.path.exists(ssl_opt_sh): + check_ssl_opt_sh(results, ssl_opt_sh) + if results.warnings or results.errors: + sys.stderr.write('{}: {} errors, {} warnings\n' + .format(sys.argv[0], results.errors, results.warnings)) + sys.exit(1 if results.errors else 0) + +if __name__ == '__main__': + main() From 7a020f3d10d665bf3128d782358ca654f444a6a2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Sep 2019 19:54:10 +0200 Subject: [PATCH 13/26] Make test case descriptions unique Remove one test case which was an exact duplicate. Tweak the description of two test cases that had the same description. --- tests/suites/test_suite_x509parse.data | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index ce49ff0fc..775e17e09 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -611,11 +611,11 @@ X509 CRT verification #26 (domain not matching multi certificate) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_SHA256_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15 x509_verify:"data_files/cert_example_multi.crt":"data_files/test-ca.crt":"data_files/crl.pem":"www.example.net":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"compat":"NULL" -X509 CRT verification #27 (domain not matching multi certificate) +X509 CRT verification #27.1 (domain not matching multi certificate: suffix) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_SHA256_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15 x509_verify:"data_files/cert_example_multi.crt":"data_files/test-ca.crt":"data_files/crl.pem":"xample.net":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"compat":"NULL" -X509 CRT verification #27 (domain not matching multi certificate) +X509 CRT verification #27.2 (domain not matching multi certificate: head junk) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_SHA256_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15 x509_verify:"data_files/cert_example_multi.crt":"data_files/test-ca.crt":"data_files/crl.pem":"bexample.net":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"compat":"NULL" @@ -1292,10 +1292,6 @@ X509 CRT ASN1 (TBS, inv Validity, notBefore length out of bounds) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"307b3066a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a300806000c045465737430021701300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_DATE + MBEDTLS_ERR_ASN1_OUT_OF_DATA -X509 CRT ASN1 (TBS, inv Validity, notBefore length out of bounds) -depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C -x509parse_crt:"307b3066a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a300806000c045465737430021701300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_DATE + MBEDTLS_ERR_ASN1_OUT_OF_DATA - X509 CRT ASN1 (TBS, inv Validity, notBefore empty) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt:"3081893074a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a3008060013045465737430101700170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210ffffffffffffffffffffffffffffffff0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_DATE From a9478bab08f97400854c387fba3af8b7c958a225 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 18 Sep 2019 18:45:35 +0200 Subject: [PATCH 14/26] Fix configuration short name in key-exchanges.pl This is testing with $kex, not without $kex, so use $kex, not "-$kex". In test-ref-configs.pl, use $conf rather than "$conf". This is purely a matter of Perl coding style. --- tests/scripts/key-exchanges.pl | 2 +- tests/scripts/test-ref-configs.pl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/key-exchanges.pl b/tests/scripts/key-exchanges.pl index bfde0693a..9cfa2d398 100755 --- a/tests/scripts/key-exchanges.pl +++ b/tests/scripts/key-exchanges.pl @@ -45,7 +45,7 @@ for my $kex (@kexes) { print "\n******************************************\n"; print "* Testing with key exchange: $kex\n"; print "******************************************\n"; - $ENV{MBEDTLS_TEST_CONFIGURATION} = "-$kex"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = $kex; # full config with all key exchanges disabled except one system( "scripts/config.pl full" ) and abort "Failed config full\n"; diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index 5e110113b..1df84f74f 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -65,7 +65,7 @@ while( my ($conf, $data) = each %configs ) { print "\n******************************************\n"; print "* Testing configuration: $conf\n"; print "******************************************\n"; - $ENV{MBEDTLS_TEST_CONFIGURATION} = "$conf"; + $ENV{MBEDTLS_TEST_CONFIGURATION} = $conf; system( "cp configs/$conf $config_h" ) and abort "Failed to activate $conf\n"; From 600bb694acb607cfa4af4be3f0873cdd99fa228b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 19 Sep 2019 21:29:11 +0200 Subject: [PATCH 15/26] Better information messages for quick checks Call them "check" rather than "test" to distinguish them from tests that build and run code, and for consistency with the component names. --- tests/scripts/all.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 4d31b0ecd..6d7edd6bb 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -606,32 +606,32 @@ pre_check_tools () { # Indicative running times are given for reference. component_check_recursion () { - msg "test: recursion.pl" # < 1s + msg "Check: recursion.pl" # < 1s record_status tests/scripts/recursion.pl library/*.c } component_check_generated_files () { - msg "test: freshness of generated source files" # < 1s + msg "Check: freshness of generated source files" # < 1s record_status tests/scripts/check-generated-files.sh } component_check_doxy_blocks () { - msg "test: doxygen markup outside doxygen blocks" # < 1s + msg "Check: doxygen markup outside doxygen blocks" # < 1s record_status tests/scripts/check-doxy-blocks.pl } component_check_files () { - msg "test: check-files.py" # < 1s + msg "Check: file sanity checks (permissions, encodings)" # < 1s record_status tests/scripts/check-files.py } component_check_names () { - msg "test/build: declared and exported names" # < 3s + msg "Check: declared and exported names (builds the library)" # < 3s record_status tests/scripts/check-names.sh -v } component_check_doxygen_warnings () { - msg "test: doxygen warnings" # ~ 3s + msg "Check: doxygen warnings (builds the documentation)" # ~ 3s record_status tests/scripts/doxygen.sh } From 895868bc82f62646349c1d49aab5cf0c165793ef Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 19 Sep 2019 21:30:05 +0200 Subject: [PATCH 16/26] all.sh: run check-test-cases.py --- tests/scripts/all.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6d7edd6bb..f32086667 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -630,6 +630,11 @@ component_check_names () { record_status tests/scripts/check-names.sh -v } +component_check_test_cases () { + msg "Check: test case descriptions" # < 1s + record_status tests/scripts/check-test-cases.py +} + component_check_doxygen_warnings () { msg "Check: doxygen warnings (builds the documentation)" # ~ 3s record_status tests/scripts/doxygen.sh From 168858f52d72eccad6873dfc6066bffd4b058398 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 20 Sep 2019 17:54:45 +0200 Subject: [PATCH 17/26] Fix regex matching run_test calls in ssl-opt.sh No descriptions were processed before due to bugs in the regex. Support \" inside double-quoted strings. --- tests/scripts/check-test-cases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py index 688ad25c9..9e67a4207 100755 --- a/tests/scripts/check-test-cases.py +++ b/tests/scripts/check-test-cases.py @@ -92,7 +92,7 @@ def check_ssl_opt_sh(results, file_name): # Assume that all run_test calls have the same simple form # with the test description entirely on the same line as the # function name. - m = re.match(r'\s+run_test\s+"([^"])"', line) + m = re.match(r'\s*run_test\s+"((?:[^\\"]|\\.)*)"', line) if not m: continue description = m.group(1) From 283df2e90c37e332b70eb93be7cc92ddfe4f66c3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 20 Sep 2019 17:56:29 +0200 Subject: [PATCH 18/26] Fix cosmetic error in warnings --- tests/scripts/check-test-cases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py index 9e67a4207..17022bf04 100755 --- a/tests/scripts/check-test-cases.py +++ b/tests/scripts/check-test-cases.py @@ -37,7 +37,7 @@ class Results: def warning(self, file_name, line_number, fmt, *args): sys.stderr.write(('{}:{}:Warning:' + fmt + '\n') - .format(file_name, line_number, args)) + .format(file_name, line_number, *args)) self.warnings += 1 def collect_test_directories(): From 32b9421f1289fa16d5b96c4b9af59b1b09095e6d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 20 Sep 2019 18:00:49 +0200 Subject: [PATCH 19/26] Factor description-checking code into a common function Behavior change: some error messages are slightly different. --- tests/scripts/check-test-cases.py | 44 ++++++++++++++----------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py index 17022bf04..b1b78e3e1 100755 --- a/tests/scripts/check-test-cases.py +++ b/tests/scripts/check-test-cases.py @@ -54,6 +54,22 @@ def collect_test_directories(): directories.append(crypto_tests_dir) return directories +def check_description(results, seen, file_name, line_number, description): + if description in seen: + results.error(file_name, line_number, + 'Duplicate description (also line {})', + seen[description]) + return + if re.search(r'[\t;]', description): + results.error(file_name, line_number, + 'Forbidden character \'{}\' in description', + re.search(r'[\t;]', description).group(0)) + if len(description) > 66: + results.warning(file_name, line_number, + 'Test description too long ({} > 66)', + len(description)) + seen[description] = line_number + def check_test_suite(results, data_file_name): in_paragraph = False descriptions = {} @@ -69,18 +85,8 @@ def check_test_suite(results, data_file_name): continue if not in_paragraph: # This is a test case description line. - if line in descriptions: - results.error(data_file_name, line_number, - 'Duplicate description (also line {}): {}', - descriptions[line], line) - else: - if re.search(r'[\t;]', line): - results.error(data_file_name, line_number, - 'Forbidden character in description') - if len(line) > 66: - results.warning(data_file_name, line_number, - 'Test description will be truncated') - descriptions[line] = line_number + check_description(results, descriptions, + data_file_name, line_number, line) in_paragraph = True def check_ssl_opt_sh(results, file_name): @@ -96,18 +102,8 @@ def check_ssl_opt_sh(results, file_name): if not m: continue description = m.group(1) - if description in descriptions: - results.error(data_file_name, line_number, - 'Duplicate description (also line {}): {}', - descriptions[line], line) - else: - if re.search(r'[\t;]', line): - results.error(data_file_name, line_number, - 'Forbidden character in description') - if len(line) > 66: - results.warning(data_file_name, line_number, - 'Test description will break visual alignment') - descriptions[line] = line_number + check_description(results, descriptions, + file_name, line_number, description) def main(): test_directories = collect_test_directories() From f12ad58a1d445a053c3711ebed2d7f6b061fb72f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 20 Sep 2019 18:02:01 +0200 Subject: [PATCH 20/26] Process input files as binary Don't die if there's a non-ASCII character and we're running in an ASCII environment. --- tests/scripts/check-test-cases.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py index b1b78e3e1..8a37a9702 100755 --- a/tests/scripts/check-test-cases.py +++ b/tests/scripts/check-test-cases.py @@ -60,10 +60,10 @@ def check_description(results, seen, file_name, line_number, description): 'Duplicate description (also line {})', seen[description]) return - if re.search(r'[\t;]', description): + if re.search(br'[\t;]', description): results.error(file_name, line_number, 'Forbidden character \'{}\' in description', - re.search(r'[\t;]', description).group(0)) + re.search(br'[\t;]', description).group(0).decode('ascii')) if len(description) > 66: results.warning(file_name, line_number, 'Test description too long ({} > 66)', @@ -73,15 +73,13 @@ def check_description(results, seen, file_name, line_number, description): def check_test_suite(results, data_file_name): in_paragraph = False descriptions = {} - line_number = 0 - with open(data_file_name) as data_file: - for line in data_file: - line_number += 1 - line = line.rstrip('\r\n') + with open(data_file_name, 'rb') as data_file: + for line_number, line in enumerate(data_file, 1): + line = line.rstrip(b'\r\n') if not line: in_paragraph = False continue - if line.startswith('#'): + if line.startswith(b'#'): continue if not in_paragraph: # This is a test case description line. @@ -91,14 +89,12 @@ def check_test_suite(results, data_file_name): def check_ssl_opt_sh(results, file_name): descriptions = {} - line_number = 0 - with open(file_name) as file_contents: - for line in file_contents: - line_number += 1 + with open(file_name, 'rb') as file_contents: + for line_number, line in enumerate(file_contents, 1): # Assume that all run_test calls have the same simple form # with the test description entirely on the same line as the # function name. - m = re.match(r'\s*run_test\s+"((?:[^\\"]|\\.)*)"', line) + m = re.match(br'\s*run_test\s+"((?:[^\\"]|\\.)*)"', line) if not m: continue description = m.group(1) From 57870e8c676c74366f58f5ec1beafd37aaafac8e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 20 Sep 2019 18:02:30 +0200 Subject: [PATCH 21/26] Reject non-ASCII characters in test case descriptions Don't require that all the tools we use to process test outcomes are Unicode-clean. --- tests/scripts/check-test-cases.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py index 8a37a9702..87a35e47e 100755 --- a/tests/scripts/check-test-cases.py +++ b/tests/scripts/check-test-cases.py @@ -64,6 +64,9 @@ def check_description(results, seen, file_name, line_number, description): results.error(file_name, line_number, 'Forbidden character \'{}\' in description', re.search(br'[\t;]', description).group(0).decode('ascii')) + if re.search(br'[^ -~]', description): + results.error(file_name, line_number, + 'Non-ASCII character in description') if len(description) > 66: results.warning(file_name, line_number, 'Test description too long ({} > 66)', From 0d8b86a1313e23e52b202507b8fab58bbc8b1912 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 20 Sep 2019 18:03:11 +0200 Subject: [PATCH 22/26] ssl-opt.sh: Fix some test case descriptions Fix copypasta in some test cases with MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES enabled. Add unique suffix to the two "DTLS fragmenting: proxy MTU: auto-reduction" test cases. --- tests/ssl-opt.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 625fa349d..5ea0a678f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1162,7 +1162,7 @@ run_test "SHA-1 forbidden by default in server certificate" \ -c "The certificate is signed with an unacceptable hash" requires_config_enabled MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES -run_test "SHA-1 forbidden by default in server certificate" \ +run_test "SHA-1 allowed by default in server certificate" \ "$P_SRV key_file=data_files/server2.key crt_file=data_files/server2.crt" \ "$P_CLI debug_level=2 allow_sha1=0" \ 0 @@ -1185,7 +1185,7 @@ run_test "SHA-1 forbidden by default in client certificate" \ -s "The certificate is signed with an unacceptable hash" requires_config_enabled MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES -run_test "SHA-1 forbidden by default in client certificate" \ +run_test "SHA-1 allowed by default in client certificate" \ "$P_SRV auth_mode=required allow_sha1=0" \ "$P_CLI key_file=data_files/cli-rsa.key crt_file=data_files/cli-rsa-sha1.crt" \ 0 @@ -7629,7 +7629,7 @@ requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C -run_test "DTLS fragmenting: proxy MTU: auto-reduction" \ +run_test "DTLS fragmenting: proxy MTU: auto-reduction (not valgrind)" \ -p "$P_PXY mtu=508" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ @@ -7653,7 +7653,7 @@ requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA requires_config_enabled MBEDTLS_AES_C requires_config_enabled MBEDTLS_GCM_C -run_test "DTLS fragmenting: proxy MTU: auto-reduction" \ +run_test "DTLS fragmenting: proxy MTU: auto-reduction (with valgrind)" \ -p "$P_PXY mtu=508" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ From 717cd76e8a3b1464397df95397f8454ca89e85cf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 27 Sep 2019 20:21:11 +0200 Subject: [PATCH 23/26] Restore MBEDTLS_TEST_OUTCOME_FILE after test_default_out_of_box Since components run in the main process, unsetting MBEDTLS_TEST_OUTCOME_FILE unset it in subsequent components as well. To avoid this, save and restore the value. (Making each component run in a subshell would be a better solution, but it would be a much bigger change.) --- tests/scripts/all.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index f32086667..1ee4eb150 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -667,6 +667,7 @@ component_test_default_out_of_box () { msg "build: make, default config (out-of-box)" # ~1min make # Disable fancy stuff + SAVE_MBEDTLS_TEST_OUTCOME_FILE="$MBEDTLS_TEST_OUTCOME_FILE" unset MBEDTLS_TEST_OUTCOME_FILE msg "test: main suites make, default config (out-of-box)" # ~10s @@ -674,6 +675,9 @@ component_test_default_out_of_box () { msg "selftest: make, default config (out-of-box)" # ~10s programs/test/selftest + + export MBEDTLS_TEST_OUTCOME_FILE="$SAVE_MBEDTLS_TEST_OUTCOME_FILE" + unset SAVE_MBEDTLS_TEST_OUTCOME_FILE } component_test_default_cmake_gcc_asan () { From e94bc87ebe8104d24d23b60487e4c5a7e480ea29 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 27 Sep 2019 20:22:41 +0200 Subject: [PATCH 24/26] Document test case descriptions --- docs/architecture/testing/test-framework.md | 30 +++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/architecture/testing/test-framework.md b/docs/architecture/testing/test-framework.md index 270a3f9be..dd2968e0f 100644 --- a/docs/architecture/testing/test-framework.md +++ b/docs/architecture/testing/test-framework.md @@ -4,6 +4,36 @@ This document is an overview of the Mbed TLS test framework and test tools. This document is incomplete. You can help by expanding it. +## Unit tests + +See https://tls.mbed.org/kb/development/test_suites + +### Unit test descriptions + +Each test case has a description which succinctly describes for a human audience what the test does. The first non-comment line of each paragraph in a `.data` file is the test description. The following rules and guidelines apply: + +* Test descriptions may not contain semicolons, line breaks and other control characters, or non-ASCII characters.
+ Rationale: keep the tools that process test descriptions (`generate_test_code.py`, [outcome file](#outcome-file) tools) simple. +* Test descriptions must be unique within a `.data` file. If you can't think of a better description, the convention is to append `#1`, `#2`, etc.
+ Rationale: make it easy to relate a failure log to the test data. Avoid confusion between cases in the [outcome file](#outcome-file). +* Test descriptions should be a maximum of **66 characters**.
+ Rationale: 66 characters is what our various tools assume (leaving room for 14 more characters on an 80-column line). Longer descriptions may be truncated or may break a visual alignment.
+ We have a lot of test cases with longer descriptions, but they should be avoided. At least please make sure that the first 66 characters describe the test uniquely. +* Make the description descriptive. “foo: x=2, y=4” is more descriptive than “foo #2”. “foo: 0 Date: Fri, 27 Sep 2019 20:33:33 +0200 Subject: [PATCH 25/26] Update the crypto submodule to be the same as development --- crypto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto b/crypto index 3f20efc03..37b5c831b 160000 --- a/crypto +++ b/crypto @@ -1 +1 @@ -Subproject commit 3f20efc03016b38f2677dadd476b21229c627c80 +Subproject commit 37b5c831b41cd41456caa979f1444234c51e4c51 From 73344622782f4396862df34934c243ad49c2d158 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 1 Oct 2019 10:36:10 +0200 Subject: [PATCH 26/26] Make hyperlink a hyperlink in every markdown flavor --- docs/architecture/testing/test-framework.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/testing/test-framework.md b/docs/architecture/testing/test-framework.md index dd2968e0f..e0e960f87 100644 --- a/docs/architecture/testing/test-framework.md +++ b/docs/architecture/testing/test-framework.md @@ -6,7 +6,7 @@ This document is incomplete. You can help by expanding it. ## Unit tests -See https://tls.mbed.org/kb/development/test_suites +See ### Unit test descriptions