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

[Backport 2.28] Add test for ECP group metadata #6204

Merged
merged 7 commits into from
Dec 13, 2022

Conversation

wernerlewis
Copy link
Contributor

Description

Trivial backport of #6203, with additional commit to include
mbedtls_ecp_group_cmp, taken from development branch.

Test cases added to check curve parameters and information for each
curve supported. Expected parameters are taken from references: SEC 2
for NIST, RFC 5639 for Brainpool, and RFC 7748 for curve25519/curve448.

Status

READY

Signed-off-by: Werner Lewis <werner.lewis@arm.com>
@wernerlewis wernerlewis added enhancement component-crypto Crypto primitives and low-level interfaces labels Aug 15, 2022
@wernerlewis wernerlewis force-pushed the ecp_group_test_2.28 branch 3 times, most recently from 83ec0bd to e5c06ae Compare August 15, 2022 14:27
Test cases added to check curve parameters and information for each
curve supported. Expected parameters are taken from references: SEC 2
for NIST, RFC 5639 for Brainpool, and RFC 7748 for curve25519/curve448.

Signed-off-by: Werner Lewis <werner.lewis@arm.com>
@daverodgman daverodgman added the priority-high High priority - will be reviewed soon label Aug 16, 2022
Spec values are now always used for test data, and conversion to
internal representation is done in the test function.

Signed-off-by: Werner Lewis <werner.lewis@arm.com>
Signed-off-by: Werner Lewis <werner.lewis@arm.com>
Signed-off-by: Werner Lewis <werner.lewis@arm.com>
Signed-off-by: Werner Lewis <werner.lewis@arm.com>
Signed-off-by: Werner Lewis <werner.lewis@arm.com>
@AndrzejKurek AndrzejKurek self-assigned this Sep 20, 2022
Copy link
Contributor

@AndrzejKurek AndrzejKurek left a comment

Choose a reason for hiding this comment

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

This is a faithful backport.

@AndrzejKurek AndrzejKurek removed their assignment Dec 1, 2022
@AndrzejKurek
Copy link
Contributor

Please note that the original PR has been merged and this backport has not.

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 as a backport of #6203

return 1;
if( grp1->T_size != grp2->T_size )
return 1;
if( grp1->T != grp2->T )
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong: all the other embedded data is compared by value, but this field is compared by reference. However, it's evidently enough for what this function is used for, and it's the same in development, so that's ok for now. Filed for later

@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports needs-ci Needs to pass CI tests labels Dec 1, 2022
@mpg mpg removed the needs-ci Needs to pass CI tests label Dec 13, 2022
@mpg mpg merged commit 97ead79 into Mbed-TLS:mbedtls-2.28 Dec 13, 2022
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 component-crypto Crypto primitives and low-level interfaces enhancement priority-high High priority - will be reviewed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants