Enable Curve448 support via the PSA API #4626
Merged
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
component-crypto
mbedtls_ecp_read_key and mbedtls_ecp_write_key are updated to include support for Curve448 as prescribed by RFC 7748 §5. Test suites have been updated to validate curve448 under Montgomery curves. Signed-off-by: Archana <[email protected]>
Enable Curve448 support Add test vectors to evaluate * RFC 7748 * a known-answer public key export test. * a known-answer ECDH (X448) test. Signed-off-by: Archana <[email protected]>
Signed-off-by: Archana <[email protected]>
TRodziewicz
suggested changes
Jul 12, 2021
ronald-cron-arm
requested changes
Jul 12, 2021
There was a problem hiding this comment.
That looks almost good to me, thanks. I have only minor comments. I would also suggest to add a "PSA generate key: ECC, Curve448, good" test similar to the "PSA generate key: ECC, Curve25519, good" one to complete the alignment of curve 25519 and curve 448 testing in test_suite_psa_crypto. Otherwise, the PR needs rebasing now.
Improve the description of some curve 448 tests Signed-off-by: Archana <[email protected]>
* Add tests to validate MSB not ok for Curve25519 and Curve448. * Add a test to generate key for for Curve448. Signed-off-by: Archana <[email protected]>
ronald-cron-arm
requested changes
Jul 12, 2021
There was a problem hiding this comment.
Thanks for addressing my comments so quickly. There are a few things that we use to do differently though:
- when a PR is in review and it has some conflicts with the branch it is targeted to we prefer to rebase the PR rather than merge the targeted branch (development in the case of this PR). Thus, could you please remove the last merge commit and rather rebase the PR commits on top of the
developmentbranch. Feel free to ask if you have some doubt on how to do that. - concerning review conversation we let the reviewer not the PR author resolve the conversations. The PR author usually just uses the rocket icon to indicate to the reviewer that a comment has been addressed.
- when you create new commits to address review comments please make commit message that explain what the commit does and group in one commit similar changes. For example, here a possibility is to have one commit to address the style issues with a commit message like "Fix coding style issue" (it would a squash of your three "Update library/ecp.c" commits. Then we could have one commit "test: psa: Improve the description of some curve 448 tests" ...
silabs-ArchanaM
referenced
this pull request
in SiliconLabs/mbedtls
Jul 13, 2021
Signed-off-by: Archana <[email protected]>
silabs-ArchanaM
force-pushed
the
arch_silabs_3399
branch
from
7091628 to
a8605bb
Compare
ronald-cron-arm
requested changes
Jul 13, 2021
silabs-ArchanaM
referenced
this pull request
in SiliconLabs/mbedtls
Jul 13, 2021
Signed-off-by: Archana <[email protected]>
silabs-ArchanaM
force-pushed
the
arch_silabs_3399
branch
from
a8605bb to
96d4572
Compare
silabs-ArchanaM
referenced
this pull request
in SiliconLabs/mbedtls
Jul 13, 2021
Signed-off-by: Archana <[email protected]>
silabs-ArchanaM
force-pushed
the
arch_silabs_3399
branch
from
96d4572 to
6a66e02
Compare
Signed-off-by: Archana <[email protected]>
silabs-ArchanaM
force-pushed
the
arch_silabs_3399
branch
from
6a66e02 to
554e64e
Compare
TRodziewicz
approved these changes
Jul 16, 2021
silabs-ArchanaM
removed
the
needs-reviewer
gilles-peskine-arm
removed
the
needs-review
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.
Description
Enabled Curve448 support via the PSA API fixing #4249.
Added missing support for validating Curve448 keys fixing #3399.
Status
READY
Requires Backporting
NO
Migrations
NO
Todos