-
Notifications
You must be signed in to change notification settings - Fork 25
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
TLS options #7
Comments
Hmm, TLS support is an interesting idea. I'm not sure if it should be baked in, or kept as part of the relevant crypto library. Certainly there's an argument for a generic stream, which can be implemented by a some_crate::TcpSocket, other_crate::TlsSocket and embedded_hal::Serial. |
I don't really mean TLS support baked in as such. I haven't given it much thought, but to give a bit of context it originated from a desire to have my MQTT Client only rely on traits from embedded-nal, while still being able to take options like
Possibly even CA's I think this makes sense, as these certificate pairs are very tightly coupled to the socket. I wouldn't expect embedded-nal to have any notion of how the TLS is set up, as that could be using any crypto library, offloading to some other chip (eg. through AT), or any other way. A crate implementing embedded-nal traits could then implement |
Ah, OK. That makes sense. I guess we should look at things like https://docs.rs/native-tls to see what sort of crypto APIs are common across different libraries. I imagine we probably want to take keys and certs as |
Yup, that makes sense i think! I can see if i can find time to give a proposal some time during next week.
That sounds fine by me, though we should probably consider |
If someone had read a bunch of bytes off the wire, it would be a shame to force them to verify them for UTF-8 correctness before handing them over, as any valid PEM is plain 7-bit ASCII anyway. So maybe we should take &[u8] for both PEM and DIR. Or have some enum: enum Certificate<'a> {
Pem(&'a [u8]),
Der(&'a [u8]),
} |
I think taking &[u8] makes great sense for both yes. And your enum probably makes sense as well, as i suspect a lot of tls frameworks needs to know what format the data is coming in. Also we need to remember an option to add a password for certificates. One proposal could be to simply copy the relevant parts of native-tls's api? I think its the exact same thing we want here? Basically we want the stuff provided by
They might not need to look the same (The certificate could very well be the enum described above) Should we distinguish between a |
Private Keys. Certificates are the public key half :)
They should both implement some generic Stream trait. It's a source of frustration that std::io isn't core::io, which means there is no real standard for implementing a stream on |
Ahh yeah, of course. Guess it went a bit too fast.
That's a good point! Regarding the std::io vs core::io, i guess that's something we will have to live with.. AFAIK there is no effort, nor plans about making it core::io, is there? Other than the core_io 3rd party crate |
Would it make sense to create core_io ref: https://github.com/jethrogb/rust-core_io |
I'm wary of relying on anything nightly-only, so if there is an impl for |
Now that https://github.com/drogue-iot/embedded-tls exists, I'm actually quite interested in leveraging TLS for devices running MQTT, and it seems to me like different traits are required for this. For an MQTT library taking in a @MathiasKoch I saw that there was some work on this on your the fork of After reviewing embedded-tls and the TLS handshake, I don't think any of the specifics of TLS belong in the embedded-nal. I think we just need an extension trait to An alternative approach is to leave the current traits as-is and allow crate authors to implement a full I'm not convinced which approach would be better, as having a separate trait might cause library authors stress in figuring out how to implement their driver given either a Perhaps the best approach is to instead add a new associated constant to
pub trait TcpClientStack {
const TLS_ENABLED: bool;
// ...
}
struct DriverTakingNal<T: TcpClientStack> {
stack: T,
}
impl<T: TcpClientStack> DriverTakingNal<T> {
pub fn connect(&mut self, remote: IpAddress) {
let socket = self.stack.socket().unwrap();
if T::TLS_ENABLED {
socket.connect((remote, 8883)).unwrap();
} else {
socket.connect((remote, 1883)).unwrap();
}
}
} Edit Edit: It seems to me like @MathiasKoch is pointing out that we may want to have specific TLS data associated with a socket as opposed to the entire network stack. Perhaps this could be done via a new API requirement on |
I think they absolutely need to be specific on a per-socket basis to be even remotely useful. This would not be possible if the TLS settings are not per socket, as the HTTP request would end up with a TLS handshake with an x.509 & hostname checking that would fail every single time. This is not even close to an advanced, far-fetched example, but merely a very common one from most IoT devices out there. To your question on progress: None, as we ended up running with our own extensions to solve this at work. I would still like to see a proper abstraction, and I personally think the Just my 2 cents. EDIT: |
This is a good point. I failed to realize that if you had two sockets, one that needed TLS and one that didn't use TLS, the one using TLS may try to establish the TLS handshake when no TLS server is running, causing the process to fail. This is a very valid concern. However, if you were able to differentiate between TLS-enabled and TLS-absent sockets, would two individual TLS-enabled sockets need different TLS parameters, or would it be feasible to associate all certificates/CAs with all TLS-enabled sockets? From what I understand, something like Chrome or Firefox would associate certificates/CAs with all TLS sockets and wouldn't have unique certificates on a per-connection basis. Would it be feasible for us to go this approach, i.e. have a method to open a TLS socket and one to open a normal socket, then have TLS configs associated with all TLS sockets? I ask about this approach because it seems to me like we could quite easily leverage something like |
Would it make sense to add a third trait to allow "upgrading" a TcpSocket to a TlsSocket somehow? Not sure on the format, but i guess it should add a way of setting certificates etc.
The text was updated successfully, but these errors were encountered: