-
Notifications
You must be signed in to change notification settings - Fork 247
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
Add Rustls provider including examples #1899
base: main
Are you sure you want to change the base?
Conversation
Fixes #1836 While Rustls will also work on H2 no examples are added for H2 |
TLS is the next major part after getting the HAL and drivers running. Many protocols and systems use/require it, especially in IoT development. Now the question lies down to; What is the roadmap? Should All three of the above mentionned TLS suites support a different set of features, and compatibility. I believe both Rustls and embedded-tls to be the future of TLS on bare-metal, since they are more lightweight and pure Rust implementation. This should be documented for new users who come across needing to use TLS on bare-metal. Rustls
embedded-tls
esp-mbedtls
|
I can build both examples for |
I totally agree that TLS is an important topic We don't have an official TLS roadmap ,yet - but I'd say both, Rustls and embedded-tls (both HW accelerated) is something I personally want. After my vacation I'll look into lifting the atomics requirement for Rustls so it will work on all our targets |
I've converted to draft until we can support all chips, hopefully it won't take too long to get upstream to allow using portable-atomic 🤞 |
Just taking another look at this, maybe we should merge this in it's current state? We know the upstream issue needs to be resolved for the non-atomic targets, but that doesn't mean we can't land support for the atomic's target now, I think? |
Did you mean to close this, or are you planning on opening a new one? |
oh - that was a mistake |
74a327f
to
60c6e15
Compare
I remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this again!
|
||
This means that ESP32-S2, ESP32-C2 and ESP32-C3 are NOT supported. | ||
|
||
## Status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we remove this and break it out into issues instead? I think we'll need a new label for the esp-rustls-provider
package.
My fear is that we'll never check this again :D, if we have it in separate issues we can at least be aware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can create the issues after the PR is merged of course, there is no rush to do it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the TODOs here - we shouldn't forget about creating separate issues after the merge (since creating them before is kind of weird?)
|
||
/// Assume the RNG is actually producing true random numbers - which is the case | ||
/// when radio peripherals are enabled | ||
struct ProbablyTrng; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too 😄
I only have micronits to pick right now - like the fact that this PR isn't about the examples I think, but the provider crate 🤔 What are the long term plans, are we to provide hardware acceleration through the provider crate, where we can? |
Good point about the PR title 👍 Yes - we definitely want to support HW-crypto long-term (it's hidden in a vague sentence in the README - should have written it in the PR description) |
60c6e15
to
6c724a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments, LGTM.
Please don't forget to file the relevant issues once this is merged 😃
Submission Checklist 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Description
This adds two examples using Rustls (a client and a server) for ESP32, ESP32-S3 and ESP32-C6.
It is impossible to compile Rustls for targets w/o atomics - there is an issue opened and there were several attempts to tackle it. Currently it looks like they might accept a PR to make forks replacing
Arc
easy and maintainable (i.e. we will need a fork for now) - they are considering to make the coreArc
free and we could build upon that (but that won't happen too soon for sure)Please note: While this seems to work fine it's just the beginning. But we need to start somewhere.
This needs to support
async
and long-term we want this to support HW-accelerationTesting
Running the examples on supported targets