diff --git a/docs/architecture/psa-migration/outcome-analysis.sh b/docs/architecture/psa-migration/outcome-analysis.sh index 9084685482..b26963b90f 100755 --- a/docs/architecture/psa-migration/outcome-analysis.sh +++ b/docs/architecture/psa-migration/outcome-analysis.sh @@ -1,42 +1,30 @@ #!/bin/sh -# This script runs tests in various revisions and configurations and analyses -# the results in order to highlight any difference in the set of tests skipped -# in the test suites of interest. +# This script runs tests before and after a PR and analyzes the results in +# order to highlight any difference in the set of tests skipped. # -# It can be used to ensure the testing criteria mentioned in strategy.md, +# It can be used to check the first testing criterion mentioned in strategy.md, # end of section "Supporting builds with drivers without the software -# implementation" are met, namely: +# implementation", namely: the sets of tests skipped in the default config and +# the full config must be the same before and after the PR. # -# - the sets of tests skipped in the default config and the full config must be -# the same before and after the PR that implements step 3; -# - the set of tests skipped in the driver-only build is the same as in an -# equivalent software-based configuration, or the difference is small enough, -# justified, and a github issue is created to track it. -# This part is verified by tests/scripts/analyze_outcomes.py +# USAGE: +# - First, commit any uncommited changes. (Also, see warning below.) +# - Then launch --> [SKIP_SSL_OPT=1] docs/architecture/psa-migration/outcome-analysis.sh +# - SKIP_SSL_OPT=1 can optionally be set to skip ssl-opt.sh tests # # WARNING: this script checks out a commit other than the head of the current # branch; it checks out the current branch again when running successfully, # but while the script is running, or if it terminates early in error, you # should be aware that you might be at a different commit than expected. # -# NOTE: This is only an example/template script, you should make a copy and -# edit it to suit your needs. The part that needs editing is at the top. -# -# Also, you can comment out parts that don't need to be re-done when +# NOTE: you can comment out parts that don't need to be re-done when # re-running this script (for example "get numbers before this PR"). -# ----- BEGIN edit this ----- -# Space-separated list of test suites to ignore: -# if SSS is in that list, test_suite_SSS and test_suite_SSS.* are ignored. -IGNORE="md mdx shax" # accelerated -IGNORE="$IGNORE entropy hmac_drbg random" # disabled (ext. RNG) -IGNORE="$IGNORE psa_crypto_init" # needs internal RNG -IGNORE="$IGNORE hkdf" # disabled in the all.sh component tested -# ----- END edit this ----- - set -eu +: ${SKIP_SSL_OPT:=0} + cleanup() { make clean git checkout -- include/mbedtls/mbedtls_config.h include/psa/crypto_config.h @@ -45,7 +33,14 @@ cleanup() { record() { export MBEDTLS_TEST_OUTCOME_FILE="$PWD/outcome-$1.csv" rm -f $MBEDTLS_TEST_OUTCOME_FILE + make check + + if [ $SKIP_SSL_OPT -eq 0 ]; then + make -C programs ssl/ssl_server2 ssl/ssl_client2 \ + test/udp_proxy test/query_compile_time_config + tests/ssl-opt.sh + fi } # save current HEAD @@ -54,21 +49,26 @@ HEAD=$(git branch --show-current) # get the numbers before this PR for default and full cleanup git checkout $(git merge-base HEAD development) + record "before-default" cleanup + scripts/config.py full record "before-full" # get the numbers now for default and full cleanup git checkout $HEAD + record "after-default" cleanup + scripts/config.py full record "after-full" +cleanup # analysis @@ -77,15 +77,19 @@ populate_suites () { make generated_files >/dev/null data_files=$(cd tests/suites && echo *.data) for data in $data_files; do - suite=${data#test_suite_} - suite=${suite%.data} - suite_base=${suite%%.*} - case " $IGNORE " in - *" $suite_base "*) :;; - *) SUITES="$SUITES $suite";; - esac + suite=${data%.data} + SUITES="$SUITES $suite" done make neat + + if [ $SKIP_SSL_OPT -eq 0 ]; then + SUITES="$SUITES ssl-opt" + extra_files=$(cd tests/opt-testcases && echo *.sh) + for extra in $extra_files; do + suite=${extra%.sh} + SUITES="$SUITES $suite" + done + fi } compare_suite () { @@ -93,7 +97,7 @@ compare_suite () { new="outcome-$2.csv" suite="$3" - pattern_suite=";test_suite_$suite;" + pattern_suite=";$suite;" total=$(grep -c "$pattern_suite" "$ref") sed_cmd="s/^.*$pattern_suite\(.*\);SKIP.*/\1/p" sed -n "$sed_cmd" "$ref" > skipped-ref @@ -101,8 +105,9 @@ compare_suite () { nb_ref=$(wc -l %4d\n" \ - $suite $total $nb_ref $nb_new + name=${suite#test_suite_} + printf "%40s: total %4d; skipped %4d -> %4d\n" \ + $name $total $nb_ref $nb_new if diff skipped-ref skipped-new | grep '^> '; then ret=1 else diff --git a/docs/architecture/psa-migration/strategy.md b/docs/architecture/psa-migration/strategy.md index 0ad5fa0a53..1542324746 100644 --- a/docs/architecture/psa-migration/strategy.md +++ b/docs/architecture/psa-migration/strategy.md @@ -386,15 +386,16 @@ are expressed (sometimes in bulk), to get things wrong in a way that would result in more tests being skipped, which is easy to miss. Care must be taken to ensure this does not happen. The following criteria can be used: -- the sets of tests skipped in the default config and the full config must be - the same before and after the PR that implements step 3; -- the set of tests skipped in the driver-only build is the same as in an - equivalent software-based configuration, or the difference is small enough, - justified, and a github issue is created to track it. - -Note that the favourable case is when the number of tests skipped is 0 in the -driver-only build. In other cases, analysis of the outcome files is needed, -see the example script `outcome-analysis.sh` in the same directory. +1. The sets of tests skipped in the default config and the full config must be + the same before and after the PR that implements step 3. This is tested +manually for each PR that changes dependency declarations by using the script +`outcome-analysis.sh` in the present directory. +2. The set of tests skipped in the driver-only build is the same as in an + equivalent software-based configuration. This is tested automatically by the +CI in the "Results analysis" stage, by running +`tests/scripts/analyze_outcomes.py`. See the +`analyze_driver_vs_reference_xxx` actions in the script and the comments above +their declaration for how to do that locally. Migrating away from the legacy API diff --git a/tests/Makefile b/tests/Makefile index f0373381ef..312607ee30 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -203,6 +203,7 @@ ifndef WINDOWS rm -f src/*.o src/drivers/*.o src/libmbed* rm -f include/test/instrument_record_status.h rm -rf libtestdriver1 + rm -f ../library/libtestdriver1.a else if exist *.c del /Q /F *.c if exist *.exe del /Q /F *.exe diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index bf40764921..2221d591b8 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -133,13 +133,14 @@ pre_check_environment () { pre_initialize_variables () { CONFIG_H='include/mbedtls/mbedtls_config.h' CRYPTO_CONFIG_H='include/psa/crypto_config.h' + CONFIG_TEST_DRIVER_H='tests/include/test/drivers/config_test_driver.h' # Files that are clobbered by some jobs will be backed up. Use a different # suffix from auxiliary scripts so that all.sh and auxiliary scripts can # independently decide when to remove the backup file. backup_suffix='.all.bak' # Files clobbered by config.py - files_to_back_up="$CONFIG_H $CRYPTO_CONFIG_H" + files_to_back_up="$CONFIG_H $CRYPTO_CONFIG_H $CONFIG_TEST_DRIVER_H" # Files clobbered by in-tree cmake files_to_back_up="$files_to_back_up Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile" @@ -2040,6 +2041,12 @@ component_test_no_use_psa_crypto_full_cmake_asan() { component_test_psa_crypto_config_accel_ecdsa () { msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA" + # Algorithms and key types to accelerate + loc_accel_list="ALG_ECDSA ALG_DETERMINISTIC_ECDSA KEY_TYPE_ECC_KEY_PAIR KEY_TYPE_ECC_PUBLIC_KEY" + + # Configure and build the test driver library + # ------------------------------------------- + # Disable ALG_STREAM_CIPHER and ALG_ECB_NO_PADDING to avoid having # partial support for cipher operations in the driver test library. scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_STREAM_CIPHER @@ -2050,32 +2057,121 @@ component_test_psa_crypto_config_accel_ecdsa () { scripts/config.py -f tests/include/test/drivers/config_test_driver.h set MBEDTLS_SHA384_C scripts/config.py -f tests/include/test/drivers/config_test_driver.h set MBEDTLS_SHA512_C - loc_accel_list="ALG_ECDSA ALG_DETERMINISTIC_ECDSA KEY_TYPE_ECC_KEY_PAIR KEY_TYPE_ECC_PUBLIC_KEY" loc_accel_flags=$( echo "$loc_accel_list" | sed 's/[^ ]* */-DLIBTESTDRIVER1_MBEDTLS_PSA_ACCEL_&/g' ) make -C tests libtestdriver1.a CFLAGS="$ASAN_CFLAGS $loc_accel_flags" LDFLAGS="$ASAN_CFLAGS" - # Restore test driver base configuration - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_SHA224_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_SHA384_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_SHA512_C + # Configure and build the test driver library + # ------------------------------------------- + # Start from default config (no USE_PSA) + driver support + TLS 1.3 scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG - scripts/config.py unset MBEDTLS_USE_PSA_CRYPTO - scripts/config.py unset MBEDTLS_SSL_PROTO_TLS1_3 + scripts/config.py set MBEDTLS_SSL_PROTO_TLS1_3 + + # Disable the module that's accelerated scripts/config.py unset MBEDTLS_ECDSA_C + + # Disable things that depend on it scripts/config.py unset MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED scripts/config.py unset MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED + # Build the library loc_accel_flags="$loc_accel_flags $( echo "$loc_accel_list" | sed 's/[^ ]* */-DMBEDTLS_PSA_ACCEL_&/g' )" make CFLAGS="$ASAN_CFLAGS -O -Werror -I../tests/include -I../tests -I../../tests -DPSA_CRYPTO_DRIVER_TEST -DMBEDTLS_TEST_LIBTESTDRIVER1 $loc_accel_flags" LDFLAGS="-ltestdriver1 $ASAN_CFLAGS" + # Make sure ECDSA was not re-enabled by accident (additive config) not grep mbedtls_ecdsa_ library/ecdsa.o + # Run the tests + # ------------- + msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA" make test } +# Auxiliary function to build config for hashes with and without drivers +config_psa_crypto_config_ecdsa_use_psa () { + DRIVER_ONLY="$1" + # start with config full for maximum coverage (also enables USE_PSA) + scripts/config.py full + # enable support for drivers and configuring PSA-only algorithms + scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG + scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS + if [ "$DRIVER_ONLY" -eq 1 ]; then + # Disable the module that's accelerated + scripts/config.py unset MBEDTLS_ECDSA_C + fi + # Disable things that depend on it + # TODO: make these work - #6862 + scripts/config.py unset MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED + scripts/config.py unset MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED +} + +# Keep in sync with component_test_psa_crypto_config_reference_ecdsa_use_psa +component_test_psa_crypto_config_accel_ecdsa_use_psa () { + msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" + + # Algorithms and key types to accelerate + loc_accel_list="ALG_ECDSA ALG_DETERMINISTIC_ECDSA KEY_TYPE_ECC_KEY_PAIR KEY_TYPE_ECC_PUBLIC_KEY" + + # Configure and build the test driver library + # ------------------------------------------- + + # Disable ALG_STREAM_CIPHER and ALG_ECB_NO_PADDING to avoid having + # partial support for cipher operations in the driver test library. + scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_STREAM_CIPHER + scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_ECB_NO_PADDING + + # All SHA-2 variants are needed for ECDSA signature tests, + # but only SHA-256 is enabled by default, so enable the others. + scripts/config.py -f tests/include/test/drivers/config_test_driver.h set MBEDTLS_SHA224_C + scripts/config.py -f tests/include/test/drivers/config_test_driver.h set MBEDTLS_SHA384_C + scripts/config.py -f tests/include/test/drivers/config_test_driver.h set MBEDTLS_SHA512_C + + loc_accel_flags=$( echo "$loc_accel_list" | sed 's/[^ ]* */-DLIBTESTDRIVER1_MBEDTLS_PSA_ACCEL_&/g' ) + make -C tests libtestdriver1.a CFLAGS="$ASAN_CFLAGS $loc_accel_flags" LDFLAGS="$ASAN_CFLAGS" + + # Configure and build the main libraries with drivers enabled + # ----------------------------------------------------------- + + # Use the same config as reference, only without built-in ECDSA + config_psa_crypto_config_ecdsa_use_psa 1 + + # Build the library + loc_accel_flags="$loc_accel_flags $( echo "$loc_accel_list" | sed 's/[^ ]* */-DMBEDTLS_PSA_ACCEL_&/g' )" + make CFLAGS="$ASAN_CFLAGS -O -Werror -I../tests/include -I../tests -I../../tests -DPSA_CRYPTO_DRIVER_TEST -DMBEDTLS_TEST_LIBTESTDRIVER1 $loc_accel_flags" LDFLAGS="-ltestdriver1 $ASAN_CFLAGS" + + # Make sure ECDSA was not re-enabled by accident (additive config) + not grep mbedtls_ecdsa_ library/ecdsa.o + + # Run the tests + # ------------- + + msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" + make test + + # TODO: ssl-opt.sh (currently doesn't pass) - #6861 +} + +# Keep in sync with component_test_psa_crypto_config_accel_ecdsa_use_psa. +# Used by tests/scripts/analyze_outcomes.py for comparison purposes. +component_test_psa_crypto_config_reference_ecdsa_use_psa () { + msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" + + # To be aligned with the accel component that needs this + scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_STREAM_CIPHER + scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_ECB_NO_PADDING + + config_psa_crypto_config_ecdsa_use_psa 0 + + make + + msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" + make test + + # TODO: ssl-opt.sh (when the accel component is ready) - #6861 +} + component_test_psa_crypto_config_accel_ecdh () { msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDH" @@ -2153,15 +2249,6 @@ component_test_psa_crypto_config_accel_rsa_signature () { loc_accel_flags=$( echo "$loc_accel_list" | sed 's/[^ ]* */-DLIBTESTDRIVER1_MBEDTLS_PSA_ACCEL_&/g' ) make -C tests libtestdriver1.a CFLAGS="$ASAN_CFLAGS $loc_accel_flags" LDFLAGS="$ASAN_CFLAGS" - # Restore test driver base configuration - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_SHA1_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_SHA224_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_SHA512_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_MD_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_PEM_PARSE_C - scripts/config.py -f tests/include/test/drivers/config_test_driver.h unset MBEDTLS_BASE64_C - - # Mbed TLS library build scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index bb44396534..eeded5f628 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -87,8 +87,8 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, ignor driver_test_passed = True if component_ref in entry: reference_test_passed = True - if(driver_test_passed is False and reference_test_passed is True): - print('{}: driver: skipped/failed; reference: passed'.format(key)) + if(reference_test_passed and not driver_test_passed): + print(key) result = False return result @@ -123,6 +123,7 @@ def do_analyze_coverage(outcome_file, args): """Perform coverage analysis.""" del args # unused outcomes = read_outcome_file(outcome_file) + print("\n*** Analyze coverage ***\n") results = analyze_outcomes(outcomes) return results.error_count == 0 @@ -131,6 +132,8 @@ def do_analyze_driver_vs_reference(outcome_file, args): ignored_tests = ['test_suite_' + x for x in args['ignored_suites']] outcomes = read_outcome_file(outcome_file) + print("\n*** Analyze driver {} vs reference {} ***\n".format( + args['component_driver'], args['component_ref'])) return analyze_driver_vs_reference(outcomes, args['component_ref'], args['component_driver'], ignored_tests) @@ -138,15 +141,38 @@ def do_analyze_driver_vs_reference(outcome_file, args): TASKS = { 'analyze_coverage': { 'test_function': do_analyze_coverage, - 'args': {}}, + 'args': {} + }, + # How to use analyze_driver_vs_reference_xxx locally: + # 1. tests/scripts/all.sh --outcome-file "$PWD/out.csv" + # 2. tests/scripts/analyze_outcomes.py out.csv analyze_driver_vs_reference_xxx 'analyze_driver_vs_reference_hash': { 'test_function': do_analyze_driver_vs_reference, 'args': { 'component_ref': 'test_psa_crypto_config_reference_hash_use_psa', 'component_driver': 'test_psa_crypto_config_accel_hash_use_psa', - 'ignored_suites': ['shax', 'mdx', # the software implementations that are being excluded - 'md', # the legacy abstraction layer that's being excluded - ]}} + 'ignored_suites': [ + 'shax', 'mdx', # the software implementations that are being excluded + 'md', # the legacy abstraction layer that's being excluded + ]}}, + 'analyze_driver_vs_reference_ecdsa': { + 'test_function': do_analyze_driver_vs_reference, + 'args': { + 'component_ref': 'test_psa_crypto_config_reference_ecdsa_use_psa', + 'component_driver': 'test_psa_crypto_config_accel_ecdsa_use_psa', + 'ignored_suites': [ + 'ecdsa', # the software implementation that's excluded + # the following lines should not be needed, + # they will be removed by upcoming work + 'psa_crypto_se_driver_hal', # #6856 + 'random', # #6856 + 'ecp', # #6856 + 'pk', # #6857 + 'x509parse', # #6858 + 'x509write', # #6858 + 'debug', # #6860 + 'ssl', # #6860 + ]}}, } def main():