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

sys/psa_crypto: correct use of (ECDSA) key_bits #20607

Merged
merged 1 commit into from May 16, 2024

Conversation

mguetschow
Copy link
Contributor

@mguetschow mguetschow commented Apr 22, 2024

Contribution description

The key_bits that are part of the psa_key_attributes_t are restricted to certain values in the PSA specification. An example is PSA_ECC_FAMILY_SECP_R1, which allows for key_bits = 256, among others.

However, in https://github.com/RIOT-OS/RIOT/blob/master/examples/psa_crypto/example_ecdsa_p256.c#L91, key_bits was set to the size of the exported key, which at least for PSA_ECC_FAMILY_SECP_R1 doesn't match the expected key_bits (as it is defined here to be 1+2*key_bits).

Fixing this revealed a unnecessary computation of the curve_bits, which actually match the key_bits according to the specification. Removed the respective macros and adapted all their usage.

Testing procedure

CI should suffice to see that compilation and the tests succeed.

Issues/PRs references

Fixes #20468.

Draft since blocked by #20545: the change in examples/psa_crypto/example_ecdsa_p256.c needs to be copied to tests/sys/psa_crypto_ecdsa*/example_ecdsa_p256.c Done ✔️

@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 22, 2024
@github-actions github-actions bot added Area: sys Area: System Area: examples Area: Example Applications labels Apr 22, 2024
@mguetschow mguetschow marked this pull request as draft April 22, 2024 10:59
@riot-ci
Copy link

riot-ci commented Apr 22, 2024

Murdock results

✔️ PASSED

15e48f0 sys/psa_crypto: correct use of (ECDSA) key_bits

Success Failures Total Runtime
10083 0 10083 13m:20s

Artifacts

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label May 14, 2024
@mguetschow mguetschow marked this pull request as ready for review May 14, 2024 15:04
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label May 15, 2024
Copy link
Contributor

@Einhornhool Einhornhool left a comment

Choose a reason for hiding this comment

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

Oh well, looks like I didn't read the specification correctly. Thank you for fixing this!

@benpicco benpicco added this pull request to the merge queue May 16, 2024
Merged via the queue into RIOT-OS:master with commit 3c4b23c May 16, 2024
28 checks passed
@mguetschow mguetschow deleted the psa-key-bits-cleanup branch May 16, 2024 11:22
@mguetschow
Copy link
Contributor Author

Thank you all! 🎉

1 similar comment
@mguetschow
Copy link
Contributor Author

Thank you all! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

examples/psa_crypto: key_bits usage doesn't match specification
4 participants