Merge pull request #6855 from mpg/driver-only-ecdsa-starter

Driver-only ECDSA starter

docs/architecture/psa-migration/outcome-analysis.sh

@@ -1,42 +1,30 @@
 #!/bin/sh
 
-# This script runs tests in various revisions and configurations and analyses
+# This script runs tests before and after a PR and analyzes the results in
-# the results in order to highlight any difference in the set of tests skipped
+# order to highlight any difference in the set of tests skipped.
-# in the test suites of interest.
 #
-# 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
+# USAGE:
-#   the same before and after the PR that implements step 3;
+# - First, commit any uncommited changes. (Also, see warning below.)
-# - the set of tests skipped in the driver-only build is the same as in an
+# - Then launch --> [SKIP_SSL_OPT=1] docs/architecture/psa-migration/outcome-analysis.sh
-#   equivalent software-based configuration, or the difference is small enough,
+#     - SKIP_SSL_OPT=1 can optionally be set to skip ssl-opt.sh tests
-#   justified, and a github issue is created to track it.
-#   This part is verified by tests/scripts/analyze_outcomes.py
 #
 # 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
+# NOTE: you can comment out parts that don't need to be re-done when
-# 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
 # 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=${data%.data}
-        suite=${suite%.data}
+        SUITES="$SUITES $suite"
-        suite_base=${suite%%.*}
-        case " $IGNORE " in
-            *" $suite_base "*) :;;
-            *) SUITES="$SUITES $suite";;
-        esac
     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 <skipped-ref)
     nb_new=$(wc -l <skipped-new)
 
-    printf "%36s: total %4d; skipped %4d -> %4d\n" \
+    name=${suite#test_suite_}
-            $suite      $total       $nb_ref $nb_new
+    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

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
+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;
+  the same before and after the PR that implements step 3. This is tested
-- the set of tests skipped in the driver-only build is the same as in an
+manually for each PR that changes dependency declarations by using the script
-  equivalent software-based configuration, or the difference is small enough,
+`outcome-analysis.sh` in the present directory.
-  justified, and a github issue is created to track it.
+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
-Note that the favourable case is when the number of tests skipped is 0 in the
+CI in the "Results analysis" stage, by running
-driver-only build. In other cases, analysis of the outcome files is needed,
+`tests/scripts/analyze_outcomes.py`. See the
-see the example script `outcome-analysis.sh` in the same directory.
+`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

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

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
+    # Configure and build the test driver library
-    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
 
+    # 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 set MBEDTLS_SSL_PROTO_TLS1_3
-    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
 }
 
+# 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

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):
+        if(reference_test_passed and not driver_test_passed):
-            print('{}: driver: skipped/failed; reference: passed'.format(key))
+            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" <component_ref> <component_driver>
+    # 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
+            'ignored_suites': [
-                               'md',  # the legacy abstraction layer that's being excluded
+                '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():