From 5a994c15f4a3699abe55507f327c762f177fa3cf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 16:46:51 +0100 Subject: [PATCH 01/17] More readable code around expression generation FOO(BAR) is an expression, not a name. Pack expression generation into a method. No behavior change. --- tests/scripts/test_psa_constant_names.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 724f8d94b..4a32851e4 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -145,6 +145,9 @@ class Inputs: except BaseException as e: raise Exception('distribute_arguments({})'.format(name)) from e + def generate_expressions(self, names): + return itertools.chain(*map(self.distribute_arguments, names)) + _argument_split_re = re.compile(r' *, *') @classmethod def _argument_split(cls, arguments): @@ -252,8 +255,8 @@ def remove_file_if_exists(filename): except OSError: pass -def run_c(options, type_word, names): - """Generate and run a program to print out numerical values for names.""" +def run_c(options, type_word, expressions): + """Generate and run a program to print out numerical values for expressions.""" if type_word == 'status': cast_to = 'long' printf_format = '%ld' @@ -278,9 +281,9 @@ def run_c(options, type_word, names): int main(void) { ''') - for name in names: + for expr in expressions: c_file.write(' printf("{}\\n", ({}) {});\n' - .format(printf_format, cast_to, name)) + .format(printf_format, cast_to, expr)) c_file.write(''' return 0; } ''') @@ -313,14 +316,14 @@ def do_test(options, inputs, type_word, names): Use inputs to figure out what arguments to pass to macros that take arguments. """ - names = sorted(itertools.chain(*map(inputs.distribute_arguments, names))) - values = run_c(options, type_word, names) + expressions = sorted(inputs.generate_expressions(names)) + values = run_c(options, type_word, expressions) output = subprocess.check_output([options.program, type_word] + values) outputs = output.decode('ascii').strip().split('\n') - errors = [(type_word, name, value, output) - for (name, value, output) in zip(names, values, outputs) - if normalize(name) != normalize(output)] - return len(names), errors + errors = [(type_word, expr, value, output) + for (expr, value, output) in zip(expressions, values, outputs) + if normalize(expr) != normalize(output)] + return len(expressions), errors def report_errors(errors): """Describe each case where the output is not as expected.""" From 5a6dc895f227182ab97009967e5e1678f9750555 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 16:48:07 +0100 Subject: [PATCH 02/17] Simplify expression normalization No need to split lines, or remove whitespace after removing whitespace. No behavior change. --- tests/scripts/test_psa_constant_names.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 4a32851e4..73b67ca01 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -307,8 +307,7 @@ def normalize(expr): """Normalize the C expression so as not to care about trivial differences. Currently "trivial differences" means whitespace. """ - expr = re.sub(NORMALIZE_STRIP_RE, '', expr, len(expr)) - return expr.strip().split('\n') + return re.sub(NORMALIZE_STRIP_RE, '', expr) def do_test(options, inputs, type_word, names): """Test psa_constant_names for the specified type. From 8f5a5018e89aa15b770bea316aa7c5e527c8996f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 16:49:10 +0100 Subject: [PATCH 03/17] Describe options in alphabetical order No behavior change. --- tests/scripts/test_psa_constant_names.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 73b67ca01..785d1a4be 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -354,15 +354,15 @@ def main(): parser.add_argument('--include', '-I', action='append', default=['include'], help='Directory for header files') - parser.add_argument('--program', - default='programs/psa/psa_constant_names', - help='Program to test') parser.add_argument('--keep-c', action='store_true', dest='keep_c', default=False, help='Keep the intermediate C file') parser.add_argument('--no-keep-c', action='store_false', dest='keep_c', help='Don\'t keep the intermediate C file (default)') + parser.add_argument('--program', + default='programs/psa/psa_constant_names', + help='Program to test') options = parser.parse_args() headers = [os.path.join(options.include[0], 'psa', h) for h in ['crypto.h', 'crypto_extra.h', 'crypto_values.h']] From 69f93b5040c593f800bb0d5c80e3fa4bd2e60c2b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 16:49:50 +0100 Subject: [PATCH 04/17] Move the names of input files to global variables No behavior change. --- tests/scripts/test_psa_constant_names.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 785d1a4be..6ae393643 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -349,6 +349,9 @@ def run_tests(options, inputs): errors += e return count, errors +HEADERS = ['psa/crypto.h', 'psa/crypto_extra.h', 'psa/crypto_values.h'] +TEST_SUITES = ['tests/suites/test_suite_psa_crypto_metadata.data'] + def main(): parser = argparse.ArgumentParser(description=globals()['__doc__']) parser.add_argument('--include', '-I', @@ -364,10 +367,8 @@ def main(): default='programs/psa/psa_constant_names', help='Program to test') options = parser.parse_args() - headers = [os.path.join(options.include[0], 'psa', h) - for h in ['crypto.h', 'crypto_extra.h', 'crypto_values.h']] - test_suites = ['tests/suites/test_suite_psa_crypto_metadata.data'] - inputs = gather_inputs(headers, test_suites) + headers = [os.path.join(options.include[0], h) for h in HEADERS] + inputs = gather_inputs(headers, TEST_SUITES) count, errors = run_tests(options, inputs) report_errors(errors) if errors == []: From 4408dfd0fc0e05c9b860957f92d6b13d27e6d9f2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 17:16:21 +0100 Subject: [PATCH 05/17] Minor docstring improvements No behavior change. --- tests/scripts/test_psa_constant_names.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 6ae393643..a40a82959 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -61,6 +61,7 @@ class read_file_lines: class Inputs: """Accumulate information about macros to test. + This includes macro names as well as information about their arguments when applicable. """ @@ -101,6 +102,7 @@ class Inputs: def gather_arguments(self): """Populate the list of values for macro arguments. + Call this after parsing all the inputs. """ self.arguments_for['hash_alg'] = sorted(self.hash_algorithms) @@ -118,6 +120,7 @@ class Inputs: def distribute_arguments(self, name): """Generate macro calls with each tested argument set. + If name is a macro without arguments, just yield "name". If name is a macro with arguments, yield a series of "name(arg1,...,argN)" where each argument takes each possible @@ -305,15 +308,21 @@ int main(void) NORMALIZE_STRIP_RE = re.compile(r'\s+') def normalize(expr): """Normalize the C expression so as not to care about trivial differences. + Currently "trivial differences" means whitespace. """ return re.sub(NORMALIZE_STRIP_RE, '', expr) def do_test(options, inputs, type_word, names): """Test psa_constant_names for the specified type. + Run program on names. Use inputs to figure out what arguments to pass to macros that take arguments. + + Return ``(count, errors)`` where ``count`` is the number of expressions + that have been tested and ``errors`` is the list of errors that were + encountered. """ expressions = sorted(inputs.generate_expressions(names)) values = run_c(options, type_word, expressions) @@ -332,6 +341,7 @@ def report_errors(errors): def run_tests(options, inputs): """Run psa_constant_names on all the gathered inputs. + Return a tuple (count, errors) where count is the total number of inputs that were tested and errors is the list of cases where the output was not as expected. From ffe2d6e71b9125d6a45d6d682699caf36efee8a3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 17:17:01 +0100 Subject: [PATCH 06/17] Move the type_word->name_set mapping into its own method No behavior change. --- tests/scripts/test_psa_constant_names.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index a40a82959..a43a3e888 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -100,6 +100,17 @@ class Inputs: 'tag_length': ['1', '63'], } + def get_names(self, type_word): + """Return the set of known names of values of the given type.""" + return { + 'status': self.statuses, + 'algorithm': self.algorithms, + 'ecc_curve': self.ecc_curves, + 'dh_group': self.dh_groups, + 'key_type': self.key_types, + 'key_usage': self.key_usage_flags, + }[type_word] + def gather_arguments(self): """Populate the list of values for macro arguments. @@ -313,7 +324,7 @@ def normalize(expr): """ return re.sub(NORMALIZE_STRIP_RE, '', expr) -def do_test(options, inputs, type_word, names): +def do_test(options, inputs, type_word): """Test psa_constant_names for the specified type. Run program on names. @@ -324,6 +335,7 @@ def do_test(options, inputs, type_word, names): that have been tested and ``errors`` is the list of errors that were encountered. """ + names = inputs.get_names(type_word) expressions = sorted(inputs.generate_expressions(names)) values = run_c(options, type_word, expressions) output = subprocess.check_output([options.program, type_word] + values) @@ -348,13 +360,9 @@ def run_tests(options, inputs): """ count = 0 errors = [] - for type_word, names in [('status', inputs.statuses), - ('algorithm', inputs.algorithms), - ('ecc_curve', inputs.ecc_curves), - ('dh_group', inputs.dh_groups), - ('key_type', inputs.key_types), - ('key_usage', inputs.key_usage_flags)]: - c, e = do_test(options, inputs, type_word, names) + for type_word in ['status', 'algorithm', 'ecc_curve', 'dh_group', + 'key_type', 'key_usage']: + c, e = do_test(options, inputs, type_word) count += c errors += e return count, errors From c231711dbc8ed339d37698afe66fce46931cdc18 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 17:17:39 +0100 Subject: [PATCH 07/17] Move value collection into its own function No behavior change. --- tests/scripts/test_psa_constant_names.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index a43a3e888..53af0a524 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -324,6 +324,17 @@ def normalize(expr): """ return re.sub(NORMALIZE_STRIP_RE, '', expr) +def collect_values(options, inputs, type_word): + """Generate expressions using known macro names and calculate their values. + + Return a list of pairs of (expr, value) where expr is an expression and + value is a string representation of its integer value. + """ + names = inputs.get_names(type_word) + expressions = sorted(inputs.generate_expressions(names)) + values = run_c(options, type_word, expressions) + return expressions, values + def do_test(options, inputs, type_word): """Test psa_constant_names for the specified type. @@ -335,9 +346,7 @@ def do_test(options, inputs, type_word): that have been tested and ``errors`` is the list of errors that were encountered. """ - names = inputs.get_names(type_word) - expressions = sorted(inputs.generate_expressions(names)) - values = run_c(options, type_word, expressions) + expressions, values = collect_values(options, inputs, type_word) output = subprocess.check_output([options.program, type_word] + values) outputs = output.decode('ascii').strip().split('\n') errors = [(type_word, expr, value, output) From b86b6d32f98a21d85c061d14b1a05254b49c6f96 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 17:26:10 +0100 Subject: [PATCH 08/17] Path options that affect run_c as separate arguments No behavior change. --- tests/scripts/test_psa_constant_names.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 53af0a524..e64040802 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -269,8 +269,10 @@ def remove_file_if_exists(filename): except OSError: pass -def run_c(options, type_word, expressions): +def run_c(type_word, expressions, include_path=None, keep_c=False): """Generate and run a program to print out numerical values for expressions.""" + if include_path is None: + include_path = [] if type_word == 'status': cast_to = 'long' printf_format = '%ld' @@ -304,9 +306,9 @@ int main(void) c_file.close() cc = os.getenv('CC', 'cc') subprocess.check_call([cc] + - ['-I' + dir for dir in options.include] + + ['-I' + dir for dir in include_path] + ['-o', exe_name, c_name]) - if options.keep_c: + if keep_c: sys.stderr.write('List of {} tests kept at {}\n' .format(type_word, c_name)) else: @@ -324,7 +326,7 @@ def normalize(expr): """ return re.sub(NORMALIZE_STRIP_RE, '', expr) -def collect_values(options, inputs, type_word): +def collect_values(inputs, type_word, include_path=None, keep_c=False): """Generate expressions using known macro names and calculate their values. Return a list of pairs of (expr, value) where expr is an expression and @@ -332,7 +334,8 @@ def collect_values(options, inputs, type_word): """ names = inputs.get_names(type_word) expressions = sorted(inputs.generate_expressions(names)) - values = run_c(options, type_word, expressions) + values = run_c(type_word, expressions, + include_path=include_path, keep_c=keep_c) return expressions, values def do_test(options, inputs, type_word): @@ -346,7 +349,9 @@ def do_test(options, inputs, type_word): that have been tested and ``errors`` is the list of errors that were encountered. """ - expressions, values = collect_values(options, inputs, type_word) + expressions, values = collect_values(inputs, type_word, + include_path=options.include, + keep_c=options.keep_c) output = subprocess.check_output([options.program, type_word] + values) outputs = output.decode('ascii').strip().split('\n') errors = [(type_word, expr, value, output) From 2460933a6f546c9b50f28fd597adc9c23c9b71a8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 17:44:21 +0100 Subject: [PATCH 09/17] Move test running and reporting functions into their own class This makes the structure of the code more apparent. No behavior change. --- tests/scripts/test_psa_constant_names.py | 88 ++++++++++++------------ 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index e64040802..e261b4f56 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -338,48 +338,52 @@ def collect_values(inputs, type_word, include_path=None, keep_c=False): include_path=include_path, keep_c=keep_c) return expressions, values -def do_test(options, inputs, type_word): - """Test psa_constant_names for the specified type. +class Tests: + """An object representing tests and their results.""" - Run program on names. - Use inputs to figure out what arguments to pass to macros that - take arguments. + def __init__(self, options): + self.options = options + self.count = 0 + self.errors = [] - Return ``(count, errors)`` where ``count`` is the number of expressions - that have been tested and ``errors`` is the list of errors that were - encountered. - """ - expressions, values = collect_values(inputs, type_word, - include_path=options.include, - keep_c=options.keep_c) - output = subprocess.check_output([options.program, type_word] + values) - outputs = output.decode('ascii').strip().split('\n') - errors = [(type_word, expr, value, output) - for (expr, value, output) in zip(expressions, values, outputs) - if normalize(expr) != normalize(output)] - return len(expressions), errors + def run_one(self, inputs, type_word): + """Test psa_constant_names for the specified type. -def report_errors(errors): - """Describe each case where the output is not as expected.""" - for type_word, name, value, output in errors: - print('For {} "{}", got "{}" (value: {})' - .format(type_word, name, output, value)) + Run the program on the names for this type. + Use the inputs to figure out what arguments to pass to macros that + take arguments. + """ + expressions, values = collect_values(inputs, type_word, + include_path=self.options.include, + keep_c=self.options.keep_c) + output = subprocess.check_output([self.options.program, type_word] + + values) + outputs = output.decode('ascii').strip().split('\n') + self.count += len(expressions) + for expr, value, output in zip(expressions, values, outputs): + if normalize(expr) != normalize(output): + self.errors.append((type_word, expr, value, output)) -def run_tests(options, inputs): - """Run psa_constant_names on all the gathered inputs. + def run_all(self, inputs): + """Run psa_constant_names on all the gathered inputs.""" + for type_word in ['status', 'algorithm', 'ecc_curve', 'dh_group', + 'key_type', 'key_usage']: + self.run_one(inputs, type_word) - Return a tuple (count, errors) where count is the total number of inputs - that were tested and errors is the list of cases where the output was - not as expected. - """ - count = 0 - errors = [] - for type_word in ['status', 'algorithm', 'ecc_curve', 'dh_group', - 'key_type', 'key_usage']: - c, e = do_test(options, inputs, type_word) - count += c - errors += e - return count, errors + def report(self, out): + """Describe each case where the output is not as expected. + + Write the errors to ``out``. + Also write a total. + """ + for type_word, name, value, output in self.errors: + out.write('For {} "{}", got "{}" (value: {})\n' + .format(type_word, name, output, value)) + out.write('{} test cases'.format(self.count)) + if self.errors: + out.write(', {} FAIL\n'.format(len(self.errors))) + else: + out.write(' PASS\n') HEADERS = ['psa/crypto.h', 'psa/crypto_extra.h', 'psa/crypto_values.h'] TEST_SUITES = ['tests/suites/test_suite_psa_crypto_metadata.data'] @@ -401,12 +405,10 @@ def main(): options = parser.parse_args() headers = [os.path.join(options.include[0], h) for h in HEADERS] inputs = gather_inputs(headers, TEST_SUITES) - count, errors = run_tests(options, inputs) - report_errors(errors) - if errors == []: - print('{} test cases PASS'.format(count)) - else: - print('{} test cases, {} FAIL'.format(count, len(errors))) + tests = Tests(options) + tests.run_all(inputs) + tests.report(sys.stdout) + if tests.errors: exit(1) if __name__ == '__main__': From a5000f1dc65f5cc530c0438284dc6d7df82f59e1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 17:51:11 +0100 Subject: [PATCH 10/17] Make a class for error data No behavior change. --- tests/scripts/test_psa_constant_names.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index e261b4f56..5780f25b1 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -8,6 +8,7 @@ or 1 (with a Python backtrace) if there was an operational error. """ import argparse +from collections import namedtuple import itertools import os import platform @@ -341,6 +342,9 @@ def collect_values(inputs, type_word, include_path=None, keep_c=False): class Tests: """An object representing tests and their results.""" + Error = namedtuple('Error', + ['type', 'expression', 'value', 'output']) + def __init__(self, options): self.options = options self.count = 0 @@ -362,7 +366,10 @@ class Tests: self.count += len(expressions) for expr, value, output in zip(expressions, values, outputs): if normalize(expr) != normalize(output): - self.errors.append((type_word, expr, value, output)) + self.errors.append(self.Error(type=type_word, + expression=expr, + value=value, + output=output)) def run_all(self, inputs): """Run psa_constant_names on all the gathered inputs.""" @@ -376,9 +383,10 @@ class Tests: Write the errors to ``out``. Also write a total. """ - for type_word, name, value, output in self.errors: + for error in self.errors: out.write('For {} "{}", got "{}" (value: {})\n' - .format(type_word, name, output, value)) + .format(error.type, error.expression, + error.output, error.value)) out.write('{} test cases'.format(self.count)) if self.errors: out.write(', {} FAIL\n'.format(len(self.errors))) From 84a45817a45004002e7b644dc02345c0c971b39c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 19:50:33 +0100 Subject: [PATCH 11/17] Allow gather_inputs to work with a derived Inputs class No behavior change. --- tests/scripts/test_psa_constant_names.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 5780f25b1..ee909cf81 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -251,9 +251,9 @@ class Inputs: if m: self.add_test_case_line(m.group(1), m.group(2)) -def gather_inputs(headers, test_suites): +def gather_inputs(headers, test_suites, inputs_class=Inputs): """Read the list of inputs to test psa_constant_names with.""" - inputs = Inputs() + inputs = inputs_class() for header in headers: inputs.parse_header(header) for test_cases in test_suites: From 8c8694c14de578034c06f3971749e2d3967786cf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 19:22:45 +0100 Subject: [PATCH 12/17] add_test_case_line: data-driven dispatch No behavior change. --- tests/scripts/test_psa_constant_names.py | 30 ++++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index ee909cf81..5b86b247d 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -61,6 +61,7 @@ class read_file_lines: from exc_value class Inputs: + # pylint: disable=too-many-instance-attributes """Accumulate information about macros to test. This includes macro names as well as information about their arguments @@ -93,6 +94,16 @@ class Inputs: 'KEY_TYPE': self.key_types, 'KEY_USAGE': self.key_usage_flags, } + # Test functions + self.table_by_test_function = { + 'key_type': self.key_types, + 'ecc_key_types': self.ecc_curves, + 'dh_key_types': self.dh_groups, + 'hash_algorithm': self.hash_algorithms, + 'mac_algorithm': self.mac_algorithms, + 'hmac_algorithm': self.mac_algorithms, + 'aead_algorithm': self.aead_algorithms, + } # macro name -> list of argument names self.argspecs = {} # argument name -> list of values @@ -220,24 +231,17 @@ class Inputs: def add_test_case_line(self, function, argument): """Parse a test case data line, looking for algorithm metadata tests.""" + sets = [] if function.endswith('_algorithm'): # As above, ECDH and FFDH algorithms are excluded for now. # Support for them will be added in the future. if 'ECDH' in argument or 'FFDH' in argument: return - self.algorithms.add(argument) - if function == 'hash_algorithm': - self.hash_algorithms.add(argument) - elif function in ['mac_algorithm', 'hmac_algorithm']: - self.mac_algorithms.add(argument) - elif function == 'aead_algorithm': - self.aead_algorithms.add(argument) - elif function == 'key_type': - self.key_types.add(argument) - elif function == 'ecc_key_types': - self.ecc_curves.add(argument) - elif function == 'dh_key_types': - self.dh_groups.add(argument) + sets.append(self.algorithms) + if function in self.table_by_test_function: + sets.append(self.table_by_test_function[function]) + for s in sets: + s.add(argument) # Regex matching a *.data line containing a test function call and # its arguments. The actual definition is partly positional, but this From 98a710c5b2a3e35f5dbe55d003aadffca655f08f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 18:58:36 +0100 Subject: [PATCH 13/17] Fix the collection of ECC curves and DH groups PSA_ECC_CURVE_xxx and PSA_DH_GROUP_xxx were not collected from headers, only from test suites. --- tests/scripts/test_psa_constant_names.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 5b86b247d..af536866c 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -89,8 +89,8 @@ class Inputs: self.table_by_prefix = { 'ERROR': self.statuses, 'ALG': self.algorithms, - 'CURVE': self.ecc_curves, - 'GROUP': self.dh_groups, + 'ECC_CURVE': self.ecc_curves, + 'DH_GROUP': self.dh_groups, 'KEY_TYPE': self.key_types, 'KEY_USAGE': self.key_usage_flags, } @@ -183,7 +183,7 @@ class Inputs: # Groups: 1=macro name, 2=type, 3=argument list (optional). _header_line_re = \ re.compile(r'#define +' + - r'(PSA_((?:KEY_)?[A-Z]+)_\w+)' + + r'(PSA_((?:(?:DH|ECC|KEY)_)?[A-Z]+)_\w+)' + r'(?:\(([^\n()]*)\))?') # Regex of macro names to exclude. _excluded_name_re = re.compile(r'_(?:GET|IS|OF)_|_(?:BASE|FLAG|MASK)\Z') From 2bcfc714d2fcaa502186a307056fceea973a3ee7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 19:49:26 +0100 Subject: [PATCH 14/17] Error out if a test case uses an unknown macro name Insist that test cases must only use macro names that are declared in a header. This may catch errors such as not parsing the intended files. Make this check easily overridden in a derived class. --- tests/scripts/test_psa_constant_names.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index af536866c..8f393a1ab 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -69,6 +69,7 @@ class Inputs: """ def __init__(self): + self.all_declared = set() # Sets of names per type self.statuses = set(['PSA_SUCCESS']) self.algorithms = set(['0xffffffff']) @@ -213,6 +214,7 @@ class Inputs: if not m: return name = m.group(1) + self.all_declared.add(name) if re.search(self._excluded_name_re, name) or \ name in self._excluded_names: return @@ -229,6 +231,19 @@ class Inputs: for line in lines: self.parse_header_line(line) + _macro_identifier_re = r'[A-Z]\w+' + def generate_undeclared_names(self, expr): + for name in re.findall(self._macro_identifier_re, expr): + if name not in self.all_declared: + yield name + + def accept_test_case_line(self, function, argument): + #pylint: disable=unused-argument + undeclared = list(self.generate_undeclared_names(argument)) + if undeclared: + raise Exception('Undeclared names in test case', undeclared) + return True + def add_test_case_line(self, function, argument): """Parse a test case data line, looking for algorithm metadata tests.""" sets = [] @@ -240,8 +255,9 @@ class Inputs: sets.append(self.algorithms) if function in self.table_by_test_function: sets.append(self.table_by_test_function[function]) - for s in sets: - s.add(argument) + if self.accept_test_case_line(function, argument): + for s in sets: + s.add(argument) # Regex matching a *.data line containing a test function call and # its arguments. The actual definition is partly positional, but this From 79616687383c9ad8afe99d39d835ffd3d6145ea4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 20:08:10 +0100 Subject: [PATCH 15/17] Support key agreement Key agreement algorithms were excluded back when they were constructed with a macro conveying the key agreement itself taking the KDF as an argument, because that was hard to support. Now the encoding has changed and key agreement algorithms are constructed with PSA_ALG_KEY_AGREEMENT taking two arguments, one that identifies the raw key agreement and one that identifies the KDF. This is easy to process, so add support. --- tests/scripts/test_psa_constant_names.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 8f393a1ab..6e7bf48b1 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -104,6 +104,8 @@ class Inputs: 'mac_algorithm': self.mac_algorithms, 'hmac_algorithm': self.mac_algorithms, 'aead_algorithm': self.aead_algorithms, + 'key_derivation_algorithm': self.kdf_algorithms, + 'key_agreement_algorithm': self.ka_algorithms, } # macro name -> list of argument names self.argspecs = {} @@ -197,10 +199,6 @@ class Inputs: # Auxiliary macro whose name doesn't fit the usual patterns for # auxiliary macros. 'PSA_ALG_AEAD_WITH_DEFAULT_TAG_LENGTH_CASE', - # PSA_ALG_ECDH and PSA_ALG_FFDH are excluded for now as the script - # currently doesn't support them. - 'PSA_ALG_ECDH', - 'PSA_ALG_FFDH', # Deprecated aliases. 'PSA_ERROR_UNKNOWN_ERROR', 'PSA_ERROR_OCCUPIED_SLOT', @@ -248,11 +246,13 @@ class Inputs: """Parse a test case data line, looking for algorithm metadata tests.""" sets = [] if function.endswith('_algorithm'): - # As above, ECDH and FFDH algorithms are excluded for now. - # Support for them will be added in the future. - if 'ECDH' in argument or 'FFDH' in argument: - return sets.append(self.algorithms) + if function == 'key_agreement_algorithm' and \ + argument.startswith('PSA_ALG_KEY_AGREEMENT('): + # We only want *raw* key agreement algorithms as such, so + # exclude ones that are already chained with a KDF. + # Keep the expression as one to test as an algorithm. + function = 'other_algorithm' if function in self.table_by_test_function: sets.append(self.table_by_test_function[function]) if self.accept_test_case_line(function, argument): From d2cea9f57c2da5ea4582bc423334dbbb5bcd6d69 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Nov 2019 20:10:13 +0100 Subject: [PATCH 16/17] Add some more KDF test cases --- .../test_suite_psa_crypto_metadata.data | 28 +++++++++++++++++++ .../test_suite_psa_crypto_metadata.function | 2 ++ 2 files changed, 30 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto_metadata.data b/tests/suites/test_suite_psa_crypto_metadata.data index e989895d2..9cdee0353 100644 --- a/tests/suites/test_suite_psa_crypto_metadata.data +++ b/tests/suites/test_suite_psa_crypto_metadata.data @@ -262,6 +262,26 @@ Key derivation: HKDF using SHA-256 depends_on:MBEDTLS_SHA256_C key_derivation_algorithm:PSA_ALG_HKDF( PSA_ALG_SHA_256 ):ALG_IS_HKDF +Key derivation: HKDF using SHA-384 +depends_on:MBEDTLS_SHA512_C +key_derivation_algorithm:PSA_ALG_HKDF( PSA_ALG_SHA_384 ):ALG_IS_HKDF + +Key derivation: TLS 1.2 PRF using SHA-256 +depends_on:MBEDTLS_SHA256_C +key_derivation_algorithm:PSA_ALG_TLS12_PRF( PSA_ALG_SHA_256 ):ALG_IS_TLS12_PRF + +Key derivation: TLS 1.2 PRF using SHA-384 +depends_on:MBEDTLS_SHA512_C +key_derivation_algorithm:PSA_ALG_TLS12_PRF( PSA_ALG_SHA_384 ):ALG_IS_TLS12_PRF + +Key derivation: TLS 1.2 PSK-to-MS using SHA-256 +depends_on:MBEDTLS_SHA256_C +key_derivation_algorithm:PSA_ALG_TLS12_PSK_TO_MS( PSA_ALG_SHA_256 ):ALG_IS_TLS12_PSK_TO_MS + +Key derivation: TLS 1.2 PSK-to-MS using SHA-384 +depends_on:MBEDTLS_SHA512_C +key_derivation_algorithm:PSA_ALG_TLS12_PSK_TO_MS( PSA_ALG_SHA_384 ):ALG_IS_TLS12_PSK_TO_MS + Key agreement: FFDH, raw output depends_on:MBEDTLS_DHM_C key_agreement_algorithm:PSA_ALG_FFDH:ALG_IS_FFDH | ALG_IS_RAW_KEY_AGREEMENT:PSA_ALG_FFDH:PSA_ALG_CATEGORY_KEY_DERIVATION @@ -270,6 +290,10 @@ Key agreement: FFDH, HKDF using SHA-256 depends_on:MBEDTLS_DHM_C key_agreement_algorithm:PSA_ALG_KEY_AGREEMENT( PSA_ALG_FFDH, PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ):ALG_IS_FFDH:PSA_ALG_FFDH:PSA_ALG_HKDF( PSA_ALG_SHA_256 ) +Key agreement: FFDH, HKDF using SHA-384 +depends_on:MBEDTLS_DHM_C +key_agreement_algorithm:PSA_ALG_KEY_AGREEMENT( PSA_ALG_FFDH, PSA_ALG_HKDF( PSA_ALG_SHA_384 ) ):ALG_IS_FFDH:PSA_ALG_FFDH:PSA_ALG_HKDF( PSA_ALG_SHA_384 ) + Key agreement: ECDH, raw output depends_on:MBEDTLS_ECDH_C key_agreement_algorithm:PSA_ALG_ECDH:ALG_IS_ECDH | ALG_IS_RAW_KEY_AGREEMENT:PSA_ALG_ECDH:PSA_ALG_CATEGORY_KEY_DERIVATION @@ -278,6 +302,10 @@ Key agreement: ECDH, HKDF using SHA-256 depends_on:MBEDTLS_ECDH_C key_agreement_algorithm:PSA_ALG_KEY_AGREEMENT( PSA_ALG_ECDH, PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ):ALG_IS_ECDH:PSA_ALG_ECDH:PSA_ALG_HKDF( PSA_ALG_SHA_256 ) +Key agreement: ECDH, HKDF using SHA-384 +depends_on:MBEDTLS_ECDH_C +key_agreement_algorithm:PSA_ALG_KEY_AGREEMENT( PSA_ALG_ECDH, PSA_ALG_HKDF( PSA_ALG_SHA_384 ) ):ALG_IS_ECDH:PSA_ALG_ECDH:PSA_ALG_HKDF( PSA_ALG_SHA_384 ) + Key type: raw data key_type:PSA_KEY_TYPE_RAW_DATA:KEY_TYPE_IS_UNSTRUCTURED diff --git a/tests/suites/test_suite_psa_crypto_metadata.function b/tests/suites/test_suite_psa_crypto_metadata.function index a9f1b3938..3a9347e2f 100644 --- a/tests/suites/test_suite_psa_crypto_metadata.function +++ b/tests/suites/test_suite_psa_crypto_metadata.function @@ -37,6 +37,8 @@ #define ALG_IS_WILDCARD ( 1u << 19 ) #define ALG_IS_RAW_KEY_AGREEMENT ( 1u << 20 ) #define ALG_IS_AEAD_ON_BLOCK_CIPHER ( 1u << 21 ) +#define ALG_IS_TLS12_PRF ( 1u << 22 ) +#define ALG_IS_TLS12_PSK_TO_MS ( 1u << 23 ) /* Flags for key type classification macros. There is a flag for every * key type classification macro PSA_KEY_TYPE_IS_xxx except for some that From 8fa1348276fc07a322de309aa79f85b8d5709493 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 25 Nov 2019 17:10:12 +0100 Subject: [PATCH 17/17] Enumerate metadata test functions explicitly When gathering test cases from test_suite_psa_crypto_metadata, look up the test function explicitly. This way test_psa_constant_names will error out if we add a new test function that needs coverage here. This change highlights an omission in the previous version: asymmetric_signature_wildcard was silently ignored as a source of algorithm expressions to test. Fix that. --- tests/scripts/test_psa_constant_names.py | 28 ++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 6e7bf48b1..89319870d 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -97,15 +97,22 @@ class Inputs: } # Test functions self.table_by_test_function = { - 'key_type': self.key_types, - 'ecc_key_types': self.ecc_curves, - 'dh_key_types': self.dh_groups, - 'hash_algorithm': self.hash_algorithms, - 'mac_algorithm': self.mac_algorithms, - 'hmac_algorithm': self.mac_algorithms, - 'aead_algorithm': self.aead_algorithms, - 'key_derivation_algorithm': self.kdf_algorithms, - 'key_agreement_algorithm': self.ka_algorithms, + # Any function ending in _algorithm also gets added to + # self.algorithms. + 'key_type': [self.key_types], + 'ecc_key_types': [self.ecc_curves], + 'dh_key_types': [self.dh_groups], + 'hash_algorithm': [self.hash_algorithms], + 'mac_algorithm': [self.mac_algorithms], + 'cipher_algorithm': [], + 'hmac_algorithm': [self.mac_algorithms], + 'aead_algorithm': [self.aead_algorithms], + 'key_derivation_algorithm': [self.kdf_algorithms], + 'key_agreement_algorithm': [self.ka_algorithms], + 'asymmetric_signature_algorithm': [], + 'asymmetric_signature_wildcard': [self.algorithms], + 'asymmetric_encryption_algorithm': [], + 'other_algorithm': [], } # macro name -> list of argument names self.argspecs = {} @@ -253,8 +260,7 @@ class Inputs: # exclude ones that are already chained with a KDF. # Keep the expression as one to test as an algorithm. function = 'other_algorithm' - if function in self.table_by_test_function: - sets.append(self.table_by_test_function[function]) + sets += self.table_by_test_function[function] if self.accept_test_case_line(function, argument): for s in sets: s.add(argument)