Skip to content

[tf-psa-crypto] Restrict MBEDTLS_X509_RSASSA_PSS_SUPPORT #253

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
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Apr 10, 2025

Description

This helps resolving Mbed-TLS/mbedtls#8154

PR checklist

@valeriosetti valeriosetti self-assigned this Apr 10, 2025
@valeriosetti valeriosetti force-pushed the issue8154-tfpsacrypto branch from d838540 to a2eff51 Compare April 10, 2025 13:43
@valeriosetti valeriosetti force-pushed the issue8154-tfpsacrypto branch 3 times, most recently from cf1266c to 40b6da6 Compare April 11, 2025 13:27
@valeriosetti valeriosetti requested a review from mpg April 29, 2025 08:18
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM, just a few suggestions for improvements.

@@ -1118,16 +1104,15 @@ void pk_rsa_verify_ext_test_vec(data_t *message_str, int digest,
* PSA or the Mbed TLS API, depending on the PSS options used.
* So, it may return either INVALID_PADDING or INVALID_SIGNATURE.
*/
fprintf(stderr, "result=%d, ret=%d\n", result, ret);
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover from debugging.

Also, looks like we could just update the data file with the correct error code now that we're always using PSA, and then we no longer need this if and comment here.

int ret;

mbedtls_pk_init(&pk);
MD_OR_USE_PSA_INIT();
USE_PSA_INIT();
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: could even be PSA_INIT() (and PSA_DONE()), we know we're always using PSA now.

@valeriosetti valeriosetti requested a review from mpg April 29, 2025 13:31
@valeriosetti valeriosetti added size-s Estimated task size: small (~2d) 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 priority-high High priority - will be reviewed soon labels Apr 29, 2025
This commit also removes the legacy path which was not using PSA API.

Signed-off-by: Valerio Setti <[email protected]>
Since the option is now ignored in mbedtls_pk_verify_ext(), we can
simplify test code by not managing this extra parameter and just pass
NULL instead.

Signed-off-by: Valerio Setti <[email protected]>
Since the legacy path in mbedtls_pk_verify_ext() has been removed and
PSA is the only solution to perform RSA signature validation, there is
no need to adapt return values for legacy/PSA paths.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti valeriosetti force-pushed the issue8154-tfpsacrypto branch from 7dc025b to 1dd184c Compare April 29, 2025 14:12
@valeriosetti
Copy link
Contributor Author

I had to rebase on development to fix this kind of CI failures:

[2025-04-29T13:50:08.248Z] make[1]: *** No rule to make target '../tf-psa-crypto/programs/test/which_aes.c', needed by '../tf-psa-crypto/programs/test/which_aes'.  Stop.

@bjwtaylor bjwtaylor self-requested a review April 29, 2025 14:14
@bjwtaylor
Copy link

LGTM, just need to resolve Manuel's comments and get the CI to pass.

@valeriosetti valeriosetti removed the needs-reviewer This PR needs someone to pick it up for review label Apr 29, 2025
Copy link
Contributor

@mpg mpg 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!

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review Every commit must be reviewed by at least two team members priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Projects
Development

Successfully merging this pull request may close these issues.

3 participants