rumqttd: adding optional native-tls support.

[jaredwolff]

By using the use-native-tls feature, this crate can now use tokio-native-tls vs tokio-rustls. Discussed this in #257. Turns out it was trivial to add since tokio uses the same API for both packages. The only thing that seems like a bummer right now is that it requires use of a pkcs12 cert rather than 3 separate ones.

[jaredwolff]

@tekjar looks like CI needs openssl or a way to build it.

[tekjar]

Thanks @jaredwolff . I'll try to find some time. I'm a bit occupied with other work for the whole month

[jaredwolff]

@tekjar no worries. I'm kicking the tires here to make sure it's working ok.

[jaredwolff]

Looks like this is running cargo test with --all-features will bork the build. Created separate builds for (default) rustls and native-tls. Wasn't sure if there was a better way to do it.

[tekjar]

This is a very useful addition. Thanks @jaredwolff

Cargo features are only additive. In this case, I think we are trying to create mutually exclusive features. This is a problem. If this library is included in the dependency tree multiple times, using native-tls feature is going to globally enable native-tls (overwriting other dependency's wish to use rustls)

What we need is a single feature flag native-tls and the ability to dynamically choose nativetls or rustls at runtime

[jaredwolff]

(overwriting other dependency's wish to use rustls)

Is this really the case? I thought if rustls is 'disabled' in this crate, for example, you can still use it outside. I could be wrong here.

reqwest comes to mind as a pertinent example. The package gives you the choice to either use the default (native-tls) or rustls. Sean did something similar in his CI setup by building the mutually exclusive features separately.

What we need is a single feature flag native-tls and the ability to dynamically choose nativetls or rustls at runtime

It seems like a runtime option could increase the amount of unused cruft that you need to compile when building the rumqttd package. If you layer rumqttd on top of other projects it gets hefty fast. Right now one of my sub projects is 350+ dependencies. This is more of a developer quality of life thing more than anything but it does matter for folks!

[tekjar]

I'll check reqwest in depth tomorrow but reqwest features are still additive

cargo build --all-features
cargo test --all-features

builds and works fine

Is this really the case? I thought if rustls is 'disabled' in this crate, for example, you can still use it outside. I could be wrong here.

Yeah. This is a well know problem. Here is a good explanation. In short, multiple dependencies using rumqtt library and enabling different features shouldn't cause compilation errors or silently switch tls implementation underneath

[jaredwolff]

I'll check reqwest in depth tomorrow but reqwest features are still additive

Hmm ok! I'll look into more how he did it as well so I can modify the PR as necessary.

Here is a good explanation. In short, multiple dependencies using rumqtt library and enabling different features shouldn't cause compilation errors or silently switch tls implementation underneath

Agh! Ok now I get it. It doesn't necessarily matter about other deps using a rustls or native-tls it's whether or not someone is using rumqttd with both features. I guess there hypothetically be a need to use rustls and native-tls at the same time. Looking at the resulting documentation this is clicking now! Thanks @tekjar

[jaredwolff]

@tekjar updated the code so it works in all 4 cases:

• No features
• Rustls
• Native TLS
• Both enabled

I'm going to kick the tires a bit and will push any other changes I find to make this work.

[tekjar]

@jaredwolff This is nice. Do you think we'll need both the feature flags? I think it's fine to leave rustls as is and have just one feature flag to enable native-tls.

What you've done makes sense when you want disable rustls and enable native-tls. In our usecase, we usually want a rustls server on a port always but also run native-tls on a different port for microcontrollers that can't work with rustls

[jaredwolff]

Do you think we'll need both the feature flags?

In my use case it's handy. I don't use Rustls anywhere else since I use a separate ACME reverse proxy for web stuff.

[tekjar]

Cool. We'll merge as is then. This work is very useful for us as well. Thanks for your contribution

[jaredwolff]

Awesome @tekjar! I have to make some small tweaks but those will come in another PR. 😎