Skip to content
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

Switch generate_psa_test.py to automatic dependencies for negative test cases #157

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jan 15, 2025

This completes the forward port of Mbed-TLS/mbedtls#9025. This is mostly happening in the framework, but a follow-up to #144 is also needed here.

The CI is expected to fail here due to needing an updated framework in mbedtls. The CI should pass in the consuming PR.

PR checklist

@gilles-peskine-arm gilles-peskine-arm added priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) needs-preceding-pr Requires another PR to be merged first needs-ci Needs to pass CI tests labels Jan 15, 2025
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-negative-crypto branch from af1f05f to 99aa44c Compare January 16, 2025 18:58
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests labels Jan 17, 2025
Copy link
Contributor

@waleed-elmelegy-arm waleed-elmelegy-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Fix psa_key_agreement_iop_setup() (and psa_key_agreement_iop_complete())
happily parsing key material as an ECC private key even when it is different
(e.g. an ECC public key), and happily performing ECDH even when passed a
different algorithm (e.g. FFDH), when nothing else goes wrong.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
…ible

Detect a bad algorithm later, so that psa_key_agreement_iop_setup() returns
the same error as psa_key_agreement() in more error cases. This simplifies
testing, and in particular makes test_suite_psa_crypto pass without
requiring changes in `psa_exercise_key.c`.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-storage-test-cases-never-supported-negative-crypto branch from 99aa44c to 137b577 Compare January 20, 2025 14:59
@gilles-peskine-arm
Copy link
Contributor Author

I've rebased on top of the latest development and made a new commit for the framework update, to resolve the submodule update conflict. No other changes.

The CI still needs Mbed-TLS/mbedtls#9909 to pass.

Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@davidhorstmann-arm davidhorstmann-arm added needs-ci Needs to pass CI tests and removed needs-ci Needs to pass CI tests labels Jan 20, 2025
@gilles-peskine-arm gilles-peskine-arm removed the needs-preceding-pr Requires another PR to be merged first label Jan 20, 2025
@waleed-elmelegy-arm waleed-elmelegy-arm removed the needs-review Every commit must be reviewed by at least two team members label Jan 20, 2025
@waleed-elmelegy-arm waleed-elmelegy-arm added approved Design and code approved - may be waiting for CI or backports needs-ci Needs to pass CI tests and removed needs-reviewer This PR needs someone to pick it up for review labels Jan 20, 2025
@gilles-peskine-arm
Copy link
Contributor Author

The CI has passed on Mbed-TLS/mbedtls#9909 at Mbed-TLS/mbedtls@13c418d, thus I'm merging.

@gilles-peskine-arm gilles-peskine-arm merged commit 1bc29c9 into Mbed-TLS:development Jan 20, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants