From 462e3a9931da96c044660f60ef69fced0b8093ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 27 Dec 2022 12:35:11 +0100 Subject: [PATCH 01/12] all.sh: restore config_test_driver.h automatically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d166f779a..c949a6a9d 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" @@ -1978,11 +1979,6 @@ component_test_psa_crypto_config_accel_ecdsa () { 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 - scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG scripts/config.py unset MBEDTLS_USE_PSA_CRYPTO @@ -2077,15 +2073,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 From 200fd0f099b5782c50b663e6224ead5bd935e9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 27 Dec 2022 13:03:10 +0100 Subject: [PATCH 02/12] Add comments to accel_ecdsa component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slightly re-organize (accel list at the top). No need to disable USE_PSA or TLS 1.3 because they're already that way in the default config. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c949a6a9d..82ad6823e 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1965,6 +1965,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 @@ -1975,23 +1981,33 @@ 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" + # Configure and build the test driver library + # ------------------------------------------- + + # Start from default config (no USE_PSA or TLS 1.3) + driver support 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 + + # 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 } From 6d7db93bbb753e73a8f4ca62caeea4c0c66a3414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 28 Dec 2022 09:48:01 +0100 Subject: [PATCH 03/12] Enable TLS 1.3 in accelerated ECDSA test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- 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 82ad6823e..1d76d3000 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1987,9 +1987,10 @@ component_test_psa_crypto_config_accel_ecdsa () { # Configure and build the test driver library # ------------------------------------------- - # Start from default config (no USE_PSA or TLS 1.3) + driver support + # 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 set MBEDTLS_SSL_PROTO_TLS1_3 # Disable the module that's accelerated scripts/config.py unset MBEDTLS_ECDSA_C From 171c45feda859db4bb880d3cb4185ff09319cff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 29 Dec 2022 11:31:35 +0100 Subject: [PATCH 04/12] Add component accel_ecdsa_use_psa MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the basis for future work, we'll want to make sure everything passes in this component. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1d76d3000..c0c35a251 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2013,6 +2013,56 @@ component_test_psa_crypto_config_accel_ecdsa () { make test } +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 + + # SHA384 needed for some ECDSA signature tests. + 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 test driver library + # ------------------------------------------- + + # Start from full config (inc. USE_PSA + TLS 1.3) + driver support + scripts/config.py full + scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS + scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG + + # 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 + USE_PSA" + make test +} + component_test_psa_crypto_config_accel_ecdh () { msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDH" From 10e3963aa40bc0cf5af26a7c17f09c7e4d08b903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 29 Dec 2022 12:29:09 +0100 Subject: [PATCH 05/12] Add comparison of accel_ecdsa against reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For now, ignore test suites that don't have parity even is they should. The purpose is just to prepare the infrastructure and map the work. Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 54 ++++++++++++++++++++++++------- tests/scripts/analyze_outcomes.py | 31 +++++++++++++++--- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c0c35a251..ff1efe795 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2013,6 +2013,25 @@ component_test_psa_crypto_config_accel_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 + 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" @@ -2037,17 +2056,8 @@ component_test_psa_crypto_config_accel_ecdsa_use_psa () { # Configure and build the test driver library # ------------------------------------------- - # Start from full config (inc. USE_PSA + TLS 1.3) + driver support - scripts/config.py full - scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS - scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG - - # 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 + # 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' )" @@ -2061,6 +2071,28 @@ component_test_psa_crypto_config_accel_ecdsa_use_psa () { msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" make test + + # TODO: ssl-opt.sh (currently doesn't pass) + # TODO: is some subset of compat.sh needed? +} + +# 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) } component_test_psa_crypto_config_accel_ecdh () { diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index bb4439653..b360431ef 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -138,15 +138,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', + 'random', + 'ecp', + 'pk', + 'x509parse', + 'x509write', + 'debug', + 'ssl', + ]}}, } def main(): From 222bc85c6c09b9da9eeac2e42b49be9004d73b72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 29 Dec 2022 13:47:25 +0100 Subject: [PATCH 06/12] Update outcome analysis script & documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the script only makes before-after comparison, it no longer makes sense to ignore some test suites. Signed-off-by: Manuel Pégourié-Gonnard --- .../psa-migration/outcome-analysis.sh | 38 ++++--------------- docs/architecture/psa-migration/strategy.md | 19 +++++----- 2 files changed, 17 insertions(+), 40 deletions(-) diff --git a/docs/architecture/psa-migration/outcome-analysis.sh b/docs/architecture/psa-migration/outcome-analysis.sh index 908468548..f7ca01b45 100755 --- a/docs/architecture/psa-migration/outcome-analysis.sh +++ b/docs/architecture/psa-migration/outcome-analysis.sh @@ -1,40 +1,21 @@ #!/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: -# -# - 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 +# implementation", namely: the sets of tests skipped in the default config and +# the full config must be the same before and after the PR. # # 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 cleanup() { @@ -69,7 +50,6 @@ cleanup scripts/config.py full record "after-full" - # analysis populate_suites () { @@ -79,11 +59,7 @@ populate_suites () { 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 + SUITES="$SUITES $suite" done make neat } diff --git a/docs/architecture/psa-migration/strategy.md b/docs/architecture/psa-migration/strategy.md index 0ad5fa0a5..07fc48859 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.csv`. 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 From 6bbeba6a445a6f664b1a0e2c372572e4feb40a02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 29 Dec 2022 14:02:05 +0100 Subject: [PATCH 07/12] Add ssl-opt.sh support to outcome-analysis.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit But make it optional as it makes things much slower. Signed-off-by: Manuel Pégourié-Gonnard --- .../psa-migration/outcome-analysis.sh | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/docs/architecture/psa-migration/outcome-analysis.sh b/docs/architecture/psa-migration/outcome-analysis.sh index f7ca01b45..6603c3ae5 100755 --- a/docs/architecture/psa-migration/outcome-analysis.sh +++ b/docs/architecture/psa-migration/outcome-analysis.sh @@ -8,6 +8,11 @@ # implementation", namely: the sets of tests skipped in the default config and # the full config must be the same before and after the PR. # +# USAGE: +# - First, commit any uncommited changes. (Also, see warning below.) +# - including ssl-opt.sh: docs/architecture/psa-migration/outcome-analysis.sh +# - or: SKIP_SSL_OPT=1 docs/architecture/psa-migration/outcome-analysis.sh +# # 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 @@ -18,6 +23,8 @@ set -eu +: ${SKIP_SSL_OPT:=0} + cleanup() { make clean git checkout -- include/mbedtls/mbedtls_config.h include/psa/crypto_config.h @@ -26,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 @@ -35,21 +49,27 @@ 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 populate_suites () { @@ -57,11 +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=${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 () { @@ -69,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 @@ -77,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 From 8510105b5dba02d758ad94b8479fb7c8cec6aa9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 29 Dec 2022 16:04:35 +0100 Subject: [PATCH 08/12] Remove libtestdriver1 with 'make clean' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's a bit strange for tests/Makefile to clean up in library, but OTOH it's also tests/Makefile that copies this file there. Regardless, there was no place that cleaned up this file, and it needs to be removed somewhere. Signed-off-by: Manuel Pégourié-Gonnard --- tests/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile b/tests/Makefile index f0373381e..312607ee3 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 From c6967d21b9b9f7f911a8d9190003fe2ab20ef97a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 30 Dec 2022 13:40:34 +0100 Subject: [PATCH 09/12] Tune output format of analyze_outcomes.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The part "driver: skipped/failed, reference: passed" didn't add any information, but used up space on the screen and made the output slightly harder to parse. OTOH, now that we have multiple analyze_vs_reference tasks, we should print out which one we're doing, so that that output makes sense in case of a failure on the CI (which runs all tasks). Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/analyze_outcomes.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index b360431ef..482d64e7c 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) From 5a2e02635aade3d83eab18d800c093f9cdc5345b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 23 Jan 2023 12:51:52 +0100 Subject: [PATCH 10/12] Improve a few comments & documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- docs/architecture/psa-migration/outcome-analysis.sh | 4 ++-- docs/architecture/psa-migration/strategy.md | 2 +- tests/scripts/all.sh | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/architecture/psa-migration/outcome-analysis.sh b/docs/architecture/psa-migration/outcome-analysis.sh index 6603c3ae5..b26963b90 100755 --- a/docs/architecture/psa-migration/outcome-analysis.sh +++ b/docs/architecture/psa-migration/outcome-analysis.sh @@ -10,8 +10,8 @@ # # USAGE: # - First, commit any uncommited changes. (Also, see warning below.) -# - including ssl-opt.sh: docs/architecture/psa-migration/outcome-analysis.sh -# - or: SKIP_SSL_OPT=1 docs/architecture/psa-migration/outcome-analysis.sh +# - 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, diff --git a/docs/architecture/psa-migration/strategy.md b/docs/architecture/psa-migration/strategy.md index 07fc48859..154232474 100644 --- a/docs/architecture/psa-migration/strategy.md +++ b/docs/architecture/psa-migration/strategy.md @@ -393,7 +393,7 @@ manually for each PR that changes dependency declarations by using the script 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.csv`. See the +`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. diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index ff1efe795..73191b64d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2053,8 +2053,8 @@ component_test_psa_crypto_config_accel_ecdsa_use_psa () { 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 test driver library - # ------------------------------------------- + # 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 From bc19a0b0d8e04162259df0201b1617edf72f0c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 23 Jan 2023 12:54:24 +0100 Subject: [PATCH 11/12] Fix missing SHA-224 in test driver build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 73191b64d..2412c60f1 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2046,7 +2046,9 @@ component_test_psa_crypto_config_accel_ecdsa_use_psa () { 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 - # SHA384 needed for some ECDSA signature tests. + # 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 From d84902f4efcfbde7bfa5d1de49df93801a79c536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 23 Jan 2023 13:03:13 +0100 Subject: [PATCH 12/12] Add issue numbers to TODO comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the python script I didn't use the word TODO because pylint doesn't like that, but morally it's the same. I removed the comment about "do we need a subset of compat.sh?" because it turns out that `ssl-opt.sh` is already exercising all the key exchanges: % sed -n 's/.*force_ciphersuite=TLS-\([^ ]*\)-WITH.*/\1/p' tests/ssl-opt.sh | sort -u DHE-PSK DHE-RSA ECDH-ECDSA ECDHE-ECDSA ECDHE-PSK ECDHE-RSA ECJPAKE PSK RSA RSA-PSK (the only omission is ECDH-RSA which is not of interest here and does not actually differ from ECDH-ECDSA). So, we don't need a subset of compat.sh because we're already getting enough testing from ssl-opt.sh (not to mention test_suite_ssl). Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 7 +++---- tests/scripts/analyze_outcomes.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2412c60f1..fa82733ed 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2026,7 +2026,7 @@ config_psa_crypto_config_ecdsa_use_psa () { scripts/config.py unset MBEDTLS_ECDSA_C fi # Disable things that depend on it - # TODO: make these work + # 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 } @@ -2074,8 +2074,7 @@ component_test_psa_crypto_config_accel_ecdsa_use_psa () { msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" make test - # TODO: ssl-opt.sh (currently doesn't pass) - # TODO: is some subset of compat.sh needed? + # TODO: ssl-opt.sh (currently doesn't pass) - #6861 } # Keep in sync with component_test_psa_crypto_config_accel_ecdsa_use_psa. @@ -2094,7 +2093,7 @@ component_test_psa_crypto_config_reference_ecdsa_use_psa () { msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated ECDSA + USE_PSA" make test - # TODO: ssl-opt.sh (when the accel component is ready) + # TODO: ssl-opt.sh (when the accel component is ready) - #6861 } component_test_psa_crypto_config_accel_ecdh () { diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 482d64e7c..eeded5f62 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -164,14 +164,14 @@ TASKS = { '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', - 'random', - 'ecp', - 'pk', - 'x509parse', - 'x509write', - 'debug', - 'ssl', + 'psa_crypto_se_driver_hal', # #6856 + 'random', # #6856 + 'ecp', # #6856 + 'pk', # #6857 + 'x509parse', # #6858 + 'x509write', # #6858 + 'debug', # #6860 + 'ssl', # #6860 ]}}, }