From 0125a0c2760cfbc60cc1acae0ee4ab033117c5f7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Dec 2021 13:25:57 +0100 Subject: [PATCH 01/38] Add type annotations and pass mypy Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-run-tests | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/bin/mbedtls-run-tests b/tools/bin/mbedtls-run-tests index e73f8cf..9e6fc96 100755 --- a/tools/bin/mbedtls-run-tests +++ b/tools/bin/mbedtls-run-tests @@ -12,12 +12,12 @@ import os import re import subprocess import tempfile - +import typing class TestCaseFilter: """Test case filter.""" - def __init__(self, options): + def __init__(self, options: argparse.Namespace) -> None: """Set up a test case filter. See `main` for what options are valid. @@ -44,7 +44,7 @@ class TestCaseFilter: not re.match(self.exclude, description)) -def extract_description(stanza): +def extract_description(stanza: str) -> typing.Optional[str]: """Extract the description from a .data stanza.""" m = re.match(r'(?:\n|#[^\n]*\n)*([^#\n][^\n]*)\n', stanza + '\n') if m: @@ -52,7 +52,9 @@ def extract_description(stanza): else: return None -def filter_test_cases(all_datax, tcf, temp_datax): +def filter_test_cases(all_datax: str, + tcf: TestCaseFilter, + temp_datax: typing.IO[bytes]) -> None: """Filter test cases. Filter test cases from datax_file based on their description according @@ -71,7 +73,12 @@ def filter_test_cases(all_datax, tcf, temp_datax): temp_datax.write(line) temp_datax.flush() -def run_exe(keep_temp, precommand, exe, extra_options, all_datax, tcf): +def run_exe(keep_temp: bool, + precommand: typing.List[str], + exe: str, + extra_options: typing.List[str], + all_datax: str, + tcf: TestCaseFilter) -> int: """Run one Mbed TLS test suites based on the specified options. Return the subprocess's exit status (should be 0 for success, 1 for @@ -95,7 +102,7 @@ def run_exe(keep_temp, precommand, exe, extra_options, all_datax, tcf): outcome = subprocess.run(cmd, cwd=directory) return outcome.returncode -def find_data_file(exe): +def find_data_file(exe: str) -> str: """Return the .datax file for the specified test suite executable.""" directory = os.path.dirname(exe) basename = os.path.basename(exe) @@ -109,7 +116,7 @@ def find_data_file(exe): else: raise Exception('.datax file not found for ' + exe) -def run(options): +def run(options: argparse.Namespace) -> int: """Run Mbed TLS test suites based on the specified options. See `main` for what options are valid. @@ -131,7 +138,7 @@ def run(options): global_status = max(global_status, 120) return global_status -def main(): +def main() -> None: """Process the command line and run Mbed TLS tests.""" parser = argparse.ArgumentParser(description=__doc__) parser.add_argument('--command', '-c', From a1b89bd82edf87bfd0bda40f78673280e5f6935f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Dec 2021 13:26:14 +0100 Subject: [PATCH 02/38] Add type annotations and pass mypy Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 448 ++++++++++++++++++++------------ 1 file changed, 285 insertions(+), 163 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 38860d8..be242ed 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -2,7 +2,10 @@ """Generate a makefile for Mbed Crypto or Mbed TLS. """ +import abc import argparse +import collections +import enum import glob import itertools import os @@ -12,12 +15,40 @@ import shutil import subprocess import sys import tempfile +import typing +from typing import Dict, Iterable, List, Optional, Set, Tuple + + +T = typing.TypeVar('T') +V = typing.TypeVar('V') + +# The typing_extensions module is necessary for type annotations that are +# checked with mypy. It is only used for type annotations or to define +# things that are themselves only used for type annotations. It is not +# available on a default Python <3.8 installation. Therefore, try loading +# what we need from it for the sake of mypy (which depends on, or comes +# with, typing_extensions), and if not define substitutes that lack the +# static type information but are good enough at runtime. +try: + from typing_extensions import Protocol #pylint: disable=import-error +except ImportError: + class Protocol: #type: ignore + #pylint: disable=too-few-public-methods + pass + +class Writable(Protocol): + """Abstract class for typing hints.""" + # pylint: disable=no-self-use,too-few-public-methods,unused-argument + def write(self, text: str) -> typing.Any: + ... -def sjoin(*args): + +def sjoin(*args: str) -> str: """Join the arguments (strings) with a single space between each.""" return ' '.join(args) -def append_to_value(d, key, *values): +def append_to_value(d: Dict[T, List[V]], + key: T, *values: V) -> None: """Append to a value in a dictionary. Append values to d[key]. Create an empty list if d[key] does not exist. @@ -25,7 +56,12 @@ def append_to_value(d, key, *values): lst = d.setdefault(key, []) lst += values -def are_same_existing_files(*files): +def are_same_existing_files(*files: str) -> bool: + """Whether all the given files are the same file. + + Symbolic links are considered the same as their target. + Non-existent paths are not considered the same as anything, even themselves. + """ for file1 in files: if not os.path.exists(file1): return False @@ -45,8 +81,9 @@ class EnvironmentOption: * default: a default value if the variable is not in the environment. """ - def __init__(self, var, default='', help=None, - option=None): + def __init__(self, var: str, default='', + help: Optional[str] = None, + option: Optional[str] = None) -> None: self.var = var self.attr = var self.option = ('--' + var.lower().replace('_', '-') @@ -117,6 +154,11 @@ _environment_options = [ 'Options to always pass to ${CC}'), ] + +# For typing purposes. To be refined. +Options = argparse.Namespace + + """The list of potential submodules. A submodule is a subdirectory of the source tree which has the same @@ -124,6 +166,7 @@ general structure as the source tree. """ _submodule_names = ['crypto'] + class SourceFile: """A description of a file path in the source tree. @@ -132,44 +175,44 @@ class SourceFile: not in a submodule), and the path inside the submodule. """ - def __init__(self, root, submodule, inner_path): + def __init__(self, root: str, submodule: str, inner_path: str) -> None: self.root = root self.submodule = submodule self.inner_path = inner_path - def sort_key(self): + def sort_key(self) -> Tuple[str, bool, str]: # Compare by inner path first, then by submodule. # The empty submodule comes last. return (self.inner_path, not self.submodule, self.submodule) - def __lt__(self, other): + def __lt__(self, other: 'SourceFile') -> bool: if self.root != other.root: raise TypeError("Cannot compare source files under different roots" , self, other) return self.sort_key() < other.sort_key() - def relative_path(self): + def relative_path(self) -> str: """Path to the file from the root of the source tree.""" return os.path.join(self.submodule, self.inner_path) - def source_dir(self): + def source_dir(self) -> str: """Path to the directory containing the file, from the root of the source tree.""" return os.path.dirname(self.relative_path()) - def real_path(self): + def real_path(self) -> str: """A path at which the file can be opened during makefile generation.""" return os.path.join(self.root, self.submodule, self.inner_path) - def make_path(self): + def make_path(self) -> str: """A path to the file that is valid in the makefile.""" if self.submodule: return '/'.join(['$(SOURCE_DIR)', self.submodule, self.inner_path]) else: return '$(SOURCE_DIR)/' + self.inner_path - def target_dir(self): + def target_dir(self) -> str: """The target directory for build products of this source file. This is the path to the directory containing the source file @@ -177,11 +220,11 @@ class SourceFile: """ return os.path.dirname(self.inner_path) - def base(self): + def base(self) -> str: """The path to the file inside the submodule, without the extension.""" return os.path.splitext(self.inner_path)[0] - def target(self, extension): + def target(self, extension) -> str: """A build target for this source file, with the specified extension.""" return self.base() + extension @@ -192,35 +235,65 @@ class GeneratedFile(SourceFile): inside the build tree rather than a file inside the source tree. """ - def __init__(self, path): + def __init__(self, path: str) -> None: super().__init__('.', '', path) - def make_path(self): + def make_path(self) -> str: return self.inner_path -class ClassicTestGenerator: + +class TestGeneratorInterface: + """Test generator script description interface.""" + + __metaclass__ = abc.ABCMeta + + @abc.abstractmethod + def target(self, c_file: str) -> str: + """A space-separated list of generated files.""" + ... + + @abc.abstractmethod + def script(self, source_dir: str) -> str: + """The path to the generator script.""" + ... + + @abc.abstractmethod + def function_files(self, function_file: str) -> List[str]: + """The full list of .function files used to generate the given suite. + + This always includes the argument, and can include other files + (helpers.function, etc.). + """ + ... + + @abc.abstractmethod + def command(self, function_file: str, data_file: str) -> str: + """The build command to generate the given test suite.""" + ... + +class ClassicTestGenerator(TestGeneratorInterface): """Test generator script description for the classic (<<2.13) test generator (generate_code.pl).""" - def __init__(self, options): + def __init__(self, options: Options) -> None: self.options = options @staticmethod - def target(c_file): + def target(c_file: str) -> str: return c_file @staticmethod - def script(_source_dir): + def script(_source_dir: str) -> str: return 'tests/scripts/generate_code.pl' @staticmethod - def function_files(function_file): + def function_files(function_file: str) -> List[str]: return ['tests/suites/helpers.function', 'tests/suites/main_test.function', function_file] @staticmethod - def command(function_file, data_file): + def command(function_file: str, data_file: str) -> str: source_dir = os.path.dirname(function_file) if source_dir != os.path.dirname(data_file): raise Exception('Function file and data file are not in the same directory', @@ -237,24 +310,24 @@ class ClassicTestGenerator: os.path.splitext(os.path.basename(function_file))[0], os.path.splitext(os.path.basename(data_file))[0]) -class OnTargetTestGenerator: +class OnTargetTestGenerator(TestGeneratorInterface): """Test generator script description for the >=2.13 test generator with on-target testing support (generate_test_code.py).""" - def __init__(self, options): + def __init__(self, options: Options) -> None: self.options = options @staticmethod - def target(c_file): + def target(c_file: str) -> str: datax_file = os.path.splitext(c_file)[0] + '.datax' return sjoin(c_file, datax_file) @staticmethod - def script(source_dir): + def script(source_dir: str) -> str: return os.path.dirname(source_dir) + '/scripts/generate_test_code.py' @staticmethod - def function_files(function_file, on_target=False): + def function_files(function_file: str, on_target=False) -> List[str]: source_dir = os.path.dirname(function_file) return (['{}/{}.function'.format(source_dir, helper) for helper in ['helpers', 'main_test', @@ -262,7 +335,7 @@ class OnTargetTestGenerator: [function_file]) @classmethod - def command(cls, function_file, data_file): + def command(cls, function_file: str, data_file: str) -> str: source_dir = os.path.dirname(function_file) suite_path = '$(SOURCE_DIR_FROM_TESTS)/' + source_dir return sjoin('$(PYTHON)', @@ -291,7 +364,7 @@ class MakefileMaker: 'library/psa_crypto_driver_wrappers.h', ]) - def __init__(self, options, source_path): + def __init__(self, options: Options, source_path: str) -> None: """Initialize a makefile generator. options is the command line option object. @@ -313,11 +386,11 @@ class MakefileMaker: self.object_extension = self.options.object_extension self.shared_library_extension = self.options.shared_library_extension self.source_path = source_path - self.out = None - self.static_libraries = None - self.help = {} - self.clean = [] - self.variables_to_export = set() + self.out = None #type: Optional[Writable] + self.static_libraries = None #type: Optional[List[str]] + self.help = {} #type: Dict[str, str] # target: help_text + self.clean = [] #type: List[str] # file names or patterns to clean + self.variables_to_export = set() #type: Set[str] if options.QEMU_LD_PREFIX: self.variables_to_export.add('QEMU_LD_PREFIX') self.dependency_cache = { @@ -329,17 +402,19 @@ class MakefileMaker: self.submodules = [submodule for submodule in _submodule_names if self.source_exists(submodule)] if self.source_exists('tests/scripts/generate_test_code.py'): - self.test_generator = OnTargetTestGenerator(options) + self.test_generator = OnTargetTestGenerator(options) #type: TestGeneratorInterface else: self.test_generator = ClassicTestGenerator(options) # Unset fields that are only meaningful at certain later times. # Setting them here makes Pylint happy, but having set them here # makes it harder to diagnose if some method is buggy and attempts # to use a field whose value isn't actually known. + # (TODO: this was back before mypy. Now it's probably simpler to + # allow None, since mypy forces to assert 'is not None' explicitly?) del self.static_libraries # Set when generating the library targets del self.out # Set only while writing the output file - def get_file_submodule(self, filename): + def get_file_submodule(self, filename: str) -> Tuple[Optional[str], str]: """Break up a path into submodule and inner path. More precisely, given a path filename from the root of the source @@ -355,7 +430,7 @@ class MakefileMaker: return submodule, filename[len(submodule) + 1:] return None, filename - def crypto_file_path(self, filename): + def crypto_file_path(self, filename: str) -> str: """Return the path to a crypto file. Look for the file at the given path in the crypto submodule, and if @@ -367,7 +442,7 @@ class MakefileMaker: filename = in_crypto return '$(SOURCE_DIR)/' + filename - def source_exists(self, filename): + def source_exists(self, filename: str) -> bool: """Test if the given path exists in the source tree. This function does not try different submodules. If the file is @@ -375,15 +450,16 @@ class MakefileMaker: """ return os.path.exists(os.path.join(self.options.source, filename)) - def line(self, text): + def line(self, text: str) -> None: """Emit a makefile line.""" + assert self.out is not None self.out.write(text + '\n') - def words(self, *words): + def words(self, *words: str) -> None: """Emit a makefile line obtain by joining the words with spaces.""" self.line(' '.join(words)) - def assign(self, name, *value_words): + def assign(self, name: str, *value_words: str) -> None: """Emit a makefile line that contains an assignment. The assignment is to the variable called name, and its value @@ -392,6 +468,9 @@ class MakefileMaker: nonempty_words = [word for word in value_words if word] self.line(' '.join([name, '='] + nonempty_words)) + # TODO: what should be the type of format? Mypy can check format strings + # since https://github.com/python/mypy/pull/7418, but how do I indicate + # that template is a formatting template? def format(self, template, *args): """Emit a makefile line containing the given formatted template.""" self.line(template.format(*args)) @@ -400,7 +479,7 @@ class MakefileMaker: """Emit a makefile comment line containing the given formatted template.""" self.format('## ' + template, *args) - def add_dependencies(self, name, *dependencies): + def add_dependencies(self, name: str, *dependencies: str) -> None: """Generate dependencies for name.""" parts = (name + ':',) + dependencies simple = ' '.join(parts) @@ -411,7 +490,7 @@ class MakefileMaker: _glob2re_re = re.compile(r'[.^$*+?{}\|()]') @staticmethod - def _glob2re_repl(m): + def _glob2re_repl(m: typing.Match[str]) -> str: if m.group(0) == '*': return '[^/]*' elif m.group(0) == '?': @@ -419,14 +498,14 @@ class MakefileMaker: else: return '\\' + m.group(0) @classmethod - def glob2re(cls, pattern): + def glob2re(cls, pattern: str) -> str: # Simple glob pattern to regex translator. Does not support # character sets properly, which is ok because we don't use them. # We don't use fnmatch or PurePath.match because they let # '*' and '?' match '/'. return re.sub(cls._glob2re_re, cls._glob2re_repl, pattern) - def is_already_cleaned(self, name): + def is_already_cleaned(self, name: str) -> bool: if not self.clean: return False regex = ''.join(['\A(?:', @@ -434,9 +513,9 @@ class MakefileMaker: for patterns in self.clean for pattern in patterns.split()]), ')\Z']) - return re.match(regex, name) + return re.match(regex, name) is not None - def add_clean(self, *elements): + def add_clean(self, *elements: str) -> None: """Add one or more element to the list of things to clean. These can be file paths or wildcard patterns. They can contain @@ -447,8 +526,12 @@ class MakefileMaker: """ self.clean.append(' '.join(elements)) - def target(self, name, dependencies, commands, - help=None, phony=False, clean=None, short=None): + def target(self, name: str, dependencies: Iterable[str], + commands: Iterable[str], + help: Optional[str] = None, + phony=False, + clean: Optional[bool] = None, + short: Optional[str] = None) -> None: """Generate a makefile rule. * name: the target(s) of the rule. This is a string. If there are @@ -490,7 +573,7 @@ class MakefileMaker: if clean: self.add_clean(name) - def setenv_command(self): + def setenv_command(self) -> str: """Generate a shell command to export some environment variables. The values of these variables must not contain the character ``'`` @@ -505,7 +588,7 @@ class MakefileMaker: for name in sorted(self.variables_to_export)]) + '; ') - def environment_option_subsection(self): + def environment_option_subsection(self) -> None: """Generate the assignments to customizable options.""" self.comment('Tool settings') for envopt in _environment_options: @@ -514,7 +597,7 @@ class MakefileMaker: self.assign(envopt.var, getattr(self.options, envopt.attr)) - def settings_section(self): + def settings_section(self) -> None: """Generate assignments to customizable and internal variables. Some additional section-specified variables are assigned in each @@ -557,7 +640,7 @@ class MakefileMaker: self.assign('SOURCE_DIR_FROM_TESTS', '../$(SOURCE_DIR)') self.assign('VALGRIND_LOG_DIR_FROM_TESTS', '.') - def include_path(self, filename): + def include_path(self, filename: str) -> List[str]: """Return the include path for filename. filename must be a path relative to the root of the source tree. @@ -580,12 +663,13 @@ class MakefileMaker: dirs.append(os.path.join(submodule, subdir)) return dirs - def include_path_options(self, filename): + def include_path_options(self, filename: str) -> str: """Return the include path options (-I ...) for filename.""" return ' '.join(['-I $(SOURCE_DIR)/' + dir for dir in self.include_path(filename)]) - def collect_c_dependencies(self, c_file, stack=frozenset()): + def collect_c_dependencies(self, c_file: str, + stack=frozenset()) -> typing.FrozenSet[str]: """Find the build dependencies of the specified C source file. c_file must be an existing C file in the source tree. @@ -607,7 +691,7 @@ class MakefileMaker: if c_file in self.dependency_cache: return self.dependency_cache[c_file] if c_file in stack: - return set() + return frozenset() stack |= {c_file} include_path = ([os.path.dirname(c_file)] + self.include_path(c_file)) dependencies = set() @@ -629,10 +713,11 @@ class MakefileMaker: for dep in frozenset(dependencies): dependencies |= self.collect_c_dependencies(dep, stack) dependencies |= extra - self.dependency_cache[c_file] = dependencies - return dependencies + frozen = frozenset(dependencies) + self.dependency_cache[c_file] = frozen + return frozen - def is_generated(self, filename): + def is_generated(self, filename: str) -> bool: """Whether the specified C file is generated. Implemented with heuristics based on the name. @@ -650,7 +735,7 @@ class MakefileMaker: #return True return False - def is_include_only(self, filename): + def is_include_only(self, filename: str) -> bool: """Whether the specified C file is only meant for use in "#include" directive. Implemented with heuristics based on the name. @@ -660,7 +745,7 @@ class MakefileMaker: 'ssl_test_common_source.c', } - def c_with_dependencies(self, c_file): + def c_with_dependencies(self, c_file: str) -> List[str]: """A list of C dependencies in makefile syntax. Generate the depdendencies of c_file with collect_c_dependencies, @@ -677,7 +762,7 @@ class MakefileMaker: '$(SOURCE_DIR)/' + filename) for filename in sorted(deps) + [c_file]] - def c_dependencies_only(self, c_files): + def c_dependencies_only(self, c_files: Iterable[str]) -> List[str]: """A list of C dependencies in makefile syntax. Generate the depdendencies of each element of c_files with @@ -687,11 +772,12 @@ class MakefileMaker: The elements of c_files themselves are included not in the resulting list unless they are a dependency of another element. """ - deps = set.union(*[self.collect_c_dependencies(c_file) - for c_file in c_files]) + deps = frozenset.union(*[self.collect_c_dependencies(c_file) + for c_file in c_files]) return ['$(SOURCE_DIR)/' + filename for filename in sorted(deps)] - def object_target(self, section, src, extra): + def object_target(self, section: str, + src: SourceFile, extra: Iterable[str]) -> None: c_file = src.make_path() dependencies = self.c_with_dependencies(src.relative_path()) for short, switch, extension in [ @@ -705,15 +791,32 @@ class MakefileMaker: '$(COMMON_FLAGS)', '$(CFLAGS)', ' '.join(['$({}_CFLAGS)'.format(section)] + - extra), + list(extra)), '-o $@', switch, c_file)], short=(short + c_file)) - _potential_libraries = ['crypto', 'x509', 'tls'] + class Library(enum.Enum): + CRYPTO = 1 + X509 = 2 + TLS = 3 - @staticmethod - def library_of(module): + def __lt__(self, other: 'MakefileMaker.Library') -> bool: + return self.value < other.value + + def core(self) -> str: + return self.name.lower() + + def libname(self) -> str: + return 'libmbed' + self.core() + + # For tracing when a library is accidentally implicitly converted to a string + def __str__(self): + import pdb; pdb.set_trace() + return super().__str__() + + @classmethod + def library_of(cls, module: str) -> Library: """Identify which Mbed TLS library contains the specified module. This function bases the result on known module names, defaulting @@ -722,35 +825,41 @@ class MakefileMaker: module = os.path.basename(module) if module.startswith('x509') or \ module in ['certs', 'pkcs11']: - return 'x509' + return cls.Library.X509 elif module.startswith('ssl') or \ module in ['debug', 'net', 'net_sockets']: - return 'tls' + return cls.Library.TLS else: - return 'crypto' + return cls.Library.CRYPTO - @staticmethod - def dash_l_lib(lib): + @classmethod + def dash_l_lib(cls, lib: typing.Union[Library, str]) -> str: """Return the -l option to link with the specified library.""" - base = os.path.splitext(os.path.basename(lib))[0] + if isinstance(lib, cls.Library): + base = lib.libname() + else: + base = os.path.splitext(os.path.basename(lib))[0] if base.startswith('lib'): base = base[3:] - if not base.startswith('mbed'): - base = 'mbed' + base return '-l' + base - def psa_crypto_driver_wrappers_subsection(self, contents): + def psa_crypto_driver_wrappers_subsection( + self, + contents: Dict[Library, List[str]] + ) -> None: generated = 'library/psa_crypto_driver_wrappers.c' script_path = self.crypto_file_path('scripts/psa_crypto_driver_wrappers.py') sources = [self.crypto_file_path(drv) for drv in self.options.psa_driver] - contents['crypto'].append(os.path.splitext(generated)[0]) + contents[self.Library.CRYPTO].append(os.path.splitext(generated)[0]) self.target(generated, [script_path] + sources, [sjoin(script_path, '-o $@', *sources)]) self.object_target('LIBRARY', GeneratedFile(generated), []) - def list_source_files(self, root, *patterns): + def list_source_files(self, + root: str, + *patterns: str) -> Iterable[SourceFile]: """List the source files matching any of the specified patterns. Look for the specified wildcard pattern under all submodules, including @@ -783,7 +892,7 @@ class MakefileMaker: all_sources[base] = src return sorted(all_sources.values()) - def library_section(self): + def library_section(self) -> None: """Generate the section of the makefile for the library directory. The targets are object files for library modules and @@ -804,26 +913,25 @@ class MakefileMaker: modules = self.list_source_files(self.options.source, 'library/*.c') for module in modules: self.object_target('LIBRARY', module, []) - contents = {} + contents = collections.defaultdict(list) # Enumerate libraries and the rules to build them - for lib in self._potential_libraries: - contents[lib] = [] for module in modules: contents[self.library_of(module.base())].append(module.base()) if self.options.psa_driver: self.psa_crypto_driver_wrappers_subsection(contents) - libraries = [lib for lib in self._potential_libraries - if contents[lib]] + libraries = sorted(contents.keys()) for lib in libraries: - self.format('libmbed{}_modules = {}', lib, ' '.join(contents[lib])) - self.format('libmbed{}_objects = $(libmbed{}_modules:={})', - lib, lib, self.object_extension) + libname = lib.libname() + self.format('{}_modules = {}', libname, ' '.join(contents[lib])) + self.format('{}_objects = $({}_modules:={})', + libname, libname, self.object_extension) self.static_libraries = [] shared_libraries = [] for idx, lib in enumerate(libraries): - objects = '$(libmbed{}_objects)'.format(lib) - static = 'library/libmbed{}{}'.format(lib, self.library_extension) - shared = 'library/libmbed{}{}'.format(lib, self.shared_library_extension) + libname = lib.libname() + objects = '$({}_objects)'.format(libname) + static = 'library/{}{}'.format(libname, self.library_extension) + shared = 'library/{}{}'.format(libname, self.shared_library_extension) self.static_libraries.append(static) shared_libraries.append(shared) self.target(static, [objects], @@ -832,13 +940,14 @@ class MakefileMaker: dependent_libraries = libraries[:idx] if dependent_libraries: dash_l_dependent = ('-L . ' + - sjoin(*[self.dash_l_lib(lib) - for lib in dependent_libraries])) + sjoin(*[self.dash_l_lib(dep) + for dep in dependent_libraries])) else: dash_l_dependent = '' - self.target(shared, [objects] + ['library/libmbed{}{}' - .format(lib, self.shared_library_extension) - for lib in dependent_libraries], + self.target(shared, [objects] + ['library/{}{}' + .format(dep.libname(), + self.shared_library_extension) + for dep in dependent_libraries], [sjoin('$(CC)', '$(COMMON_FLAGS)', '$(LDFLAGS)', @@ -884,13 +993,13 @@ class MakefileMaker: 'programs/fuzz/common', 'programs/fuzz/onefile', })) - def auxiliary_objects(self, base): + def auxiliary_objects(self, base: str) -> Iterable[str]: if base.startswith('programs/fuzz'): return ['programs/fuzz/common', 'programs/fuzz/onefile'] else: return self._auxiliary_objects.get(base, []) - def program_libraries(self, app): + def program_libraries(self, app: str) -> List[Library]: """Return the list of libraries that app uses. app is the base of the main file of a sample program (directory @@ -907,14 +1016,14 @@ class MakefileMaker: subdir == 'fuzz' or basename in {'cert_app', 'dh_client', 'dh_server', 'udp_proxy'} ): - return ['crypto', 'x509', 'tls'] + return [self.Library.CRYPTO, self.Library.X509, self.Library.TLS] if (subdir == 'x509' or (basename == 'selftest' and self.source_exists('library/x509.c')) ): - return ['crypto', 'x509'] - return ['crypto'] + return [self.Library.CRYPTO, self.Library.X509] + return [self.Library.CRYPTO] - def extra_link_flags(self, app): + def extra_link_flags(self, app: str) -> Iterable[str]: """Return the list of extra link flags for app. app is the base of the main file of a sample program (directory @@ -930,7 +1039,8 @@ class MakefileMaker: flags.append('$(LDFLAGS_L_DLOPEN)') return flags - def add_run_target(self, program, executable=None): + def add_run_target(self, program: str, + executable: Optional[str] = None) -> None: if executable is None: executable = program + self.executable_extension self.target(program + '.run', @@ -942,7 +1052,8 @@ class MakefileMaker: ['$(SETENV)$(RUN) ' + executable + ' $(RUNS)', 'mv gmon.out $@']) - def program_subsection(self, src, executables): + def program_subsection(self, src: SourceFile, + executables: List[str]) -> None: """Emit the makefile rules for the given sample program. src is a SourceFile object refering to a source file under programs/. @@ -979,7 +1090,7 @@ class MakefileMaker: else: object_deps.append('$(test_common_objects)') libs = list(reversed(self.program_libraries(base))) - lib_files = ['library/libmbed{}{}'.format(lib, self.library_extension) + lib_files = ['library/{}{}'.format(lib.libname(), self.library_extension) for lib in libs] dash_l_libs = [self.dash_l_lib(lib) for lib in libs] self.target(exe_file, @@ -990,14 +1101,15 @@ class MakefileMaker: '$(COMMON_FLAGS)', '$(LDFLAGS)', '$(PROGRAMS_LDFLAGS)', - sjoin(*(dash_l_libs + self.extra_link_flags(base))), + sjoin(*(dash_l_libs + + list(self.extra_link_flags(base)))), '$(EXTRA_LIBS)', '-o $@')], clean=False, short='LD $@') executables.append(exe_file) - def programs_section(self): + def programs_section(self) -> None: """Emit the makefile rules to build the sample programs.""" self.comment('Sample programs') self.assign('PROGRAMS_CFLAGS', @@ -1011,7 +1123,7 @@ class MakefileMaker: for ext in (self.assembly_extension, self.object_extension)]) programs = self.list_source_files(self.options.source, 'programs/*/*.c') - executables = [] + executables = [] #type: List[str] for src in programs: self.program_subsection(src, executables) dirs = set(src.target_dir() for src in programs) @@ -1029,7 +1141,7 @@ class MakefileMaker: self.add_run_target('programs/test/selftest') # TODO: *_demo.sh - def define_tests_common_objects(self): + def define_tests_common_objects(self) -> None: """Emit the definition of tests_common_objects. These objects are needed for unit tests and for sample programs @@ -1046,7 +1158,9 @@ class MakefileMaker: tests_common_objects.append(object_file) self.assign('test_common_objects', *tests_common_objects) - def test_subsection(self, src, executables, groups): + def test_subsection(self, src: SourceFile, + executables: List[str], + groups: Dict[str, List[str]]): """Emit the makefile rules to build one test suite. src is a SourceFile object for a .data file. @@ -1127,7 +1241,7 @@ class MakefileMaker: short='VALGRIND tests/' + exe_basename, phony=True) - def test_group_targets(self, groups): + def test_group_targets(self, groups: Dict[str, List[str]]) -> None: """Emit run targets for test groups. A test group is a group of test executables that share the same @@ -1149,7 +1263,7 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi short='', phony=True) - def tests_section(self): + def tests_section(self) -> None: """Emit makefile rules to build and run test suites.""" self.comment('Test targets') self.assign('TESTS_CFLAGS', @@ -1161,8 +1275,10 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi '-L library', '$(TESTS_EXTRA_LDFLAGS)') self.assign('TESTS_EXTRA_OBJECTS') + assert self.static_libraries is not None self.assign('test_libs', - *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)], + *[self.dash_l_lib(lib) + for lib in reversed(self.static_libraries)], '$(EXTRA_LIBS)') self.assign('test_build_deps', '$(test_common_objects)', *self.static_libraries) @@ -1173,8 +1289,8 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi self.add_clean('tests/*.c', 'tests/*.datax') data_files = self.list_source_files(self.options.source, 'tests/suites/*.data') - executables = [] - groups = {} + executables = [] #type: List[str] + groups = {} #type: Dict[str, List[str]] for src in data_files: self.test_subsection(src, executables, groups) self.assign('test_apps', *executables) @@ -1216,7 +1332,7 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi rm -f files.info tests.info all.info final.info descriptions """ - def misc_section(self): + def misc_section(self) -> None: """Emit miscellaneous other targets. """ if self.options.coverage_targets: @@ -1225,22 +1341,23 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi [line.strip() for line in self.COVERAGE_RULE.split('\n')], help='Generate test coverage report') - def help_lines(self): + def help_lines(self) -> Iterable[str]: """Return the lines of text to show for the 'help' target.""" return ['{:<14} : {}'.format(name, self.help[name]) for name in sorted(self.help.keys())] - def variables_help_lines(self): + def variables_help_lines(self) -> Iterable[str]: """Return the lines of text to show for the 'help-variables' target.""" env_opts = [(envopt.var, envopt.help) - for envopt in _environment_options] + for envopt in _environment_options + if envopt.help is not None] ad_hoc = [ ('V', 'Show commands in full if non-empty.') ] return ['{:<14} # {}'.format(name, text) for (name, text) in sorted(env_opts + ad_hoc)] - def output_all(self): + def output_all(self) -> None: """Emit the whole makefile.""" self.comment('Generated by {}', ' '.join(sys.argv)) self.comment('Do not edit this file! All modifications will be lost.') @@ -1286,7 +1403,7 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi self.line('') self.comment('End of generated file.') - def generate(self): + def generate(self) -> None: """Generate the makefile.""" destination = os.path.join(self.options.dir, 'Makefile') temp_file = destination + '.new' @@ -1298,13 +1415,14 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi del self.out os.replace(temp_file, destination) + class ConfigMaker: """Parent class for config.h or mbedtls_config.h builders. Typical usage: ChildClass(options).run() """ - def __init__(self, options): + def __init__(self, options: Options) -> None: """Initialize a config header builder with the given command line options.""" self.options = options self.source_file = options.config_file @@ -1325,30 +1443,30 @@ class ConfigMaker: self.target_file = os.path.join(options.dir, 'include', 'mbedtls', basename) - def start(self): + def start(self) -> None: """Builder-specific method which is called first.""" raise NotImplementedError - def set(self, name, value=None): + def set(self, name: str, value: Optional[str] = None) -> None: """Builder-specific method to set name to value.""" raise Exception("Configuration method {} does not support setting options" - .format(options.config_mode)) + .format(self.options.config_mode)) - def unset(self, name): + def unset(self, name: str) -> None: """Builder-specific method to unset name.""" raise Exception("Configuration method {} does not support unsetting options" - .format(options.config_mode)) + .format(self.options.config_mode)) - def batch(self, name): + def batch(self, name: str) -> None: """Builder-specific method to set the configuration with the given name.""" raise Exception("Configuration method {} does not support batch-setting options" - .format(options.config_mode)) + .format(self.options.config_mode)) - def finish(self): + def finish(self) -> None: """Builder-specific method which is called last.""" raise NotImplementedError - def run(self): + def run(self) -> None: """Go ahead and generate the config header.""" self.start() if self.options.config_name is not None: @@ -1371,16 +1489,16 @@ class ConfigMaker: value = spec[m.end('sep'):] self.set(name, value) self.finish() -_config_classes = {} +_config_classes = {} #type: Dict[str, typing.Type[ConfigMaker]] class ConfigCopy(ConfigMaker): """ConfigMaker implementation that copies the config header and runs the config script.""" - def start(self): + def start(self) -> None: if not are_same_existing_files(self.source_file, self.target_file): shutil.copyfile(self.source_file, self.target_file) - def run_config_script(self, *args): + def run_config_script(self, *args: str) -> None: if os.path.exists(os.path.join(self.options.source, 'scripts', 'config.py')): cmd = [sys.executable, 'scripts/config.py'] @@ -1389,30 +1507,30 @@ class ConfigCopy(ConfigMaker): cmd += ['-f', os.path.abspath(self.target_file)] + list(args) subprocess.check_call(cmd, cwd=self.options.source) - def set(self, name, value=None): + def set(self, name: str, value: Optional[str] = None) -> None: if value is None: self.run_config_script('set', name) else: self.run_config_script('set', name, value) - def unset(self, name): + def unset(self, name: str) -> None: self.run_config_script('unset', name) - def batch(self, name): + def batch(self, name: str) -> None: self.run_config_script(name) - def finish(self): + def finish(self) -> None: pass _config_classes['copy'] = ConfigCopy class ConfigInclude(ConfigMaker): """ConfigMaker implementation that makes a config script that #includes the base one.""" - def __init__(self, *args): + def __init__(self, *args) -> None: super().__init__(*args) - self.lines = [] + self.lines = [] #type: List[str] - def start(self): + def start(self) -> None: source_path = self.source_file_path if not os.path.isabs(source_path): source_path = os.path.join(os.pardir, os.pardir, source_path) @@ -1420,16 +1538,16 @@ class ConfigInclude(ConfigMaker): self.lines.append('#include "{}"'.format(source_path)) self.lines.append('') - def set(self, name, value=None): + def set(self, name: str, value: Optional[str] = None) -> None: if value: self.lines.append('#define ' + name + ' ' + value) else: self.lines.append('#define ' + name) - def unset(self, name): + def unset(self, name: str) -> None: self.lines.append('#undef ' + name) - def finish(self): + def finish(self) -> None: if self.version < 3: self.lines.append('') self.lines.append('#undef MBEDTLS_CHECK_CONFIG_H') @@ -1443,6 +1561,7 @@ class ConfigInclude(ConfigMaker): out.write(line + '\n') _config_classes['include'] = ConfigInclude + class BuildTreeMaker: """Prepare an Mbed TLS/Crypto build tree. @@ -1454,7 +1573,7 @@ class BuildTreeMaker: Typical usage: BuildTreeMaker(options).run() """ - def __init__(self, options): + def __init__(self, options: Options) -> None: self.options = options self.source_path = os.path.abspath(options.source) options.in_tree_build = are_same_existing_files(self.options.source, @@ -1468,7 +1587,7 @@ class BuildTreeMaker: options.config_mode = 'copy' self.config = _config_classes[options.config_mode](options) - def programs_subdirs(self): + def programs_subdirs(self) -> Iterable[str]: """Detect subdirectories for sample programs.""" tops = ([self.options.source] + [os.path.join(self.options.source, submodule) @@ -1478,13 +1597,13 @@ class BuildTreeMaker: for d in glob.glob(os.path.join(top, 'programs', '*')) if os.path.isdir(d)] - def make_subdir(self, subdir): + def make_subdir(self, subdir: List[str]) -> None: """Create the given subdirectory of the build tree.""" path = os.path.join(self.options.dir, *subdir) if not os.path.exists(path): os.makedirs(path) - def make_link(self, target, link): + def make_link(self, target: str, link: str) -> None: """Create a symbolic link called link pointing to target. link is a path relative to the build directory. @@ -1495,13 +1614,15 @@ class BuildTreeMaker: if not os.path.lexists(link_path): os.symlink(target, link_path) - def link_to_source(self, sub_link, link): + def link_to_source(self, + sub_link: typing.List[str], + link: typing.List[str]) -> None: """Create a symbolic link in the build tree to sub_link under the source.""" self.make_link(os.path.join(*([os.pardir] * (len(link) - 1) + ['source'] + sub_link)), os.path.join(*link)) - def link_to_source_maybe(self, link): + def link_to_source_maybe(self, link: typing.List[str]) -> None: """Create a symbolic link in the build tree to a target of the same name in the source tree in any submodule. @@ -1513,7 +1634,7 @@ class BuildTreeMaker: if os.path.exists(os.path.join(self.options.source, *sub_link)): self.link_to_source(sub_link, link) - def link_test_suites(self): + def link_test_suites(self) -> None: # This is a hack due to the weird paths produced by the test generator. # The test generator generates a file with #file directives that # point to "suites/xxx.function". Since we run the compiler from the @@ -1526,7 +1647,7 @@ class BuildTreeMaker: # in a submodule. self.make_link('source/tests/suites', 'suites') - def prepare_source(self): + def prepare_source(self) -> None: """Generate extra source files in the source tree. This is necessary in the Mbed TLS development branch since before 3.0: @@ -1549,7 +1670,7 @@ class BuildTreeMaker: subprocess.check_call([make_command, 'generated_files'], cwd=self.source_path) - def run(self): + def run(self) -> None: """Go ahead and prepate the build tree.""" for subdir in ([['include', 'mbedtls'], ['library'], @@ -1580,7 +1701,7 @@ set for this preset. The field _help in a description has a special meaning: it's the documentation of the preset. """ _preset_options = { - '': {}, # empty preset = use defaults + '': argparse.Namespace(), # empty preset = use defaults 'asan': argparse.Namespace( _help='Clang with ASan+UBSan, current configuration', config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], @@ -1665,7 +1786,7 @@ _preset_options = { config_unset=['MBEDTLS_AESNI_C'], CFLAGS='-g -O3', ), -} +} #type: Dict[str, argparse.Namespace] """Default values for some options. @@ -1684,13 +1805,14 @@ _default_options = { 'source': os.curdir, } -def set_default_option(options, attr, value): +def set_default_option(options: Options, attr: str, + value: typing.Any) -> None: if getattr(options, attr) is None: setattr(options, attr, value) elif isinstance(value, list): setattr(options, attr, value + getattr(options, attr)) -def handle_cross(options): +def handle_cross(options: Options) -> None: """Update settings to handle --cross.""" if options.cross is None: return @@ -1700,7 +1822,7 @@ def handle_cross(options): set_default_option(options, 'CC', options.cross + '-gcc') set_default_option(options, 'dir', 'build-' + options.cross) -def set_default_options(options): +def set_default_options(options: Options) -> None: """Apply the preset if any and set default for remaining options. We set defaults via this function rather than via the `default` @@ -1729,7 +1851,7 @@ def set_default_options(options): for envopt in _environment_options: set_default_option(options, envopt.attr, envopt.default) -def preset_help(): +def preset_help() -> str: """Return a documentation string for the presets.""" return '\n'.join(['Presets:'] + ['{}: {}'.format(name, _preset_options[name]._help) @@ -1737,7 +1859,7 @@ def preset_help(): if hasattr(_preset_options[name], '_help')] + ['']) -def arg_type_bool(arg): +def arg_type_bool(arg: typing.Union[bool, str]) -> bool: """Boolean argument type for argparse.add_argument.""" if not isinstance(arg, str): return arg @@ -1749,7 +1871,7 @@ def arg_type_bool(arg): else: raise argparse.ArgumentTypeError('invalid boolean value: ' + repr(arg)) -def main(): +def main() -> None: """Process the command line and prepare a build tree accordingly.""" parser = argparse.ArgumentParser(formatter_class=argparse.RawDescriptionHelpFormatter, description=__doc__, From 660e4935a2279b9073f9e5fae61fa1f52f1d5d9c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 23:19:43 +0100 Subject: [PATCH 03/38] New config mode: explicit Make a config file that only sets the options given explicitly, without copying or including a base file. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 35 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index be242ed..b11dff8 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1523,19 +1523,16 @@ class ConfigCopy(ConfigMaker): pass _config_classes['copy'] = ConfigCopy -class ConfigInclude(ConfigMaker): - """ConfigMaker implementation that makes a config script that #includes the base one.""" +class ConfigExplicit(ConfigMaker): + """ConfigMaker implementation that makes a config script with only explicitly set options.""" def __init__(self, *args) -> None: super().__init__(*args) self.lines = [] #type: List[str] def start(self) -> None: - source_path = self.source_file_path - if not os.path.isabs(source_path): - source_path = os.path.join(os.pardir, os.pardir, source_path) - self.lines.append('#ifndef MBEDTLS_CHECK_CONFIG_H') - self.lines.append('#include "{}"'.format(source_path)) + self.lines.append('#ifndef MBEDTLS_CONFIG_H') + self.lines.append('#define MBEDTLS_CONFIG_H') self.lines.append('') def set(self, name: str, value: Optional[str] = None) -> None: @@ -1547,6 +1544,25 @@ class ConfigInclude(ConfigMaker): def unset(self, name: str) -> None: self.lines.append('#undef ' + name) + def finish(self) -> None: + self.lines.append('') + self.lines.append('#endif') + with open(self.target_file, 'w') as out: + for line in self.lines: + out.write(line + '\n') +_config_classes['explicit'] = ConfigExplicit + +class ConfigInclude(ConfigExplicit): + """ConfigMaker implementation that makes a config script that #includes the base one.""" + + def start(self) -> None: + source_path = self.source_file_path + if not os.path.isabs(source_path): + source_path = os.path.join(os.pardir, os.pardir, source_path) + self.lines.append('#ifndef MBEDTLS_CHECK_CONFIG_H') + self.lines.append('#include "{}"'.format(source_path)) + self.lines.append('') + def finish(self) -> None: if self.version < 3: self.lines.append('') @@ -1555,10 +1571,7 @@ class ConfigInclude(ConfigMaker): self.lines.append('#define mbedtls_iso_c_forbids_empty_translation_units mbedtls_iso_c_forbids_empty_translation_units2') self.lines.append('#include "mbedtls/check_config.h"') self.lines.append('#undef mbedtls_iso_c_forbids_empty_translation_units') - self.lines.append('#endif') - with open(self.target_file, 'w') as out: - for line in self.lines: - out.write(line + '\n') + super().finish() _config_classes['include'] = ConfigInclude From b738dffdefcede708d6265e12000d00bda8810af Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 23:20:40 +0100 Subject: [PATCH 04/38] New source subdirectory in 3.1+: tests/opt-testcases Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b11dff8..df3760c 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1698,6 +1698,7 @@ class BuildTreeMaker: ['tests', 'configs'], ['tests', 'compat.sh'], ['tests', 'data_files'], + ['tests', 'opt-testcases'], ['tests', 'scripts'], ['tests', 'ssl-opt.sh']]: self.link_to_source_maybe(link) From b4196fff1b819470761545e595835eed437762b2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Mar 2022 23:20:54 +0100 Subject: [PATCH 05/38] New preset: ecc-heap Similar to scripts/ecc-heap.sh. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index df3760c..5372f03 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1736,6 +1736,34 @@ _preset_options = { CFLAGS='-O0', COMMON_FLAGS='-g3', ), + 'ecc-heap': argparse.Namespace( + _help='Like scripts/ecc-heap.sh', + config_mode='explicit', + config_set=[ + 'MBEDTLS_PLATFORM_C', + 'MBEDTLS_PLATFORM_MEMORY', + 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', + 'MBEDTLS_MEMORY_DEBUG', + 'MBEDTLS_TIMING_C', + 'MBEDTLS_BIGNUM_C', + 'MBEDTLS_ECP_C', + 'MBEDTLS_ASN1_PARSE_C', + 'MBEDTLS_ASN1_WRITE_C', + 'MBEDTLS_ECDSA_C', + 'MBEDTLS_ECDH_C', + 'MBEDTLS_ECP_DP_SECP192R1_ENABLED', + 'MBEDTLS_ECP_DP_SECP224R1_ENABLED', + 'MBEDTLS_ECP_DP_SECP256R1_ENABLED', + 'MBEDTLS_ECP_DP_SECP384R1_ENABLED', + 'MBEDTLS_ECP_DP_SECP521R1_ENABLED', + 'MBEDTLS_ECP_DP_CURVE25519_ENABLED', + 'MBEDTLS_SHA256_C', #necessary for the ECDSA benchmark + 'MBEDTLS_SHA224_C', #In 3.0, SHA256 requires SHA224 + 'MBEDTLS_ECP_WINDOW_SIZE=4', + 'MBEDTLS_ECP_FIXED_POINT_OPTIM=1', + ], + CFLAGS='-O2', + ), 'full': argparse.Namespace( _help='Full configuration', config_name='full', From 49a8ddcf86da39c2282af37af24d85078caa4254 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:16:04 +0200 Subject: [PATCH 06/38] Improve support for 3rdparty "submodules" Fix the build with Everest enabled. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 75 ++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 5372f03..bfa3648 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -164,7 +164,7 @@ Options = argparse.Namespace A submodule is a subdirectory of the source tree which has the same general structure as the source tree. """ -_submodule_names = ['crypto'] +SUBMODULE_NAMES = ['crypto'] class SourceFile: @@ -364,6 +364,24 @@ class MakefileMaker: 'library/psa_crypto_driver_wrappers.h', ]) + @staticmethod + def gather_submodules(source_dir) -> Iterable[str]: + """Iterate over the existing submodules under source_dir. + + 3rdparty directories are considered submodules, as well as + names registered in SUBMODULE_NAMES. + """ + for m in SUBMODULE_NAMES: + if os.path.isdir(os.path.join(source_dir, m)): + yield m + for d in glob.glob(os.path.join(source_dir, '3rdparty', '*')): + continue #TODO + if os.path.isdir(os.path.join(d, 'include')) or \ + os.path.isdir(os.path.join(d, 'library')) or \ + os.path.isdir(os.path.join(d, 'programs')) or \ + os.path.isdir(os.path.join(d, 'tests')): + yield d[len(source_dir)+1:] + def __init__(self, options: Options, source_path: str) -> None: """Initialize a makefile generator. @@ -399,8 +417,7 @@ class MakefileMaker: # dependencies change over time. 'library/psa_crypto_driver_wrappers.c': self.PSA_CRYPTO_DRIVER_WRAPPERS_DEPENDENCIES, } - self.submodules = [submodule for submodule in _submodule_names - if self.source_exists(submodule)] + self.submodules = list(self.gather_submodules(self.options.source)) if self.source_exists('tests/scripts/generate_test_code.py'): self.test_generator = OnTargetTestGenerator(options) #type: TestGeneratorInterface else: @@ -661,6 +678,11 @@ class MakefileMaker: dirs.append(subdir) if submodule is not None: dirs.append(os.path.join(submodule, subdir)) + if submodule == '3rdparty/everest': + dirs.append(os.path.join(submodule, 'include/everest')) + if '/kremlib/' in filename: + dirs.append(os.path.join(submodule, 'include/everest/kremlib')) + print(filename, dirs) return dirs def include_path_options(self, filename: str) -> str: @@ -777,9 +799,16 @@ class MakefileMaker: return ['$(SOURCE_DIR)/' + filename for filename in sorted(deps)] def object_target(self, section: str, - src: SourceFile, extra: Iterable[str]) -> None: - c_file = src.make_path() - dependencies = self.c_with_dependencies(src.relative_path()) + src: SourceFile, + deps: Iterable[str] = (), + auto_deps: bool = True, + extra_flags: str = '') -> None: + c_path = src.make_path() + dependencies = list(deps) + if auto_deps: + dependencies += self.c_with_dependencies(src.relative_path()) + else: + dependencies += [c_path] for short, switch, extension in [ ('CC -S ', '-S', self.assembly_extension), ('CC ', '-c', self.object_extension), @@ -790,11 +819,18 @@ class MakefileMaker: '$(WARNING_CFLAGS)', '$(COMMON_FLAGS)', '$(CFLAGS)', - ' '.join(['$({}_CFLAGS)'.format(section)] + - list(extra)), + ' '.join(['$({}_CFLAGS)'.format(section)]), + # TODO: some files need extra -I (currently: + # 3rdparty/everest/library/**/*.c need + # 3rdparty/everest/include/everst and some + # need .../kremlib as well). This is currently + # handled in the include_path method, but + # we don't call it here, only for a section + # as a whole. + extra_flags.strip(), '-o $@', - switch, c_file)], - short=(short + c_file)) + switch, c_path)], + short=(short + c_path)) class Library(enum.Enum): CRYPTO = 1 @@ -855,7 +891,7 @@ class MakefileMaker: self.target(generated, [script_path] + sources, [sjoin(script_path, '-o $@', *sources)]) - self.object_target('LIBRARY', GeneratedFile(generated), []) + self.object_target('LIBRARY', GeneratedFile(generated)) def list_source_files(self, root: str, @@ -871,7 +907,7 @@ class MakefileMaker: """ # FIXME: for error.c at least, we need the root, not the submodule. all_sources = {} - for submodule in _submodule_names + ['']: + for submodule in self.submodules + ['']: submodule_root = os.path.join(root, submodule) start = len(submodule_root) if submodule: @@ -912,7 +948,7 @@ class MakefileMaker: # Enumerate modules and emit the rules to build them modules = self.list_source_files(self.options.source, 'library/*.c') for module in modules: - self.object_target('LIBRARY', module, []) + self.object_target('LIBRARY', module) contents = collections.defaultdict(list) # Enumerate libraries and the rules to build them for module in modules: @@ -1076,9 +1112,9 @@ class MakefileMaker: [script_path], short='Gen $@') self.add_clean(base + '_generated.c') - extra_includes = ['-I', src.target_dir()] # for generated files + extra_flags = '-I ' + src.target_dir() # for generated files object_file = src.target(self.object_extension) - self.object_target('PROGRAMS', src, extra_includes) + self.object_target('PROGRAMS', src, extra_flags=extra_flags) if base in self._auxiliary_sources: return exe_file = src.target(self.executable_extension) @@ -1153,7 +1189,7 @@ class MakefileMaker: 'tests/src/drivers/*.c') tests_common_objects = [] for src in tests_common_sources: - self.object_target('TESTS', src, []) + self.object_target('TESTS', src) object_file = src.target(self.object_extension) tests_common_objects.append(object_file) self.assign('test_common_objects', *tests_common_objects) @@ -1599,12 +1635,13 @@ class BuildTreeMaker: else: options.config_mode = 'copy' self.config = _config_classes[options.config_mode](options) + self.submodules = self.makefile.submodules def programs_subdirs(self) -> Iterable[str]: """Detect subdirectories for sample programs.""" tops = ([self.options.source] + [os.path.join(self.options.source, submodule) - for submodule in _submodule_names]) + for submodule in self.submodules]) return [os.path.basename(d) for top in tops for d in glob.glob(os.path.join(top, 'programs', '*')) @@ -1640,9 +1677,9 @@ class BuildTreeMaker: name in the source tree in any submodule. Check the root first, then the submodules in the order given by - _submodule_names. + SUBMODULE_NAMES. """ - for submodule in [''] + _submodule_names: + for submodule in [''] + SUBMODULE_NAMES: sub_link = [submodule] + link if os.path.exists(os.path.join(self.options.source, *sub_link)): self.link_to_source(sub_link, link) From 99f539d2ef297bd1b87e4e3a1e2acc32a74076e2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:18:37 +0200 Subject: [PATCH 07/38] Add pkcs7 to the list of x509 modules Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index bfa3648..25aab4c 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -860,7 +860,7 @@ class MakefileMaker: """ module = os.path.basename(module) if module.startswith('x509') or \ - module in ['certs', 'pkcs11']: + module in ['certs', 'pkcs7', 'pkcs11']: return cls.Library.X509 elif module.startswith('ssl') or \ module in ['debug', 'net', 'net_sockets']: From d66dd9733781ba2db46ed3dac80743ad83d3bb14 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:19:17 +0200 Subject: [PATCH 08/38] Support zeroize test program properly Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 25aab4c..b70efb8 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -985,6 +985,7 @@ class MakefileMaker: self.shared_library_extension) for dep in dependent_libraries], [sjoin('$(CC)', + '$(WARNING_CFLAGS)', '$(COMMON_FLAGS)', '$(LDFLAGS)', '$(DLLFLAGS)', @@ -1073,6 +1074,8 @@ class MakefileMaker: flags.append('$(LDFLAGS_L_THREADS)') if 'dlopen' in app: flags.append('$(LDFLAGS_L_DLOPEN)') + if 'zeroize' in app: + flags.append('-g3') return flags def add_run_target(self, program: str, @@ -1113,6 +1116,8 @@ class MakefileMaker: short='Gen $@') self.add_clean(base + '_generated.c') extra_flags = '-I ' + src.target_dir() # for generated files + if 'zeroize' in base: + extra_flags += ' -g3' object_file = src.target(self.object_extension) self.object_target('PROGRAMS', src, extra_flags=extra_flags) if base in self._auxiliary_sources: @@ -1134,6 +1139,7 @@ class MakefileMaker: [sjoin('$(CC)', object_file, sjoin(*object_deps), + '$(WARNING_CFLAGS)', '$(COMMON_FLAGS)', '$(LDFLAGS)', '$(PROGRAMS_LDFLAGS)', @@ -1176,6 +1182,9 @@ class MakefileMaker: self.add_run_target('programs/test/benchmark') self.add_run_target('programs/test/selftest') # TODO: *_demo.sh + self.target('test_zeroize', + ['programs/test/zeroize'], + ['gdb -x ../tests/scripts/test_zeroize.gdb -nw -batch -nx']) def define_tests_common_objects(self) -> None: """Emit the definition of tests_common_objects. @@ -1731,6 +1740,7 @@ class BuildTreeMaker: if not self.options.in_tree_build and not os.path.exists(source_link): os.symlink(self.source_path, source_link) for link in [['include', 'psa'], # hack for psa_constant_names.py + ['programs', 'test', 'zeroize.c'], ['scripts'], ['tests', 'configs'], ['tests', 'compat.sh'], From 822b172fb2f5e13c83748b9a3f0ea0142e0ba04a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:20:24 +0200 Subject: [PATCH 09/38] Fix dependencies of the dlopen test program Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b70efb8..7203d09 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1126,14 +1126,18 @@ class MakefileMaker: object_deps = [dep + self.object_extension for dep in self.auxiliary_objects(base) if self.source_exists(dep + '.c')] - if base == 'programs/test/dlopen': - object_deps.append('library/platform.o library/platform_util.o') - else: - object_deps.append('$(test_common_objects)') libs = list(reversed(self.program_libraries(base))) lib_files = ['library/{}{}'.format(lib.libname(), self.library_extension) for lib in libs] dash_l_libs = [self.dash_l_lib(lib) for lib in libs] + if base == 'programs/test/dlopen': + object_deps.append('library/platform.o library/platform_util.o') + else: + object_deps.append('$(test_crypto_objects)') + if self.Library.X509 in libs: + object_deps.append('$(test_x509_objects)') + if self.Library.TLS in libs: + object_deps.append('$(test_ssl_objects)') self.target(exe_file, [object_file] + object_deps + lib_files, [sjoin('$(CC)', From 60614354f8aa35ca16515f0f10ab8e778004fa02 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:21:19 +0200 Subject: [PATCH 10/38] Use Clang for ASAn+UBSan builds Clang finds more things than GCC Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 7203d09..951e8a8 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1821,12 +1821,13 @@ _preset_options = { config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], ), 'full-asan': argparse.Namespace( - _help='Full configuration with GCC+ASan+UBSan', + _help='Full configuration with Clang+ASan+UBSan', config_name='full', config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], - CC='gcc', + CC='clang', CFLAGS='-O', - COMMON_FLAGS='-fsanitize=address,undefined -fno-common -g3', + COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', + WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', ), 'full-debug': argparse.Namespace( _help='Full configuration, debug build', From 6d834a9b6abe8cbea965675e6758d7352c1e275f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:23:51 +0200 Subject: [PATCH 11/38] New preset 'cf' for constant-flow testing with MSan Signed-off-by: Gilles Peskine Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 951e8a8..c4c87eb 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1774,6 +1774,15 @@ _preset_options = { CFLAGS='-O', COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', ), + 'cf': argparse.Namespace( + _help='Constant flow with MSan, current configuration', + config_set=['MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN'], + config_unset=['MBEDTLS_AESNI_C'], + CC='clang', + CFLAGS='-O', + COMMON_FLAGS='-fsanitize=memory -g3', + WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', + ), 'coverage': argparse.Namespace( _help='Build with coverage instrumentation', config_name='full', From eb5917ed1cfeac7428ec6a5bb7af2a321adeaec9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:24:24 +0200 Subject: [PATCH 12/38] New preset 'opt' for a straightforward performance-optimized build Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index c4c87eb..856cb91 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1876,6 +1876,10 @@ _preset_options = { CFLAGS='-O', COMMON_FLAGS='-fsanitize=memory -g3', ), + 'opt': argparse.Namespace( + _help='Optimized build', + CFLAGS='-O3', + ), 'thumb': argparse.Namespace( _help='Default configuration for arm-linux-gnueabi', CC='arm-linux-gnueabi-gcc', From 4a2fd1ae2a20fe70a95f525f04fba05e4ce37129 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:24:59 +0200 Subject: [PATCH 13/38] New preset 'noplatform' for a configuration without platform stuff Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 856cb91..38f5ad3 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1876,6 +1876,29 @@ _preset_options = { CFLAGS='-O', COMMON_FLAGS='-fsanitize=memory -g3', ), + 'noplatform': argparse.Namespace( + _help='Full except platform/fsio/net', + config_name='full', + config_unset=[ + 'MBEDTLS_PLATFORM_C', + 'MBEDTLS_NET_C', + 'MBEDTLS_PLATFORM_MEMORY', + 'MBEDTLS_PLATFORM_PRINTF_ALT', + 'MBEDTLS_PLATFORM_FPRINTF_ALT', + 'MBEDTLS_PLATFORM_SNPRINTF_ALT', + 'MBEDTLS_PLATFORM_VSNPRINTF_ALT', + 'MBEDTLS_PLATFORM_TIME_ALT', + 'MBEDTLS_PLATFORM_EXIT_ALT', + 'MBEDTLS_PLATFORM_SETBUF_ALT', + 'MBEDTLS_PLATFORM_NV_SEED_ALT', + 'MBEDTLS_ENTROPY_NV_SEED', + 'MBEDTLS_FS_IO', + 'MBEDTLS_PSA_CRYPTO_SE_C', + 'MBEDTLS_PSA_CRYPTO_STORAGE_C', + 'MBEDTLS_PSA_ITS_FILE_C', + ], + WARNING_CFLAGS='-Werror -Wall -Wextra -std=c99 -D_DEFAULT_SOURCE', + ), 'opt': argparse.Namespace( _help='Optimized build', CFLAGS='-O3', From 860243110c8866b0ce2d0a49c5a7a4aa4c0225d6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:25:42 +0200 Subject: [PATCH 14/38] Add more Clang warnings to 'asan' and 'msan' presets Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 38f5ad3..9702c0a 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1773,6 +1773,7 @@ _preset_options = { CC='clang', CFLAGS='-O', COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', + WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', ), 'cf': argparse.Namespace( _help='Constant flow with MSan, current configuration', @@ -1875,6 +1876,7 @@ _preset_options = { CC='clang', CFLAGS='-O', COMMON_FLAGS='-fsanitize=memory -g3', + WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', ), 'noplatform': argparse.Namespace( _help='Full except platform/fsio/net', From 578f793e3eeefb2d35a4b82363b3cd735486c373 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:26:14 +0200 Subject: [PATCH 15/38] Fix test suite running when there is both foo.data and foo.bar.data Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 43 +++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 9702c0a..8516407 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1233,6 +1233,7 @@ class MakefileMaker: function_file = os.path.join(source_dir, function_base + '.function') function_files = self.test_generator.function_files(function_file) c_file = os.path.join('tests', base + '.c') + object_file = os.path.join('tests', base + self.object_extension) exe_basename = base + self.executable_extension exe_file = os.path.join('tests', exe_basename) if function_base in groups: @@ -1245,20 +1246,22 @@ class MakefileMaker: [data_file])], ['cd tests && ' + generate_command], short='Gen $@') + self.object_target('TESTS', GeneratedFile(c_file), + auto_deps=False, + deps=self.c_with_dependencies(function_file), + extra_flags='$(TESTS_CFLAGS)') self.target(exe_file, (self.c_dependencies_only(function_files) + - ['$(lib)', '$(test_build_deps)', c_file]), + ['$(lib)', '$(test_build_deps)', object_file]), [sjoin('$(CC)', + object_file, '$(WARNING_CFLAGS)', '$(COMMON_FLAGS)', - '$(CFLAGS)', - '$(TESTS_CFLAGS)', - '$(TESTS_EXTRA_OBJECTS)', - c_file, '$(LDFLAGS)', '$(TESTS_LDFLAGS)', '$(test_common_objects)', '$(test_libs)', + '$(TESTS_EXTRA_OBJECTS)', '-o $@')], clean=False, short='CC $@') @@ -1290,27 +1293,25 @@ class MakefileMaker: short='VALGRIND tests/' + exe_basename, phony=True) - def test_group_targets(self, groups: Dict[str, List[str]]) -> None: - """Emit run targets for test groups. + def test_group_target(self, base : str, executables : List[str]) -> None: + """Emit run targets for a test group. A test group is a group of test executables that share the same - .function file. The groups parameter is a dictionary mapping group - names to the list of executables that they contain. + .function file. """ use_run_test_suites = False - for base, executables in groups.items(): - shell_code = ''' + shell_code = ''' failures=; for x in {}; do -./$$x || failures="$$failures $$x"; +$(RUN) ./$$x $(RUNS) || failures="$$failures $$x"; done; if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi '''.format(' '.join([re.sub(r'.*/', r'', exe) for exe in executables])) - self.target('tests/' + base + '.run', - groups[base] + ['tests/seedfile'], - ['$(SETENV)cd tests && ' + shell_code], - short='', - phony=True) + self.target('tests/' + base + '.run', + executables + ['tests/seedfile'], + ['$(SETENV)cd tests && ' + shell_code], + short='', + phony=True) def tests_section(self) -> None: """Emit makefile rules to build and run test suites.""" @@ -1343,7 +1344,13 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi for src in data_files: self.test_subsection(src, executables, groups) self.assign('test_apps', *executables) - self.test_group_targets(groups) + for base in sorted(groups.keys()): + # If there's both a foo.data and a foo.bar.data, the + # foo.run targets runs the foo executable, and we can't reuse + # that name for a group. + if 'tests/' + base in executables: + continue + self.test_group_target(base, groups[base]) self.target('tests', ['$(test_apps)'], [], help='Build the host tests.', From 67671676846a40ebfe1f6817fd6573ae3bce8db9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Jul 2023 14:27:19 +0200 Subject: [PATCH 16/38] Support test helper files under src/test_helpers Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 8516407..7d98efe 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1199,13 +1199,27 @@ class MakefileMaker: """ tests_common_sources = self.list_source_files(self.options.source, 'tests/src/*.c', - 'tests/src/drivers/*.c') - tests_common_objects = [] + 'tests/src/drivers/*.c', + 'tests/src/test_helpers/*.c') + tests_crypto_objects = [] + tests_x509_objects = [] + tests_ssl_objects = [] for src in tests_common_sources: self.object_target('TESTS', src) object_file = src.target(self.object_extension) - tests_common_objects.append(object_file) - self.assign('test_common_objects', *tests_common_objects) + if 'ssl' in object_file: + tests_ssl_objects.append(object_file) + elif 'x509' in object_file: + tests_x509_objects.append(object_file) + else: + tests_crypto_objects.append(object_file) + self.assign('test_crypto_objects', *tests_crypto_objects) + self.assign('test_x509_objects', *tests_x509_objects) + self.assign('test_ssl_objects', *tests_ssl_objects) + self.assign('test_common_objects', + '$(test_crypto_objects)', + '$(test_x509_objects)', + '$(test_ssl_objects)') def test_subsection(self, src: SourceFile, executables: List[str], @@ -1744,7 +1758,8 @@ class BuildTreeMaker: """Go ahead and prepate the build tree.""" for subdir in ([['include', 'mbedtls'], ['library'], - ['tests', 'src', 'drivers']] + + ['tests', 'src', 'drivers'], + ['tests', 'src', 'test_helpers']] + [['programs', d] for d in self.programs_subdirs()]): self.make_subdir(subdir) source_link = os.path.join(self.options.dir, 'source') From a86e8f41dc2aa66eed4c4dedbbe448bbe003efef Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Jun 2024 20:23:46 +0200 Subject: [PATCH 17/38] Completion for many mbedtls sample programs * pk: dh_genprime, gen_key, key_app, key_app_writer. * ssl: ssl_client2, ssl_mail_client, ssl_server2. * x509: pem2der, cert_app, cert_req, cert_write, crl_app, req_app. Signed-off-by: Gilles Peskine --- tools/zsh/_mbedtls_programs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 tools/zsh/_mbedtls_programs diff --git a/tools/zsh/_mbedtls_programs b/tools/zsh/_mbedtls_programs new file mode 100644 index 0000000..6a40ace --- /dev/null +++ b/tools/zsh/_mbedtls_programs @@ -0,0 +1,30 @@ +#compdef dh_genprime gen_key key_app key_app_writer ssl_client2 ssl_mail_client ssl_server2 pem2der cert_app cert_req cert_write crl_app req_app +## Completion for Mbed TLS SSL sample and test programs. + +_ssl_client2 () { + local param + local -a params values + params=("${(@)${(@)${(@M)${(@f)$(_call_program help $words[1] help)}:# #[a-z][0-9A-Z_a-z]#=*}%%=*}## ##}") + if [[ $PREFIX$SUFFIX == *=* ]]; then + IPREFIX="${PREFIX%%\=*}=" + PREFIX="${PREFIX#*=}" + param=${${IPREFIX%=}##[!0-9A-Z_a-z]##} + case $param in + auth_mode) + _values $param none optional required;; + force_ciphersuite) + values=("${(@M)${=$(_call_program help_ciphersuites $words[1] help_ciphersuites)}:#TLS-*}") + _values $param $values;; + *path*|output_file) + _files -/;; + *file*|*_crt|*_key) + _files;; + *version) + _values $param ssl3 tls10 tls11 tls12 tls13 dtls10 dtls12;; + esac + else + _values -s= parameter $params + fi +} + +_ssl_client2 "$@" From 3340323839efa80082c8f11c9175ddcff2e7882f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Jun 2024 20:25:25 +0200 Subject: [PATCH 18/38] Improve config.py completion Recognize -f/--file option. Recognize PSA_xxx symbols as well as MBEDTLS_xxx. Signed-off-by: Gilles Peskine --- tools/zsh/_config.pl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/zsh/_config.pl b/tools/zsh/_config.pl index 08ba7bc..e9bc7f0 100644 --- a/tools/zsh/_config.pl +++ b/tools/zsh/_config.pl @@ -1,9 +1,12 @@ #compdef config.pl config.py -## Completion for scripts/config.pl and scripts/config.pl in Mbed TLS. +## Completion for scripts/config.pl and scripts/config.py in Mbed TLS. _config_pl_symbols () { local -a identifiers - identifiers=("${(@f)$(_call_program _config_pl_symbols sed -n \''s!^/*\**#define \(MBEDTLS_[0-9A-Z_a-z][0-9A-Z_a-z]*\).*!\1!p'\' \$config_h)}") + identifiers=("${(@f)$(_call_program _config_pl_symbols 'sed -n \ + -e '\''s!^/*\**#define \(MBEDTLS_[0-9A-Z_a-z][0-9A-Z_a-z]*\).*!\1!p'\'' \ + -e '\''s!^/*\**#define \(PSA_[0-9A-Z_a-z][0-9A-Z_a-z]*\).*!\1!p'\'' \ + $config_h')}") _describe -t symbols 'config.h symbols' identifiers } @@ -26,6 +29,11 @@ {'-o','--force'}'[define symbol even if not present]' \ '1:config.pl command:->command' \ '*::config.pl commands:->param' + if (($+opt_args[--file])); then + config_h=$opt_args[--file] + elif (($+opt_args[-f])); then + config_h=$opt_args[-f] + fi case $state in (command) _describe -t commands 'config.pl command' commands_with_descriptions;; From 7a8b82543cfd6ad5e6a463df9c07730f9fb04b6f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Jun 2024 20:26:33 +0200 Subject: [PATCH 19/38] New target: ssl-opt Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 7d98efe..38fded4 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1185,6 +1185,16 @@ class MakefileMaker: self.add_clean('$(programs)') self.add_run_target('programs/test/benchmark') self.add_run_target('programs/test/selftest') + self.target('ssl-opt', + ['programs/ssl/seedfile', + 'programs/ssl/ssl_client2$(EXEXT)', + 'programs/ssl/ssl_server2$(EXEXT)', + 'programs/test/query_compile_time_config$(EXEXT)', + 'programs/test/udp_proxy$(EXEXT)', + 'tests/seedfile'], + [], + help='Build the programs needed for ssl-opt.sh', + phony=True) # TODO: *_demo.sh self.target('test_zeroize', ['programs/test/zeroize'], From ec7454cf6a19ec2f62d620e0a81c0c1a77c52d59 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Jun 2024 20:27:25 +0200 Subject: [PATCH 20/38] Support Mbed TLS 3.5+ generated files Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 38fded4..036f8c1 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -720,7 +720,7 @@ class MakefileMaker: extra = set() with open(os.path.join(self.options.source, c_file)) as stream: for line in stream: - m = re.match(r'#include ["<](.*)[">]', line) + m = re.match(r' *# *include *["<](.*?)[">]', line) if m is None: continue filename = m.group(1) @@ -749,6 +749,8 @@ class MakefileMaker: """ if 'ssl_debug_helpers_generated.' in filename: return False + if 'value_names_generated' in filename: + return False if '_generated.' in filename: return True if 'psa_crypto_driver_wrappers.c' in filename: @@ -1640,6 +1642,7 @@ class ConfigInclude(ConfigExplicit): if not os.path.isabs(source_path): source_path = os.path.join(os.pardir, os.pardir, source_path) self.lines.append('#ifndef MBEDTLS_CHECK_CONFIG_H') + #TODO: in 2.x, MBEDTLS_USER_CONFIG_FILE needs to go here! self.lines.append('#include "{}"'.format(source_path)) self.lines.append('') From 7ad50495f6027f9a55e5dd6f4b0d3bade3efa7aa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Jun 2024 20:29:22 +0200 Subject: [PATCH 21/38] New presets asan-gcc, full-asan-gcc, tsan Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 42 ++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 036f8c1..c739a2c 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1793,6 +1793,8 @@ class BuildTreeMaker: self.makefile.generate() self.config.run() +CLANG_WARNING_CFLAGS = '-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE' + """Named presets. This is a dictionary mapping preset names to their descriptions. The @@ -1806,18 +1808,25 @@ _preset_options = { _help='Clang with ASan+UBSan, current configuration', config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], CC='clang', - CFLAGS='-O', + CFLAGS='-O2', + COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', + WARNING_CFLAGS=CLANG_WARNING_CFLAGS, + ), + 'asan-gcc': argparse.Namespace( + _help='GCC with ASan+UBSan, current configuration', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='gcc', + CFLAGS='-O2', COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', - WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', ), 'cf': argparse.Namespace( _help='Constant flow with MSan, current configuration', config_set=['MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN'], config_unset=['MBEDTLS_AESNI_C'], CC='clang', - CFLAGS='-O', + CFLAGS='-O2', COMMON_FLAGS='-fsanitize=memory -g3', - WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', + WARNING_CFLAGS=CLANG_WARNING_CFLAGS, ), 'coverage': argparse.Namespace( _help='Build with coverage instrumentation', @@ -1870,9 +1879,17 @@ _preset_options = { config_name='full', config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], CC='clang', - CFLAGS='-O', + CFLAGS='-O2', + COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', + WARNING_CFLAGS=CLANG_WARNING_CFLAGS, + ), + 'full-asan-gcc': argparse.Namespace( + _help='Full configuration with GCC+ASan+UBSan', + config_name='full', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='gcc', + CFLAGS='-O2', COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', - WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', ), 'full-debug': argparse.Namespace( _help='Full configuration, debug build', @@ -1909,9 +1926,9 @@ _preset_options = { config_unset=['MBEDTLS_AESNI_C', 'MBEDTLS_MEMORY_BUFFER_ALLOC_C'], CC='clang', - CFLAGS='-O', + CFLAGS='-O2', COMMON_FLAGS='-fsanitize=memory -g3', - WARNING_CFLAGS='-Werror -Wall -Wextra -Wdocumentation -Wno-documentation-deprecated-sync -std=c99 -D_DEFAULT_SOURCE', + WARNING_CFLAGS=CLANG_WARNING_CFLAGS, ), 'noplatform': argparse.Namespace( _help='Full except platform/fsio/net', @@ -1946,6 +1963,15 @@ _preset_options = { CFLAGS='-Os', COMMON_FLAGS='-mthumb', ), + 'tsan': argparse.Namespace( + _help='Clang with TSan, current configuration + pthread', + config_set=['MBEDTLS_THREADING_C', 'MBEDTLS_THREADING_PTHREAD'], + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='clang', + CFLAGS='-O2', + COMMON_FLAGS='-fsanitize=thread -fno-sanitize-recover=all -fno-common -g3', + WARNING_CFLAGS=CLANG_WARNING_CFLAGS, + ), 'valgrind': argparse.Namespace( # This is misleading: it doesn't actually run programs through # valgrind when you run e.g. `make check` From fcd34412da0283fc7af40d626c11f6c19b4015d7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Jun 2024 20:29:35 +0200 Subject: [PATCH 22/38] New preset alt Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index c739a2c..fccbb11 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1804,6 +1804,51 @@ it's the documentation of the preset. """ _preset_options = { '': argparse.Namespace(), # empty preset = use defaults + 'alt': argparse.Namespace( + _help='Build with ALT implementations', + config_name='full', + config_set=[ + 'MBEDTLS_AES_ALT', + 'MBEDTLS_ARIA_ALT', + 'MBEDTLS_CAMELLIA_ALT', + 'MBEDTLS_CCM_ALT', + 'MBEDTLS_CHACHA20_ALT', + 'MBEDTLS_CHACHAPOLY_ALT', + 'MBEDTLS_CMAC_ALT', + 'MBEDTLS_DES_ALT', + 'MBEDTLS_DHM_ALT', + 'MBEDTLS_ECJPAKE_ALT', + 'MBEDTLS_ECP_ALT', + 'MBEDTLS_GCM_ALT', + #'MBEDTLS_MD2_ALT', + #'MBEDTLS_MD4_ALT', + 'MBEDTLS_MD5_ALT', + 'MBEDTLS_NIST_KW_ALT', + 'MBEDTLS_POLY1305_ALT', + 'MBEDTLS_RIPEMD160_ALT', + 'MBEDTLS_RSA_ALT', + 'MBEDTLS_SHA1_ALT', + 'MBEDTLS_SHA256_ALT', + 'MBEDTLS_SHA512_ALT', + 'MBEDTLS_THREADING_ALT', + 'MBEDTLS_TIMING_ALT', + ], + config_unset=[ + 'MBEDTLS_AESCE_C', + 'MBEDTLS_AESNI_C', + 'MBEDTLS_DEBUG_C', + 'MBEDTLS_PADLOCK_C', + 'MBEDTLS_ECP_RESTARTABLE', + 'MBEDTLS_THREADING_PTHREAD', + 'MBEDTLS_PK_PARSE_EC_EXTENDED', + 'MBEDTLS_SHA256_USE_ARMV8_A_CRYPTO_IF_PRESENT', + 'MBEDTLS_SHA256_USE_ARMV8_A_CRYPTO_ONLY', + 'MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT', + 'MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY', + ], + default_target='lib', + LIBRARY_EXTRA_CFLAGS='-I $(SOURCE_DIR)/tests/include/alt-dummy' + ), 'asan': argparse.Namespace( _help='Clang with ASan+UBSan, current configuration', config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], From c6a50c57d0a482ff090baaee1e6b34e9b1cb131a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Jun 2024 12:28:35 +0200 Subject: [PATCH 23/38] Support framework after generate_test_code.py move Support mbedtls after https://github.com/Mbed-TLS/mbedtls/pull/9200 Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index fccbb11..ed22fce 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -322,9 +322,12 @@ class OnTargetTestGenerator(TestGeneratorInterface): datax_file = os.path.splitext(c_file)[0] + '.datax' return sjoin(c_file, datax_file) - @staticmethod - def script(source_dir: str) -> str: - return os.path.dirname(source_dir) + '/scripts/generate_test_code.py' + def script(self, source_dir: str) -> str: + if os.path.exists(os.path.join(self.options.source, source_dir, + '../scripts/generate_test_code.py')): + return os.path.dirname(source_dir) + '/scripts/generate_test_code.py' + else: + return 'framework/scripts/generate_test_code.py' @staticmethod def function_files(function_file: str, on_target=False) -> List[str]: @@ -334,12 +337,11 @@ class OnTargetTestGenerator(TestGeneratorInterface): 'target_test' if on_target else 'host_test']] + [function_file]) - @classmethod - def command(cls, function_file: str, data_file: str) -> str: + def command(self, function_file: str, data_file: str) -> str: source_dir = os.path.dirname(function_file) suite_path = '$(SOURCE_DIR_FROM_TESTS)/' + source_dir return sjoin('$(PYTHON)', - '$(SOURCE_DIR_FROM_TESTS)/' + cls.script(source_dir), + '$(SOURCE_DIR_FROM_TESTS)/' + self.script(source_dir), '-f $(SOURCE_DIR_FROM_TESTS)/' + function_file, '-d $(SOURCE_DIR_FROM_TESTS)/' + data_file, '-t', suite_path + '/main_test.function', @@ -418,7 +420,8 @@ class MakefileMaker: 'library/psa_crypto_driver_wrappers.c': self.PSA_CRYPTO_DRIVER_WRAPPERS_DEPENDENCIES, } self.submodules = list(self.gather_submodules(self.options.source)) - if self.source_exists('tests/scripts/generate_test_code.py'): + if self.source_exists('tests/scripts/generate_test_code.py') or \ + self.source_exists('framework/scripts/generate_test_code.py'): self.test_generator = OnTargetTestGenerator(options) #type: TestGeneratorInterface else: self.test_generator = ClassicTestGenerator(options) From f145d54a4c8e9c0db6f8da2451cd73c3054f110a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Jun 2024 12:54:23 +0200 Subject: [PATCH 24/38] m0plus preset: use baremetal_size rather than baremetal The point of that preset is for code size measurements, so baremetal_size is the configuration to use, now that it exists. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index ed22fce..dbe335f 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1956,7 +1956,11 @@ _preset_options = { ), 'm0plus': argparse.Namespace( _help='Baremetal configuration for Cortex-M0+ target', - config_name='baremetal', + # 'baremetal_size' only exists since shortly before mbedtls-3.2.0 + # and mbedtls-2.28.1. In older versions, use the 'baremetal' + # configuration and unset + # 'MBEDTLS_DEBUG_C,MBEDTLS_SELF_TEST,MBEDTLS_TEST_HOOKS'. + config_name='baremetal_size', default_target='lib', CC='arm-none-eabi-gcc', CFLAGS='-Os', From 2d9350d062d2fe34be9a42f4d10c44cb699d9642 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Jun 2024 15:22:09 +0200 Subject: [PATCH 25/38] Support framework after move of include/psa/*.h to tf-psa-crypto Support mbedtls after https://github.com/Mbed-TLS/mbedtls/pull/9247 Signed-off-by: Gilles Peskine Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index dbe335f..664324a 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -164,7 +164,7 @@ Options = argparse.Namespace A submodule is a subdirectory of the source tree which has the same general structure as the source tree. """ -SUBMODULE_NAMES = ['crypto'] +SUBMODULE_NAMES = ['crypto', 'tf-psa-crypto'] class SourceFile: @@ -457,9 +457,10 @@ class MakefileMaker: it exists, return its path from the root of the source tree. Otherwise return filename unchanged. """ - in_crypto = os.path.join('crypto', filename) - if os.path.exists(in_crypto): - filename = in_crypto + for submodule in SUBMODULE_NAMES: + in_submodule = os.path.join(submodule, filename) + if os.path.exists(in_submodule): + filename = in_submodule return '$(SOURCE_DIR)/' + filename def source_exists(self, filename: str) -> bool: @@ -1789,7 +1790,8 @@ class BuildTreeMaker: ['tests', 'data_files'], ['tests', 'opt-testcases'], ['tests', 'scripts'], - ['tests', 'ssl-opt.sh']]: + ['tests', 'ssl-opt.sh'], + ['tf-psa-crypto']]: self.link_to_source_maybe(link) self.link_test_suites() self.prepare_source() From 6b4a5a435706a8d9f850aa28d39556e8ce07bcc2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 17 Jul 2024 20:18:01 +0200 Subject: [PATCH 26/38] Fix ambiguous "generated by" line and add "prepare" target Add shell quoting to the "generated by" line so that it can be copy-pasted into a shell, even if some arguments contain spaces and other special characters. Add a new target `prepare`, alias `dep`: `make prepare` or `make dep` regenerates the makefile. This might not yet work in all arrangements of the build directory relative to the source directory; it works at least when `mbedtls-prepare-build` is invoked inside the source directory. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 664324a..5f28afe 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -11,6 +11,7 @@ import itertools import os import platform import re +import shlex import shutil import subprocess import sys @@ -1443,9 +1444,25 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi return ['{:<14} # {}'.format(name, text) for (name, text) in sorted(env_opts + ad_hoc)] + QUOTE_TO_DELAY_RE = re.compile(r'(\')(--[-0-9A-Z_a-z]+=)') + def quote_arg_for_shell(self, arg) -> str: + """Quote a string for use in the platform's shell.""" + quoted = shlex.quote(arg) + # Be a bit nicer: `'--foo=hello world'` -> `--foo='hello world'` + quoted = self.QUOTE_TO_DELAY_RE.sub(r'\2\1', quoted) + return quoted + + def generation_command_for_shell(self) -> str: + """The shell command used to create the build tree.""" + return ' '.join(self.quote_arg_for_shell(arg) for arg in sys.argv) + + def generation_command_for_make(self) -> str: + """The shell command used to create the build tree, quoted for make.""" + return self.generation_command_for_shell().replace('$', '$$') + def output_all(self) -> None: """Emit the whole makefile.""" - self.comment('Generated by {}', ' '.join(sys.argv)) + self.comment('Generated by ' + self.generation_command_for_shell()) self.comment('Do not edit this file! All modifications will be lost.') self.line('') self.settings_section() @@ -1457,6 +1474,13 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi help='Build the library, the tests and the sample programs.', phony=True) self.line('') + self.target('prepare', [], + ['cd $(SOURCE_DIR) && ' + + self.generation_command_for_make()], + help='Regenerate this makefile.', + phony=True) + self.target('dep', ['prepare'], [], phony=True) + self.line('') self.target('pwd', [], ['pwd'], phony=True, short='PWD') # for testing self.line('') self.library_section() From 5b3bed2842a36a4d5e690b58c5ce3abbb880199d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 17 Jul 2024 20:21:25 +0200 Subject: [PATCH 27/38] Support move of crypto headers and C sources to tf-psa-crypto The build works on development as of cb854d5d19e05339448afb03839bee7f7e3ecd23. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 107 +++++++++++++++++++++++--------- 1 file changed, 76 insertions(+), 31 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 5f28afe..c044923 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -671,7 +671,21 @@ class MakefileMaker: """ dirs = [] submodule, base = self.get_file_submodule(filename) - subdirs = ['include', 'include/mbedtls', 'library'] + # When there is a tf-psa-crypto submodule, we need the toplevel + # include in addition to the one in the subdmodule. The calls + # to list_source_files only return matches from the submodule if + # present. + subdirs = ['include'] + for pattern in [ + 'include', + 'drivers/*/include', + 'library', + 'core', + 'drivers/*/library', + 'drivers/*/src', + ]: + matches = self.list_source_files(self.options.source, pattern) + subdirs += [source.relative_path() for source in matches] if base.startswith('tests') or base.startswith('programs'): subdirs.append('tests') if self.source_exists('tests/include'): @@ -683,11 +697,15 @@ class MakefileMaker: dirs.append(subdir) if submodule is not None: dirs.append(os.path.join(submodule, subdir)) - if submodule == '3rdparty/everest': - dirs.append(os.path.join(submodule, 'include/everest')) - if '/kremlib/' in filename: - dirs.append(os.path.join(submodule, 'include/everest/kremlib')) - print(filename, dirs) + if submodule == '3rdparty/everest': + dirs.append(os.path.join(submodule, 'include/everest')) + if '/kremlib/' in filename: + dirs.append(os.path.join(submodule, 'include/everest/kremlib')) + elif '/everest/' in filename: + prefix = '' + if 'tf-psa-crypto/' in filename: + prefix = 'tf-psa-crypto/' + dirs.append(prefix + 'drivers/everest/include/everest') return dirs def include_path_options(self, filename: str) -> str: @@ -826,14 +844,12 @@ class MakefileMaker: '$(WARNING_CFLAGS)', '$(COMMON_FLAGS)', '$(CFLAGS)', - ' '.join(['$({}_CFLAGS)'.format(section)]), - # TODO: some files need extra -I (currently: - # 3rdparty/everest/library/**/*.c need - # 3rdparty/everest/include/everst and some - # need .../kremlib as well). This is currently - # handled in the include_path method, but - # we don't call it here, only for a section - # as a whole. + '$({}_CFLAGS)'.format(section), + # TODO: The section CFLAGS already contain some + # -I options. But in the library, we need + # extra -I options for some files (e.g. everest). + # So we add partially redundant -I options here. + self.include_path_options(c_path), extra_flags.strip(), '-o $@', switch, c_path)], @@ -953,7 +969,11 @@ class MakefileMaker: self.object_extension, self.shared_library_extension)]) # Enumerate modules and emit the rules to build them - modules = self.list_source_files(self.options.source, 'library/*.c') + modules = self.list_source_files(self.options.source, + 'core/*.c', + 'drivers/*/*.c', + 'drivers/*/*/*.c', + 'library/*.c') for module in modules: self.object_target('LIBRARY', module) contents = collections.defaultdict(list) @@ -1138,7 +1158,14 @@ class MakefileMaker: for lib in libs] dash_l_libs = [self.dash_l_lib(lib) for lib in libs] if base == 'programs/test/dlopen': - object_deps.append('library/platform.o library/platform_util.o') + for base in ['platform', 'platform_util']: + for lib in ['library', 'drivers/builtin/src']: + if self.source_exists(os.path.join(lib, base) + '.c'): + object_deps.append(os.path.join(lib, base) + self.object_extension) + break + if self.source_exists(os.path.join('tf-psa-crypto', lib, base) + '.c'): + object_deps.append(os.path.join(lib, base) + self.object_extension) + break else: object_deps.append('$(test_crypto_objects)') if self.Library.X509 in libs: @@ -1797,25 +1824,43 @@ class BuildTreeMaker: def run(self) -> None: """Go ahead and prepate the build tree.""" - for subdir in ([['include', 'mbedtls'], - ['library'], - ['tests', 'src', 'drivers'], - ['tests', 'src', 'test_helpers']] + - [['programs', d] for d in self.programs_subdirs()]): + for subdir in ( + [ + ['framework'], + ['include', 'mbedtls'], + ['library'], + ['tests', 'src', 'drivers'], + ['tests', 'src', 'test_helpers'], + ] + + [['programs', d] for d in self.programs_subdirs()] + ): self.make_subdir(subdir) + if os.path.exists(os.path.join(self.options.source, 'tf-psa-crypto')): + for subdir in [ + ['core'], + ['drivers', 'builtin', 'src'], + ['drivers', 'everest', 'library'], + ['drivers', 'everest', 'library', 'kremlib'], + ['drivers', 'everest', 'library', 'legacy'], + ['drivers', 'p256-m', 'p256-m'], + ]: + self.make_subdir(subdir) source_link = os.path.join(self.options.dir, 'source') if not self.options.in_tree_build and not os.path.exists(source_link): os.symlink(self.source_path, source_link) - for link in [['include', 'psa'], # hack for psa_constant_names.py - ['programs', 'test', 'zeroize.c'], - ['scripts'], - ['tests', 'configs'], - ['tests', 'compat.sh'], - ['tests', 'data_files'], - ['tests', 'opt-testcases'], - ['tests', 'scripts'], - ['tests', 'ssl-opt.sh'], - ['tf-psa-crypto']]: + for link in [ + ['framework', 'data_files'], + ['include', 'psa'], # hack for psa_constant_names.py + ['programs', 'test', 'zeroize.c'], + ['scripts'], + ['tests', 'configs'], + ['tests', 'compat.sh'], + ['tests', 'data_files'], + ['tests', 'opt-testcases'], + ['tests', 'scripts'], + ['tests', 'ssl-opt.sh'], + ['tf-psa-crypto'], + ]: self.link_to_source_maybe(link) self.link_test_suites() self.prepare_source() From cff5d81cb10d30619f6fc2681b74474f95799642 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 26 Sep 2024 12:29:59 +0200 Subject: [PATCH 28/38] Fix build failures on Everest even when it's disabled Fix build failures on Everest files that can happen even when Everst is disabled (observed in development after Everest moved to tf-psa-crypto, but that might occur in <=3.6 as well in some configurations). The build failures are genuine, but our official build system skips those files because they get the code from Hacl_Curve25519_joined.c instead. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index c044923..d301161 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -782,15 +782,19 @@ class MakefileMaker: #return True return False + INCLUDE_ONLY_C_FILES = frozenset([ + 'FStar_UInt128_extracted.c', # included by Hacl_Curve25519_joined.c + 'FStar_UInt64_FStar_UInt32_FStar_UInt16_FStar_UInt8.c', # included by Hacl_Curve25519_joined.c + 'Hacl_Curve25519.c', # included by Hacl_Curve25519_joined.c + 'psa_constant_names_generated.c', # included by psa_constant_names.c + 'ssl_test_common_source.c', # included by ssl_*2.c + ]) def is_include_only(self, filename: str) -> bool: """Whether the specified C file is only meant for use in "#include" directive. Implemented with heuristics based on the name. """ - return os.path.basename(filename) in { - 'psa_constant_names_generated.c', - 'ssl_test_common_source.c', - } + return os.path.basename(filename) in self.INCLUDE_ONLY_C_FILES def c_with_dependencies(self, c_file: str) -> List[str]: """A list of C dependencies in makefile syntax. From 358e1685f60c92ee6caab4c0ede8b7ac57c4675d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 26 Sep 2024 12:32:28 +0200 Subject: [PATCH 29/38] Add an IAR preset Tested on mbedtls-3.6.1. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index d301161..1ad815a 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -2029,6 +2029,16 @@ _preset_options = { CFLAGS='-Os', COMMON_FLAGS='-mthumb', ), + 'iar-thumb': argparse.Namespace( + _help='Baremetal+ configuration built with IAR', + config_name='baremetal', + config_unset=['MBEDTLS_PLATFORM_FPRINTF_ALT', + 'MBEDTLS_PLATFORM_SETBUF_ALT'], + default_target='lib', + CC='iccarm', + CFLAGS='-Ohz --cpu_mode=thumb --cpu=Cortex-M0', + WARNING_CFLAGS='--warnings_are_errors', + ), 'm0plus': argparse.Namespace( _help='Baremetal configuration for Cortex-M0+ target', # 'baremetal_size' only exists since shortly before mbedtls-3.2.0 From 5e6dc1afed9352da1979412cff6aca7626f32516 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 26 Sep 2024 12:35:18 +0200 Subject: [PATCH 30/38] Support test helpers in the tf-psa-crypto subtree Needed for mbedtls during repo split work between 3.6 and 4.0. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 1ad815a..b57fc9f 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -330,10 +330,23 @@ class OnTargetTestGenerator(TestGeneratorInterface): else: return 'framework/scripts/generate_test_code.py' - @staticmethod - def function_files(function_file: str, on_target=False) -> List[str]: + def helpers_dir(self, source_dir: str) -> str: + if os.path.exists(os.path.join(self.options.source, + source_dir, + 'helpers.function')): + return source_dir + elif os.path.exists(os.path.join(self.options.source, + 'tf-psa-crypto', + source_dir, + 'helpers.function')): + # tf-psa-crypto transition + return 'tf-psa-crypto/' + source_dir + else: + raise Exception('Unable to locate test helpers for ' + source_dir) + + def function_files(self, function_file: str, on_target=False) -> List[str]: source_dir = os.path.dirname(function_file) - return (['{}/{}.function'.format(source_dir, helper) + return (['{}/{}.function'.format(self.helpers_dir(source_dir), helper) for helper in ['helpers', 'main_test', 'target_test' if on_target else 'host_test']] + [function_file]) @@ -341,13 +354,14 @@ class OnTargetTestGenerator(TestGeneratorInterface): def command(self, function_file: str, data_file: str) -> str: source_dir = os.path.dirname(function_file) suite_path = '$(SOURCE_DIR_FROM_TESTS)/' + source_dir + helpers_path = '$(SOURCE_DIR_FROM_TESTS)/' + self.helpers_dir(source_dir) return sjoin('$(PYTHON)', '$(SOURCE_DIR_FROM_TESTS)/' + self.script(source_dir), '-f $(SOURCE_DIR_FROM_TESTS)/' + function_file, '-d $(SOURCE_DIR_FROM_TESTS)/' + data_file, - '-t', suite_path + '/main_test.function', - '-p', suite_path + '/host_test.function', - '--helpers-file', suite_path + '/helpers.function', + '-t', helpers_path + '/main_test.function', + '-p', helpers_path + '/host_test.function', + '--helpers-file', helpers_path + '/helpers.function', '-s', suite_path, '-o .') From 5076e360af99eb03102e80c64661cc623719db8e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 26 Sep 2024 12:41:08 +0200 Subject: [PATCH 31/38] Have a run target for all the programs Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b57fc9f..9ad540b 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1206,6 +1206,7 @@ class MakefileMaker: clean=False, short='LD $@') executables.append(exe_file) + self.add_run_target(src.base()) def programs_section(self) -> None: """Emit the makefile rules to build the sample programs.""" @@ -1235,8 +1236,6 @@ class MakefileMaker: help='Build the sample programs.', phony=True) self.add_clean('$(programs)') - self.add_run_target('programs/test/benchmark') - self.add_run_target('programs/test/selftest') self.target('ssl-opt', ['programs/ssl/seedfile', 'programs/ssl/ssl_client2$(EXEXT)', From f376765a3c39bd64b8fcb1d9c2280668f29847b6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 26 Sep 2024 12:44:35 +0200 Subject: [PATCH 32/38] Build all the SSL sample programs for ssl-opt Needed after https://github.com/Mbed-TLS/mbedtls/pull/9638, no big deal before. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 9ad540b..f06f817 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1237,11 +1237,11 @@ class MakefileMaker: phony=True) self.add_clean('$(programs)') self.target('ssl-opt', - ['programs/ssl/seedfile', - 'programs/ssl/ssl_client2$(EXEXT)', - 'programs/ssl/ssl_server2$(EXEXT)', - 'programs/test/query_compile_time_config$(EXEXT)', + [program for program in executables + if program.startswith('programs/ssl')] + + ['programs/test/query_compile_time_config$(EXEXT)', 'programs/test/udp_proxy$(EXEXT)', + 'programs/ssl/seedfile', 'tests/seedfile'], [], help='Build the programs needed for ssl-opt.sh', From 16f85cb317f0fe24811d1f06faf049a19bb1eafb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Oct 2024 19:52:43 +0200 Subject: [PATCH 33/38] New method MakefileMaker.library_modules Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index f06f817..921cb0d 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -969,6 +969,14 @@ class MakefileMaker: all_sources[base] = src return sorted(all_sources.values()) + def library_modules(self) -> Iterable[SourceFile]: + """Generate the list of C source files for the library.""" + return self.list_source_files(self.options.source, + 'core/*.c', + 'drivers/*/*.c', + 'drivers/*/*/*.c', + 'library/*.c') + def library_section(self) -> None: """Generate the section of the makefile for the library directory. @@ -987,11 +995,7 @@ class MakefileMaker: self.object_extension, self.shared_library_extension)]) # Enumerate modules and emit the rules to build them - modules = self.list_source_files(self.options.source, - 'core/*.c', - 'drivers/*/*.c', - 'drivers/*/*/*.c', - 'library/*.c') + modules = self.library_modules() for module in modules: self.object_target('LIBRARY', module) contents = collections.defaultdict(list) From 8ba447aa87e0a0bdc9c77a65a9bced7fbf83860c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Oct 2024 19:52:55 +0200 Subject: [PATCH 34/38] Support libtestdriver1 builds Initial commit to support builds with libtestdriver1. The following command results in a configuration that should be the same as `component_test_psa_crypto_config_accel_ecdsa`: ``` mbedtls-prepare-build -d build-accel-ecdsa-sha1-debug -p debug --config-set=MBEDTLS_PSA_CRYPTO_CONFIG --config-unset=MBEDTLS_PSA_CRYPTO_SE_C --accel-list={ALG_ECDSA,ALG_DETERMINISTIC_ECDSA,KEY_TYPE_ECC_PUBLIC_KEY,KEY_TYPE_ECC_KEY_PAIR_{BASIC,IMPORT,EXPORT,GENERATE,DERIVE},ECC_{SECP_R1_{192,224,256,384,521},SECP_K1_{192,224,256},BRAINPOOL_P_R1_{256,384,512},MONTGOMERY_{255,448}}} --config-unset=MBEDTLS_ECDSA_C,MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED,MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED --libtestdriver1-extra-list=ALG_SHA{_1,{,3}_{224,256,384,512}} ``` Known limitations: * Barely tested. * Only tested with a commit that's close to de4d5b78558666d2e258d95e6c5875f9c72687ed (development soon after the 3.6.1 release). * Only static library builds are supported. * Only configurations based on the default configuration are supported. In particular, a configuration with threading (e.g. derived from `full`) requires setting `MBEDTLS_THREADING_C` and `MBEDTLS_THREADING_PTHREAD` manually in `--libtestdriver1-extra-cflags`. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 309 +++++++++++++++++++++++++++++++- 1 file changed, 300 insertions(+), 9 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 921cb0d..cfe23c1 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -123,6 +123,8 @@ _environment_options = [ 'Options to pass to ${CC} when linking a program that uses threads'), EnvironmentOption('LIBRARY_EXTRA_CFLAGS', '', 'Options to pass to ${CC} when compiling library sources'), + EnvironmentOption('LIBTESTDRIVER1_EXTRA_CFLAGS', '', + 'Options to pass to ${CC} when compiling libtestdriver1'), EnvironmentOption('PERL', 'perl', 'Perl interpreter'), EnvironmentOption('PROGRAMS_EXTRA_CFLAGS', '', @@ -724,6 +726,11 @@ class MakefileMaker: def include_path_options(self, filename: str) -> str: """Return the include path options (-I ...) for filename.""" + if filename.startswith('tests/libtestdriver1'): + inc = ['tests/libtestdriver1'] + if '/p256-m' in filename: + inc.append(inc[0] + '/include-testdriver1/p256-m') + return sjoin(*('-I ' + dir for dir in inc)) return ' '.join(['-I $(SOURCE_DIR)/' + dir for dir in self.include_path(filename)]) @@ -850,6 +857,10 @@ class MakefileMaker: dependencies = list(deps) if auto_deps: dependencies += self.c_with_dependencies(src.relative_path()) + if self.options.accel_list: + # TODO: refine dependencies (same as in + # libtestdriver1_source_targets()) + dependencies.append('$(libtestdriver1_headers)') else: dependencies += [c_path] for short, switch, extension in [ @@ -874,9 +885,10 @@ class MakefileMaker: short=(short + c_path)) class Library(enum.Enum): - CRYPTO = 1 - X509 = 2 - TLS = 3 + TESTDRIVER1 = 1 + CRYPTO = 2 + X509 = 3 + TLS = 4 def __lt__(self, other: 'MakefileMaker.Library') -> bool: return self.value < other.value @@ -885,6 +897,8 @@ class MakefileMaker: return self.name.lower() def libname(self) -> str: + if self == self.TESTDRIVER1: + return 'libtestdriver1' return 'libmbed' + self.core() # For tracing when a library is accidentally implicitly converted to a string @@ -909,10 +923,9 @@ class MakefileMaker: else: return cls.Library.CRYPTO - @classmethod - def dash_l_lib(cls, lib: typing.Union[Library, str]) -> str: + def dash_l_lib(self, lib: typing.Union[Library, str]) -> str: """Return the -l option to link with the specified library.""" - if isinstance(lib, cls.Library): + if isinstance(lib, self.Library): base = lib.libname() else: base = os.path.splitext(os.path.basename(lib))[0] @@ -920,6 +933,211 @@ class MakefileMaker: base = base[3:] return '-l' + base + def build_libtestdriver1_rewrite_code(self) -> Optional[str]: + """Construct the perl code to rewrite libtestdriver1. + + Pass this code to ``perl -p -e``. + """ + return '; '.join([r's[(\x23 *include *["<])((?:everest|mbedtls|p256-m|psa)/)][\1include-testdriver1/\2]', + r'next if /\x23 *include\b/', + r's/\b(?=(?:mbedtls|psa)_)/libtestdriver1_/g', + r's/\b(?=(?:MBEDTLS|PSA)_)/LIBTESTDRIVER1_/g']) + + CONFIG_TEST_DRIVER_H = 'tests/include/test/drivers/config_test_driver.h' + CONFIG_TEST_DRIVER_EXTENSION_H = 'tests/include/test/drivers/crypto_config_test_driver_extension.h' + + def libtestdriver1_c_target(self, src: SourceFile) -> Optional[str]: + """Generate a target for a libtestdriver1 C source file. + + Return the path to the intermediate file within the build tree. + + We put all header files under tests/libtestdriver1/include + and all .c files under tests/libtestdriver1/library, + merging any files from submodules. + """ + short_path = src.relative_path() + basename = os.path.basename(short_path) + if basename == 'mbedtls_config.h': + # The legacy configuration is a special case, largely independent + # of the legacy configuration of the library. + # TODO: MBEDTLS_THREADING_C and MBEDTLS_THREADING_PTHREAD should + # be aligned with the library. + src = self.list_source_files(self.options.source, + self.CONFIG_TEST_DRIVER_H)[0] + if basename == 'crypto_config.h': + # The crypto configuration is a special case: it needs to have + # the library configuration with modifications described by + # CONFIG_TEST_DRIVER_EXTENSION_H. This is handled in + # test_driver_config_subsection(). + return None + elif short_path == 'library/common.h' and \ + os.path.exists(os.path.join(self.options.source, + 'tf-psa-crypto', 'core', 'common.h')): + # There is a common.h both in tf-psa-crypto and in mbedtls. + # We're building crypto files, so pick common.h from crypto. + return None + elif '/include/' in short_path: + short_path = re.sub(r'.*/include/', r'include-testdriver1/', short_path) + elif re.search(r'/p256-m/[^/]+\.h\Z', short_path): + short_path = 'include-testdriver1/p256-m/' + basename + elif short_path.startswith('include/'): + short_path = 'include-testdriver1' + short_path[7:] + else: + short_path = os.path.join('library', basename) + target_path = 'tests/libtestdriver1/' + short_path + self.target(target_path, + [src.make_path()], + [sjoin('$(PERL) -p -e', + "'$(libtestdriver1_rewrite)'", + src.make_path(), + '>$@')]) + return target_path + + def test_driver_auxiliary_files_subsection(self) -> None: + """Generate libtestdriver1 auxiliary files.""" + if os.path.exists(os.path.join(self.options.source, 'tf-psa-crypto')): + # File included by + # include/psa/crypto_driver_contexts_primitives.h + with open(os.path.join(self.options.dir, + 'tests', 'libtestdriver1', + 'libtestdriver1', 'tf-psa-crypto', + 'include', 'psa', 'crypto.h'), + 'w') as out: + out.write("""/* Generated by mbedtls-prepare-build */ +#include "../../../../include-testdriver1/psa/crypto.h" +/* End of generated file */ +""") + + def test_driver_config_subsection(self) -> None: + """Generate libtestdriver1 configuration files.""" + include_dir = os.path.join(self.options.dir, + 'tests', 'libtestdriver1', + 'include-testdriver1') + for source_path in ['include/psa/crypto_config.h', + self.CONFIG_TEST_DRIVER_EXTENSION_H]: + src = self.list_source_files(self.options.source, source_path)[0] + basename = os.path.basename(source_path) + if basename == 'crypto_config.h': + basename = 'base_crypto_config.h' + self.target('tests/libtestdriver1/include-testdriver1/psa/' + basename, + [src.make_path()], + [sjoin('$(PERL) -p -e', + "'$(libtestdriver1_rewrite)'", + src.make_path(), + '>$@')]) + with open(os.path.join(include_dir, 'psa', 'crypto_config.h'), + 'w') as out: + out.write("""/* Generated by mbedtls-prepare-build */ +#include "base_crypto_config.h" +#include "crypto_config_test_driver_extension.h" +/* End of generated file */ +""") + + def libtestdriver1_source_targets(self) -> None: + """Targets for source files in libtestdriver1. + """ + header_files = self.list_source_files(self.options.source, + 'include/*/*.h', + 'library/*.h', + 'drivers/*/*.h', + 'drivers/*/*/*.h', + 'drivers/*/*/*/*.h', + 'core/*.h') + intermediate_h_list = [] + # Add special cases from test_driver_config_subsection() + intermediate_h_list += \ + [GeneratedFile(os.path.join('tests/libtestdriver1/include-testdriver1/psa', basename)) + for basename in ['base_crypto_config.h', + os.path.basename(self.CONFIG_TEST_DRIVER_EXTENSION_H)]] + intermediate_c_list = [] + for src in header_files: + target = self.libtestdriver1_c_target(src) + if target: + intermediate_h_list.append(GeneratedFile(target)) + crypto_modules = [src + for src in self.library_modules() + if self.library_of(src.base()) == self.Library.CRYPTO] + for src in crypto_modules: + target = self.libtestdriver1_c_target(src) + if target: + intermediate_c_list.append(GeneratedFile(target)) + self.assign('libtestdriver1_headers', + sjoin(*(src.inner_path + for src in intermediate_h_list))) + self.assign('libtestdriver1_module_sources', + sjoin(*(src.inner_path + for src in intermediate_c_list))) + self.assign('libtestdriver1_modules', + sjoin(*(src.base() + for src in intermediate_c_list))) + self.assign('libtestdriver1_objects', + '$(libtestdriver1_modules:={})' + .format(self.object_extension)) + # TODO: actual dependencies .o -> .h. + # c_with_dependencies() intrinsically doesn't work on files that + # don't exist at makefile generation time. + for src in intermediate_c_list: + self.object_target('LIBTESTDRIVER1', src, + deps=['$(libtestdriver1_headers)'], + auto_deps=False) + self.target('libtestdriver1-sources', + ['$(libtestdriver1_headers)', + '$(libtestdriver1_module_sources)'], + [], + phony=True) + + def libtestdriver1_section(self) -> None: + """Generate the section of the makefile for libtestdriver1. + + The targets are object files for library modules and + static library files. + + Unlike the rest of the build, we leverage the official Makefile, + to adapt for its evolution (e.g. before/after the creation of the + framework, before/after the creation of tf-psa-crypto). + """ + self.comment('Targets for libtestdriver1') + libtestdriver1_rewrite_code = self.build_libtestdriver1_rewrite_code() + if libtestdriver1_rewrite_code is not None: + self.assign('libtestdriver1_rewrite', libtestdriver1_rewrite_code) + self.libtestdriver1_source_targets() + self.assign('LIBTESTDRIVER1_D_ACCEL', + sjoin(*('-DLIBTESTDRIVER1_MBEDTLS_PSA_ACCEL_' + accel + for accel in self.options.accel_list))) + self.assign('LIBTESTDRIVER1_CONSUMER_D_ACCEL', + sjoin(*('-DMBEDTLS_PSA_ACCEL_' + accel + for accel in self.options.accel_list))) + self.assign('LIBTESTDRIVER1_D_EXTRA', + sjoin(*('-DLIBTESTDRIVER1_MBEDTLS_PSA_ACCEL_' + accel + for accel in self.options.libtestdriver1_extra_list))) + if self.options.accel_list: + self.assign('USE_LIBTESTDRIVER1_CFLAGS', + sjoin('-I tests/libtestdriver1 -I $(SOURCE_DIR)/tests/include', + '-DPSA_CRYPTO_DRIVER_TEST', + '-DMBEDTLS_TEST_LIBTESTDRIVER1', + '$(LIBTESTDRIVER1_D_ACCEL)', + '$(LIBTESTDRIVER1_CONSUMER_D_ACCEL)')) + else: + self.assign('USE_LIBTESTDRIVER1_CFLAGS', '') + self.assign('LIBTESTDRIVER1_CFLAGS', + sjoin('$(LIBTESTDRIVER1_D_ACCEL)', + '$(LIBTESTDRIVER1_D_EXTRA)', + '$(LIBTESTDRIVER1_EXTRA_CFLAGS)')) + self.test_driver_auxiliary_files_subsection() + self.test_driver_config_subsection() + #TODO: more dependencies (adapt $(libmbedcrypt_objects)?) + prep_dependencies = [self.CONFIG_TEST_DRIVER_H, + self.CONFIG_TEST_DRIVER_EXTENSION_H] + self.target('library/libtestdriver1.a', + ['$(libtestdriver1_objects)'], + ['$(AR) $(ARFLAGS) $@ $(libtestdriver1_objects)'], + short='AR $@') + self.target('libtestdriver1', + ['library/libtestdriver1.a'], + [], + help='Build libtestdriver1.a', + phony=True) + def psa_crypto_driver_wrappers_subsection( self, contents: Dict[Library, List[str]] @@ -988,6 +1206,7 @@ class MakefileMaker: '-I include/mbedtls', # must come first, for the config header '-I include', self.include_path_options('library/*'), + '$(USE_LIBTESTDRIVER1_CFLAGS)', '$(LIBRARY_EXTRA_CFLAGS)') self.add_clean(*['library/*' + ext for ext in (self.assembly_extension, @@ -1011,6 +1230,8 @@ class MakefileMaker: self.format('{}_objects = $({}_modules:={})', libname, libname, self.object_extension) self.static_libraries = [] + if self.options.accel_list: + self.static_libraries.append('library/libtestdriver1.a') shared_libraries = [] for idx, lib in enumerate(libraries): libname = lib.libname() @@ -1096,18 +1317,22 @@ class MakefileMaker: """ basename = os.path.basename(app) subdir = os.path.basename(os.path.dirname(app)) + libs = [] + if self.options.accel_list: + libs.append(self.Library.TESTDRIVER1) + libs.append(self.Library.CRYPTO) if basename == 'dlopen': return [] if (subdir == 'ssl' or basename.startswith('ssl') or subdir == 'fuzz' or basename in {'cert_app', 'dh_client', 'dh_server', 'udp_proxy'} ): - return [self.Library.CRYPTO, self.Library.X509, self.Library.TLS] + return libs + [self.Library.X509, self.Library.TLS] if (subdir == 'x509' or (basename == 'selftest' and self.source_exists('library/x509.c')) ): - return [self.Library.CRYPTO, self.Library.X509] - return [self.Library.CRYPTO] + return libs + [self.Library.X509] + return libs def extra_link_flags(self, app: str) -> Iterable[str]: """Return the list of extra link flags for app. @@ -1218,6 +1443,7 @@ class MakefileMaker: self.assign('PROGRAMS_CFLAGS', '-I include', self.include_path_options('programs/*/*'), + '$(USE_LIBTESTDRIVER1_CFLAGS)', '$(PROGRAMS_EXTRA_CFLAGS)') self.assign('PROGRAMS_LDFLAGS', '-L library', @@ -1399,6 +1625,7 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi '-Wno-unused-function', '-I include', self.include_path_options('tests/*'), + '$(USE_LIBTESTDRIVER1_CFLAGS)', '$(TESTS_EXTRA_CFLAGS)') self.assign('TESTS_LDFLAGS', '-L library', @@ -1531,6 +1758,8 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi self.line('') self.target('pwd', [], ['pwd'], phony=True, short='PWD') # for testing self.line('') + self.libtestdriver1_section() + self.line('') self.library_section() self.line('') self.define_tests_common_objects() @@ -1546,6 +1775,14 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi help='Remove all generated files.', short='RM {generated files}', phony=True) + self.target('libtestdriver1-clean', [], + ['$(RM) ' + patterns + for patterns in self.clean + if patterns.startswith('tests/libtestdriver1/')] + + ['$(RM) library/libtestdriver1.a'], + help='Remove libtestdriver1 source and object files.', + short='RM tests/libtestdriver1/** library/libtestdriver1.a', + phony=True) self.line('') self.target('help-variables', [], ['@echo "{}"'.format(line) for line in self.variables_help_lines()], @@ -1850,6 +2087,12 @@ class BuildTreeMaker: ['framework'], ['include', 'mbedtls'], ['library'], + ['tests', 'include', 'test', 'drivers'], + ['tests', 'libtestdriver1', 'include-testdriver1', 'everest'], + ['tests', 'libtestdriver1', 'include-testdriver1', 'mbedtls'], + ['tests', 'libtestdriver1', 'include-testdriver1', 'p256-m'], + ['tests', 'libtestdriver1', 'include-testdriver1', 'psa'], + ['tests', 'libtestdriver1', 'library'], ['tests', 'src', 'drivers'], ['tests', 'src', 'test_helpers'], ] + @@ -1864,6 +2107,8 @@ class BuildTreeMaker: ['drivers', 'everest', 'library', 'kremlib'], ['drivers', 'everest', 'library', 'legacy'], ['drivers', 'p256-m', 'p256-m'], + ['tests', 'libtestdriver1', + 'libtestdriver1', 'tf-psa-crypto', 'include', 'psa'], ]: self.make_subdir(subdir) source_link = os.path.join(self.options.dir, 'source') @@ -2198,6 +2443,41 @@ def set_default_options(options: Options) -> None: for envopt in _environment_options: set_default_option(options, envopt.attr, envopt.default) +_USER_LIST_SEPARATOR_RE = re.compile(r'[\s,]+') +def split_user_list(user_list: Iterable[str]) -> Iterable[str]: + """In a list of strings, split comma- or whitespace-separated pieces of each element.""" + for element in user_list: + yield from _USER_LIST_SEPARATOR_RE.split(element) + +def normalize_psa_mechanism(name: str, prefix: str) -> str: + """Normalize a PSA mechanism name. + + Ensure that each word starts with the given prefix: PSA_ or PSA_WANT_ + or MBEDTLS_PSA_BUILTIN_ or MBEDTLS_PSA_ACCEL_. Any such prefix is first + removed from the word. + """ + name = re.sub(r'\A(?:MBEDTLS_PSA_BUILTIN_|MBEDTLS_PSA_ACCEL|PSA_|PSA_WANT_)', + r'', name) + return prefix + name + +def normalize_psa_mechanism_list(user_list: List[str], prefix: str) -> List[str]: + """Normalize a list of PSA mechanisms. + + 1. Separate into comma/whitespace-separated words. + 2. Ensure that each word starts with the given prefix: PSA_ or PSA_WANT_ + or MBEDTLS_PSA_BUILTIN_ or MBEDTLS_PSA_ACCEL_. Any such prefix is first + removed from the word. + """ + return [normalize_psa_mechanism(mechanism, prefix) + for mechanism in split_user_list(user_list) + if mechanism] + +def normalize_options(options) -> None: + """Normalize some options that can be passed in easier-to-type ways.""" + options.accel_list = normalize_psa_mechanism_list(options.accel_list, '') + options.libtestdriver1_extra_list = \ + normalize_psa_mechanism_list(options.libtestdriver1_extra_list, '') + def preset_help() -> str: """Return a documentation string for the presets.""" return '\n'.join(['Presets:'] + @@ -2227,6 +2507,11 @@ def main() -> None: parser.add_argument(envopt.option, dest=envopt.attr, help='{} ({})'.format(envopt.help, envopt.var)) + parser.add_argument('--accel-list', + action='append', default=[], + help='Algorithms to accelerate through libtestdriver1.' + ' This is loc_accel_list in all.sh.' + ' No libtestdriver1 if empty.') parser.add_argument('--assembly-extension', help='File extension for assembly files') parser.add_argument('--config-file', @@ -2258,6 +2543,11 @@ def main() -> None: help='Whether to use makefile variable for file extensions') parser.add_argument('--library-extension', help='File extension for static libraries') + parser.add_argument('--libtestdriver1-extra-list', + action='append', default=[], + help='Extra algorithms to enable in libtestdriver1.' + ' This is loc_extra_list in all.sh.' + ' Ignored if --accel-list is empty.') parser.add_argument('--object-extension', help='File extension for object files') parser.add_argument('--preset', '-p', @@ -2274,6 +2564,7 @@ def main() -> None: action='append', default=[], help='Extra variable to define in the makefile') options = parser.parse_args() + normalize_options(options) set_default_options(options) builder = BuildTreeMaker(options) builder.run() From 3b70ffddf102acde5380be1374cbe6a8a5cfeb6c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 9 Oct 2024 20:05:12 +0200 Subject: [PATCH 35/38] Fix overly broad declared type Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index cfe23c1..b99a325 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1154,7 +1154,7 @@ class MakefileMaker: def list_source_files(self, root: str, - *patterns: str) -> Iterable[SourceFile]: + *patterns: str) -> List[SourceFile]: """List the source files matching any of the specified patterns. Look for the specified wildcard pattern under all submodules, including From 856b217b7046eb64002560474d60de46e39427e9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 9 Oct 2024 20:18:05 +0200 Subject: [PATCH 36/38] Protect against accidentally overwriting the source Makefile More generally, refuse to overwrite an existing file unless it has "Generated by" in the first line. Make an exception for the config header, which we commonly expect to be modified by test scripts. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 47 ++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b99a325..d440f32 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -71,6 +71,37 @@ def are_same_existing_files(*files: str) -> bool: return False return True +def open_write_safe(filename: str, + temp_filename: Optional[str] = None, + **kwargs) -> typing.IO[str]: + # TODO: this function should be a protocol class that creates a temporary + # file and renames it when closing on success. + """Open the given file for writing, but only if it looks generated. + + If temp_filename is given, open temp_filename for writing, but expect + the content to go into filename eventually. + + The goal of this function is to only allow overwriting files that were + generated by a previous run of this script. + + Not protected against race conditions. + """ + if os.path.exists(filename): + with open(filename, 'r') as inp: + line = inp.readline() + if not ('Generated by' in line): + raise Exception('Not overwriting {} which looks hand-written' + .format(filename)) + if temp_filename is not None and os.path.exists(temp_filename): + with open(temp_filename, 'r') as inp: + line = inp.readline() + if not ('Generated by' in line or + (not line and temp_filename is not None)): + raise Exception('Not overwriting {} which looks hand-written' + .format(temp_filename)) + return open(filename if temp_filename is None else temp_filename, 'w') + + class EnvironmentOption: """A description of options that set makefile variables. @@ -998,11 +1029,10 @@ class MakefileMaker: if os.path.exists(os.path.join(self.options.source, 'tf-psa-crypto')): # File included by # include/psa/crypto_driver_contexts_primitives.h - with open(os.path.join(self.options.dir, + with open_write_safe(os.path.join(self.options.dir, 'tests', 'libtestdriver1', 'libtestdriver1', 'tf-psa-crypto', - 'include', 'psa', 'crypto.h'), - 'w') as out: + 'include', 'psa', 'crypto.h')) as out: out.write("""/* Generated by mbedtls-prepare-build */ #include "../../../../include-testdriver1/psa/crypto.h" /* End of generated file */ @@ -1025,8 +1055,7 @@ class MakefileMaker: "'$(libtestdriver1_rewrite)'", src.make_path(), '>$@')]) - with open(os.path.join(include_dir, 'psa', 'crypto_config.h'), - 'w') as out: + with open_write_safe(os.path.join(include_dir, 'psa', 'crypto_config.h')) as out: out.write("""/* Generated by mbedtls-prepare-build */ #include "base_crypto_config.h" #include "crypto_config_test_driver_extension.h" @@ -1802,7 +1831,7 @@ if [ -n "$$failures" ]; then echo; echo "Failures:$$failures"; false; fi """Generate the makefile.""" destination = os.path.join(self.options.dir, 'Makefile') temp_file = destination + '.new' - with open(temp_file, 'w') as out: + with open_write_safe(destination, temp_file) as out: try: self.out = out self.output_all() @@ -1926,6 +1955,7 @@ class ConfigExplicit(ConfigMaker): self.lines = [] #type: List[str] def start(self) -> None: + self.lines.append('/* Generated by mbedtls-prepare-build */') self.lines.append('#ifndef MBEDTLS_CONFIG_H') self.lines.append('#define MBEDTLS_CONFIG_H') self.lines.append('') @@ -1942,6 +1972,11 @@ class ConfigExplicit(ConfigMaker): def finish(self) -> None: self.lines.append('') self.lines.append('#endif') + # Overwrite the configuration file unconditionally. We're used + # to that happening. Eventually we should call open_write_safe(), + # but allow a transition period where we don't check for + # "Generated by", so that existing build trees made before + # this class added a "Generated by" comment line can be reused. with open(self.target_file, 'w') as out: for line in self.lines: out.write(line + '\n') From 2383eeaf87dac78fa93f9516e24edf3ab857af6a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Oct 2024 13:20:12 +0200 Subject: [PATCH 37/38] Fix dodgy regex quoting Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index d440f32..bb6daaf 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -561,9 +561,9 @@ class MakefileMaker: @staticmethod def _glob2re_repl(m: typing.Match[str]) -> str: if m.group(0) == '*': - return '[^/]*' + return r'[^/]*' elif m.group(0) == '?': - return '[^/]' + return r'[^/]' else: return '\\' + m.group(0) @classmethod @@ -577,11 +577,11 @@ class MakefileMaker: def is_already_cleaned(self, name: str) -> bool: if not self.clean: return False - regex = ''.join(['\A(?:', + regex = ''.join([r'\A(?:', '|'.join([self.glob2re(pattern) for patterns in self.clean for pattern in patterns.split()]), - ')\Z']) + r')\Z']) return re.match(regex, name) is not None def add_clean(self, *elements: str) -> None: From 16c7ec755c3c4cad4acf921574b36c15a21e811c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Oct 2024 13:20:26 +0200 Subject: [PATCH 38/38] Make pylint slightly happier Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index bb6daaf..97f3585 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -89,7 +89,7 @@ def open_write_safe(filename: str, if os.path.exists(filename): with open(filename, 'r') as inp: line = inp.readline() - if not ('Generated by' in line): + if 'Generated by' not in line: raise Exception('Not overwriting {} which looks hand-written' .format(filename)) if temp_filename is not None and os.path.exists(temp_filename): @@ -312,22 +312,18 @@ class ClassicTestGenerator(TestGeneratorInterface): def __init__(self, options: Options) -> None: self.options = options - @staticmethod - def target(c_file: str) -> str: + def target(self, c_file: str) -> str: return c_file - @staticmethod - def script(_source_dir: str) -> str: + def script(self, _source_dir: str) -> str: return 'tests/scripts/generate_code.pl' - @staticmethod - def function_files(function_file: str) -> List[str]: + def function_files(self, function_file: str) -> List[str]: return ['tests/suites/helpers.function', 'tests/suites/main_test.function', function_file] - @staticmethod - def command(function_file: str, data_file: str) -> str: + def command(self, function_file: str, data_file: str) -> str: source_dir = os.path.dirname(function_file) if source_dir != os.path.dirname(data_file): raise Exception('Function file and data file are not in the same directory', @@ -351,8 +347,7 @@ class OnTargetTestGenerator(TestGeneratorInterface): def __init__(self, options: Options) -> None: self.options = options - @staticmethod - def target(c_file: str) -> str: + def target(self, c_file: str) -> str: datax_file = os.path.splitext(c_file)[0] + '.datax' return sjoin(c_file, datax_file)