Add native tls WSS support.

[xiaocq2001]

Type of change

Resolves WSS part for #866.

New feature (non-breaking change which adds functionality)

• Added native_tls websocket support

Checklist:

• Formatted with cargo fmt
• Make an entry to CHANGELOG.md if it's relevant to the users of the library. If it's not relevant mention why.

[coveralls]

Pull Request Test Coverage Report for Build 9563575705

Details

• 6 of 7 (85.71%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 36.142%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
rumqttc/src/v5/eventloop.rs	0	1	0.0%

Totals
Change from base Build 9561486268:	0.02%
Covered Lines:	5975
Relevant Lines:	16532

---

💛 - Coveralls

[swanandx]

there is PR #742 which tackles same problem. Last time when I tried we faced some issues ( mentioned in discussion there ).

here as this PR says "use-native-tls" overwrites "use-rustls", I think it would be better to not overwrite and let users choose what they want to use through TlsConfig, wdyt?

[xiaocq2001]

there is PR #742 which tackles same problem. Last time when I tried we faced some issues ( mentioned in discussion there ).

here as this PR says "use-native-tls" overwrites "use-rustls", I think it would be better to not overwrite and let users choose what they want to use through TlsConfig, wdyt?

Do you mean through runtime check instead of through cfg like following:

            let connector = match &tls_config {
                #[cfg(feature = "use-native-tls")]
                TlsConfiguration::SimpleNative { .. } | TlsConfiguration::Native | TlsConfiguration::NativeConnector(_) => {
                    tls::native_tls_connector(&tls_config).await?
                }
                #[cfg(feature = "use-rustls")]
                TlsConfiguration::Simple { .. } | TlsConfiguration::Rustls(_) => {
                    tls::rustls_connector(&tls_config).await?
                }
                #[allow(unreachable_patterns)]
                _ => unreachable!("This cannot be called for other TLS backends than native-tls and rustls"),
            };

Seems native_tls_connector and rustls_connector can be merged together:

#[cfg(feature = "use-rustls")]
type ResultConnector = Result<RustlsConnector, Error>;
#[cfg(feature = "use-native-tls")]
type ResultConnector = Result<NativeTlsConnector, Error>;
#[cfg(any(feature = "use-rustls", feature = "use-native-tls"))]
pub async fn tls_connector(tls_config: &TlsConfiguration) -> ResultConnector {
    match tls_config {
        #[cfg(feature = "use-rustls")]
        TlsConfiguration::Simple { ca, alpn, client_auth } => {
            // Add ca to root store if the connection is TLS
            let mut root_cert_store = RootCertStore::empty();
            let certs = rustls_pemfile::certs(&mut BufReader::new(Cursor::new(ca)))
                .collect::<Result<Vec<_>, _>>()?;

            root_cert_store.add_parsable_certificates(certs);

            if root_cert_store.is_empty() {
                return Err(Error::NoValidCertInChain);
            }

            let config = ClientConfig::builder().with_root_certificates(root_cert_store);

            // Add der encoded client cert and key
            let mut config = if let Some(client) = client_auth.as_ref() {
                let certs =
                    rustls_pemfile::certs(&mut BufReader::new(Cursor::new(client.0.clone())))
                        .collect::<Result<Vec<_>, _>>()?;
                if certs.is_empty() {
                    return Err(Error::NoValidClientCertInChain);
                }

                // Create buffer for key file
                let mut key_buffer = BufReader::new(Cursor::new(client.1.clone()));

                // Read PEM items until we find a valid key.
                let key = loop {
                    let item = rustls_pemfile::read_one(&mut key_buffer)?;
                    match item {
                        Some(Item::Sec1Key(key)) => {
                            break key.into();
                        }
                        Some(Item::Pkcs1Key(key)) => {
                            break key.into();
                        }
                        Some(Item::Pkcs8Key(key)) => {
                            break key.into();
                        }
                        None => return Err(Error::NoValidKeyInChain),
                        _ => {}
                    }
                };

                config.with_client_auth_cert(certs, key)?
            } else {
                config.with_no_client_auth()
            };

            // Set ALPN
            if let Some(alpn) = alpn.as_ref() {
                config.alpn_protocols.extend_from_slice(alpn);
            }

            Ok(RustlsConnector::from(Arc::new(config)))
        }
        #[cfg(feature = "use-rustls")]
        TlsConfiguration::Rustls(tls_client_config) => Ok(RustlsConnector::from(tls_client_config.clone())),
        #[cfg(feature = "use-native-tls")]
        TlsConfiguration::SimpleNative { ca, client_auth } => {
            let cert = native_tls::Certificate::from_pem(ca)?;

            let mut connector_builder = native_tls::TlsConnector::builder();
            connector_builder.add_root_certificate(cert);

            if let Some((der, password)) = client_auth {
                let identity = Identity::from_pkcs12(der, password)?;
                connector_builder.identity(identity);
            }

            Ok(connector_builder.build()?.into())
        }
        #[cfg(feature = "use-native-tls")]
        TlsConfiguration::Native => Ok(native_tls::TlsConnector::new()?.into()),
        #[cfg(feature = "use-native-tls")]
        TlsConfiguration::NativeConnector(connector) => Ok(connector.to_owned().into()),
        #[allow(unreachable_patterns)]
        _ => unreachable!("This cannot be called for other TLS backends than Rustls/Native TLS"),
    }
}

[xiaocq2001]

Actually it seems if in toml the use-rustls is in default section, there is no way to remove it, even I use --no-default-features.

cargo run --example wss --no-default-features --features="websocket"
    Finished dev [unoptimized] target(s) in 0.16s
     Running `target/debug/examples/wss`
use-native-tls: false
use-rustls    : true

cargo run --example wss --no-default-features --features="websocket use-native-tls"
   Compiling rumqttc v0.24.0 (/home/cxiao/workspace/rumqtt/rumqttc)
    Finished dev [unoptimized] target(s) in 3.46s
     Running `target/debug/examples/wss`
use-native-tls: true
use-rustls    : true

That's why I try to configure use-native-tls overwrites use-rustls.

[xiaocq2001]

I tried to enable just based on feature, the following test fails:

cargo clippy --manifest-path rumqttc/Cargo.toml --all-features`

Is there a way to keep these two features mutually exclusive?

Any way, I keep the implementation of cfg handling (disable rustls code when native-tls enabled) but changed comment a bit, please check.

[coveralls]

Pull Request Test Coverage Report for Build 9578244851

Details

• 7 of 9 (77.78%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 36.142%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
rumqttc/src/v5/eventloop.rs	0	2	0.0%

Totals
Change from base Build 9561486268:	0.02%
Covered Lines:	5975
Relevant Lines:	16532

---

💛 - Coveralls

[swanandx]

Is there a way to keep these two features mutually exclusive?

we can. but it's better to make them additive. i.e. being able to turn on both without any issues.

[xiaocq2001]

Is there a way to keep these two features mutually exclusive?

we can. but it's better to make them additive. i.e. being able to turn on both without any issues.

Currently we are using async-tungstenite, where different TLS features are mutually exclusive (async-tungstenite/src/tokio.rs#L14C1-L27C9). Where rustls only works when native-tls is not enabled. There is also discuss opened sdroege/async-tungstenite#78. We need to align with such settings, before we can find a better way for additive implement.

IMHO using both TLS implement at the same time is not necessary and could lead to conflicts and unexpected behavior, as they are intended to be alternative options to each other.

[xiaocq2001]

Any comment?

IMHO we can leave additive option as an improvement (maybe based on sdroege/async-tungstenite#78 (comment)) but focus on supporting Websocket with native-tls support here.
At least rustls and native-tls works and all tests are good now.

[coveralls]

Pull Request Test Coverage Report for Build 9657739921

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 7 of 9 (77.78%) changed or added relevant lines in 2 files are covered.
• 203 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.03%) to 36.149%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
rumqttc/src/v5/eventloop.rs	0	2	0.0%

Files with Coverage Reduction	New Missed Lines	%
rumqttc/src/client.rs	43	27.57%
rumqttc/src/v5/client.rs	63	13.63%
rumqttc/src/v5/mod.rs	97	44.78%

Totals
Change from base Build 9561486268:	0.03%
Covered Lines:	5975
Relevant Lines:	16529

---

💛 - Coveralls

[xiaocq2001]

Any comments?

[xiaocq2001]

@swanandx , can we offer the functionality first align with async-tungstenite, and update later when it's updated or there is better lib with additive feature configuration?

[xiaocq2001]

Hi, is there any chance to have the feature and leave improvements for additive configuration support in the future?

[xiaocq2001]

Any comments?