-
Notifications
You must be signed in to change notification settings - Fork 36
Split test_psa_compliance.py #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
gilles-peskine-arm
wants to merge
14
commits into
Mbed-TLS:main
Choose a base branch
from
gilles-peskine-arm:compliance-split-framework
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3008a4a
Put the command line entry point in a function
gilles-peskine-arm 26cb1a1
Move the bulk of test_psa_compliance.py to a module
gilles-peskine-arm 43c6cbc
Move branch-dependent defaults to the calling script
gilles-peskine-arm 4ff6a49
Support applying a patch to the compliance suite
gilles-peskine-arm af6b189
Teach crypto_knowledge the categories of SP800_100 and SPAKE2P algori…
gilles-peskine-arm 565b4c3
Skip all knowledge of SPAKE2+ keys for now
gilles-peskine-arm b4035db
Fix [] used as a default argument value (mutable default values are d…
gilles-peskine-arm 954783c
macro_collector: be stricter about unusual macros
gilles-peskine-arm ca98305
Treat xxx_HAS_xxx as not a constructor, like xxx_IS_xxx etc.
gilles-peskine-arm 1295140
Read patch files instead of taking a single patch as input
gilles-peskine-arm 088a99e
Allow framework and consuming branch to have Python scripts with the …
gilles-peskine-arm 581da21
Allow patch files to have trailing whitespace
gilles-peskine-arm aebb1fa
Sort patch files for reproducibility
gilles-peskine-arm 44ea713
Simplify passing a file to subprocess stdin
gilles-peskine-arm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,189 @@ | ||
"""Run the PSA Crypto API compliance test suite. | ||
Clone the repo and check out the commit specified by PSA_ARCH_TEST_REPO and PSA_ARCH_TEST_REF, | ||
then compile and run the test suite. The clone is stored at <repository root>/psa-arch-tests. | ||
Known defects in either the test suite or mbedtls / TF-PSA-Crypto - identified by their test | ||
number - are ignored, while unexpected failures AND successes are reported as errors, to help | ||
keep the list of known defects as up to date as possible. | ||
""" | ||
|
||
# Copyright The Mbed TLS Contributors | ||
# SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later | ||
|
||
import argparse | ||
import glob | ||
import os | ||
import re | ||
import shutil | ||
import subprocess | ||
import sys | ||
from typing import List, Optional | ||
from pathlib import Path | ||
|
||
from . import build_tree | ||
|
||
PSA_ARCH_TESTS_REPO = 'https://github.com/ARM-software/psa-arch-tests.git' | ||
|
||
#pylint: disable=too-many-branches,too-many-statements,too-many-locals | ||
def test_compliance(library_build_dir: str, | ||
psa_arch_tests_ref: str, | ||
patch_files: List[str], | ||
expected_failures: List[int]) -> int: | ||
"""Check out and run compliance tests. | ||
|
||
library_build_dir: path where our library will be built. | ||
psa_arch_tests_ref: tag or sha to use for the arch-tests. | ||
patch: patch to apply to the arch-tests with ``patch -p1``. | ||
expected_failures: default list of expected failures. | ||
""" | ||
root_dir = os.getcwd() | ||
install_dir = Path(library_build_dir + "/install_dir").resolve() | ||
tmp_env = os.environ | ||
tmp_env['CC'] = 'gcc' | ||
subprocess.check_call(['cmake', '.', '-GUnix Makefiles', | ||
'-B' + library_build_dir, | ||
'-DCMAKE_INSTALL_PREFIX=' + str(install_dir)], | ||
env=tmp_env) | ||
subprocess.check_call(['cmake', '--build', library_build_dir, '--target', 'install']) | ||
|
||
if build_tree.is_mbedtls_3_6(): | ||
crypto_library_path = install_dir.joinpath("lib/libmbedcrypto.a") | ||
else: | ||
crypto_library_path = install_dir.joinpath("lib/libtfpsacrypto.a") | ||
|
||
psa_arch_tests_dir = 'psa-arch-tests' | ||
os.makedirs(psa_arch_tests_dir, exist_ok=True) | ||
try: | ||
os.chdir(psa_arch_tests_dir) | ||
|
||
# Reuse existing local clone | ||
subprocess.check_call(['git', 'init']) | ||
subprocess.check_call(['git', 'fetch', PSA_ARCH_TESTS_REPO, psa_arch_tests_ref]) | ||
subprocess.check_call(['git', 'checkout', '--force', 'FETCH_HEAD']) | ||
|
||
if patch_files: | ||
subprocess.check_call(['git', 'reset', '--hard']) | ||
for patch_file in patch_files: | ||
with open(os.path.join(root_dir, patch_file), 'rb') as patch: | ||
subprocess.check_call(['patch', '-p1'], | ||
stdin=patch) | ||
|
||
build_dir = 'api-tests/build' | ||
try: | ||
shutil.rmtree(build_dir) | ||
except FileNotFoundError: | ||
pass | ||
os.mkdir(build_dir) | ||
os.chdir(build_dir) | ||
|
||
#pylint: disable=bad-continuation | ||
subprocess.check_call([ | ||
'cmake', '..', | ||
'-GUnix Makefiles', | ||
'-DTARGET=tgt_dev_apis_stdc', | ||
'-DTOOLCHAIN=HOST_GCC', | ||
'-DSUITE=CRYPTO', | ||
'-DPSA_CRYPTO_LIB_FILENAME={}'.format(str(crypto_library_path)), | ||
'-DPSA_INCLUDE_PATHS=' + str(install_dir.joinpath("include")) | ||
]) | ||
|
||
subprocess.check_call(['cmake', '--build', '.']) | ||
|
||
proc = subprocess.Popen(['./psa-arch-tests-crypto'], | ||
bufsize=1, stdout=subprocess.PIPE, universal_newlines=True) | ||
|
||
test_re = re.compile( | ||
'^TEST: (?P<test_num>[0-9]*)|' | ||
'^TEST RESULT: (?P<test_result>FAILED|PASSED)' | ||
) | ||
test = -1 | ||
unexpected_successes = expected_failures.copy() | ||
expected_failures.clear() | ||
unexpected_failures = [] # type: List[int] | ||
if proc.stdout is None: | ||
return 1 | ||
|
||
for line in proc.stdout: | ||
print(line, end='') | ||
match = test_re.match(line) | ||
if match is not None: | ||
groupdict = match.groupdict() | ||
test_num = groupdict['test_num'] | ||
if test_num is not None: | ||
test = int(test_num) | ||
elif groupdict['test_result'] == 'FAILED': | ||
try: | ||
unexpected_successes.remove(test) | ||
expected_failures.append(test) | ||
print('Expected failure, ignoring') | ||
except KeyError: | ||
unexpected_failures.append(test) | ||
print('ERROR: Unexpected failure') | ||
elif test in unexpected_successes: | ||
print('ERROR: Unexpected success') | ||
proc.wait() | ||
|
||
print() | ||
print('***** test_psa_compliance.py report ******') | ||
print() | ||
print('Expected failures:', ', '.join(str(i) for i in expected_failures)) | ||
print('Unexpected failures:', ', '.join(str(i) for i in unexpected_failures)) | ||
print('Unexpected successes:', ', '.join(str(i) for i in sorted(unexpected_successes))) | ||
print() | ||
if unexpected_successes or unexpected_failures: | ||
if unexpected_successes: | ||
print('Unexpected successes encountered.') | ||
print('Please remove the corresponding tests from ' | ||
'EXPECTED_FAILURES in tests/scripts/compliance_test.py') | ||
print() | ||
print('FAILED') | ||
return 1 | ||
else: | ||
print('SUCCESS') | ||
return 0 | ||
finally: | ||
os.chdir(root_dir) | ||
|
||
def main(psa_arch_tests_ref: str, | ||
expected_failures: Optional[List[int]] = None) -> None: | ||
"""Command line entry point. | ||
|
||
psa_arch_tests_ref: tag or sha to use for the arch-tests. | ||
expected_failures: default list of expected failures. | ||
""" | ||
build_dir = 'out_of_source_build' | ||
default_patch_directory = os.path.join(build_tree.guess_project_root(), | ||
'scripts/data_files/psa-arch-tests') | ||
|
||
# pylint: disable=invalid-name | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument('--build-dir', nargs=1, | ||
help='path to Mbed TLS / TF-PSA-Crypto build directory') | ||
parser.add_argument('--expected-failures', nargs='+', | ||
help='''set the list of test codes which are expected to fail | ||
from the command line. If omitted the list given by | ||
EXPECTED_FAILURES (inside the script) is used.''') | ||
parser.add_argument('--patch-directory', nargs=1, | ||
default=default_patch_directory, | ||
mpg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
help='Directory containing patches (*.patch) to apply to psa-arch-tests') | ||
args = parser.parse_args() | ||
|
||
if args.build_dir is not None: | ||
build_dir = args.build_dir[0] | ||
|
||
if expected_failures is None: | ||
expected_failures = [] | ||
if args.expected_failures is not None: | ||
expected_failures_list = [int(i) for i in args.expected_failures] | ||
else: | ||
expected_failures_list = expected_failures | ||
|
||
if args.patch_directory: | ||
patch_file_glob = os.path.join(args.patch_directory, '*.patch') | ||
patch_files = sorted(glob.glob(patch_file_glob)) | ||
else: | ||
patch_files = [] | ||
|
||
sys.exit(test_compliance(build_dir, | ||
psa_arch_tests_ref, | ||
patch_files, | ||
expected_failures_list)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the logic behind running
git reset
only if we have a patch to apply. It seems to me thatgit reset
is needed if we had a patch to apply the last time we ran this script (and we are re-using the repo), which might not be the same as we have one now. Also, I don't think callinggit reset
when not strictly needed does any harm. So, I'd be inclined to just call it unconditionally. Am I missing something?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to minimize changes in 3.6 which doesn't need any of the new fancy stuff. Also, arguably, it makes things worse for local debugging where you might want to edit the cached psa-arch-test tree until you get it right. On the other hand, I agree with you that I would have made
git reset
unconditional if I was doing this from scratch. I'll defer to Bence's preference on that since he know this script's history a lot better than we do.