Skip to content

Conversation

valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Jun 5, 2025

Description

Helps resolving Mbed-TLS/TF-PSA-Crypto#298
This is PR 1 out of 3 for this series to solve that issue

Shadow PR with everything: Mbed-TLS/mbedtls#10228

PR checklist

@valeriosetti valeriosetti changed the title [framework] [framework] PK: try storing all private RSA keys in PSA Jun 5, 2025
@valeriosetti valeriosetti self-assigned this Jun 10, 2025
@valeriosetti valeriosetti added needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Jun 10, 2025
Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti valeriosetti changed the title [framework] PK: try storing all private RSA keys in PSA [framework] PK: try storing all private RSA keys in PSA (1/3) Jun 12, 2025
@valeriosetti valeriosetti removed their assignment Jun 12, 2025
Signed-off-by: Valerio Setti <vsetti@baylibre.com>
The previous key had only 518 bits for E. Being not a multiple of 8
this didn't allow the key to be imported into PSA.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
Copy link
Contributor Author

Only Windows failures left in the CI, but all of them are related to the CI, not to the code, so I'm going to remove the need-ci label.

@valeriosetti valeriosetti added needs-review Every commit must be reviewed by at least two team members, and removed needs-ci Needs to pass CI tests labels Jun 13, 2025
@gilles-peskine-arm gilles-peskine-arm self-requested a review June 18, 2025 18:44
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-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, assuming that Mbed-TLS/TF-PSA-Crypto#308 does the right thing (I haven't reviewed it yet).

Copy link
Contributor

@felixc-arm felixc-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 with very minor/optional nit 🙂

Comment on lines 1397 to 1404
case MBEDTLS_PK_RSA:
#if defined(MBEDTLS_PK_USE_PSA_RSA_DATA)
TEST_ASSERT(PSA_KEY_TYPE_IS_RSA(psa_type));
pk_public = pk->pub_raw;
pk_public_length = pk->pub_raw_len;
break;
#else /* MBEDTLS_PK_USE_PSA_RSA_DATA */
TEST_ASSERT(PSA_KEY_TYPE_IS_RSA(psa_type));
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit - could probably have moved the duplicated TEST_ASSERT out of the #if defined but only a minor code style thing.

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Jun 24, 2025
@felixc-arm felixc-arm added approved Design and code approved - may be waiting for CI or backports and removed 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 labels Jun 24, 2025
@mpg mpg merged commit 893ad9e into Mbed-TLS:main Jun 25, 2025
2 of 3 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) Jun 25, 2025
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 priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Development

Successfully merging this pull request may close these issues.

4 participants