From 5fa986c8cb77c081ab5046cdd8e519c1a1f4560b Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 10 Nov 2023 14:05:09 +0000 Subject: [PATCH 01/41] Move handling of mutex->is_valid into threading_helpers.c This is now a field only used for testing. Signed-off-by: Paul Elliott --- include/mbedtls/threading.h | 9 ++++++--- library/threading.c | 21 ++++++++++----------- tests/src/threading_helpers.c | 18 +++++++++++++----- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/include/mbedtls/threading.h b/include/mbedtls/threading.h index ed16a23b1a..c136ea0b0d 100644 --- a/include/mbedtls/threading.h +++ b/include/mbedtls/threading.h @@ -28,10 +28,13 @@ extern "C" { #include typedef struct mbedtls_threading_mutex_t { pthread_mutex_t MBEDTLS_PRIVATE(mutex); - /* is_valid is 0 after a failed init or a free, and nonzero after a - * successful init. This field is not considered part of the public - * API of Mbed TLS and may change without notice. */ + + /* is_valid is controlled by code in tests/src/threading_helpers - it will + * be 0 after a failed init or a free, and nonzero after a successful init. + * This field is for testing only and thus not considered part of the + * public API of Mbed TLS and may change without notice. */ char MBEDTLS_PRIVATE(is_valid); + } mbedtls_threading_mutex_t; #endif diff --git a/library/threading.c b/library/threading.c index 52fe8fca99..d97f0cfe78 100644 --- a/library/threading.c +++ b/library/threading.c @@ -56,28 +56,27 @@ static void threading_mutex_init_pthread(mbedtls_threading_mutex_t *mutex) return; } - /* A nonzero value of is_valid indicates a successfully initialized - * mutex. This is a workaround for not being able to return an error - * code for this function. The lock/unlock functions return an error - * if is_valid is nonzero. The Mbed TLS unit test code uses this field - * to distinguish more states of the mutex; see - * tests/src/threading_helpers for details. */ - mutex->is_valid = pthread_mutex_init(&mutex->mutex, NULL) == 0; + /* One problem here is that calling lock on a pthread mutex without first + * having initialised it is undefined behaviour. Obviously we cannot check + * this here in a thread safe manner without a significant performance + * hit, so state transitions are checked in tests only via the is_valid + * varaible. Please make sure any new mutex that gets added is exercised in + * tests; see tests/src/threading_helpers for more details. */ + (void) pthread_mutex_init(&mutex->mutex, NULL); } static void threading_mutex_free_pthread(mbedtls_threading_mutex_t *mutex) { - if (mutex == NULL || !mutex->is_valid) { + if (mutex == NULL) { return; } (void) pthread_mutex_destroy(&mutex->mutex); - mutex->is_valid = 0; } static int threading_mutex_lock_pthread(mbedtls_threading_mutex_t *mutex) { - if (mutex == NULL || !mutex->is_valid) { + if (mutex == NULL) { return MBEDTLS_ERR_THREADING_BAD_INPUT_DATA; } @@ -90,7 +89,7 @@ static int threading_mutex_lock_pthread(mbedtls_threading_mutex_t *mutex) static int threading_mutex_unlock_pthread(mbedtls_threading_mutex_t *mutex) { - if (mutex == NULL || !mutex->is_valid) { + if (mutex == NULL) { return MBEDTLS_ERR_THREADING_BAD_INPUT_DATA; } diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index 6f405b00c6..0ea1e98d82 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -64,9 +64,9 @@ enum value_of_mutex_is_valid_field { * compatibility with threading_mutex_init_pthread() and * threading_mutex_free_pthread(). MUTEX_LOCKED could be any nonzero * value. */ - MUTEX_FREED = 0, //!< Set by threading_mutex_free_pthread - MUTEX_IDLE = 1, //!< Set by threading_mutex_init_pthread and by our unlock - MUTEX_LOCKED = 2, //!< Set by our lock + MUTEX_FREED = 0, //! < Set by mbedtls_test_wrap_mutex_free + MUTEX_IDLE = 1, //! < Set by mbedtls_test_wrap_mutex_init and by mbedtls_test_wrap_mutex_unlock + MUTEX_LOCKED = 2, //! < Set by mbedtls_test_wrap_mutex_lock }; typedef struct { @@ -101,8 +101,12 @@ static void mbedtls_test_mutex_usage_error(mbedtls_threading_mutex_t *mutex, static void mbedtls_test_wrap_mutex_init(mbedtls_threading_mutex_t *mutex) { mutex_functions.init(mutex); - if (mutex->is_valid) { + + if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { + mutex->state = MUTEX_IDLE; ++live_mutexes; + + mutex_functions.unlock(&mbedtls_test_mutex_mutex); } } @@ -123,7 +127,11 @@ static void mbedtls_test_wrap_mutex_free(mbedtls_threading_mutex_t *mutex) mbedtls_test_mutex_usage_error(mutex, "corrupted state"); break; } + + /* Mark mutex as free'd first, because we need to release the mutex. If + * free fails, this could end up with inconsistent state. */ if (mutex->is_valid) { + mutex->is_valid = MUTEX_FREED; --live_mutexes; } mutex_functions.free(mutex); @@ -138,7 +146,7 @@ static int mbedtls_test_wrap_mutex_lock(mbedtls_threading_mutex_t *mutex) break; case MUTEX_IDLE: if (ret == 0) { - mutex->is_valid = 2; + mutex->is_valid = MUTEX_LOCKED; } break; case MUTEX_LOCKED: From 3774637518fdb218ae899e991113d4459095de88 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Sun, 12 Nov 2023 19:05:57 +0000 Subject: [PATCH 02/41] Make threading helpers tests thread safe Signed-off-by: Paul Elliott --- include/mbedtls/threading.h | 9 +-- tests/src/threading_helpers.c | 115 +++++++++++++++++++--------------- 2 files changed, 69 insertions(+), 55 deletions(-) diff --git a/include/mbedtls/threading.h b/include/mbedtls/threading.h index c136ea0b0d..cdfa7d69e5 100644 --- a/include/mbedtls/threading.h +++ b/include/mbedtls/threading.h @@ -29,10 +29,11 @@ extern "C" { typedef struct mbedtls_threading_mutex_t { pthread_mutex_t MBEDTLS_PRIVATE(mutex); - /* is_valid is controlled by code in tests/src/threading_helpers - it will - * be 0 after a failed init or a free, and nonzero after a successful init. - * This field is for testing only and thus not considered part of the - * public API of Mbed TLS and may change without notice. */ + /* WARNING - is_valid should only be accessed when holding the mutex lock in + * tests/src/threading_helpers.c, otherwise corruption can occur. + * is_valid will be 0 after a failed init or a free, and nonzero after a + * successful init. This field is for testing only and thus not considered + * part of the public API of Mbed TLS and may change without notice.*/ char MBEDTLS_PRIVATE(is_valid); } mbedtls_threading_mutex_t; diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index 0ea1e98d82..0ffffbfd54 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -77,6 +77,8 @@ typedef struct { } mutex_functions_t; static mutex_functions_t mutex_functions; +mbedtls_threading_mutex_t mbedtls_test_mutex_mutex; + /** The total number of calls to mbedtls_mutex_init(), minus the total number * of calls to mbedtls_mutex_free(). * @@ -88,6 +90,7 @@ static void mbedtls_test_mutex_usage_error(mbedtls_threading_mutex_t *mutex, const char *msg) { (void) mutex; + if (mbedtls_test_info.mutex_usage_error == NULL) { mbedtls_test_info.mutex_usage_error = msg; } @@ -112,73 +115,81 @@ static void mbedtls_test_wrap_mutex_init(mbedtls_threading_mutex_t *mutex) static void mbedtls_test_wrap_mutex_free(mbedtls_threading_mutex_t *mutex) { - switch (mutex->is_valid) { - case MUTEX_FREED: - mbedtls_test_mutex_usage_error(mutex, "free without init or double free"); - break; - case MUTEX_IDLE: - /* Do nothing. The underlying free function will reset is_valid - * to 0. */ - break; - case MUTEX_LOCKED: - mbedtls_test_mutex_usage_error(mutex, "free without unlock"); - break; - default: - mbedtls_test_mutex_usage_error(mutex, "corrupted state"); - break; - } + if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - /* Mark mutex as free'd first, because we need to release the mutex. If - * free fails, this could end up with inconsistent state. */ - if (mutex->is_valid) { - mutex->is_valid = MUTEX_FREED; - --live_mutexes; + switch (mutex->is_valid) { + case MUTEX_FREED: + mbedtls_test_mutex_usage_error(mutex, "free without init or double free"); + break; + case MUTEX_IDLE: + mutex->is_valid = MUTEX_FREED; + --live_mutexes; + break; + case MUTEX_LOCKED: + mbedtls_test_mutex_usage_error(mutex, "free without unlock"); + break; + default: + mbedtls_test_mutex_usage_error(mutex, "corrupted state"); + break; + } + + mutex_functions.unlock(&mbedtls_test_mutex_mutex); } mutex_functions.free(mutex); } static int mbedtls_test_wrap_mutex_lock(mbedtls_threading_mutex_t *mutex) { + /* Lock the passed in mutex first, so that the only way to change the state + * is to hold the passed in and internal mutex - otherwise we create a race + * condition. */ int ret = mutex_functions.lock(mutex); - switch (mutex->is_valid) { - case MUTEX_FREED: - mbedtls_test_mutex_usage_error(mutex, "lock without init"); - break; - case MUTEX_IDLE: - if (ret == 0) { - mutex->is_valid = MUTEX_LOCKED; - } - break; - case MUTEX_LOCKED: - mbedtls_test_mutex_usage_error(mutex, "double lock"); - break; - default: - mbedtls_test_mutex_usage_error(mutex, "corrupted state"); - break; + if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { + switch (mutex->is_valid) { + case MUTEX_FREED: + mbedtls_test_mutex_usage_error(mutex, "lock without init"); + break; + case MUTEX_IDLE: + if (ret == 0) { + mutex->is_valid = MUTEX_LOCKED; + } + break; + case MUTEX_LOCKED: + mbedtls_test_mutex_usage_error(mutex, "double lock"); + break; + default: + mbedtls_test_mutex_usage_error(mutex, "corrupted state"); + break; + } + + mutex_functions.unlock(&mbedtls_test_mutex_mutex); } return ret; } static int mbedtls_test_wrap_mutex_unlock(mbedtls_threading_mutex_t *mutex) { - int ret = mutex_functions.unlock(mutex); - switch (mutex->is_valid) { - case MUTEX_FREED: - mbedtls_test_mutex_usage_error(mutex, "unlock without init"); - break; - case MUTEX_IDLE: - mbedtls_test_mutex_usage_error(mutex, "unlock without lock"); - break; - case MUTEX_LOCKED: - if (ret == 0) { + /* Lock the internal mutex first and change state, so that the only way to + * change the state is to hold the passed in and internal mutex - otherwise + * we create a race condition. */ + if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { + switch (mutex->is_valid) { + case MUTEX_FREED: + mbedtls_test_mutex_usage_error(mutex, "unlock without init"); + break; + case MUTEX_IDLE: + mbedtls_test_mutex_usage_error(mutex, "unlock without lock"); + break; + case MUTEX_LOCKED: mutex->is_valid = MUTEX_IDLE; - } - break; - default: - mbedtls_test_mutex_usage_error(mutex, "corrupted state"); - break; + break; + default: + mbedtls_test_mutex_usage_error(mutex, "corrupted state"); + break; + } + mutex_functions.unlock(&mbedtls_test_mutex_mutex); } - return ret; + return mutex_functions.unlock(mutex); } void mbedtls_test_mutex_usage_init(void) @@ -191,6 +202,8 @@ void mbedtls_test_mutex_usage_init(void) mbedtls_mutex_free = &mbedtls_test_wrap_mutex_free; mbedtls_mutex_lock = &mbedtls_test_wrap_mutex_lock; mbedtls_mutex_unlock = &mbedtls_test_wrap_mutex_unlock; + + mutex_functions.init(&mbedtls_test_mutex_mutex); } void mbedtls_test_mutex_usage_check(void) From 9e25936241c1e6096779899ddc1fc1cb068ed93f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 15 Nov 2023 11:33:32 +0000 Subject: [PATCH 03/41] Rename mutex->is_valid to mutex->state Rename struct member to make it more representative of its current use. Signed-off-by: Paul Elliott --- include/mbedtls/threading.h | 6 +++--- tests/src/threading_helpers.c | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/threading.h b/include/mbedtls/threading.h index cdfa7d69e5..b504233bdc 100644 --- a/include/mbedtls/threading.h +++ b/include/mbedtls/threading.h @@ -29,12 +29,12 @@ extern "C" { typedef struct mbedtls_threading_mutex_t { pthread_mutex_t MBEDTLS_PRIVATE(mutex); - /* WARNING - is_valid should only be accessed when holding the mutex lock in + /* WARNING - state should only be accessed when holding the mutex lock in * tests/src/threading_helpers.c, otherwise corruption can occur. - * is_valid will be 0 after a failed init or a free, and nonzero after a + * state will be 0 after a failed init or a free, and nonzero after a * successful init. This field is for testing only and thus not considered * part of the public API of Mbed TLS and may change without notice.*/ - char MBEDTLS_PRIVATE(is_valid); + char MBEDTLS_PRIVATE(state); } mbedtls_threading_mutex_t; #endif diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index 0ffffbfd54..385a079261 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -58,8 +58,8 @@ * indicate the exact location of the problematic call. To locate the error, * use a debugger and set a breakpoint on mbedtls_test_mutex_usage_error(). */ -enum value_of_mutex_is_valid_field { - /* Potential values for the is_valid field of mbedtls_threading_mutex_t. +enum value_of_mutex_state_field { + /* Potential values for the state field of mbedtls_threading_mutex_t. * Note that MUTEX_FREED must be 0 and MUTEX_IDLE must be 1 for * compatibility with threading_mutex_init_pthread() and * threading_mutex_free_pthread(). MUTEX_LOCKED could be any nonzero @@ -117,12 +117,12 @@ static void mbedtls_test_wrap_mutex_free(mbedtls_threading_mutex_t *mutex) { if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - switch (mutex->is_valid) { + switch (mutex->state) { case MUTEX_FREED: mbedtls_test_mutex_usage_error(mutex, "free without init or double free"); break; case MUTEX_IDLE: - mutex->is_valid = MUTEX_FREED; + mutex->state = MUTEX_FREED; --live_mutexes; break; case MUTEX_LOCKED: @@ -145,13 +145,13 @@ static int mbedtls_test_wrap_mutex_lock(mbedtls_threading_mutex_t *mutex) * condition. */ int ret = mutex_functions.lock(mutex); if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - switch (mutex->is_valid) { + switch (mutex->state) { case MUTEX_FREED: mbedtls_test_mutex_usage_error(mutex, "lock without init"); break; case MUTEX_IDLE: if (ret == 0) { - mutex->is_valid = MUTEX_LOCKED; + mutex->state = MUTEX_LOCKED; } break; case MUTEX_LOCKED: @@ -173,7 +173,7 @@ static int mbedtls_test_wrap_mutex_unlock(mbedtls_threading_mutex_t *mutex) * change the state is to hold the passed in and internal mutex - otherwise * we create a race condition. */ if (mutex_functions.lock(&mbedtls_test_mutex_mutex) == 0) { - switch (mutex->is_valid) { + switch (mutex->state) { case MUTEX_FREED: mbedtls_test_mutex_usage_error(mutex, "unlock without init"); break; @@ -181,7 +181,7 @@ static int mbedtls_test_wrap_mutex_unlock(mbedtls_threading_mutex_t *mutex) mbedtls_test_mutex_usage_error(mutex, "unlock without lock"); break; case MUTEX_LOCKED: - mutex->is_valid = MUTEX_IDLE; + mutex->state = MUTEX_IDLE; break; default: mbedtls_test_mutex_usage_error(mutex, "corrupted state"); From a6cf5d67c516f1cb73815ec54db2606a27db4d80 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Wed, 22 Nov 2023 11:35:21 +0800 Subject: [PATCH 04/41] Share parsed outcomes among tasks when ananlyzing This extremely improves the performance. Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index a070b01639..ddacf2e06e 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -179,23 +179,26 @@ by a semicolon. outcomes[key].failures.append(setup) return outcomes -def do_analyze_coverage(results: Results, outcome_file, args): +def do_analyze_coverage(results: Results, outcomes_or_file, args): """Perform coverage analysis.""" results.new_section("Analyze coverage") - outcomes = read_outcome_file(outcome_file) + outcomes = read_outcome_file(outcomes_or_file) \ + if isinstance(outcomes_or_file, str) else outcomes_or_file analyze_outcomes(results, outcomes, args) -def do_analyze_driver_vs_reference(results: Results, outcome_file, args): +def do_analyze_driver_vs_reference(results: Results, outcomes_or_file, args): """Perform driver vs reference analyze.""" results.new_section("Analyze driver {} vs reference {}", args['component_driver'], args['component_ref']) - execute_reference_driver_tests(results, args['component_ref'], \ - args['component_driver'], outcome_file) - ignored_suites = ['test_suite_' + x for x in args['ignored_suites']] - outcomes = read_outcome_file(outcome_file) + if isinstance(outcomes_or_file, str): + execute_reference_driver_tests(results, args['component_ref'], \ + args['component_driver'], outcomes_or_file) + outcomes = read_outcome_file(outcomes_or_file) + else: + outcomes = outcomes_or_file analyze_driver_vs_reference(results, outcomes, args['component_ref'], args['component_driver'], @@ -493,10 +496,19 @@ def main(): KNOWN_TASKS['analyze_coverage']['args']['full_coverage'] = options.full_coverage + # If the outcome file already exists, we assume that the user wants to + # perform the comparison. + # Share the contents among tasks to improve performance. + if os.path.exists(options.outcomes): + main_results.info("Read outcome file from {}.", options.outcomes) + outcomes_or_file = read_outcome_file(options.outcomes) + else: + outcomes_or_file = options.outcomes + for task in tasks_list: test_function = KNOWN_TASKS[task]['test_function'] test_args = KNOWN_TASKS[task]['args'] - test_function(main_results, options.outcomes, test_args) + test_function(main_results, outcomes_or_file, test_args) main_results.info("Overall results: {} warnings and {} errors", main_results.warning_count, main_results.error_count) From a4428588782c60947b11a2ee703e4eceda7ac8b4 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Wed, 22 Nov 2023 19:02:15 +0800 Subject: [PATCH 05/41] Restruct the structure of outcome file presentation Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 68 ++++++++++++------------------- 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index ddacf2e06e..2cd6257d35 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -40,25 +40,6 @@ class Results: def _print_line(fmt, *args, **kwargs): sys.stderr.write((fmt + '\n').format(*args, **kwargs)) -class TestCaseOutcomes: - """The outcomes of one test case across many configurations.""" - # pylint: disable=too-few-public-methods - - def __init__(self): - # Collect a list of witnesses of the test case succeeding or failing. - # Currently we don't do anything with witnesses except count them. - # The format of a witness is determined by the read_outcome_file - # function; it's the platform and configuration joined by ';'. - self.successes = [] - self.failures = [] - - def hits(self): - """Return the number of times a test case has been run. - - This includes passes and failures, but not skips. - """ - return len(self.successes) + len(self.failures) - def execute_reference_driver_tests(results: Results, ref_component, driver_component, \ outcome_file): """Run the tests specified in ref_component and driver_component. Results @@ -82,7 +63,12 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() for key in available: - hits = outcomes[key].hits() if key in outcomes else 0 + hits = 0 + for _comp, comp_outcomes in outcomes.items(): + if key in comp_outcomes["successes"] or \ + key in comp_outcomes["failures"]: + hits += 1 + if hits == 0 and key not in allow_list: if full_coverage: results.error('Test case not executed: {}', key) @@ -117,8 +103,14 @@ def analyze_driver_vs_reference(results: Results, outcomes, - only some specific test inside a test suite, for which the corresponding output string is provided """ - seen_reference_passing = False - for key in outcomes: + ref_outcomes = outcomes.get("component_" + component_ref) + driver_outcomes = outcomes.get("component_" + component_driver) + + if ref_outcomes is None or not ref_outcomes['successes']: + results.error("no passing test in reference component: bad outcome file?") + return + + for key in ref_outcomes["successes"]: # key is like "test_suite_foo.bar;Description of test case" (full_test_suite, test_string) = key.split(';') test_suite = full_test_suite.split('.')[0] # retrieve main part of test suite name @@ -136,23 +128,11 @@ def analyze_driver_vs_reference(results: Results, outcomes, if name_matches_pattern(test_string, str_or_re): ignored = True - # 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 - seen_reference_passing = True - if reference_test_passed and not driver_test_passed and not ignored: + if not ignored and not key in driver_outcomes['successes']: results.error("PASS -> SKIP/FAIL: {}", key) - if ignored and driver_test_passed: + if ignored and key in driver_outcomes['successes']: results.error("uselessly ignored: {}", key) - if not seen_reference_passing: - results.error("no passing test in reference component: bad outcome file?") - def analyze_outcomes(results: Results, outcomes, args): """Run all analyses on the given outcome collection.""" analyze_coverage(results, outcomes, args['allow_list'], @@ -168,15 +148,19 @@ by a semicolon. outcomes = {} with open(outcome_file, 'r', encoding='utf-8') as input_file: for line in input_file: - (platform, config, suite, case, result, _cause) = line.split(';') + (_platform, config, suite, case, result, _cause) = line.split(';') key = ';'.join([suite, case]) - setup = ';'.join([platform, config]) - if key not in outcomes: - outcomes[key] = TestCaseOutcomes() + if config not in outcomes: + outcomes[config] = {"successes":[], "failures":[]} if result == 'PASS': - outcomes[key].successes.append(setup) + outcomes[config]['successes'].append(key) elif result == 'FAIL': - outcomes[key].failures.append(setup) + outcomes[config]['failures'].append(key) + + for config in outcomes: + outcomes[config]['successes'] = frozenset(outcomes[config]['successes']) + outcomes[config]['failures'] = frozenset(outcomes[config]['failures']) + return outcomes def do_analyze_coverage(results: Results, outcomes_or_file, args): From 31a9b7891adf28a7437b177cb547d2ffb58a8983 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Thu, 23 Nov 2023 14:15:37 +0800 Subject: [PATCH 06/41] Improve comments and variable naming Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 57 ++++++++++++++++++------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 2cd6257d35..0baba1b7e9 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -62,24 +62,24 @@ def execute_reference_driver_tests(results: Results, ref_component, driver_compo def analyze_coverage(results, outcomes, allow_list, full_coverage): """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() - for key in available: + for suite_case in available: hits = 0 for _comp, comp_outcomes in outcomes.items(): - if key in comp_outcomes["successes"] or \ - key in comp_outcomes["failures"]: + if suite_case in comp_outcomes["successes"] or \ + suite_case in comp_outcomes["failures"]: hits += 1 - if hits == 0 and key not in allow_list: + if hits == 0 and suite_case not in allow_list: if full_coverage: - results.error('Test case not executed: {}', key) + results.error('Test case not executed: {}', suite_case) else: - results.warning('Test case not executed: {}', key) - elif hits != 0 and key in allow_list: + results.warning('Test case not executed: {}', suite_case) + elif hits != 0 and suite_case in allow_list: # Test Case should be removed from the allow list. if full_coverage: - results.error('Allow listed test case was executed: {}', key) + results.error('Allow listed test case was executed: {}', suite_case) else: - results.warning('Allow listed test case was executed: {}', key) + results.warning('Allow listed test case was executed: {}', suite_case) def name_matches_pattern(name, str_or_re): """Check if name matches a pattern, that may be a string or regex. @@ -96,8 +96,8 @@ def name_matches_pattern(name, str_or_re): def analyze_driver_vs_reference(results: Results, outcomes, component_ref, component_driver, ignored_suites, ignored_tests=None): - """Check that all tests executed in the reference component are also - executed in the corresponding driver component. + """Check that all tests passed in the reference component are also + passed in the corresponding driver component. Skip: - full test suites provided in ignored_suites list - only some specific test inside a test suite, for which the corresponding @@ -110,9 +110,9 @@ def analyze_driver_vs_reference(results: Results, outcomes, results.error("no passing test in reference component: bad outcome file?") return - for key in ref_outcomes["successes"]: - # key is like "test_suite_foo.bar;Description of test case" - (full_test_suite, test_string) = key.split(';') + for suite_case in ref_outcomes["successes"]: + # suite_case is like "test_suite_foo.bar;Description of test case" + (full_test_suite, test_string) = suite_case.split(';') test_suite = full_test_suite.split('.')[0] # retrieve main part of test suite name # Immediately skip fully-ignored test suites @@ -128,10 +128,10 @@ def analyze_driver_vs_reference(results: Results, outcomes, if name_matches_pattern(test_string, str_or_re): ignored = True - if not ignored and not key in driver_outcomes['successes']: - results.error("PASS -> SKIP/FAIL: {}", key) - if ignored and key in driver_outcomes['successes']: - results.error("uselessly ignored: {}", key) + if not ignored and not suite_case in driver_outcomes['successes']: + results.error("PASS -> SKIP/FAIL: {}", suite_case) + if ignored and suite_case in driver_outcomes['successes']: + results.error("uselessly ignored: {}", suite_case) def analyze_outcomes(results: Results, outcomes, args): """Run all analyses on the given outcome collection.""" @@ -141,22 +141,31 @@ def analyze_outcomes(results: Results, outcomes, args): def read_outcome_file(outcome_file): """Parse an outcome file and return an outcome collection. -An outcome collection is a dictionary mapping keys to TestCaseOutcomes objects. -The keys are the test suite name and the test case description, separated -by a semicolon. +An outcome collection is a dictionary presentation of the outcome file: +``` +outcomes = { + "": { + "successes": frozenset(["", ... ]), + "failures": frozenset(["", ...]) + } + ... +} +suite_case = ";" +``` """ outcomes = {} with open(outcome_file, 'r', encoding='utf-8') as input_file: for line in input_file: (_platform, config, suite, case, result, _cause) = line.split(';') - key = ';'.join([suite, case]) + suite_case = ';'.join([suite, case]) if config not in outcomes: outcomes[config] = {"successes":[], "failures":[]} if result == 'PASS': - outcomes[config]['successes'].append(key) + outcomes[config]['successes'].append(suite_case) elif result == 'FAIL': - outcomes[config]['failures'].append(key) + outcomes[config]['failures'].append(suite_case) + # Convert `list` to `frozenset` to improve search performance for config in outcomes: outcomes[config]['successes'] = frozenset(outcomes[config]['successes']) outcomes[config]['failures'] = frozenset(outcomes[config]['failures']) From 34915fac3a5f0d051029501f007f5e24b45479e4 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 23 Nov 2023 17:20:19 +0100 Subject: [PATCH 07/41] ssl-opt.sh: Fix getting the list of supported ciphersuites. Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 42f9f5e5a7..b3f8ed4a4f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -358,7 +358,7 @@ requires_protocol_version() { # Space-separated list of ciphersuites supported by this build of # Mbed TLS. -P_CIPHERSUITES=" $($P_CLI --help 2>/dev/null | +P_CIPHERSUITES=" $($P_CLI help_ciphersuites 2>/dev/null | grep 'TLS-\|TLS1-3' | tr -s ' \n' ' ')" requires_ciphersuite_enabled() { From f25d83112389ec4b4cc23ae6c005c56fec53841f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 23 Nov 2023 18:49:43 +0000 Subject: [PATCH 08/41] Ensure mutex test mutex gets free'd Signed-off-by: Paul Elliott --- programs/ssl/ssl_test_lib.c | 3 +++ tests/include/test/helpers.h | 12 ++++++++++-- tests/src/threading_helpers.c | 10 ++++++++++ tests/suites/host_test.function | 4 ++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_test_lib.c b/programs/ssl/ssl_test_lib.c index 6e0c6153f1..b49dd67c26 100644 --- a/programs/ssl/ssl_test_lib.c +++ b/programs/ssl/ssl_test_lib.c @@ -435,6 +435,9 @@ int test_hooks_failure_detected(void) void test_hooks_free(void) { +#if defined(MBEDTLS_TEST_MUTEX_USAGE) + mbedtls_test_mutex_usage_end(); +#endif } #endif /* MBEDTLS_TEST_HOOKS */ diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index ba117fbdfc..4708df1632 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -240,10 +240,18 @@ int mbedtls_test_hexcmp(uint8_t *a, uint8_t *b, #endif #if defined(MBEDTLS_TEST_MUTEX_USAGE) -/** Permanently activate the mutex usage verification framework. See - * threading_helpers.c for information. */ +/** + * Activate the mutex usage verification framework. See threading_helpers.c for + * information. + * */ void mbedtls_test_mutex_usage_init(void); +/** + * Deactivate the mutex usage verification framework. See threading_helpers.c + * for information. + */ +void mbedtls_test_mutex_usage_end(void); + /** Call this function after executing a test case to check for mutex usage * errors. */ void mbedtls_test_mutex_usage_check(void); diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index 385a079261..434d124f18 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -228,4 +228,14 @@ void mbedtls_test_mutex_usage_check(void) mbedtls_test_info.mutex_usage_error = NULL; } +void mbedtls_test_mutex_usage_end(void) +{ + mbedtls_mutex_init = mutex_functions.init; + mbedtls_mutex_free = mutex_functions.free; + mbedtls_mutex_lock = mutex_functions.lock; + mbedtls_mutex_unlock = mutex_functions.unlock; + + mutex_functions.free(&mbedtls_test_mutex_mutex); +} + #endif /* MBEDTLS_TEST_MUTEX_USAGE */ diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index d8ff49ef17..cc286973cf 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -772,6 +772,10 @@ int execute_tests(int argc, const char **argv) mbedtls_fprintf(stdout, " (%u / %u tests (%u skipped))\n", total_tests - total_errors, total_tests, total_skipped); +#if defined(MBEDTLS_TEST_MUTEX_USAGE) + mbedtls_test_mutex_usage_end(); +#endif + #if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) && \ !defined(TEST_SUITE_MEMORY_BUFFER_ALLOC) #if defined(MBEDTLS_MEMORY_DEBUG) From 8c6d332c44bd4a212119c278f3f8a21fb420257d Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 23 Nov 2023 18:53:13 +0000 Subject: [PATCH 09/41] Fix comment typos Signed-off-by: Paul Elliott --- library/threading.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/threading.c b/library/threading.c index d97f0cfe78..873b5077b8 100644 --- a/library/threading.c +++ b/library/threading.c @@ -59,9 +59,9 @@ static void threading_mutex_init_pthread(mbedtls_threading_mutex_t *mutex) /* One problem here is that calling lock on a pthread mutex without first * having initialised it is undefined behaviour. Obviously we cannot check * this here in a thread safe manner without a significant performance - * hit, so state transitions are checked in tests only via the is_valid - * varaible. Please make sure any new mutex that gets added is exercised in - * tests; see tests/src/threading_helpers for more details. */ + * hit, so state transitions are checked in tests only via the state + * variable. Please make sure any new mutex that gets added is exercised in + * tests; see tests/src/threading_helpers.c for more details. */ (void) pthread_mutex_init(&mutex->mutex, NULL); } From 392ed3fe7fe9d0ed7769fbb3ad9a6329114a7e1e Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 24 Nov 2023 15:48:28 +0000 Subject: [PATCH 10/41] Add better documentation for mbedtls_test_mutex_mutex Signed-off-by: Paul Elliott --- tests/src/threading_helpers.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/src/threading_helpers.c b/tests/src/threading_helpers.c index 434d124f18..5fbf65b2da 100644 --- a/tests/src/threading_helpers.c +++ b/tests/src/threading_helpers.c @@ -77,12 +77,30 @@ typedef struct { } mutex_functions_t; static mutex_functions_t mutex_functions; +/** + * The mutex used to guard live_mutexes below and access to the status variable + * in every mbedtls_threading_mutex_t. + * Note that we are not reporting any errors when locking and unlocking this + * mutex. This is for a couple of reasons: + * + * 1. We have no real way of reporting any errors with this mutex - we cannot + * report it back to the caller, as the failure was not that of the mutex + * passed in. We could fail the test, but again this would indicate a problem + * with the test code that did not exist. + * + * 2. Any failure to lock is unlikely to be intermittent, and will thus not + * give false test results - the overall result would be to turn off the + * testing. This is not a situation that is likely to happen with normal + * testing and we still have TSan to fall back on should this happen. + */ mbedtls_threading_mutex_t mbedtls_test_mutex_mutex; -/** The total number of calls to mbedtls_mutex_init(), minus the total number - * of calls to mbedtls_mutex_free(). +/** + * The total number of calls to mbedtls_mutex_init(), minus the total number + * of calls to mbedtls_mutex_free(). * - * Reset to 0 after each test case. + * Do not read or write without holding mbedtls_test_mutex_mutex (above). Reset + * to 0 after each test case. */ static int live_mutexes; From dd1d6a7cca72bd65ac54dba85b1e7bf9a2f4cef3 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Mon, 27 Nov 2023 17:57:31 +0800 Subject: [PATCH 11/41] Improve readability of the script Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 0baba1b7e9..4d13676089 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -64,7 +64,7 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): available = check_test_cases.collect_available_test_cases() for suite_case in available: hits = 0 - for _comp, comp_outcomes in outcomes.items(): + for comp_outcomes in outcomes.values(): if suite_case in comp_outcomes["successes"] or \ suite_case in comp_outcomes["failures"]: hits += 1 @@ -96,8 +96,8 @@ def name_matches_pattern(name, str_or_re): def analyze_driver_vs_reference(results: Results, outcomes, component_ref, component_driver, ignored_suites, ignored_tests=None): - """Check that all tests passed in the reference component are also - passed in the corresponding driver component. + """Check that all tests passing in the reference component are also + passing in the corresponding driver component. Skip: - full test suites provided in ignored_suites list - only some specific test inside a test suite, for which the corresponding @@ -144,7 +144,7 @@ def read_outcome_file(outcome_file): An outcome collection is a dictionary presentation of the outcome file: ``` outcomes = { - "": { + "": { "successes": frozenset(["", ... ]), "failures": frozenset(["", ...]) } @@ -156,19 +156,19 @@ suite_case = ";" outcomes = {} with open(outcome_file, 'r', encoding='utf-8') as input_file: for line in input_file: - (_platform, config, suite, case, result, _cause) = line.split(';') + (_platform, component, suite, case, result, _cause) = line.split(';') suite_case = ';'.join([suite, case]) - if config not in outcomes: - outcomes[config] = {"successes":[], "failures":[]} + if component not in outcomes: + outcomes[component] = {"successes":[], "failures":[]} if result == 'PASS': - outcomes[config]['successes'].append(suite_case) + outcomes[component]['successes'].append(suite_case) elif result == 'FAIL': - outcomes[config]['failures'].append(suite_case) + outcomes[component]['failures'].append(suite_case) # Convert `list` to `frozenset` to improve search performance - for config in outcomes: - outcomes[config]['successes'] = frozenset(outcomes[config]['successes']) - outcomes[config]['failures'] = frozenset(outcomes[config]['failures']) + for component in outcomes: + outcomes[component]['successes'] = frozenset(outcomes[component]['successes']) + outcomes[component]['failures'] = frozenset(outcomes[component]['failures']) return outcomes @@ -489,9 +489,9 @@ def main(): KNOWN_TASKS['analyze_coverage']['args']['full_coverage'] = options.full_coverage - # If the outcome file already exists, we assume that the user wants to - # perform the comparison. - # Share the contents among tasks to improve performance. + # If the outcome file exists, parse it once and share the result + # among tasks to improve performance. + # Otherwise, it will be generated by do_analyze_driver_vs_reference. if os.path.exists(options.outcomes): main_results.info("Read outcome file from {}.", options.outcomes) outcomes_or_file = read_outcome_file(options.outcomes) From f28cf594b1f297e2d6354c3de6f85d0ea2a32dca Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 10:56:29 +0800 Subject: [PATCH 12/41] Break the loop when case hits We don't care about the number of hits of the test cases, so break the iteration when the case hits. Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 4d13676089..488c96bbad 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -68,6 +68,7 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): if suite_case in comp_outcomes["successes"] or \ suite_case in comp_outcomes["failures"]: hits += 1 + break if hits == 0 and suite_case not in allow_list: if full_coverage: From 59b9efc6dd89e761ac961798123363774c8e074d Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 11:15:00 +0800 Subject: [PATCH 13/41] Check if driver_component is missing Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 488c96bbad..2515b309e9 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -107,7 +107,11 @@ def analyze_driver_vs_reference(results: Results, outcomes, ref_outcomes = outcomes.get("component_" + component_ref) driver_outcomes = outcomes.get("component_" + component_driver) - if ref_outcomes is None or not ref_outcomes['successes']: + if ref_outcomes is None or driver_outcomes is None: + results.error("required components are missing: bad outcome file?") + return + + if not ref_outcomes['successes']: results.error("no passing test in reference component: bad outcome file?") return From 28ae4648a61504acdfde9758e81368f8a7ec54bd Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 11:35:19 +0800 Subject: [PATCH 14/41] Use mutable set all the time Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 2515b309e9..890c70dd64 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -164,16 +164,11 @@ suite_case = ";" (_platform, component, suite, case, result, _cause) = line.split(';') suite_case = ';'.join([suite, case]) if component not in outcomes: - outcomes[component] = {"successes":[], "failures":[]} + outcomes[component] = {"successes":set(), "failures":set()} if result == 'PASS': - outcomes[component]['successes'].append(suite_case) + outcomes[component]['successes'].add(suite_case) elif result == 'FAIL': - outcomes[component]['failures'].append(suite_case) - - # Convert `list` to `frozenset` to improve search performance - for component in outcomes: - outcomes[component]['successes'] = frozenset(outcomes[component]['successes']) - outcomes[component]['failures'] = frozenset(outcomes[component]['failures']) + outcomes[component]['failures'].add(suite_case) return outcomes From 18908ec2767a1557b908137ad37ebecc52eca932 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 12:11:52 +0800 Subject: [PATCH 15/41] Define named tuple for component outcomes Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 42 +++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 890c70dd64..b52952458b 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -12,9 +12,14 @@ import traceback import re import subprocess import os +import typing import check_test_cases +ComponentOutcomes = typing.NamedTuple('ComponentOutcomes', + [('successes', typing.Set[str]), + ('failures', typing.Set[str])]) + class Results: """Process analysis results.""" @@ -65,8 +70,8 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): for suite_case in available: hits = 0 for comp_outcomes in outcomes.values(): - if suite_case in comp_outcomes["successes"] or \ - suite_case in comp_outcomes["failures"]: + if suite_case in comp_outcomes.successes or \ + suite_case in comp_outcomes.failures: hits += 1 break @@ -111,11 +116,11 @@ def analyze_driver_vs_reference(results: Results, outcomes, results.error("required components are missing: bad outcome file?") return - if not ref_outcomes['successes']: + if not ref_outcomes.successes: results.error("no passing test in reference component: bad outcome file?") return - for suite_case in ref_outcomes["successes"]: + for suite_case in ref_outcomes.successes: # suite_case is like "test_suite_foo.bar;Description of test case" (full_test_suite, test_string) = suite_case.split(';') test_suite = full_test_suite.split('.')[0] # retrieve main part of test suite name @@ -133,9 +138,9 @@ def analyze_driver_vs_reference(results: Results, outcomes, if name_matches_pattern(test_string, str_or_re): ignored = True - if not ignored and not suite_case in driver_outcomes['successes']: + if not ignored and not suite_case in driver_outcomes.successes: results.error("PASS -> SKIP/FAIL: {}", suite_case) - if ignored and suite_case in driver_outcomes['successes']: + if ignored and suite_case in driver_outcomes.successes: results.error("uselessly ignored: {}", suite_case) def analyze_outcomes(results: Results, outcomes, args): @@ -149,12 +154,23 @@ def read_outcome_file(outcome_file): An outcome collection is a dictionary presentation of the outcome file: ``` outcomes = { - "": { - "successes": frozenset(["", ... ]), - "failures": frozenset(["", ...]) - } + "": ComponentOutcomes, ... } + +CompoentOutcomes is a named tuple which is defined as: + +ComponentOutcomes( + successes = { + , + ... + }, + failures = { + , + ... + } +) + suite_case = ";" ``` """ @@ -164,11 +180,11 @@ suite_case = ";" (_platform, component, suite, case, result, _cause) = line.split(';') suite_case = ';'.join([suite, case]) if component not in outcomes: - outcomes[component] = {"successes":set(), "failures":set()} + outcomes[component] = ComponentOutcomes(set(), set()) if result == 'PASS': - outcomes[component]['successes'].add(suite_case) + outcomes[component].successes.add(suite_case) elif result == 'FAIL': - outcomes[component]['failures'].add(suite_case) + outcomes[component].failures.add(suite_case) return outcomes From da3c206ebde6c29904fb46a61ec7534f90c0d08e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 28 Nov 2023 14:28:03 +0800 Subject: [PATCH 16/41] fix build warning with arm64 gcc 5.4 GCC 5.4 reports below warning on Arm64 ``` warning: 'vst1q_u8' is static but used in inline function 'mbedtls_xor' which is not static ``` This inline function miss `static`, others have the keyword Signed-off-by: Jerry Yu --- library/common.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/common.h b/library/common.h index c20f6b260e..ec30a7da1b 100644 --- a/library/common.h +++ b/library/common.h @@ -165,7 +165,10 @@ static inline const unsigned char *mbedtls_buffer_offset_const( * \param b Pointer to input (buffer of at least \p n bytes) * \param n Number of bytes to process. */ -inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned char *b, size_t n) +static inline void mbedtls_xor(unsigned char *r, + const unsigned char *a, + const unsigned char *b, + size_t n) { size_t i = 0; #if defined(MBEDTLS_EFFICIENT_UNALIGNED_ACCESS) From 20e3ca391ed30347ed611e9bfe83600f3455ed4d Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 15:30:03 +0800 Subject: [PATCH 17/41] Run tests for ref_vs_driver outside task function Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 45 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index b52952458b..4e925a18e4 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -50,11 +50,7 @@ def execute_reference_driver_tests(results: Results, ref_component, driver_compo """Run the tests specified in ref_component and driver_component. Results are stored in the output_file and they will be used for the following coverage analysis""" - # If the outcome file already exists, we assume that the user wants to - # perform the comparison analysis again without repeating the tests. - if os.path.exists(outcome_file): - results.info("Outcome file ({}) already exists. Tests will be skipped.", outcome_file) - return + results.new_section("Test {} and {}", ref_component, driver_component) shell_command = "tests/scripts/all.sh --outcome-file " + outcome_file + \ " " + ref_component + " " + driver_component @@ -188,27 +184,18 @@ suite_case = ";" return outcomes -def do_analyze_coverage(results: Results, outcomes_or_file, args): +def do_analyze_coverage(results: Results, outcomes, args): """Perform coverage analysis.""" results.new_section("Analyze coverage") - outcomes = read_outcome_file(outcomes_or_file) \ - if isinstance(outcomes_or_file, str) else outcomes_or_file analyze_outcomes(results, outcomes, args) -def do_analyze_driver_vs_reference(results: Results, outcomes_or_file, args): +def do_analyze_driver_vs_reference(results: Results, outcomes, args): """Perform driver vs reference analyze.""" results.new_section("Analyze driver {} vs reference {}", args['component_driver'], args['component_ref']) ignored_suites = ['test_suite_' + x for x in args['ignored_suites']] - if isinstance(outcomes_or_file, str): - execute_reference_driver_tests(results, args['component_ref'], \ - args['component_driver'], outcomes_or_file) - outcomes = read_outcome_file(outcomes_or_file) - else: - outcomes = outcomes_or_file - analyze_driver_vs_reference(results, outcomes, args['component_ref'], args['component_driver'], ignored_suites, args['ignored_tests']) @@ -507,17 +494,29 @@ def main(): # If the outcome file exists, parse it once and share the result # among tasks to improve performance. - # Otherwise, it will be generated by do_analyze_driver_vs_reference. - if os.path.exists(options.outcomes): - main_results.info("Read outcome file from {}.", options.outcomes) - outcomes_or_file = read_outcome_file(options.outcomes) - else: - outcomes_or_file = options.outcomes + # Otherwise, it will be generated by execute_reference_driver_tests. + if not os.path.exists(options.outcomes): + if len(tasks_list) > 1: + sys.stderr.write("mutiple tasks found, please provide a valid outcomes file.\n") + sys.exit(2) + + task_name = tasks_list[0] + task = KNOWN_TASKS[task_name] + if task['test_function'] != do_analyze_driver_vs_reference: + sys.stderr.write("please provide valid outcomes file for {}.\n".format(task_name)) + sys.exit(2) + + execute_reference_driver_tests(main_results, + task['args']['component_ref'], + task['args']['component_driver'], + options.outcomes) + + outcomes = read_outcome_file(options.outcomes) for task in tasks_list: test_function = KNOWN_TASKS[task]['test_function'] test_args = KNOWN_TASKS[task]['args'] - test_function(main_results, outcomes_or_file, test_args) + test_function(main_results, outcomes, test_args) main_results.info("Overall results: {} warnings and {} errors", main_results.warning_count, main_results.error_count) From c2e8f3a0800d720ea92a93e5c1911e9691694a3b Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 17:22:04 +0800 Subject: [PATCH 18/41] Add type annotations to analyze_outcomes.py Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 74 +++++++++++++++---------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 4e925a18e4..018d941113 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -16,10 +16,32 @@ import typing import check_test_cases + +# `CompoentOutcomes` is a named tuple which is defined as: +# ComponentOutcomes( +# successes = { +# "", +# ... +# }, +# failures = { +# "", +# ... +# } +# ) +# suite_case = ";" ComponentOutcomes = typing.NamedTuple('ComponentOutcomes', [('successes', typing.Set[str]), ('failures', typing.Set[str])]) +# `Outcomes` is a representation of the outcomes file, +# which defined as: +# Outcomes = { +# "": ComponentOutcomes, +# ... +# } +Outcomes = typing.Dict[str, ComponentOutcomes] + + class Results: """Process analysis results.""" @@ -45,8 +67,8 @@ class Results: def _print_line(fmt, *args, **kwargs): sys.stderr.write((fmt + '\n').format(*args, **kwargs)) -def execute_reference_driver_tests(results: Results, ref_component, driver_component, \ - outcome_file): +def execute_reference_driver_tests(results: Results, ref_component: str, driver_component: str, \ + outcome_file: str) -> None: """Run the tests specified in ref_component and driver_component. Results are stored in the output_file and they will be used for the following coverage analysis""" @@ -60,7 +82,8 @@ def execute_reference_driver_tests(results: Results, ref_component, driver_compo if ret_val != 0: results.error("failed to run reference/driver components") -def analyze_coverage(results, outcomes, allow_list, full_coverage): +def analyze_coverage(results: Results, outcomes: Outcomes, + allow_list: typing.List[str], full_coverage: bool) -> None: """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() for suite_case in available: @@ -83,7 +106,7 @@ def analyze_coverage(results, outcomes, allow_list, full_coverage): else: results.warning('Allow listed test case was executed: {}', suite_case) -def name_matches_pattern(name, str_or_re): +def name_matches_pattern(name: str, str_or_re) -> bool: """Check if name matches a pattern, that may be a string or regex. - If the pattern is a string, name must be equal to match. - If the pattern is a regex, name must fully match. @@ -91,13 +114,13 @@ def name_matches_pattern(name, str_or_re): # The CI's python is too old for re.Pattern #if isinstance(str_or_re, re.Pattern): if not isinstance(str_or_re, str): - return str_or_re.fullmatch(name) + return str_or_re.fullmatch(name) is not None else: return str_or_re == name -def analyze_driver_vs_reference(results: Results, outcomes, - component_ref, component_driver, - ignored_suites, ignored_tests=None): +def analyze_driver_vs_reference(results: Results, outcomes: Outcomes, + component_ref: str, component_driver: str, + ignored_suites: typing.List[str], ignored_tests=None) -> None: """Check that all tests passing in the reference component are also passing in the corresponding driver component. Skip: @@ -139,37 +162,14 @@ def analyze_driver_vs_reference(results: Results, outcomes, if ignored and suite_case in driver_outcomes.successes: results.error("uselessly ignored: {}", suite_case) -def analyze_outcomes(results: Results, outcomes, args): +def analyze_outcomes(results: Results, outcomes: Outcomes, args) -> None: """Run all analyses on the given outcome collection.""" analyze_coverage(results, outcomes, args['allow_list'], args['full_coverage']) -def read_outcome_file(outcome_file): +def read_outcome_file(outcome_file: str) -> Outcomes: """Parse an outcome file and return an outcome collection. - -An outcome collection is a dictionary presentation of the outcome file: -``` -outcomes = { - "": ComponentOutcomes, - ... -} - -CompoentOutcomes is a named tuple which is defined as: - -ComponentOutcomes( - successes = { - , - ... - }, - failures = { - , - ... - } -) - -suite_case = ";" -``` -""" + """ outcomes = {} with open(outcome_file, 'r', encoding='utf-8') as input_file: for line in input_file: @@ -184,12 +184,12 @@ suite_case = ";" return outcomes -def do_analyze_coverage(results: Results, outcomes, args): +def do_analyze_coverage(results: Results, outcomes: Outcomes, args) -> None: """Perform coverage analysis.""" results.new_section("Analyze coverage") analyze_outcomes(results, outcomes, args) -def do_analyze_driver_vs_reference(results: Results, outcomes, args): +def do_analyze_driver_vs_reference(results: Results, outcomes: Outcomes, args) -> None: """Perform driver vs reference analyze.""" results.new_section("Analyze driver {} vs reference {}", args['component_driver'], args['component_ref']) @@ -502,7 +502,7 @@ def main(): task_name = tasks_list[0] task = KNOWN_TASKS[task_name] - if task['test_function'] != do_analyze_driver_vs_reference: + if task['test_function'] != do_analyze_driver_vs_reference: # pylint: disable=comparison-with-callable sys.stderr.write("please provide valid outcomes file for {}.\n".format(task_name)) sys.exit(2) From 451ec8a4bca2eea57304013ade53b403d59a3b5a Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Tue, 28 Nov 2023 17:59:05 +0800 Subject: [PATCH 19/41] Add comment to read_outcome_file in analyze_outcomes.py Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 018d941113..02aac225fc 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -174,6 +174,9 @@ def read_outcome_file(outcome_file: str) -> Outcomes: with open(outcome_file, 'r', encoding='utf-8') as input_file: for line in input_file: (_platform, component, suite, case, result, _cause) = line.split(';') + # Note that `component` is not unique. If a test case passes on Linux + # and fails on FreeBSD, it'll end up in both the successes set and + # the failures set. suite_case = ';'.join([suite, case]) if component not in outcomes: outcomes[component] = ComponentOutcomes(set(), set()) From 897bb77c0cfa8a1d37aea77a171c6c88bbcd49a9 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 15 Nov 2023 16:34:20 +0000 Subject: [PATCH 20/41] Update tf-m tests in all.sh for P256-M Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 5c2f1fd791..bd6b966c2e 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -3254,14 +3254,6 @@ common_tfm_config () { # # Enable filesystem I/O for the benefit of PK parse/write tests. echo "#define MBEDTLS_FS_IO" >> "$CONFIG_H" - - # Config adjustments for features that are not supported - # when using only drivers / by p256-m - # - # Disable all the features that auto-enable ECP_LIGHT (see config_adjust_legacy_crypto.h) - scripts/config.py -f "$CRYPTO_CONFIG_H" unset PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_DERIVE - # Disable deterministic ECDSA as p256-m only does randomized - scripts/config.py -f "$CRYPTO_CONFIG_H" unset PSA_WANT_ALG_DETERMINISTIC_ECDSA } # Keep this in sync with component_test_tfm_config() as they are both meant @@ -3271,8 +3263,8 @@ component_test_tfm_config_p256m_driver_accel_ec () { common_tfm_config - # Build crypto library specifying we want to use P256M code for EC operations - make CFLAGS="$ASAN_CFLAGS -DMBEDTLS_PSA_P256M_DRIVER_ENABLED -I../tests/include/spe" LDFLAGS="$ASAN_CFLAGS" + # Build crypto library + make CFLAGS="$ASAN_CFLAGS -I../tests/include/spe" LDFLAGS="$ASAN_CFLAGS" # Make sure any built-in EC alg was not re-enabled by accident (additive config) not grep mbedtls_ecdsa_ library/ecdsa.o @@ -3283,6 +3275,8 @@ component_test_tfm_config_p256m_driver_accel_ec () { not grep mbedtls_rsa_ library/rsa.o not grep mbedtls_dhm_ library/dhm.o not grep mbedtls_mpi_ library/bignum.o + # Check that p256m was built + grep -q p256_ecdsa_verify library/libmbedcrypto.a # Run the tests msg "test: TF-M config + p256m driver + accel ECDH(E)/ECDSA" @@ -3295,9 +3289,16 @@ component_test_tfm_config_p256m_driver_accel_ec () { component_test_tfm_config() { common_tfm_config + # Disable P256M driver, which is on by default, so that analyze_outcomes + # can compare this test with test_tfm_config_p256m_driver_accel_ec + echo "#undef MBEDTLS_PSA_P256M_DRIVER_ENABLED" >> "$CONFIG_H" + msg "build: TF-M config" make CFLAGS='-Werror -Wall -Wextra -I../tests/include/spe' tests + # Check that p256m was not built + not grep p256_ecdsa_verify library/libmbedcrypto.a + msg "test: TF-M config" make test } From be5489ae9835aa7e8ffcad771bcb6050a298be64 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 27 Nov 2023 10:30:03 +0000 Subject: [PATCH 21/41] Simplify test for building P256-M Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index bd6b966c2e..e15fb2afb9 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -3276,7 +3276,7 @@ component_test_tfm_config_p256m_driver_accel_ec () { not grep mbedtls_dhm_ library/dhm.o not grep mbedtls_mpi_ library/bignum.o # Check that p256m was built - grep -q p256_ecdsa_verify library/libmbedcrypto.a + grep -q p256_ecdsa_ library/libmbedcrypto.a # Run the tests msg "test: TF-M config + p256m driver + accel ECDH(E)/ECDSA" @@ -3297,7 +3297,7 @@ component_test_tfm_config() { make CFLAGS='-Werror -Wall -Wextra -I../tests/include/spe' tests # Check that p256m was not built - not grep p256_ecdsa_verify library/libmbedcrypto.a + not grep p256_ecdsa_ library/libmbedcrypto.a msg "test: TF-M config" make test From a326eb990d086e344660aea23928c6dd98c7020c Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 27 Nov 2023 10:56:41 +0000 Subject: [PATCH 22/41] We no longer need to undef ALT defines Signed-off-by: Dave Rodgman --- configs/config-tfm.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 191e4c4f41..4fa08caf62 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -21,16 +21,6 @@ /* MBEDTLS_PSA_CRYPTO_SPM needs third-party files, so disable it. */ #undef MBEDTLS_PSA_CRYPTO_SPM -/* TF-M provides its own dummy implementations to save code size. - * We don't have any way to disable the tests that need these feature, - * so we just keep AES decryption enabled. We will resolve this through - * an official way to disable AES decryption, then this deviation - * will no longer be needed: - * https://github.com/Mbed-TLS/mbedtls/issues/7368 - */ -#undef MBEDTLS_AES_SETKEY_DEC_ALT -#undef MBEDTLS_AES_DECRYPT_ALT - /* Use built-in platform entropy functions (TF-M provides its own). */ #undef MBEDTLS_NO_PLATFORM_ENTROPY From 4edcf693e7d96ef4b6678f7d8b27529f8090d1dd Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 15 Nov 2023 12:23:29 +0000 Subject: [PATCH 23/41] Use latest TF-M config with bare-minimum changes Move all changes local to Mbed TLS into config-tfm.h (except for commenting out a couple of #include's). Signed-off-by: Dave Rodgman --- configs/config-tfm.h | 38 +++ configs/ext/crypto_config_profile_medium.h | 44 ++-- .../tfm_mbedcrypto_config_profile_medium.h | 222 +++++++----------- 3 files changed, 139 insertions(+), 165 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 4fa08caf62..d987b63313 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -28,3 +28,41 @@ * but using the native allocator is faster and works better with * memory management analysis frameworks such as ASan. */ #undef MBEDTLS_MEMORY_BUFFER_ALLOC_C + +// This macro is enabled in TFM Medium but is disabled here because it is +// incompatible with baremetal builds in Mbed TLS. +#undef MBEDTLS_PSA_CRYPTO_STORAGE_C + +// This macro is enabled in TFM Medium but is disabled here because it is +// incompatible with baremetal builds in Mbed TLS. +#undef MBEDTLS_ENTROPY_NV_SEED + +// These platform-related TF-M settings are not useful here. +#undef MBEDTLS_PLATFORM_NO_STD_FUNCTIONS +#undef MBEDTLS_PLATFORM_STD_MEM_HDR +#undef MBEDTLS_PLATFORM_SNPRINTF_MACRO +#undef MBEDTLS_PLATFORM_PRINTF_ALT +#undef MBEDTLS_PLATFORM_STD_EXIT_SUCCESS +#undef MBEDTLS_PLATFORM_STD_EXIT_FAILURE + +// We expect TF-M to pick this up soon +#define MBEDTLS_BLOCK_CIPHER_NO_DECRYPT + +/*********************************************************************** + * Local changes to crypto config below this delimiter + **********************************************************************/ + +/* Between Mbed TLS 3.4 and 3.5, the PSA_WANT_KEY_TYPE_RSA_KEY_PAIR macro + * (commented-out above) has been replaced with the following new macros: */ +//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC 1 +//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT 1 +//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT 1 +//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE 1 +//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_DERIVE 1 /* Not supported */ + +/* Between Mbed TLS 3.4 and 3.5, the following macros have been added: */ +//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_BASIC 1 +//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_IMPORT 1 +//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_EXPORT 1 +//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_GENERATE 1 +//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_DERIVE 1 // Not supported diff --git a/configs/ext/crypto_config_profile_medium.h b/configs/ext/crypto_config_profile_medium.h index 682835a064..63ed4701de 100644 --- a/configs/ext/crypto_config_profile_medium.h +++ b/configs/ext/crypto_config_profile_medium.h @@ -50,7 +50,7 @@ //#define PSA_WANT_ALG_CFB 1 //#define PSA_WANT_ALG_CHACHA20_POLY1305 1 //#define PSA_WANT_ALG_CTR 1 -#define PSA_WANT_ALG_DETERMINISTIC_ECDSA 1 +//#define PSA_WANT_ALG_DETERMINISTIC_ECDSA 1 //#define PSA_WANT_ALG_ECB_NO_PADDING 1 #define PSA_WANT_ALG_ECDH 1 #define PSA_WANT_ALG_ECDSA 1 @@ -105,33 +105,27 @@ //#define PSA_WANT_KEY_TYPE_CAMELLIA 1 //#define PSA_WANT_KEY_TYPE_CHACHA20 1 //#define PSA_WANT_KEY_TYPE_DES 1 -#define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_BASIC 1 +//#define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR 1 /* Deprecated */ +#define PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY 1 +#define PSA_WANT_KEY_TYPE_RAW_DATA 1 +//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR 1 /* Deprecated */ +//#define PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY 1 + +/* + * The following symbols extend and deprecate the legacy + * PSA_WANT_KEY_TYPE_xxx_KEY_PAIR ones. They include the usage of that key in + * the name's suffix. "_USE" is the most generic and it can be used to describe + * a generic suport, whereas other ones add more features on top of that and + * they are more specific. + */ +#define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_BASIC 1 #define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_IMPORT 1 #define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_EXPORT 1 #define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_GENERATE 1 -#define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_DERIVE 1 -#define PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY 1 -#define PSA_WANT_KEY_TYPE_RAW_DATA 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR 1 -//#define PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY 1 +//#define PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_DERIVE 1 -/*********************************************************************** - * Local edits below this delimiter - **********************************************************************/ - -/* Between Mbed TLS 3.4 and 3.5, the PSA_WANT_KEY_TYPE_RSA_KEY_PAIR macro - * (commented-out above) has been replaced with the following new macros: */ -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_DERIVE 1 /* Not supported */ - -/* Between Mbed TLS 3.4 and 3.5, the following macros have been added: */ -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_BASIC 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_IMPORT 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_EXPORT 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_GENERATE 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_DERIVE 1 // Not supported +#ifdef CRYPTO_HW_ACCELERATOR +#include "crypto_accelerator_config.h" +#endif #endif /* PROFILE_M_PSA_CRYPTO_CONFIG_H */ diff --git a/configs/ext/tfm_mbedcrypto_config_profile_medium.h b/configs/ext/tfm_mbedcrypto_config_profile_medium.h index 53243dd938..c435b5957f 100644 --- a/configs/ext/tfm_mbedcrypto_config_profile_medium.h +++ b/configs/ext/tfm_mbedcrypto_config_profile_medium.h @@ -8,13 +8,29 @@ * memory footprint. */ /* - * Copyright The Mbed TLS Contributors - * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + * Copyright (C) 2006-2023, 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) */ #ifndef PROFILE_M_MBEDTLS_CONFIG_H #define PROFILE_M_MBEDTLS_CONFIG_H +//#include "config_tfm.h" + #if defined(_MSC_VER) && !defined(_CRT_SECURE_NO_DEPRECATE) #define _CRT_SECURE_NO_DEPRECATE 1 #endif @@ -80,44 +96,6 @@ * \{ */ -/** - * \def MBEDTLS_MD2_PROCESS_ALT - * - * MBEDTLS__FUNCTION_NAME__ALT: Uncomment a macro to let mbed TLS use you - * alternate core implementation of symmetric crypto or hash function. Keep in - * mind that function prototypes should remain the same. - * - * This replaces only one function. The header file from mbed TLS is still - * used, in contrast to the MBEDTLS__MODULE_NAME__ALT flags. - * - * Example: In case you uncomment MBEDTLS_SHA256_PROCESS_ALT, mbed TLS will - * no longer provide the mbedtls_sha1_process() function, but it will still provide - * the other function (using your mbedtls_sha1_process() function) and the definition - * of mbedtls_sha1_context, so your implementation of mbedtls_sha1_process must be compatible - * with this definition. - * - * \note Because of a signature change, the core AES encryption and decryption routines are - * currently named mbedtls_aes_internal_encrypt and mbedtls_aes_internal_decrypt, - * respectively. When setting up alternative implementations, these functions should - * be overridden, but the wrapper functions mbedtls_aes_decrypt and mbedtls_aes_encrypt - * must stay untouched. - * - * \note If you use the AES_xxx_ALT macros, then is is recommended to also set - * MBEDTLS_AES_ROM_TABLES in order to help the linker garbage-collect the AES - * tables. - * - * Uncomment a macro to enable alternate implementation of the corresponding - * function. - * - * \warning MD2, MD4, MD5, DES and SHA-1 are considered weak and their use - * constitutes a security risk. If possible, we recommend avoiding - * dependencies on them, and considering stronger message digests - * and ciphers instead. - * - */ -#define MBEDTLS_AES_SETKEY_DEC_ALT -#define MBEDTLS_AES_DECRYPT_ALT - /** * \def MBEDTLS_AES_ROM_TABLES * @@ -171,21 +149,6 @@ */ #define MBEDTLS_ECP_NIST_OPTIM -/** - * \def MBEDTLS_ERROR_STRERROR_DUMMY - * - * Enable a dummy error function to make use of mbedtls_strerror() in - * third party libraries easier when MBEDTLS_ERROR_C is disabled - * (no effect when MBEDTLS_ERROR_C is enabled). - * - * You can safely disable this if MBEDTLS_ERROR_C is enabled, or if you're - * not using mbedtls_strerror() or error_strerror() in your application. - * - * Disable if you run into name conflicts and want to really remove the - * mbedtls_strerror() - */ -#define MBEDTLS_ERROR_STRERROR_DUMMY - /** * \def MBEDTLS_NO_PLATFORM_ENTROPY * @@ -223,26 +186,7 @@ * \note The entropy collector will write to the seed file before entropy is * given to an external source, to update it. */ -// This macro is enabled in TFM Medium but is disabled here because it is -// incompatible with baremetal builds in Mbed TLS. -//#define MBEDTLS_ENTROPY_NV_SEED - -/* MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER - * - * Enable key identifiers that encode a key owner identifier. - * - * This is only meaningful when building the library as part of a - * multi-client service. When you activate this option, you must provide an - * implementation of the type mbedtls_key_owner_id_t and a translation from - * mbedtls_svc_key_id_t to file name in all the storage backends that you - * you wish to support. - * - * Note that while this define has been removed from TF-M's copy of this config - * file, TF-M still passes this option to Mbed TLS during the build via CMake. - * Therefore we keep it in our copy. See discussion on PR #7426 for more info. - * - */ -#define MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER +#define MBEDTLS_ENTROPY_NV_SEED /** * \def MBEDTLS_PSA_CRYPTO_SPM @@ -326,26 +270,21 @@ #define MBEDTLS_AES_C /** - * \def MBEDTLS_BLOCK_CIPHER_NO_DECRYPT + * \def MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH * - * Remove decryption operation for AES, ARIA and Camellia block cipher. + * Use only 128-bit keys in AES operations to save ROM. * - * \note This feature is incompatible with insecure block cipher, - * MBEDTLS_DES_C, and cipher modes which always require decryption - * operation, MBEDTLS_CIPHER_MODE_CBC, MBEDTLS_CIPHER_MODE_XTS and - * MBEDTLS_NIST_KW_C. When #MBEDTLS_PSA_CRYPTO_CONFIG is enabled, - * this feature is incompatible with following supported PSA equivalence, - * PSA_WANT_ALG_ECB_NO_PADDING, PSA_WANT_ALG_CBC_NO_PADDING, - * PSA_WANT_ALG_CBC_PKCS7 and PSA_WANT_KEY_TYPE_DES. + * Uncomment this macro to remove support for AES operations that use 192- + * or 256-bit keys. + * + * Uncommenting this macro reduces the size of AES code by ~300 bytes + * on v8-M/Thumb2. * * Module: library/aes.c - * library/aesce.c - * library/aesni.c - * library/aria.c - * library/camellia.c - * library/cipher.c + * + * Requires: MBEDTLS_AES_C */ -#define MBEDTLS_BLOCK_CIPHER_NO_DECRYPT +#define MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH /** * \def MBEDTLS_CIPHER_C @@ -388,18 +327,6 @@ */ #define MBEDTLS_ENTROPY_C -/** - * \def MBEDTLS_ERROR_C - * - * Enable error code to error string conversion. - * - * Module: library/error.c - * Caller: - * - * This module enables mbedtls_strerror(). - */ -#define MBEDTLS_ERROR_C - /** * \def MBEDTLS_HKDF_C * @@ -413,40 +340,7 @@ * This module adds support for the Hashed Message Authentication Code * (HMAC)-based key derivation function (HKDF). */ -#define MBEDTLS_HKDF_C /* Used for HUK deriviation */ - -/** - * \def MBEDTLS_MD_C - * - * Enable the generic layer for message digest (hashing) and HMAC. - * - * Requires: one of: MBEDTLS_MD5_C, MBEDTLS_RIPEMD160_C, MBEDTLS_SHA1_C, - * MBEDTLS_SHA224_C, MBEDTLS_SHA256_C, MBEDTLS_SHA384_C, - * MBEDTLS_SHA512_C, or MBEDTLS_PSA_CRYPTO_C with at least - * one hash. - * Module: library/md.c - * Caller: library/constant_time.c - * library/ecdsa.c - * library/ecjpake.c - * library/hkdf.c - * library/hmac_drbg.c - * library/pk.c - * library/pkcs5.c - * library/pkcs12.c - * library/psa_crypto_ecp.c - * library/psa_crypto_rsa.c - * library/rsa.c - * library/ssl_cookie.c - * library/ssl_msg.c - * library/ssl_tls.c - * library/x509.c - * library/x509_crt.c - * library/x509write_crt.c - * library/x509write_csr.c - * - * Uncomment to enable generic message digest wrappers. - */ -#define MBEDTLS_MD_C +//#define MBEDTLS_HKDF_C /* Used for HUK deriviation */ /** * \def MBEDTLS_MEMORY_BUFFER_ALLOC_C @@ -484,6 +378,15 @@ */ #define MBEDTLS_PLATFORM_C +#define MBEDTLS_PLATFORM_NO_STD_FUNCTIONS +#define MBEDTLS_PLATFORM_STD_MEM_HDR + +#include + +#define MBEDTLS_PLATFORM_SNPRINTF_MACRO snprintf +#define MBEDTLS_PLATFORM_PRINTF_ALT +#define MBEDTLS_PLATFORM_STD_EXIT_SUCCESS EXIT_SUCCESS +#define MBEDTLS_PLATFORM_STD_EXIT_FAILURE EXIT_FAILURE /** * \def MBEDTLS_PSA_CRYPTO_C @@ -508,9 +411,7 @@ * either MBEDTLS_PSA_ITS_FILE_C or a native implementation of * the PSA ITS interface */ -// This macro is enabled in TFM Medium but is disabled here because it is -// incompatible with baremetal builds in Mbed TLS. -//#define MBEDTLS_PSA_CRYPTO_STORAGE_C +#define MBEDTLS_PSA_CRYPTO_STORAGE_C /* \} name SECTION: mbed TLS modules */ @@ -614,6 +515,47 @@ /* ECP options */ #define MBEDTLS_ECP_FIXED_POINT_OPTIM 0 /**< Disable fixed-point speed-up */ +/** + * Uncomment to enable p256-m. This is an alternative implementation of + * key generation, ECDH and (randomized) ECDSA on the curve SECP256R1. + * Compared to the default implementation: + * + * - p256-m has a much smaller code size and RAM footprint. + * - p256-m is only available via the PSA API. This includes the pk module + * when #MBEDTLS_USE_PSA_CRYPTO is enabled. + * - p256-m does not support deterministic ECDSA, EC-JPAKE, custom protocols + * over the core arithmetic, or deterministic derivation of keys. + * + * We recommend enabling this option if your application uses the PSA API + * and the only elliptic curve support it needs is ECDH and ECDSA over + * SECP256R1. + * + * If you enable this option, you do not need to enable any ECC-related + * MBEDTLS_xxx option. You do need to separately request support for the + * cryptographic mechanisms through the PSA API: + * - #MBEDTLS_PSA_CRYPTO_C and #MBEDTLS_PSA_CRYPTO_CONFIG for PSA-based + * configuration; + * - #MBEDTLS_USE_PSA_CRYPTO if you want to use p256-m from PK, X.509 or TLS; + * - #PSA_WANT_ECC_SECP_R1_256; + * - #PSA_WANT_ALG_ECDH and/or #PSA_WANT_ALG_ECDSA as needed; + * - #PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY, #PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_BASIC, + * #PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_IMPORT, + * #PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_EXPORT and/or + * #PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_GENERATE as needed. + * + * \note To benefit from the smaller code size of p256-m, make sure that you + * do not enable any ECC-related option not supported by p256-m: this + * would cause the built-in ECC implementation to be built as well, in + * order to provide the required option. + * Make sure #PSA_WANT_ALG_DETERMINISTIC_ECDSA, #PSA_WANT_ALG_JPAKE and + * #PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_DERIVE, and curves other than + * SECP256R1 are disabled as they are not supported by this driver. + * Also, avoid defining #MBEDTLS_PK_PARSE_EC_COMPRESSED or + * #MBEDTLS_PK_PARSE_EC_EXTENDED as those currently require a subset of + * the built-in ECC implementation, see docs/driver-only-builds.md. + */ +#define MBEDTLS_PSA_P256M_DRIVER_ENABLED + /* \} name SECTION: Customisation configuration options */ #if CRYPTO_NV_SEED @@ -621,7 +563,7 @@ #endif /* CRYPTO_NV_SEED */ #if !defined(CRYPTO_HW_ACCELERATOR) && defined(MBEDTLS_ENTROPY_NV_SEED) -#include "mbedtls_entropy_nv_seed_config.h" +//#include "mbedtls_entropy_nv_seed_config.h" #endif #ifdef CRYPTO_HW_ACCELERATOR From 6632a12fa3e3ab8e0fa7b11c61a4b9b1e7e4c363 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 20 Nov 2023 16:03:56 +0100 Subject: [PATCH 24/41] all.sh: re-enable CCM/GCM in test_full_no_cipher_with_crypto[_config]() Signed-off-by: Valerio Setti --- tests/scripts/all.sh | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 08136c0147..93a32aefee 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1583,9 +1583,7 @@ common_test_full_no_cipher_with_psa_crypto () { # on CIPHER_C so we disable them. # This does not hold for KEY_TYPE_CHACHA20 and ALG_CHACHA20_POLY1305 # so we keep them enabled. - scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_CCM scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_CCM_STAR_NO_TAG - scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_GCM scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_CMAC scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_CBC_NO_PADDING scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_CBC_PKCS7 @@ -1594,27 +1592,19 @@ common_test_full_no_cipher_with_psa_crypto () { scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_ECB_NO_PADDING scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_OFB scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_ALG_STREAM_CIPHER - scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_KEY_TYPE_AES scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_KEY_TYPE_DES - scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_KEY_TYPE_CAMELLIA - scripts/config.py -f $CRYPTO_CONFIG_H unset PSA_WANT_KEY_TYPE_ARIA else # Don't pull in cipher via PSA mechanisms scripts/config.py unset MBEDTLS_PSA_CRYPTO_CONFIG # Disable cipher modes/keys that make PSA depend on CIPHER_C. # Keep CHACHA20 and CHACHAPOLY enabled since they do not depend on CIPHER_C. scripts/config.py unset-all MBEDTLS_CIPHER_MODE - scripts/config.py unset MBEDTLS_AES_C scripts/config.py unset MBEDTLS_DES_C - scripts/config.py unset MBEDTLS_ARIA_C - scripts/config.py unset MBEDTLS_CAMELLIA_C # Dependencies on AES_C scripts/config.py unset MBEDTLS_CTR_DRBG_C fi # The following modules directly depends on CIPHER_C - scripts/config.py unset MBEDTLS_CCM_C scripts/config.py unset MBEDTLS_CMAC_C - scripts/config.py unset MBEDTLS_GCM_C scripts/config.py unset MBEDTLS_NIST_KW_C scripts/config.py unset MBEDTLS_PKCS12_C scripts/config.py unset MBEDTLS_PKCS5_C From b1cf8aeda445757209323b8e3eb864140c83adb8 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 20 Nov 2023 16:24:00 +0100 Subject: [PATCH 25/41] adjust_psa_from_legacy: add required CIPHER_C dependencies Some PSA_WANT symbols do not have a 1:1 matching with legacy ones. For example, previous to this commit: - CCM_C enabled both PSA_WANT_ALG_CCM and PSA_WANT_ALG_CCM_STAR_NO_TAG even thought the two are not equivalent (authenticated VS non-authenticated). - there was no legacy equivalent for ECB_NO_PADDING What it is common to both PSA_WANT_ALG_CCM_STAR_NO_TAG and PSA_WANT_ALG_ECB_NO_PADDING is the fact that the builtin implementation depends on CIPHER_C. Therefore this commits adds this guards to select whether or not to enable the above mentioned PSA_WANT symbols. Signed-off-by: Valerio Setti --- include/mbedtls/config_adjust_psa_from_legacy.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/config_adjust_psa_from_legacy.h b/include/mbedtls/config_adjust_psa_from_legacy.h index 60b00c1e4a..b841875cf8 100644 --- a/include/mbedtls/config_adjust_psa_from_legacy.h +++ b/include/mbedtls/config_adjust_psa_from_legacy.h @@ -25,9 +25,11 @@ #if defined(MBEDTLS_CCM_C) #define MBEDTLS_PSA_BUILTIN_ALG_CCM 1 -#define MBEDTLS_PSA_BUILTIN_ALG_CCM_STAR_NO_TAG 1 #define PSA_WANT_ALG_CCM 1 +#if defined(MBEDTLS_CIPHER_C) +#define MBEDTLS_PSA_BUILTIN_ALG_CCM_STAR_NO_TAG 1 #define PSA_WANT_ALG_CCM_STAR_NO_TAG 1 +#endif /* MBEDTLS_CIPHER_C */ #endif /* MBEDTLS_CCM_C */ #if defined(MBEDTLS_CMAC_C) @@ -247,8 +249,9 @@ #endif #endif -#if defined(MBEDTLS_AES_C) || defined(MBEDTLS_DES_C) || \ - defined(MBEDTLS_ARIA_C) || defined(MBEDTLS_CAMELLIA_C) +#if (defined(MBEDTLS_AES_C) || defined(MBEDTLS_DES_C) || \ + defined(MBEDTLS_ARIA_C) || defined(MBEDTLS_CAMELLIA_C)) && \ + defined(MBEDTLS_CIPHER_C) #define MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING 1 #define PSA_WANT_ALG_ECB_NO_PADDING 1 #endif From 919e3fa729ed71cb075c1af8c2fc030b2067fa03 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 20 Nov 2023 16:30:05 +0100 Subject: [PATCH 26/41] check_config: fix guards for PSA builtin implementation of cipher/AEAD While the PSA builtin implementation of cipher still depends on CIPHER_C, the same is no more true for AEADs. When CIPHER_C is not defined, BLOCK_CIPHER_C is used instead, thus making it possible to support AEADs without CIPHER_C. Signed-off-by: Valerio Setti --- include/mbedtls/check_config.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 792e7d70df..9b5b6467ea 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -785,9 +785,8 @@ #error "MBEDTLS_PSA_CRYPTO_C defined, but not all prerequisites (missing RNG)" #endif -#if defined(MBEDTLS_PSA_CRYPTO_C) && \ - (defined(PSA_HAVE_SOFT_BLOCK_CIPHER) || defined(PSA_HAVE_SOFT_BLOCK_AEAD)) && \ - !defined(MBEDTLS_CIPHER_C) +#if defined(MBEDTLS_PSA_CRYPTO_C) && defined(PSA_HAVE_SOFT_BLOCK_MODE) && \ + defined(PSA_HAVE_SOFT_BLOCK_CIPHER) && !defined(MBEDTLS_CIPHER_C) #error "MBEDTLS_PSA_CRYPTO_C defined, but not all prerequisites" #endif From 5b73de8ddb6db3ffde86f53a87a3059491127186 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 28 Nov 2023 15:49:25 +0100 Subject: [PATCH 27/41] ssl-opt.sh: Add a check of the list of supported ciphersuites Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b3f8ed4a4f..f92642dece 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -358,9 +358,18 @@ requires_protocol_version() { # Space-separated list of ciphersuites supported by this build of # Mbed TLS. -P_CIPHERSUITES=" $($P_CLI help_ciphersuites 2>/dev/null | - grep 'TLS-\|TLS1-3' | - tr -s ' \n' ' ')" +P_CIPHERSUITES="" +if [ "$LIST_TESTS" -eq 0 ]; then + P_CIPHERSUITES=" $($P_CLI help_ciphersuites 2>/dev/null | + grep 'TLS-\|TLS1-3' | + tr -s ' \n' ' ')" + + if [ -z "${P_CIPHERSUITES# }" ]; then + echo >&2 "$0: fatal error: no cipher suites found!" + exit 125 + fi +fi + requires_ciphersuite_enabled() { case $P_CIPHERSUITES in *" $1 "*) :;; From 41bc42ac1b1a11496b202e7f2559c0bd6f664638 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 28 Nov 2023 15:03:57 +0100 Subject: [PATCH 28/41] ssl-opt.sh: Fix some symmetric crypto dependencies Fix some dependencies on symmetric crypto that were not correct in case of driver but not builtin support. Revealed by "Analyze driver test_psa_crypto_config_accel_cipher_aead vs reference test_psa_crypto_config_reference_cipher_aead" in analyze_outcomes.py. Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 56 +++++++++++++++++------------------------------- 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index f92642dece..ac7860c9ad 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2341,7 +2341,7 @@ run_test "Opaque key for server authentication: invalid alg: ecdh with RSA ke requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_hash_alg SHA_256 -requires_config_enabled MBEDTLS_CCM_C +requires_config_enabled PSA_WANT_ALG_CCM run_test "Opaque key for server authentication: invalid alg: ECDHE-ECDSA with ecdh" \ "$P_SRV key_opaque=1 crt_file=data_files/server5.crt \ key_file=data_files/server5.key key_opaque_algs=ecdh,none \ @@ -2395,7 +2395,7 @@ run_test "Opaque keys for server authentication: EC keys with different algs, requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_hash_alg SHA_384 -requires_config_enabled MBEDTLS_CCM_C +requires_config_enabled PSA_WANT_ALG_CCM requires_config_disabled MBEDTLS_X509_REMOVE_INFO run_test "Opaque keys for server authentication: EC + RSA, force ECDHE-ECDSA" \ "$P_SRV key_opaque=1 crt_file=data_files/server5.crt \ @@ -2575,7 +2575,7 @@ requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_384 -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_config_disabled MBEDTLS_X509_REMOVE_INFO run_test "Opaque keys for server authentication: EC + RSA, force DHE-RSA" \ "$P_SRV auth_mode=required key_opaque=1 crt_file=data_files/server5.crt \ @@ -9124,8 +9124,7 @@ run_test "SSL async private: renegotiation: server-initiated, decrypt" \ # Tests for ECC extensions (rfc 4492) -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_KEY_EXCHANGE_RSA_ENABLED run_test "Force a non ECC ciphersuite in the client side" \ @@ -9137,8 +9136,7 @@ run_test "Force a non ECC ciphersuite in the client side" \ -S "found supported elliptic curves extension" \ -S "found supported point formats extension" -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_KEY_EXCHANGE_RSA_ENABLED run_test "Force a non ECC ciphersuite in the server side" \ @@ -9148,8 +9146,7 @@ run_test "Force a non ECC ciphersuite in the server side" \ -C "found supported_point_formats extension" \ -S "server hello, supported_point_formats extension" -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 run_test "Force an ECC ciphersuite in the client side" \ "$P_SRV debug_level=3" \ @@ -9160,8 +9157,7 @@ run_test "Force an ECC ciphersuite in the client side" \ -s "found supported elliptic curves extension" \ -s "found supported point formats extension" -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 run_test "Force an ECC ciphersuite in the server side" \ "$P_SRV debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ @@ -9686,8 +9682,7 @@ run_test "DTLS fragmenting: both (MTU=1024)" \ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: both (MTU=512)" \ -p "$P_PXY mtu=512" \ @@ -9716,8 +9711,7 @@ run_test "DTLS fragmenting: both (MTU=512)" \ not_with_valgrind requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU: auto-reduction (not valgrind)" \ -p "$P_PXY mtu=508" \ @@ -9739,8 +9733,7 @@ run_test "DTLS fragmenting: proxy MTU: auto-reduction (not valgrind)" \ only_with_valgrind requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU: auto-reduction (with valgrind)" \ -p "$P_PXY mtu=508" \ @@ -9791,8 +9784,7 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake (MTU=1024)" \ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, simple handshake (MTU=512)" \ -p "$P_PXY mtu=512" \ @@ -9840,8 +9832,7 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=1024)" \ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=512)" \ -p "$P_PXY mtu=512" \ @@ -9875,8 +9866,7 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=512)" \ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ -p "$P_PXY mtu=1450" \ @@ -9904,7 +9894,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled MBEDTLS_CHACHAPOLY_C +requires_config_enabled PSA_WANT_ALG_CHACHA20_POLY1305 requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ -p "$P_PXY mtu=512" \ @@ -9934,8 +9924,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-GCM renego" \ -p "$P_PXY mtu=512" \ @@ -9965,8 +9954,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CCM_C +requires_config_enabled PSA_WANT_ALG_CCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-CCM renego" \ -p "$P_PXY mtu=1024" \ @@ -9996,8 +9984,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_config_enabled MBEDTLS_SSL_ENCRYPT_THEN_MAC requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-CBC EtM renego" \ @@ -10028,8 +10015,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ -p "$P_PXY mtu=1024" \ @@ -10055,8 +10041,7 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ # Forcing ciphersuite for this test to fit the MTU of 512 with full config. requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM client_needs_more_time 2 requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU + 3d" \ @@ -10078,8 +10063,7 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ # Forcing ciphersuite for this test to fit the MTU of 512 with full config. requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled MBEDTLS_AES_C -requires_config_enabled MBEDTLS_GCM_C +requires_config_enabled PSA_WANT_ALG_GCM client_needs_more_time 2 requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU + 3d, nbio" \ From 82d7a875ff53578e74c0703ef623f63ad7ee37e1 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 28 Nov 2023 10:06:33 +0000 Subject: [PATCH 29/41] Update tests to refer to our tf-m config wrapper Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index e15fb2afb9..177736a51a 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -4123,8 +4123,10 @@ support_build_tfm_armcc () { component_build_tfm_armcc() { # test the TF-M configuration can build cleanly with various warning flags enabled - cp configs/ext/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" - cp configs/ext/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" + cp configs/config-tfm.h "$CONFIG_H" + + # MBEDTLS_NO_PLATFORM_ENTROPY is needed as we are building for baremetal + ./scripts/config.py --force set MBEDTLS_NO_PLATFORM_ENTROPY msg "build: TF-M config, armclang armv7-m thumb2" armc6_build_test "--target=arm-arm-none-eabi -march=armv7-m -mthumb -Os -std=c99 -Werror -Wall -Wextra -Wwrite-strings -Wpointer-arith -Wimplicit-fallthrough -Wshadow -Wvla -Wformat=2 -Wno-format-nonliteral -Wshadow -Wasm-operand-widths -Wunused -I../tests/include/spe" @@ -4136,8 +4138,7 @@ component_build_tfm() { # TF-M configuration needs a TF-M platform. A tweaked version of # the configuration that works on mainstream platforms is in # configs/config-tfm.h, tested via test-ref-configs.pl. - cp configs/ext/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" - cp configs/ext/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" + cp configs/config-tfm.h "$CONFIG_H" msg "build: TF-M config, clang, armv7-m thumb2" make lib CC="clang" CFLAGS="--target=arm-linux-gnueabihf -march=armv7-m -mthumb -Os -std=c99 -Werror -Wall -Wextra -Wwrite-strings -Wpointer-arith -Wimplicit-fallthrough -Wshadow -Wvla -Wformat=2 -Wno-format-nonliteral -Wshadow -Wasm-operand-widths -Wunused -I../tests/include/spe" From c89f7817e174dc7c3fbcf113c0d90dfb86658771 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 28 Nov 2023 13:55:20 +0000 Subject: [PATCH 30/41] Use common license header Signed-off-by: Dave Rodgman --- .../ext/tfm_mbedcrypto_config_profile_medium.h | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/configs/ext/tfm_mbedcrypto_config_profile_medium.h b/configs/ext/tfm_mbedcrypto_config_profile_medium.h index c435b5957f..beebddf5af 100644 --- a/configs/ext/tfm_mbedcrypto_config_profile_medium.h +++ b/configs/ext/tfm_mbedcrypto_config_profile_medium.h @@ -8,22 +8,8 @@ * memory footprint. */ /* - * Copyright (C) 2006-2023, 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) + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later */ #ifndef PROFILE_M_MBEDTLS_CONFIG_H From 29ad2d7609183ff0a82abbfdeb2d979f9b16b039 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 28 Nov 2023 17:43:49 +0100 Subject: [PATCH 31/41] ssl-opt.sh: Remove unnecessary symmetric crypto dependencies Same test cases as in the previous commit. Remove the redundant symmetric crypto dependency. The dependency is ensured by the fact that: 1) the test case forces a cipher suite 2) ssl-opt.sh enforces automatically that the forced ciphersuite is available. 3) The fact that the forced ciphersuite is available implies that the symmetric cipher algorithm it uses is available as well. Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ac7860c9ad..b0d94bde01 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2341,7 +2341,6 @@ run_test "Opaque key for server authentication: invalid alg: ecdh with RSA ke requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_hash_alg SHA_256 -requires_config_enabled PSA_WANT_ALG_CCM run_test "Opaque key for server authentication: invalid alg: ECDHE-ECDSA with ecdh" \ "$P_SRV key_opaque=1 crt_file=data_files/server5.crt \ key_file=data_files/server5.key key_opaque_algs=ecdh,none \ @@ -2395,7 +2394,6 @@ run_test "Opaque keys for server authentication: EC keys with different algs, requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_hash_alg SHA_384 -requires_config_enabled PSA_WANT_ALG_CCM requires_config_disabled MBEDTLS_X509_REMOVE_INFO run_test "Opaque keys for server authentication: EC + RSA, force ECDHE-ECDSA" \ "$P_SRV key_opaque=1 crt_file=data_files/server5.crt \ @@ -2575,7 +2573,6 @@ requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_384 -requires_config_enabled PSA_WANT_ALG_GCM requires_config_disabled MBEDTLS_X509_REMOVE_INFO run_test "Opaque keys for server authentication: EC + RSA, force DHE-RSA" \ "$P_SRV auth_mode=required key_opaque=1 crt_file=data_files/server5.crt \ @@ -9124,7 +9121,6 @@ run_test "SSL async private: renegotiation: server-initiated, decrypt" \ # Tests for ECC extensions (rfc 4492) -requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_KEY_EXCHANGE_RSA_ENABLED run_test "Force a non ECC ciphersuite in the client side" \ @@ -9136,7 +9132,6 @@ run_test "Force a non ECC ciphersuite in the client side" \ -S "found supported elliptic curves extension" \ -S "found supported point formats extension" -requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_KEY_EXCHANGE_RSA_ENABLED run_test "Force a non ECC ciphersuite in the server side" \ @@ -9146,7 +9141,6 @@ run_test "Force a non ECC ciphersuite in the server side" \ -C "found supported_point_formats extension" \ -S "server hello, supported_point_formats extension" -requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 run_test "Force an ECC ciphersuite in the client side" \ "$P_SRV debug_level=3" \ @@ -9157,7 +9151,6 @@ run_test "Force an ECC ciphersuite in the client side" \ -s "found supported elliptic curves extension" \ -s "found supported point formats extension" -requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_hash_alg SHA_256 run_test "Force an ECC ciphersuite in the server side" \ "$P_SRV debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ @@ -9682,7 +9675,6 @@ run_test "DTLS fragmenting: both (MTU=1024)" \ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: both (MTU=512)" \ -p "$P_PXY mtu=512" \ @@ -9711,7 +9703,6 @@ run_test "DTLS fragmenting: both (MTU=512)" \ not_with_valgrind requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU: auto-reduction (not valgrind)" \ -p "$P_PXY mtu=508" \ @@ -9733,7 +9724,6 @@ run_test "DTLS fragmenting: proxy MTU: auto-reduction (not valgrind)" \ only_with_valgrind requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU: auto-reduction (with valgrind)" \ -p "$P_PXY mtu=508" \ @@ -9784,7 +9774,6 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake (MTU=1024)" \ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, simple handshake (MTU=512)" \ -p "$P_PXY mtu=512" \ @@ -9832,7 +9821,6 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=1024)" \ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=512)" \ -p "$P_PXY mtu=512" \ @@ -9866,7 +9854,6 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio (MTU=512)" \ not_with_valgrind # spurious autoreduction due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ -p "$P_PXY mtu=1450" \ @@ -9894,7 +9881,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled PSA_WANT_ALG_CHACHA20_POLY1305 requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ -p "$P_PXY mtu=512" \ @@ -9924,7 +9910,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled PSA_WANT_ALG_GCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-GCM renego" \ -p "$P_PXY mtu=512" \ @@ -9954,7 +9939,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled PSA_WANT_ALG_CCM requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-CCM renego" \ -p "$P_PXY mtu=1024" \ @@ -9984,7 +9968,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_config_enabled MBEDTLS_SSL_ENCRYPT_THEN_MAC requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-CBC EtM renego" \ @@ -10015,7 +9998,6 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_hash_alg SHA_256 requires_config_enabled MBEDTLS_SSL_RENEGOTIATION -requires_config_enabled PSA_WANT_ALG_CBC_NO_PADDING requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ -p "$P_PXY mtu=1024" \ @@ -10041,7 +10023,6 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ # Forcing ciphersuite for this test to fit the MTU of 512 with full config. requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM client_needs_more_time 2 requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU + 3d" \ @@ -10063,7 +10044,6 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ # Forcing ciphersuite for this test to fit the MTU of 512 with full config. requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C -requires_config_enabled PSA_WANT_ALG_GCM client_needs_more_time 2 requires_max_content_len 2048 run_test "DTLS fragmenting: proxy MTU + 3d, nbio" \ From 60f76663c0ffffad28973e2ba2d7fe09ee8c0dfa Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 28 Nov 2023 17:52:42 +0100 Subject: [PATCH 32/41] Align forced ciphersuite with test description Signed-off-by: Ronald Cron --- tests/ssl-opt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b0d94bde01..4762285b00 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9894,7 +9894,7 @@ run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ exchanges=2 renegotiation=1 renegotiate=1 \ - force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-CHACHA20-POLY1305-SHA256 \ hs_timeout=10000-60000 \ mtu=512" \ 0 \ From 550cd6f9b2a5773060ec87926dcdcdd26148d1a3 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Wed, 29 Nov 2023 09:17:59 +0800 Subject: [PATCH 33/41] Use boolean `hit` instead of int `hits` Also fix a typo in the comments. Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 02aac225fc..52059bda09 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -17,7 +17,7 @@ import typing import check_test_cases -# `CompoentOutcomes` is a named tuple which is defined as: +# `ComponentOutcomes` is a named tuple which is defined as: # ComponentOutcomes( # successes = { # "", @@ -87,19 +87,19 @@ def analyze_coverage(results: Results, outcomes: Outcomes, """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() for suite_case in available: - hits = 0 + hit = False for comp_outcomes in outcomes.values(): if suite_case in comp_outcomes.successes or \ suite_case in comp_outcomes.failures: - hits += 1 + hit = True break - if hits == 0 and suite_case not in allow_list: + if hit == 0 and suite_case not in allow_list: if full_coverage: results.error('Test case not executed: {}', suite_case) else: results.warning('Test case not executed: {}', suite_case) - elif hits != 0 and suite_case in allow_list: + elif hit != 0 and suite_case in allow_list: # Test Case should be removed from the allow list. if full_coverage: results.error('Allow listed test case was executed: {}', suite_case) From 5b96b819803d69b2be8b7afb65d500f92faeb98f Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 29 Nov 2023 10:25:00 +0800 Subject: [PATCH 34/41] Revert "fix build warning with arm64 gcc 5.4" This reverts commit da3c206ebde6c29904fb46a61ec7534f90c0d08e. Signed-off-by: Jerry Yu --- library/common.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/common.h b/library/common.h index ec30a7da1b..c20f6b260e 100644 --- a/library/common.h +++ b/library/common.h @@ -165,10 +165,7 @@ static inline const unsigned char *mbedtls_buffer_offset_const( * \param b Pointer to input (buffer of at least \p n bytes) * \param n Number of bytes to process. */ -static inline void mbedtls_xor(unsigned char *r, - const unsigned char *a, - const unsigned char *b, - size_t n) +inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned char *b, size_t n) { size_t i = 0; #if defined(MBEDTLS_EFFICIENT_UNALIGNED_ACCESS) From 71fada10e5b05e328d3a7ae75ea5a0edaf9d271a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 29 Nov 2023 10:38:07 +0800 Subject: [PATCH 35/41] Guards neon path Old GCC(<7.3) reports warning in NEON path Signed-off-by: Jerry Yu --- library/common.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/common.h b/library/common.h index c20f6b260e..78d71fda26 100644 --- a/library/common.h +++ b/library/common.h @@ -169,7 +169,9 @@ inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned { size_t i = 0; #if defined(MBEDTLS_EFFICIENT_UNALIGNED_ACCESS) -#if defined(__ARM_NEON) +#if defined(__ARM_NEON) && \ + (defined(__GNUC__) && !defined(__clang__) && \ + __GNUC__ >= 7 && __GNUC_MINOR__ >= 3) for (; (i + 16) <= n; i += 16) { uint8x16_t v1 = vld1q_u8(a + i); uint8x16_t v2 = vld1q_u8(b + i); From e743aa74b50470e6b1d4df55766f0fefb80b942b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 29 Nov 2023 15:54:32 +0800 Subject: [PATCH 36/41] add non-gcc arm_neon support Signed-off-by: Jerry Yu --- library/common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/common.h b/library/common.h index 78d71fda26..cc58bf4f62 100644 --- a/library/common.h +++ b/library/common.h @@ -170,8 +170,8 @@ inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned size_t i = 0; #if defined(MBEDTLS_EFFICIENT_UNALIGNED_ACCESS) #if defined(__ARM_NEON) && \ - (defined(__GNUC__) && !defined(__clang__) && \ - __GNUC__ >= 7 && __GNUC_MINOR__ >= 3) + (!defined(MBEDTLS_COMPILER_IS_GCC) || \ + (defined(MBEDTLS_COMPILER_IS_GCC) && __GNUC__ >= 7 && __GNUC_MINOR__ >= 3)) for (; (i + 16) <= n; i += 16) { uint8x16_t v1 = vld1q_u8(a + i); uint8x16_t v2 = vld1q_u8(b + i); From 92787e42c42433b6a9cde0dd5a79b5baf011815a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 29 Nov 2023 16:30:38 +0800 Subject: [PATCH 37/41] fix wrong gcc version check Signed-off-by: Jerry Yu --- library/common.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/library/common.h b/library/common.h index cc58bf4f62..bd5a0c3923 100644 --- a/library/common.h +++ b/library/common.h @@ -23,6 +23,15 @@ #include #endif /* __ARM_NEON */ + +#if defined(__GNUC__) && !defined(__ARMCC_VERSION) && !defined(__clang__) \ + && !defined(__llvm__) && !defined(__INTEL_COMPILER) +/* Defined if the compiler really is gcc and not clang, etc */ +#define MBEDTLS_COMPILER_IS_GCC +#define MBEDTLS_GCC_VERSION \ + (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) +#endif + /** Helper to define a function as static except when building invasive tests. * * If a function is only used inside its own source file and should be @@ -171,7 +180,7 @@ inline void mbedtls_xor(unsigned char *r, const unsigned char *a, const unsigned #if defined(MBEDTLS_EFFICIENT_UNALIGNED_ACCESS) #if defined(__ARM_NEON) && \ (!defined(MBEDTLS_COMPILER_IS_GCC) || \ - (defined(MBEDTLS_COMPILER_IS_GCC) && __GNUC__ >= 7 && __GNUC_MINOR__ >= 3)) + (defined(MBEDTLS_COMPILER_IS_GCC) && MBEDTLS_GCC_VERSION >= 70300)) for (; (i + 16) <= n; i += 16) { uint8x16_t v1 = vld1q_u8(a + i); uint8x16_t v2 = vld1q_u8(b + i); @@ -326,12 +335,6 @@ static inline void mbedtls_xor_no_simd(unsigned char *r, #define MBEDTLS_ASSUME(x) do { } while (0) #endif -#if defined(__GNUC__) && !defined(__ARMCC_VERSION) && !defined(__clang__) \ - && !defined(__llvm__) && !defined(__INTEL_COMPILER) -/* Defined if the compiler really is gcc and not clang, etc */ -#define MBEDTLS_COMPILER_IS_GCC -#endif - /* For gcc -Os, override with -O2 for a given function. * * This will not affect behaviour for other optimisation settings, e.g. -O0. From 2d9b7d491af6560ba7db0ed6a39b7c256d322a2e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 29 Nov 2023 09:42:44 +0000 Subject: [PATCH 38/41] Remove references to 3.4 Signed-off-by: Dave Rodgman --- configs/config-tfm.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index d987b63313..a21d041cdd 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -51,18 +51,3 @@ /*********************************************************************** * Local changes to crypto config below this delimiter **********************************************************************/ - -/* Between Mbed TLS 3.4 and 3.5, the PSA_WANT_KEY_TYPE_RSA_KEY_PAIR macro - * (commented-out above) has been replaced with the following new macros: */ -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE 1 -//#define PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_DERIVE 1 /* Not supported */ - -/* Between Mbed TLS 3.4 and 3.5, the following macros have been added: */ -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_BASIC 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_IMPORT 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_EXPORT 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_GENERATE 1 -//#define PSA_WANT_KEY_TYPE_DH_KEY_PAIR_DERIVE 1 // Not supported From e4cf9b6f95264784d5b768d63bbb63c6fb74601f Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 29 Nov 2023 09:43:20 +0000 Subject: [PATCH 39/41] Move MBEDTLS_BLOCK_CIPHER_NO_DECRYPT to correct section Signed-off-by: Dave Rodgman --- configs/config-tfm.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index a21d041cdd..1925cdcb2b 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -45,9 +45,9 @@ #undef MBEDTLS_PLATFORM_STD_EXIT_SUCCESS #undef MBEDTLS_PLATFORM_STD_EXIT_FAILURE -// We expect TF-M to pick this up soon -#define MBEDTLS_BLOCK_CIPHER_NO_DECRYPT - /*********************************************************************** * Local changes to crypto config below this delimiter **********************************************************************/ + +// We expect TF-M to pick this up soon +#define MBEDTLS_BLOCK_CIPHER_NO_DECRYPT From 51e72456f9dd7bfc3a4eee53de046f4390a3d83b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 29 Nov 2023 09:44:44 +0000 Subject: [PATCH 40/41] Automatically set MBEDTLS_NO_PLATFORM_ENTROPY in TF-M config Signed-off-by: Dave Rodgman --- configs/config-tfm.h | 9 +++++++++ tests/scripts/all.sh | 3 --- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 1925cdcb2b..85b677b4cc 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -45,6 +45,15 @@ #undef MBEDTLS_PLATFORM_STD_EXIT_SUCCESS #undef MBEDTLS_PLATFORM_STD_EXIT_FAILURE +/* + * In order to get an example config that works cleanly out-of-the-box + * for both baremetal and non-baremetal builds, we detect baremetal builds + * and set this variable automatically. + */ +#if defined(__IAR_SYSTEMS_ICC__) || defined(__ARM_EABI__) +#define MBEDTLS_NO_PLATFORM_ENTROPY +#endif + /*********************************************************************** * Local changes to crypto config below this delimiter **********************************************************************/ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 177736a51a..036bdceac2 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -4125,9 +4125,6 @@ component_build_tfm_armcc() { # test the TF-M configuration can build cleanly with various warning flags enabled cp configs/config-tfm.h "$CONFIG_H" - # MBEDTLS_NO_PLATFORM_ENTROPY is needed as we are building for baremetal - ./scripts/config.py --force set MBEDTLS_NO_PLATFORM_ENTROPY - msg "build: TF-M config, armclang armv7-m thumb2" armc6_build_test "--target=arm-arm-none-eabi -march=armv7-m -mthumb -Os -std=c99 -Werror -Wall -Wextra -Wwrite-strings -Wpointer-arith -Wimplicit-fallthrough -Wshadow -Wvla -Wformat=2 -Wno-format-nonliteral -Wshadow -Wasm-operand-widths -Wunused -I../tests/include/spe" } From 5dcfd0c613eb31a20e02d901fdb26d72262b9835 Mon Sep 17 00:00:00 2001 From: Pengyu Lv Date: Wed, 29 Nov 2023 18:03:28 +0800 Subject: [PATCH 41/41] Some improvements Signed-off-by: Pengyu Lv --- tests/scripts/analyze_outcomes.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 52059bda09..0d8289bdaa 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -87,19 +87,16 @@ def analyze_coverage(results: Results, outcomes: Outcomes, """Check that all available test cases are executed at least once.""" available = check_test_cases.collect_available_test_cases() for suite_case in available: - hit = False - for comp_outcomes in outcomes.values(): - if suite_case in comp_outcomes.successes or \ - suite_case in comp_outcomes.failures: - hit = True - break + hit = any(suite_case in comp_outcomes.successes or + suite_case in comp_outcomes.failures + for comp_outcomes in outcomes.values()) - if hit == 0 and suite_case not in allow_list: + if not hit and suite_case not in allow_list: if full_coverage: results.error('Test case not executed: {}', suite_case) else: results.warning('Test case not executed: {}', suite_case) - elif hit != 0 and suite_case in allow_list: + elif hit and suite_case in allow_list: # Test Case should be removed from the allow list. if full_coverage: results.error('Allow listed test case was executed: {}', suite_case)