Implement native-tls compatible APIs

[jethrogb]

To make sure it's easy for people to swap in MbedTLS where they were using some other TLS library before, we need to make sure native-tls could be implemented in terms of MbedTLS.

[jack-fortanix]

One problem will be, native-tls assumes that PKCS12 is used as the container for keys and certs. In fact it supports no other method of configuration (there is no way to say pass say a certificate chain + a PKCS8 private key). Mbedtls doesn't support PKCS12 at all.

Last year I worked on a project that used mbedtls that needed PKCS12, I hacked up something that happened to be able to generate valid PKCS12 files + parse them but it was fragile and (with mbedtls's quite primitive ASN1 library) very verbose, it was easily 1K lines of C.

If PKCS12 support proves required (as I think it would be for native-tls) it would be easier and safer to implement a PKCS12 library in Rust in a new crate, using yasna for ASN1 and mbedtls crate for the crypto ops.

[jack-fortanix]

rust-native-tls requires that the stream types implement both Sync and Send. Already (with threading support enabled), mbedtls::Context is Send, but not Sync. This appears to be correct, in that the underlying ssl_context does not take a lock. I suppose the fix is, within rust-native-tls, add a mutex which serializes access to the ssl_context and then implement Sync for that wrapper type.

This assumes it is safe to invoke a shared ssl_context from multiple threads in a serialized way. Which it should be unless they are doing something weird, like using thread IDs for something, but as with the un-movable AES context sometimes mbedtls is kind of odd. So I'll review the code to make sure there are nothing like that.

[jack-fortanix]

I'm clearly misunderstanding what rust-native-tls expects wrt Sync, rust-openssl defines the wrapper around SSL as Sync with no external lock, but OpenSSL's SSL is very much not safe to access concurrently from multiple threads.

[jack-fortanix]

Things missing that native-tls expects

• Being able to distinguish server vs client endpoints. The getters in Config are commented out with "need bitfield support". This is needed to implement tls_server_end_point which returns a hash of the server certificate. This is used for channel binding.
• Explicit shutdown API. Right now, it happens as a result of the Context being Droped.

Things that are missing but I think I can work around:

• Ability to Clone a Config and Context, because TlsConnector and TlsAcceptor are also Clone.
• No lifetimes. native-tls clearly expects reference counting, as is done eg in OpenSSL, because its TlsStream does not have a lifetime binding it to the life of the TlsConnector/TlsAcceptor. Since adding lifetimes to the public native-tls API would break downstream applications, that is presumably a no-go as far as upstream is concerned.

I think I will refactor my code so that the TlsConnector and TlsAcceptor basically do nothing and just cache the same data that was provided by the TlsConnectorBuilder/TlsAcceptorBuilder, then all state (eg our Config/Context) is created in connect and accept and owned by the TlsStream. That prevents lifetime issues and allows clone to work on the TlsConnector/TlsAcceptor.

Also an issue:

• Very likely for upstream acceptance: some way to use rust-mbedtls in a way that isn't a total cluster#### of unsafe and raw pointers. Improve Config/Context API #4

[MabezDev]

I'm currently taking another stab at this, as we want to use mbedtls for our os (espidf). With the changes made since the first attempt you folks made, its been relatively clean to implement. The final hurdle are the quite strict trait bounds on Context's accept & establish methods.

As far as I understand it, these trait bounds are required because unlike the openssl crate, Context is always Send + Sync, even if the stream (io) is not. This is due to the indirection of Box'ing the stream as dyn Any, essentially decoupling the stream from the Context.

I assume it was done this way for a reason, but I don't know enough about the internals of mbedtls to understand why, hopefully you can help me out here. Would it be possible to drop the indirection and move the stream into the Context with a generic type parameter?

[jethrogb]

@xinyufort FYI

[AdrianCX]

note: making Context generic is also covered via pr: #163

[MabezDev]

Happy new year folks! :)

Is there anything we can do to push this forward? Is @AdrianCX's PR suitable? Would it be possible to separate the generic context element of the PR into another, if other parts of the PR are blocking?

I also see that there is another request for this in #178, seems this is also needed for use with the ureq library.