Merge pull request #6466 from mprse/driver-only-hash-ci

Driver-only hashes: test coverage in the CI

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

@@ -13,6 +13,7 @@
 # - 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
 #
 # 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,
@@ -26,30 +27,12 @@
 # re-running this script (for example "get numbers before this PR").
 
 # ----- BEGIN edit this -----
-# The component in all.sh that builds and tests with drivers.
-DRIVER_COMPONENT=test_psa_crypto_config_accel_hash_use_psa
-# A similar configuration to that of the component, except without drivers,
-# for comparison.
-reference_config () {
-    # start with full
-    scripts/config.py full
-    # use PSA config and disable driver-less algs as in the component
-    scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG
-    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
-    # disable options as in the component
-    # (no need to disable whole modules, we'll just skip their test suite)
-    scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC
-    scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_DETERMINISTIC_ECDSA
-}
 # 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
-# Compare only "reference vs driver" or also "before vs after"?
-BEFORE_AFTER=1 # 0 or 1
 # ----- END edit this -----
 
 set -eu
@@ -65,38 +48,27 @@ record() {
     make check
 }
 
-if [ "$BEFORE_AFTER" -eq 1 ]; then
+# save current HEAD
-    # save current HEAD
+HEAD=$(git branch --show-current)
-    HEAD=$(git branch --show-current)
 
-    # get the numbers before this PR for default and full
+# 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"
-fi
-
-# get the numbers now for driver-only and reference
 cleanup
-reference_config
+git checkout $(git merge-base HEAD development)
-record "reference"
+record "before-default"
 
 cleanup
-export MBEDTLS_TEST_OUTCOME_FILE="$PWD/outcome-drivers.csv"
+scripts/config.py full
-export SKIP_SSL_OPT_COMPAT_SH=1
+record "before-full"
-tests/scripts/all.sh -k test_psa_crypto_config_accel_hash_use_psa
+
+# get the numbers now for default and full
+cleanup
+git checkout $HEAD
+record "after-default"
+
+cleanup
+scripts/config.py full
+record "after-full"
+
 
 # analysis
 
@@ -156,8 +128,5 @@ compare_builds () {
 }
 
 populate_suites
-if [ "$BEFORE_AFTER" -eq 1 ]; then
+compare_builds before-default after-default
-    compare_builds before-default after-default
+compare_builds before-full after-full
-    compare_builds before-full after-full
-fi
-compare_builds reference drivers

tests/scripts/all.sh

@@ -2065,6 +2065,47 @@ component_test_psa_crypto_config_accel_hash () {
     make test
 }
 
+# Auxiliary function to build config for hashes with and without drivers
+config_psa_crypto_hash_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 built-in implementation of hashes
+        scripts/config.py unset MBEDTLS_MD5_C
+        scripts/config.py unset MBEDTLS_RIPEMD160_C
+        scripts/config.py unset MBEDTLS_SHA1_C
+        scripts/config.py unset MBEDTLS_SHA224_C
+        scripts/config.py unset MBEDTLS_SHA256_C # see external RNG below
+        scripts/config.py unset MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT
+        scripts/config.py unset MBEDTLS_SHA384_C
+        scripts/config.py unset MBEDTLS_SHA512_C
+        scripts/config.py unset MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT
+    fi
+    # Use an external RNG as currently internal RNGs depend on entropy.c
+    # which in turn hard-depends on SHA256_C (or SHA512_C).
+    # See component_test_psa_external_rng_no_drbg_use_psa.
+    scripts/config.py set MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG
+    scripts/config.py unset MBEDTLS_ENTROPY_C
+    scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED # depends on ENTROPY_C
+    scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT # depends on former
+    # Also unset MD_C and things that depend on it;
+    # see component_test_crypto_full_no_md.
+    if [ "$DRIVER_ONLY" -eq 1 ]; then
+        scripts/config.py unset MBEDTLS_MD_C
+    fi
+    scripts/config.py unset MBEDTLS_HKDF_C # has independent PSA implementation
+    scripts/config.py unset MBEDTLS_HMAC_DRBG_C
+    scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC
+    scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_DETERMINISTIC_ECDSA
+}
+
+# Note that component_test_psa_crypto_config_reference_hash_use_psa
+# is related to this component and both components need to be kept in sync.
+# For details please see comments for component_test_psa_crypto_config_reference_hash_use_psa.
 component_test_psa_crypto_config_accel_hash_use_psa () {
     msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated hash and USE_PSA"
 
@@ -2077,35 +2118,7 @@ component_test_psa_crypto_config_accel_hash_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"
 
-    # start with config full for maximum coverage (also enables USE_PSA)
+    config_psa_crypto_hash_use_psa 1
-    scripts/config.py full
-    # enable support for drivers and configuring PSA-only algorithms
-    scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS
-    scripts/config.py set MBEDTLS_PSA_CRYPTO_CONFIG
-    # disable the built-in implementation of hashes
-    scripts/config.py unset MBEDTLS_MD5_C
-    scripts/config.py unset MBEDTLS_RIPEMD160_C
-    scripts/config.py unset MBEDTLS_SHA1_C
-    scripts/config.py unset MBEDTLS_SHA224_C
-    scripts/config.py unset MBEDTLS_SHA256_C # see external RNG below
-    scripts/config.py unset MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT
-    scripts/config.py unset MBEDTLS_SHA384_C
-    scripts/config.py unset MBEDTLS_SHA512_C
-    scripts/config.py unset MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT
-    # Use an external RNG as currently internal RNGs depend on entropy.c
-    # which in turn hard-depends on SHA256_C (or SHA512_C).
-    # See component_test_psa_external_rng_no_drbg_use_psa.
-    scripts/config.py set MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG
-    scripts/config.py unset MBEDTLS_ENTROPY_C
-    scripts/config.py unset MBEDTLS_ENTROPY_NV_SEED # depends on ENTROPY_C
-    scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT # depends on former
-    # Also unset MD_C and things that depend on it;
-    # see component_test_crypto_full_no_md.
-    scripts/config.py unset MBEDTLS_MD_C
-    scripts/config.py unset MBEDTLS_HKDF_C # has independent PSA implementation
-    scripts/config.py unset MBEDTLS_HMAC_DRBG_C
-    scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC
-    scripts/config.py -f include/psa/crypto_config.h unset PSA_WANT_ALG_DETERMINISTIC_ECDSA
 
     loc_accel_flags="$loc_accel_flags $( echo "$loc_accel_list" | sed 's/[^ ]* */-DMBEDTLS_PSA_ACCEL_&/g' )"
     make CFLAGS="$ASAN_CFLAGS -Werror -I../tests/include -I../tests -I../../tests -DPSA_CRYPTO_DRIVER_TEST -DMBEDTLS_TEST_LIBTESTDRIVER1 $loc_accel_flags" LDFLAGS="-ltestdriver1 $ASAN_CFLAGS" all
@@ -2122,16 +2135,32 @@ component_test_psa_crypto_config_accel_hash_use_psa () {
     msg "test: MBEDTLS_PSA_CRYPTO_CONFIG with accelerated hash and USE_PSA"
     make test
 
-    # hidden option: when running outcome-analysis.sh, we can skip this
+    msg "test: ssl-opt.sh, MBEDTLS_PSA_CRYPTO_CONFIG with accelerated hash and USE_PSA"
-    if [ "${SKIP_SSL_OPT_COMPAT_SH-unset}" = "unset" ]; then
+    tests/ssl-opt.sh
-        msg "test: ssl-opt.sh, MBEDTLS_PSA_CRYPTO_CONFIG with accelerated hash and USE_PSA"
-        tests/ssl-opt.sh
 
-        msg "test: compat.sh, MBEDTLS_PSA_CRYPTO_CONFIG with accelerated hash and USE_PSA"
+    msg "test: compat.sh, MBEDTLS_PSA_CRYPTO_CONFIG without accelerated hash and USE_PSA"
-        tests/compat.sh
+    tests/compat.sh
-    else
+}
-        echo "skip ssl-opt.sh and compat.sh"
+
-    fi
+# This component provides reference configuration for test_psa_crypto_config_accel_hash_use_psa
+# without accelerated hash. The outcome from both components are used by the analyze_outcomes.py
+# script to find regression in test coverage when accelerated hash is used (tests and ssl-opt).
+# Both components need to be kept in sync.
+component_test_psa_crypto_config_reference_hash_use_psa() {
+    msg "test: MBEDTLS_PSA_CRYPTO_CONFIG without accelerated hash and 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
+
+    config_psa_crypto_hash_use_psa 0
+
+    make
+
+    msg "test: MBEDTLS_PSA_CRYPTO_CONFIG without accelerated hash and USE_PSA"
+    make test
+
+    msg "test: ssl-opt.sh, MBEDTLS_PSA_CRYPTO_CONFIG without accelerated hash and USE_PSA"
+    tests/ssl-opt.sh
 }
 
 component_test_psa_crypto_config_accel_cipher () {

tests/scripts/analyze_outcomes.py

@@ -9,6 +9,7 @@ less likely to be useful.
 import argparse
 import sys
 import traceback
+import re
 
 import check_test_cases
 
@@ -60,6 +61,37 @@ def analyze_coverage(results, outcomes):
             # fixed this branch to have full coverage of test cases.
             results.warning('Test case not executed: {}', key)
 
+def analyze_driver_vs_reference(outcomes, component_ref, component_driver, ignored_tests):
+    """Check that all tests executed in the reference component are also
+    executed in the corresponding driver component.
+    Skip test suites provided in ignored_tests list.
+    """
+    available = check_test_cases.collect_available_test_cases()
+    result = True
+
+    for key in available:
+        # Skip ignored test suites
+        test_suite = key.split(';')[0] # retrieve test suit name
+        test_suite = test_suite.split('.')[0] # retrieve main part of test suit name
+        if test_suite in ignored_tests:
+            continue
+        # Continue if test was not executed by any component
+        hits = outcomes[key].hits() if key in outcomes else 0
+        if hits == 0:
+            continue
+        # Search for tests that run in reference component and not in driver component
+        driver_test_passed = False
+        reference_test_passed = False
+        for entry in outcomes[key].successes:
+            if component_driver in entry:
+                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))
+            result = False
+    return result
+
 def analyze_outcomes(outcomes):
     """Run all analyses on the given outcome collection."""
     results = Results()
@@ -87,20 +119,75 @@ by a semicolon.
                 outcomes[key].failures.append(setup)
     return outcomes
 
-def analyze_outcome_file(outcome_file):
+def do_analyze_coverage(outcome_file, args):
-    """Analyze the given outcome file."""
+    """Perform coverage analysis."""
+    del args # unused
     outcomes = read_outcome_file(outcome_file)
-    return analyze_outcomes(outcomes)
+    results = analyze_outcomes(outcomes)
+    return results.error_count == 0
+
+def do_analyze_driver_vs_reference(outcome_file, args):
+    """Perform driver vs reference analyze."""
+    ignored_tests = ['test_suite_' + x for x in args['ignored_suites']]
+
+    outcomes = read_outcome_file(outcome_file)
+    return analyze_driver_vs_reference(outcomes, args['component_ref'],
+                                       args['component_driver'], ignored_tests)
+
+# List of tasks with a function that can handle this task and additional arguments if required
+TASKS = {
+    'analyze_coverage':                 {
+        'test_function': do_analyze_coverage,
+        'args': {}},
+    '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
+                              ]}}
+}
 
 def main():
     try:
         parser = argparse.ArgumentParser(description=__doc__)
         parser.add_argument('outcomes', metavar='OUTCOMES.CSV',
                             help='Outcome file to analyze')
+        parser.add_argument('task', default='all', nargs='?',
+                            help='Analysis to be done. By default, run all tasks. '
+                                 'With one or more TASK, run only those. '
+                                 'TASK can be the name of a single task or '
+                                 'comma/space-separated list of tasks. ')
+        parser.add_argument('--list', action='store_true',
+                            help='List all available tasks and exit.')
         options = parser.parse_args()
-        results = analyze_outcome_file(options.outcomes)
+
-        if results.error_count > 0:
+        if options.list:
+            for task in TASKS:
+                print(task)
+            sys.exit(0)
+
+        result = True
+
+        if options.task == 'all':
+            tasks = TASKS.keys()
+        else:
+            tasks = re.split(r'[, ]+', options.task)
+
+            for task in tasks:
+                if task not in TASKS:
+                    print('Error: invalid task: {}'.format(task))
+                    sys.exit(1)
+
+        for task in TASKS:
+            if task in tasks:
+                if not TASKS[task]['test_function'](options.outcomes, TASKS[task]['args']):
+                    result = False
+
+        if result is False:
             sys.exit(1)
+        print("SUCCESS :-)")
     except Exception: # pylint: disable=broad-except
         # Print the backtrace and exit explicitly with our chosen status.
         traceback.print_exc()