AtherEnergy / rumqtt

Pure rust mqtt cilent
The Unlicense
202 stars 71 forks source link

Make TLS support optional and add Rustls as an option #59

Closed rschifflin closed 5 years ago

rschifflin commented 6 years ago

@tekjar Let me know if this PR is useful to you. This removes the hard dependency on OpenSSL, so those who aren't using TLS don't have to bring it in. It adds two choices for TLS via features: tls-openssl, which brings in openssl, and tls-rustls, which brings in Rustls.

This is a breaking change, as existing users would now have to opt in to their TLS implementation, although users who aren't using the TLS-related helper methods (.set_ca, .should_verify_ca, .set_client_cert) wouldn't break.

Another approach that would avoid limiting TLS choices to just openssl and Rustls would be to remove the deps entirely and have the end user bring in a companion crate (like adding a rumqtt-openssl or rumqtt-rustls or rumqtt-some-other-tls crate to their Cargo.toml) satisfying some trait we define for composing a NetworkStream.

Let me know what you think

tekjar commented 6 years ago

Hi @rschifflin. Thanks alot for this PR. I was planning to do this but I was busy with full async implementation and you saved the effort for me. However, it'll take me some time to verify this. I'll try to merge this next week

dbrgn commented 6 years ago

Ah, I overlooked that when opening #74. Might be related :)

tiziano88 commented 6 years ago

Any chance this may be merged at some point? cross compiling openssl for arm is a pain, and I am sure many people would like to use this library to build apps for raspberry pi. :)

tekjar commented 6 years ago

@rschifflin @tiziano88 @dbrgn Sorry for being super late. The new master has rustls by default and a [unimplemented] feature flag to use native tls.

tekjar commented 5 years ago

Thank you for taking time :). Rustls is the default starting from 0.30