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: split algorithm definitions #20906

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

Wer-Wolf
Copy link
Contributor

Contribution description

This PR serves as the foundation for the future reorganization of the PSA headers. The final goal is to split all definitions into separate header files, so backends can for example use psa_algorithm_t without pulling in the definition for psa_mac_operation_t.

The first step for this is spliting the definition of psa_algorithm_t into a separate file. The remaining algorithm definitions are also split into separate files to reduce the size of the crypto_values.h file.

This PR also adds a pair of missing HKDF algorithm definitions and fixes an issue inside the definition of PSA_ALG_IS_WILDCARD.

Testing procedure

The changes where tested using the PSA crypto unit tests and appear to work.

Issues/PRs references

@github-actions github-actions bot added the Area: sys Area: System label Oct 11, 2024
@mguetschow mguetschow requested review from mguetschow and Einhornhool and removed request for mguetschow October 11, 2024 17:43
@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 Oct 11, 2024
@mguetschow mguetschow changed the title Split PSA algorithm definitions sys/psa_crypto: split algorithm definitions Oct 11, 2024
@riot-ci
Copy link

riot-ci commented Oct 11, 2024

Murdock results

✔️ PASSED

94f2b82 sys/psa_crypto: Fix definition of PSA_ALG_IS_WILDCARD

Success Failures Total Runtime
10212 0 10214 18m:43s

Artifacts

Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

LGTM and CI doesn't complain, but I'd like @Einhornhool to have a look at it before formally approving.

@mguetschow
Copy link
Contributor

Oh, you'd just have to rename some of your commits to stay shorter than 72 characters.

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.

Thank you, I really like this! I just have one comment below :)
It's not a MUST, though.

sys/include/psa_crypto/psa/crypto_values.h Show resolved Hide resolved
@Einhornhool
Copy link
Contributor

Oh, you'd just have to rename some of your commits to stay shorter than 72 characters.

I think you can also just squash them and have only one commit for these

@Wer-Wolf
Copy link
Contributor Author

If you are ok with one big commit, then i wont complain :)

@Wer-Wolf
Copy link
Contributor Author

Just a single question: what should happen if say mac.h need to include hashes.h? Should psa_mac depend on psa_mac?

@mguetschow
Copy link
Contributor

Just a single question: what should happen if say mac.h need to include hashes.h? Should psa_mac depend on psa_mac?

Good question, I'd say if its just about the headers, then no, and you could just include hashes.h in mac.h explicitly. But if the implementation also unconditionally needs hashes, then the module dependency makes sense.

Split definition of psa_algorithm_t into a separate file, together
with some basic algorithm macros. Also move the definitions of the
hash/mac/cipher/AEAD/etc algorithm macros into separate files as
well.

This allows PSA crypto backends to use this definitions without
pulling in all the other type definitions.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
The PSA crypto API specification 1.1.1 introduced two new algorithms
for HKDF. Add support for those.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
A definition of PSA_ALG_HASH_ANY does not exist, use
PSA_ALG_ANY_HASH instead.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
@Wer-Wolf
Copy link
Contributor Author

Are there any remaining issues?

Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks again!

@mguetschow mguetschow enabled auto-merge November 26, 2024 10:56
@mguetschow mguetschow added this pull request to the merge queue Nov 26, 2024
Merged via the queue into RIOT-OS:master with commit 80a0681 Nov 26, 2024
26 checks passed
@Wer-Wolf Wer-Wolf deleted the psa-headers branch November 26, 2024 19:02
@MrKevinWeiss MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants