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

no_std support with alloc #183

Closed
wants to merge 1 commit into from
Closed

Conversation

jmlepisto
Copy link

@jmlepisto jmlepisto commented Aug 19, 2024

Basic no_std support for environments where alloc is provided.

  • Restructured features to allow customized configurations with default-resolver with or without std
  • No functional changes, only restructured dependencies and related feature flags
  • Tested in the wild by building for a bare metal RISC-V target

Basically all crypto primitives are now individually selectable with use-xxx-yyy features. Users can build their own combinations of DH + Crypto + Hash with default-resolver to create minimal builds without the standard library for embedded targets. lib.rs includes a safeguard to make sure that all required components are provided at build time.

This changes the feature flag interface of this crate a bit, but existing features are left in place for the sake of backwards compatibility.

Providing no_std support for other resolvers beside the default one is not in the scope of this PR.

#62
#179

Added basic no_std support by switching to `core` and `alloc`
libraries wherever possible. Restructured features in Cargo.toml
to allow for customized setups with or without the standard library.
@jmlepisto
Copy link
Author

FYI @mcginty
We will be running these changes on our internal embedded products and will get good test coverage in the process.
So far everything looks good and there are no issues whatsoever on any of the tested platforms.

@jmlepisto
Copy link
Author

Ping @mcginty, any thoughts or comments on this? Would love to get these changes upstreamed eventually :)

@mcginty
Copy link
Owner

mcginty commented Feb 18, 2025

@jmlepisto I'm so sorry for dropping the ball for this review, it looks great though and if you're still willing to see it through I'm excited to get no_std support merged.

Copy link
Owner

@mcginty mcginty 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 great. Because of my own failure to review this in a timely fashion, I'm going to go ahead and manually merge this and resolve the conflicts, as well as layer on any extra changes needed from other feature additions.

It unfortunately will get marked as "Closed" instead of merged because GitHub doesn't offer a way to manually mark PRs as merged.

Thank you so much!

Comment on lines +62 to +76
// Make sure the user is running a supported configuration.
#[cfg(feature = "default-resolver")]
#[cfg(any(
not(any(feature = "use-curve25519-dalek")),
not(any(
feature = "use-aes-gcm",
feature = "use-chacha20poly1305",
feature = "use-xchacha20poly1305"
)),
not(any(feature = "use-sha2", feature = "use-blake2"))
))]
compile_error!(
"Valid selection of crypto primitived must be enabled when using feature 'default-resolver'.
Enable at least one DH feature, one Cipher feature and one Hash feature. Check README.md for details."
);
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, I like this solution.

@mcginty mcginty closed this in ad3dd46 Feb 18, 2025
@jmlepisto
Copy link
Author

Hi there! What a nice thing to see that you decided to include this 🎉 Don't worry about me having to wait a while. We all got our own lives outside of these projects.

PS. Your work on snow inspired me to take a crack at Noise protocol myself. Clatter is a statically allocated implementation with post-quantum support even for the most rudimentary embedded devices.

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.

2 participants