Switch generate_psa_test.py to automatic dependencies for positive test cases #122
Conversation
This file contains 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
gilles-peskine-arm
added
needs-review
gilles-peskine-arm
force-pushed
the
psa-storage-test-cases-never-supported-positive-crypto
branch
from
9e3869a to
2a10619
Compare
gilles-peskine-arm
added
needs-work
needs-ci
gilles-peskine-arm
force-pushed
the
psa-storage-test-cases-never-supported-positive-crypto
branch
2 times, most recently
from
fe3cb7b to
5eb6b15
Compare
gilles-peskine-arm
added
needs-review
davidhorstmann-arm
previously approved these changes
Jan 6, 2025
waleed-elmelegy-arm
previously approved these changes
Jan 8, 2025
waleed-elmelegy-arm
removed
needs-review
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Don't list mechanisms that are not implemented in `include/psa/crypto_config.h`, even commented out. Uncommenting them wouldn't help anyway: they don't work. Having them listed, even commented out, causes `find_dependencies_not_implemented()` in `psa_test_case.py` to consider those mechanisms to be implemented, and thus causes `generate_psa_tests.py` to generate test cases that cannot be executed. The affected mechanisms are: * `PSA_ALG_CBC_MAC` (`PSA_WANT_ALG_CBC_MAC`) * `PSA_ALG_XTS` (`PSA_WANT_ALG_XTS`) * `PSA_ECC_FAMILY_SECP_K1` 224-bit (`PSA_WANT_ECC_SECP_K1_224`) Also remove the affected mechanisms from configuration adjustment files, since that is code that can never be triggered. There were already no generated test cases for SECP224K1 because `PSA_WANT_ECC_SECP_K1_224` was already detected as a dependency that cannot be implemented, because that is not a valid size: PSA defines SECP224K1 as 225-bit, and `crypto_knowledge.py` follows suite, so `generate_psa_tests.py` saw `PSA_WANT_ECC_SECP_K1_225` in its enumeration but skipped it because it was never mentioned in `crypto_config.h`. This causes generated PSA tests to no longer include positive test cases for `PSA_ALG_CBC_MAC` and `PSA_ALG_XTS`. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Remove all code guarded by `PSA_WANT_ECC_SECP_K1_224`, which is not and will not be implemented. (It would be K1_225 anyway, but we don't intend to implement it anyway.) Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
dismissed stale reviews from waleed-elmelegy-arm and davidhorstmann-arm
via
32c82f0
gilles-peskine-arm
force-pushed
the
psa-storage-test-cases-never-supported-positive-crypto
branch
from
5eb6b15 to
32c82f0
Compare
|
I've rebased this and the consuming mbedtls PR to catch up with the framework updates. The framework repository now has the needed content so a framework submodule update is no longer necessary here. |
gilles-peskine-arm
added
needs-review
5 tasks
|
The CI has glitched. In any case it would have failed in |
davidhorstmann-arm
approved these changes
Jan 8, 2025
waleed-elmelegy-arm
approved these changes
Jan 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Mostly happening in the framework, see Mbed-TLS/mbedtls-framework#83.
Also remove all mentions of
PSA_WANT_ECC_SECP_K1_224(secp224k1 was never supported in PSA, but there were traces of it due to it being supported in the legacy API). It was simpler than fixing everything to keep all the scripts happy.PR checklist