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

feat: Add rustcrypto backend #141

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

chrysn
Copy link
Collaborator

@chrysn chrysn commented Nov 17, 2023

As it says on the tin -- this adds software based crypto backends. The added implementation is no_std itself, but using it with the out-of-thin-air constructor that the edhoc_crypto crate uses requires std (because it takes a random source from there).

There is one FIXME that I think warrants wider attention: When we get public keys passed in, who is responsible for checking that they are valid points on the curve? (So far I'm unwrapping here, which is perfectly OK if we place the responsibility in a different place, eg. in the application). This demarcation of responsibilities may well be expressed in the type system -- BytesP256ElemLen could state in its documentation that instances are always valid points (but it may make sense to use distinguished types here for "contains a public key" etc). Speaking of type system: This implementation would profit a lot if our HKDF extracts and private keys were not byte arrays but opaque types defined by the implementation. PSA will sooner or later make similar demands once people want to keep keys in a secure element. At any rate, I think those are for later.

@chrysn
Copy link
Collaborator Author

chrysn commented Nov 17, 2023

As a side note, this is an implementation of the crypto traits that could easily go up to crates.io, contributing to the #98 effort in that users could then start using edhoc-rs from crates.io components alone (if they choose to go with the rustcyrpto backed).

Relatedly, I don't think that this will be the implementation to end it all -- embedded users may prefer to use something hardware accelerated, while std users may prefer the assurances that can eventually come with the hacspec ones. But it's useful on embedded as a bridgehead tool, as a reference implementation, and possibly when we come to the point of enabling varied algorithms (as all relevant ones are easily available in this style).

@malishav
Copy link
Contributor

Wow, fabulous PR! Just quickly on your point related to point validation, this is tracked in #93. I think @geonnave was looking into it, but the basic idea is to extend the crypto trait with an additional function to verify the point, and to invoke that from message processing code.

@chrysn
Copy link
Collaborator Author

chrysn commented Nov 17, 2023

Good that that's being tracked; I've changed the FIXME into a note that refers over to that other issue for justification of the possible panic.

@geonnave geonnave merged commit 55e2504 into openwsn-berkeley:main Nov 19, 2023
28 checks passed
@chrysn chrysn deleted the crypto-rust branch November 19, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants