-
Notifications
You must be signed in to change notification settings - Fork 60
Split test_psa_compliance.py and update compliance version to 1.2 #453
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
Split test_psa_compliance.py and update compliance version to 1.2 #453
Conversation
587f31a to
0a4ed4c
Compare
| import scripts_path # pylint: disable=unused-import | ||
| from mbedtls_framework import psa_compliance | ||
|
|
||
| PSA_ARCH_TESTS_REF = 'v25.02_API1.8_CRYPTO_1.2.1' |
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.
There's a v25.08_API1.9_ADAC_1.0.2, but with that version, test_psa_compliance.py fails at the CMake step. I'm not going to investigate this, moving to 1.9 doesn't make much of a difference for us anyway since it doesn't advance the version of the crypto API.
0a4ed4c to
006adb9
Compare
| diff --git a/api-tests/platform/targets/tgt_dev_apis_stdc/nspe/pal_crypto_config.h b/api-tests/platform/targets/tgt_dev_apis_stdc/nspe/pal_crypto_config.h | ||
| index dad40ec..8d19699 100644 | ||
| --- a/api-tests/platform/targets/tgt_dev_apis_stdc/nspe/pal_crypto_config.h | ||
| +++ b/api-tests/platform/targets/tgt_dev_apis_stdc/nspe/pal_crypto_config.h |
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.
DES is disabled by default in v25.02, so no need to adjust the patch once we remove DES.
Once #475 is merged, this PR can remove the temporary definition of PSA_KEY_TYPE_DES in crypto_compat.h.
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.
Once #475 is merged, this PR can remove the temporary definition of
PSA_KEY_TYPE_DESincrypto_compat.h.
It's merged now :)
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.
Unfortunately, no, there's some test data in psa-arch-tests where PSA_KEY_TYPE_DES isn't guarded.
…owledge Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
You should be able to run `tests/scripts/test_psa_compliance.py` without having to consult some other scripts to know what options to pass. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
….2.1 Provide stubs that just return `PSA_ERROR_NOT_SUPPORTED` for missing functions that are necessary to build psa-arch-tests at `v25.02_API1.8_CRYPTO_1.2.1`: * `psa_key_derivation_verify_bytes()` * `psa_key_derivation_verify_key()` * `psa_pake_set_context()` Also provide a macro for the same reason: * `PSA_PAKE_STEP_CONFIRM` Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Some of those macros are necessary to build psa-arch-tests at `v25.02_API1.8_CRYPTO_1.2.1`. Define other tightly related macros for completeness. Signed-off-by: Gilles Peskine <[email protected]>
In `generate_psa_constants.py`, we use it for anything that `macro_collector.py` analyzes as a key type constructed from a curve. This now includes `PSA_KEY_TYPE_SPAKE2P_PUBLIC_KEY(curve)` and `PSA_KEY_TYPE_SPAKE2P_KEY_PAIR(curve)`. Signed-off-by: Gilles Peskine <[email protected]>
We can now test compliance with version 1.2 of the PSA Crypto API. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
006adb9 to
daa17da
Compare
mpg
left a comment
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.
LGTM, thanks!
| #!/usr/bin/env python3 | ||
| """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, |
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.
Nit: PSA_ARCH_TEST_REPO is no longer defined here so it's a bit weird to reference it in this script's docstring.
|
|
||
| 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 Mbed TLS - identified by their test |
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.
Nit: TF-PSA-Crypto
bensze01
left a comment
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.
LGTM, thanks!
|
The ABI check failure is unrelated, caused by me missing that the mbedtls commit won't be a merge commit in TF-PSA-Crypto Pull requests... Making the ABI-check work in TF-PSA-Crypto PR-jobs is non-goal of Mbed-TLS/mbedtls-test#209, so I'll open a new PR with the fix. |
cf4c26d
Since Mbed-TLS/TF-PSA-Crypto#453, psa-arch-tests crash on our CI and in similar environments because glibc detects a stack buffer overflow. Temporarily ignore the failure until we patch TF-PSA-Crypto. Revert this commit once this is done. Signed-off-by: Gilles Peskine <[email protected]>
Split the PSA API compliance script
test_psa_compliance.pyinto an engine plus a pre-branch runner, like with many similar scripts. Now each branch has the knowledge of which version of psa-arch-tests it runs against and how to adjust the results if needed.Then, in TF-PSA-Crypto, run against v25.02_API1.8_CRYPTO_1.2.1, which allows us to claim compliance with version 1.2 of the PSA Crypto API specification. This requires a bit of preparation:
Resolves #418.
Needs preceding PR: Mbed-TLS/mbedtls-framework#209.
PR checklist
Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.