From 40de3d3639cfbb67b68d1ab1d35d478bd9200c2a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 14 Oct 2022 15:13:45 +0200 Subject: [PATCH 01/10] Backport build_tree.py from development Copy of scripts/mbedtls_dev/build_tree.py from mbedtls-3.2.1, backported to facilitate future backports of python scripts. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/build_tree.py | 60 +++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 scripts/mbedtls_dev/build_tree.py diff --git a/scripts/mbedtls_dev/build_tree.py b/scripts/mbedtls_dev/build_tree.py new file mode 100644 index 0000000000..3920d0ed6c --- /dev/null +++ b/scripts/mbedtls_dev/build_tree.py @@ -0,0 +1,60 @@ +"""Mbed TLS build tree information and manipulation. +""" + +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import inspect + + +def looks_like_mbedtls_root(path: str) -> bool: + """Whether the given directory looks like the root of the Mbed TLS source tree.""" + return all(os.path.isdir(os.path.join(path, subdir)) + for subdir in ['include', 'library', 'programs', 'tests']) + + +def chdir_to_root() -> None: + """Detect the root of the Mbed TLS source tree and change to it. + + The current directory must be up to two levels deep inside an Mbed TLS + source tree. + """ + for d in [os.path.curdir, + os.path.pardir, + os.path.join(os.path.pardir, os.path.pardir)]: + if looks_like_mbedtls_root(d): + os.chdir(d) + return + raise Exception('Mbed TLS source tree not found') + + +def guess_mbedtls_root(): + """Guess mbedTLS source code directory. + + Return the first possible mbedTLS root directory + """ + dirs = set({}) + for frame in inspect.stack(): + path = os.path.dirname(frame.filename) + for d in ['.', os.path.pardir] \ + + [os.path.join(*([os.path.pardir]*i)) for i in range(2, 10)]: + d = os.path.abspath(os.path.join(path, d)) + if d in dirs: + continue + dirs.add(d) + if looks_like_mbedtls_root(d): + return d + raise Exception('Mbed TLS source tree not found') From 69feebd17840551f8014c14989fc780bb59bfaa5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 16 Sep 2022 21:41:47 +0200 Subject: [PATCH 02/10] More precise name for test data generation We have Python code both for test code generation (tests/scripts/generate_test_code.py) and now for test data generation. Avoid the ambiguous expression "test generation". This commit renames the Python module and adjusts all references to it. A subsequent commit will adjust the documentation. Signed-off-by: Gilles Peskine --- .../{test_generation.py => test_data_generation.py} | 0 tests/scripts/generate_bignum_tests.py | 10 +++++----- tests/scripts/generate_psa_tests.py | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) rename scripts/mbedtls_dev/{test_generation.py => test_data_generation.py} (100%) diff --git a/scripts/mbedtls_dev/test_generation.py b/scripts/mbedtls_dev/test_data_generation.py similarity index 100% rename from scripts/mbedtls_dev/test_generation.py rename to scripts/mbedtls_dev/test_data_generation.py diff --git a/tests/scripts/generate_bignum_tests.py b/tests/scripts/generate_bignum_tests.py index ceafa4a489..1cd859c4af 100755 --- a/tests/scripts/generate_bignum_tests.py +++ b/tests/scripts/generate_bignum_tests.py @@ -6,7 +6,7 @@ generate only the specified files. Class structure: -Child classes of test_generation.BaseTarget (file targets) represent an output +Child classes of test_data_generation.BaseTarget (file targets) represent an output file. These indicate where test cases will be written to, for all subclasses of this target. Multiple file targets should not reuse a `target_basename`. @@ -36,7 +36,7 @@ following: call `.create_test_case()` to yield the TestCase. Additional details and other attributes/methods are given in the documentation -of BaseTarget in test_generation.py. +of BaseTarget in test_data_generation.py. """ # Copyright The Mbed TLS Contributors @@ -63,7 +63,7 @@ from typing import Iterator, List, Tuple, TypeVar import scripts_path # pylint: disable=unused-import from mbedtls_dev import test_case -from mbedtls_dev import test_generation +from mbedtls_dev import test_data_generation T = TypeVar('T') #pylint: disable=invalid-name @@ -85,7 +85,7 @@ def combination_pairs(values: List[T]) -> List[Tuple[T, T]]: ) -class BignumTarget(test_generation.BaseTarget, metaclass=ABCMeta): +class BignumTarget(test_data_generation.BaseTarget, metaclass=ABCMeta): #pylint: disable=abstract-method """Target for bignum (mpi) test case generation.""" target_basename = 'test_suite_mpi.generated' @@ -235,4 +235,4 @@ class BignumAdd(BignumOperation): if __name__ == '__main__': # Use the section of the docstring relevant to the CLI as description - test_generation.main(sys.argv[1:], "\n".join(__doc__.splitlines()[:4])) + test_data_generation.main(sys.argv[1:], "\n".join(__doc__.splitlines()[:4])) diff --git a/tests/scripts/generate_psa_tests.py b/tests/scripts/generate_psa_tests.py index 867a6b4594..e7d4048ad8 100755 --- a/tests/scripts/generate_psa_tests.py +++ b/tests/scripts/generate_psa_tests.py @@ -30,7 +30,7 @@ from mbedtls_dev import crypto_knowledge from mbedtls_dev import macro_collector from mbedtls_dev import psa_storage from mbedtls_dev import test_case -from mbedtls_dev import test_generation +from mbedtls_dev import test_data_generation def psa_want_symbol(name: str) -> str: @@ -894,7 +894,7 @@ class StorageFormatV0(StorageFormat): yield from super().generate_all_keys() yield from self.all_keys_for_implicit_usage() -class PSATestGenerator(test_generation.TestGenerator): +class PSATestGenerator(test_data_generation.TestGenerator): """Test generator subclass including PSA targets and info.""" # Note that targets whose names contain 'test_format' have their content # validated by `abi_check.py`. @@ -919,4 +919,4 @@ class PSATestGenerator(test_generation.TestGenerator): super().generate_target(name, self.info) if __name__ == '__main__': - test_generation.main(sys.argv[1:], __doc__, PSATestGenerator) + test_data_generation.main(sys.argv[1:], __doc__, PSATestGenerator) From bd5147c1c09b87ab7dd24439863a0e17ed71b97d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 16 Sep 2022 22:02:37 +0200 Subject: [PATCH 03/10] Clarify the descriptions of test-case-data-related modules Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/test_case.py | 2 +- scripts/mbedtls_dev/test_data_generation.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/mbedtls_dev/test_case.py b/scripts/mbedtls_dev/test_case.py index d0afa592fd..8c97eefcfd 100644 --- a/scripts/mbedtls_dev/test_case.py +++ b/scripts/mbedtls_dev/test_case.py @@ -1,4 +1,4 @@ -"""Library for generating Mbed TLS test data. +"""Library for constructing an Mbed TLS test case. """ # Copyright The Mbed TLS Contributors diff --git a/scripts/mbedtls_dev/test_data_generation.py b/scripts/mbedtls_dev/test_data_generation.py index 5de0b885c8..2b90ef4b79 100644 --- a/scripts/mbedtls_dev/test_data_generation.py +++ b/scripts/mbedtls_dev/test_data_generation.py @@ -1,4 +1,7 @@ -"""Common test generation classes and main function. +"""Common code for test data generation. + +This module defines classes that are of general use to automatically +generate .data files for unit tests, as well as a main function. These are used both by generate_psa_tests.py and generate_bignum_tests.py. """ From 4881540b06ede987b110855fc69be1f8951d65b3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 14 Oct 2022 15:01:52 +0200 Subject: [PATCH 04/10] generate_*_tests.py: simplify test_suite_directory handling test_suite_directory can be changed by a command line option in the development branch but not in 2.28. Align the simplified version here with a change in the development version ("generate_*_tests.py --directory: fix handling of relative path"). Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/test_data_generation.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/scripts/mbedtls_dev/test_data_generation.py b/scripts/mbedtls_dev/test_data_generation.py index 2b90ef4b79..993cd4bdf4 100644 --- a/scripts/mbedtls_dev/test_data_generation.py +++ b/scripts/mbedtls_dev/test_data_generation.py @@ -139,9 +139,8 @@ class BaseTarget(metaclass=ABCMeta): class TestGenerator: """Generate test cases and write to data files.""" - def __init__(self, options) -> None: - self.test_suite_directory = self.get_option(options, 'directory', - 'tests/suites') + def __init__(self, _options) -> None: + self.test_suite_directory = 'tests/suites' # Update `targets` with an entry for each child class of BaseTarget. # Each entry represents a file generated by the BaseTarget framework, # and enables generating the .data files using the CLI. @@ -150,11 +149,6 @@ class TestGenerator: for subclass in BaseTarget.__subclasses__() }) - @staticmethod - def get_option(options, name: str, default: T) -> T: - value = getattr(options, name, None) - return default if value is None else value - def filename_for(self, basename: str) -> str: """The location of the data file with the specified base name.""" return posixpath.join(self.test_suite_directory, basename + '.data') From f8d031fb184c1b3997fc5ae47206f9d5519f7934 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 14 Oct 2022 15:21:49 +0200 Subject: [PATCH 05/10] generate_*_tests.py: chdir to mbedtls root Do this in 2.28 just like it's done in the development branch, so that code and command line usage that works on one branch doesn't surprisingly fail on 2.28. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/test_data_generation.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/mbedtls_dev/test_data_generation.py b/scripts/mbedtls_dev/test_data_generation.py index 993cd4bdf4..376046858b 100644 --- a/scripts/mbedtls_dev/test_data_generation.py +++ b/scripts/mbedtls_dev/test_data_generation.py @@ -29,6 +29,7 @@ import re from abc import ABCMeta, abstractmethod from typing import Callable, Dict, Iterable, Iterator, List, Type, TypeVar +from mbedtls_dev import build_tree from mbedtls_dev import test_case T = TypeVar('T') #pylint: disable=invalid-name @@ -182,6 +183,12 @@ def main(args, description: str, generator_class: Type[TestGenerator] = TestGene help='List available targets and exit') parser.add_argument('targets', nargs='*', metavar='TARGET', help='Target file to generate (default: all; "-": none)') + + # Change to the mbedtls root, to keep things simple. + # Note that if any command line options refer to paths, they need to + # be adjusted first. + build_tree.chdir_to_root() + options = parser.parse_args(args) generator = generator_class(options) if options.list: From ca980c037f0ff1e879c746aad3db6465db6e4aac Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 16 Sep 2022 22:26:38 +0200 Subject: [PATCH 06/10] Move implementation detail from docstring to comment Signed-off-by: Gilles Peskine --- tests/scripts/generate_bignum_tests.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/scripts/generate_bignum_tests.py b/tests/scripts/generate_bignum_tests.py index 1cd859c4af..091630decc 100755 --- a/tests/scripts/generate_bignum_tests.py +++ b/tests/scripts/generate_bignum_tests.py @@ -74,11 +74,9 @@ def quote_str(val) -> str: return "\"{}\"".format(val) def combination_pairs(values: List[T]) -> List[Tuple[T, T]]: - """Return all pair combinations from input values. - - The return value is cast, as older versions of mypy are unable to derive - the specific type returned by itertools.combinations_with_replacement. - """ + """Return all pair combinations from input values.""" + # The return value is cast, as older versions of mypy are unable to derive + # the specific type returned by itertools.combinations_with_replacement. return typing.cast( List[Tuple[T, T]], list(itertools.combinations_with_replacement(values, 2)) From 239765ad3e0c8c5a28910f49f9eaafd182e9d39a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 16 Sep 2022 22:35:18 +0200 Subject: [PATCH 07/10] Use relative imports when importing other modules in the same directory We were using absolute imports under the assumption that the /scripts directory is in the path. This worked in normal use because every one of our Python scripts either were in the /scripts directory, or added the /scripts directory to the module search path in order to reference mbedtls_dev. However, this broke things like ``` python3 -m unittest scripts/mbedtls_dev/psa_storage.py ``` Fix this by using relative imports. Relative imports are only supposed to be used inside a package (Python doesn't complain, but Pylint does). So make /scripts/mbedtls_dev a proper package by creating __init__.py. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/__init__.py | 3 +++ scripts/mbedtls_dev/crypto_knowledge.py | 2 +- scripts/mbedtls_dev/psa_storage.py | 2 +- scripts/mbedtls_dev/test_case.py | 2 +- scripts/mbedtls_dev/test_data_generation.py | 4 ++-- 5 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 scripts/mbedtls_dev/__init__.py diff --git a/scripts/mbedtls_dev/__init__.py b/scripts/mbedtls_dev/__init__.py new file mode 100644 index 0000000000..c5bddaf74b --- /dev/null +++ b/scripts/mbedtls_dev/__init__.py @@ -0,0 +1,3 @@ +# This file needs to exist to make mbedtls_dev a package. +# Among other things, this allows modules in this directory to make +# relative impotrs. diff --git a/scripts/mbedtls_dev/crypto_knowledge.py b/scripts/mbedtls_dev/crypto_knowledge.py index 2173d10fdd..f227a411b0 100644 --- a/scripts/mbedtls_dev/crypto_knowledge.py +++ b/scripts/mbedtls_dev/crypto_knowledge.py @@ -22,7 +22,7 @@ import enum import re from typing import FrozenSet, Iterable, List, Optional, Tuple -from mbedtls_dev.asymmetric_key_data import ASYMMETRIC_KEY_DATA +from .asymmetric_key_data import ASYMMETRIC_KEY_DATA def short_expression(original: str, level: int = 0) -> str: diff --git a/scripts/mbedtls_dev/psa_storage.py b/scripts/mbedtls_dev/psa_storage.py index a06dce13ba..bae99383dc 100644 --- a/scripts/mbedtls_dev/psa_storage.py +++ b/scripts/mbedtls_dev/psa_storage.py @@ -26,7 +26,7 @@ import struct from typing import Dict, List, Optional, Set, Union import unittest -from mbedtls_dev import c_build_helper +from . import c_build_helper class Expr: diff --git a/scripts/mbedtls_dev/test_case.py b/scripts/mbedtls_dev/test_case.py index 8c97eefcfd..8f08703678 100644 --- a/scripts/mbedtls_dev/test_case.py +++ b/scripts/mbedtls_dev/test_case.py @@ -21,7 +21,7 @@ import os import sys from typing import Iterable, List, Optional -from mbedtls_dev import typing_util +from . import typing_util def hex_string(data: bytes) -> str: return '"' + binascii.hexlify(data).decode('ascii') + '"' diff --git a/scripts/mbedtls_dev/test_data_generation.py b/scripts/mbedtls_dev/test_data_generation.py index 376046858b..9e36af32e7 100644 --- a/scripts/mbedtls_dev/test_data_generation.py +++ b/scripts/mbedtls_dev/test_data_generation.py @@ -29,8 +29,8 @@ import re from abc import ABCMeta, abstractmethod from typing import Callable, Dict, Iterable, Iterator, List, Type, TypeVar -from mbedtls_dev import build_tree -from mbedtls_dev import test_case +from . import build_tree +from . import test_case T = TypeVar('T') #pylint: disable=invalid-name From 7ff47661158e91655e5e1d5564db68932c4cb4f6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 18 Sep 2022 21:17:09 +0200 Subject: [PATCH 08/10] Unify check_repo_path We had 4 identical copies of the check_repo_path function. Replace them by a single copy in the build_tree module where it naturally belongs. Signed-off-by: Gilles Peskine --- scripts/abi_check.py | 9 +++------ scripts/mbedtls_dev/build_tree.py | 7 +++++++ tests/scripts/check_files.py | 10 ++++------ tests/scripts/check_names.py | 15 +++++---------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index c2288432ce..ac1d60ffd0 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -113,6 +113,8 @@ from types import SimpleNamespace import xml.etree.ElementTree as ET +from mbedtls_dev import build_tree + class AbiChecker: """API and ABI checker.""" @@ -150,11 +152,6 @@ class AbiChecker: self.git_command = "git" self.make_command = "make" - @staticmethod - def check_repo_path(): - if not all(os.path.isdir(d) for d in ["include", "library", "tests"]): - raise Exception("Must be run from Mbed TLS root") - def _setup_logger(self): self.log = logging.getLogger() if self.verbose: @@ -540,7 +537,7 @@ class AbiChecker: def check_for_abi_changes(self): """Generate a report of ABI differences between self.old_rev and self.new_rev.""" - self.check_repo_path() + build_tree.check_repo_path() if self.check_api or self.check_abi: self.check_abi_tools_are_installed() self._get_abi_dump_for_ref(self.old_version) diff --git a/scripts/mbedtls_dev/build_tree.py b/scripts/mbedtls_dev/build_tree.py index 3920d0ed6c..f52b785d95 100644 --- a/scripts/mbedtls_dev/build_tree.py +++ b/scripts/mbedtls_dev/build_tree.py @@ -25,6 +25,13 @@ def looks_like_mbedtls_root(path: str) -> bool: return all(os.path.isdir(os.path.join(path, subdir)) for subdir in ['include', 'library', 'programs', 'tests']) +def check_repo_path(): + """ + Check that the current working directory is the project root, and throw + an exception if not. + """ + if not all(os.path.isdir(d) for d in ["include", "library", "tests"]): + raise Exception("This script must be run from Mbed TLS root") def chdir_to_root() -> None: """Detect the root of the Mbed TLS source tree and change to it. diff --git a/tests/scripts/check_files.py b/tests/scripts/check_files.py index a0f5e1f538..5c18702def 100755 --- a/tests/scripts/check_files.py +++ b/tests/scripts/check_files.py @@ -34,6 +34,9 @@ try: except ImportError: pass +import scripts_path # pylint: disable=unused-import +from mbedtls_dev import build_tree + class FileIssueTracker: """Base class for file-wide issue tracking. @@ -338,7 +341,7 @@ class IntegrityChecker: """Instantiate the sanity checker. Check files under the current directory. Write a report of issues to log_file.""" - self.check_repo_path() + build_tree.check_repo_path() self.logger = None self.setup_logger(log_file) self.issues_to_check = [ @@ -353,11 +356,6 @@ class IntegrityChecker: MergeArtifactIssueTracker(), ] - @staticmethod - def check_repo_path(): - if not all(os.path.isdir(d) for d in ["include", "library", "tests"]): - raise Exception("Must be run from Mbed TLS root") - def setup_logger(self, log_file, level=logging.INFO): self.logger = logging.getLogger() self.logger.setLevel(level) diff --git a/tests/scripts/check_names.py b/tests/scripts/check_names.py index 875d0b0f5b..d1e87b5c52 100755 --- a/tests/scripts/check_names.py +++ b/tests/scripts/check_names.py @@ -56,6 +56,10 @@ import shutil import subprocess import logging +import scripts_path # pylint: disable=unused-import +from mbedtls_dev import build_tree + + # Naming patterns to check against. These are defined outside the NameCheck # class for ease of modification. MACRO_PATTERN = r"^(MBEDTLS|PSA)_[0-9A-Z_]*[0-9A-Z]$" @@ -218,7 +222,7 @@ class CodeParser(): """ def __init__(self, log): self.log = log - self.check_repo_path() + build_tree.check_repo_path() # Memo for storing "glob expression": set(filepaths) self.files = {} @@ -227,15 +231,6 @@ class CodeParser(): # Note that "*" can match directory separators in exclude lists. self.excluded_files = ["*/bn_mul", "*/compat-1.3.h"] - @staticmethod - def check_repo_path(): - """ - Check that the current working directory is the project root, and throw - an exception if not. - """ - if not all(os.path.isdir(d) for d in ["include", "library", "tests"]): - raise Exception("This script must be run from Mbed TLS root") - def comprehensive_parse(self): """ Comprehensive ("default") function to call each parsing function and From 5d01cc04a0d8fcea8119d57a789b8c41409a45cd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 18 Sep 2022 21:27:37 +0200 Subject: [PATCH 09/10] Don't use parallel jobs for pylint When pylint runs in parallel, it loses the ability to detect duplicated code across modules. Duplicated code is usually a bad thing, so give pylint the opportunity to let us know. This makes pylint slightly slower, but going from 2 threads to 1 does not make it anywhere close to twice as slow. On my machine, with Python 3.5, pylint -j2 takes about 12s while single-threaded pylint takes about 16s of wall clock time. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index dbf0365325..35319d3e1d 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -67,7 +67,7 @@ elif [ "$1" = "--can-mypy" ]; then fi echo 'Running pylint ...' -$PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py || { +$PYTHON -m pylint scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py || { echo >&2 "pylint reported errors" ret=1 } From 377e7e93bac2b74acc94fdd783a6db63b4c88432 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 29 Sep 2022 18:48:41 +0200 Subject: [PATCH 10/10] Documentation typo Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/mbedtls_dev/__init__.py b/scripts/mbedtls_dev/__init__.py index c5bddaf74b..15b0d60dd3 100644 --- a/scripts/mbedtls_dev/__init__.py +++ b/scripts/mbedtls_dev/__init__.py @@ -1,3 +1,3 @@ # This file needs to exist to make mbedtls_dev a package. # Among other things, this allows modules in this directory to make -# relative impotrs. +# relative imports.