From f641becaf8bd0c2be45df81e367ba1fa885ab285 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sun, 24 Dec 2023 22:58:42 +0100 Subject: [PATCH 01/13] Add ability to use rustls over native-tls --- Cargo.toml | 16 ++++- src/connecting_stream.rs | 127 ++++++++++++++++++++++++++++++++++++++- src/errors/mod.rs | 11 +++- src/io/stream.rs | 8 ++- src/lib.rs | 6 +- src/types/options.rs | 54 ++++++++++++----- 6 files changed, 198 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cfac622..4155bcf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,9 @@ exclude = ["tests/*", "examples/*"] [features] default = ["tokio_io"] -tls = ["tokio-native-tls", "native-tls"] +tls = [] # meta feature for the clickhouse-rs generic TLS code +tls-native-tls = ["tokio-native-tls", "native-tls", "tls"] +tls-rustls = ["tokio-rustls", "rustls", "rustls-pemfile", "tls"] async_std = ["async-std"] tokio_io = ["tokio"] @@ -67,6 +69,18 @@ optional = true version = "^0.3" optional = true +[dependencies.rustls] +version = "0.22.1" +optional = true + +[dependencies.rustls-pemfile] +version = "2.0" +optional = true + +[dependencies.tokio-rustls] +version = "0.25.0" +optional = true + [dependencies.chrono] version = "^0.4" default-features = false diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index cab0132..78c23cb 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -11,17 +11,30 @@ use futures_util::FutureExt; #[cfg(feature = "async_std")] use async_std::net::TcpStream; -#[cfg(feature = "tls")] +#[cfg(feature = "tls-native-tls")] use native_tls::TlsConnector; #[cfg(feature = "tokio_io")] use tokio::net::TcpStream; +#[cfg(feature = "tls-rustls")] +use { + rustls::{ + client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}, + crypto::{verify_tls12_signature, verify_tls13_signature}, + pki_types::{CertificateDer, ServerName, UnixTime}, + ClientConfig, DigitallySignedStruct, Error as TlsError, RootCertStore, + }, + std::sync::Arc, + tokio_rustls::TlsConnector, +}; use pin_project::pin_project; use url::Url; use crate::{errors::ConnectionError, io::Stream as InnerStream, Options}; -#[cfg(feature = "tls")] +#[cfg(feature = "tls-native-tls")] use tokio_native_tls::TlsStream; +#[cfg(feature = "tls-rustls")] +use tokio_rustls::client::TlsStream; type Result = std::result::Result; @@ -112,6 +125,57 @@ pub(crate) struct ConnectingStream { state: State, } +#[derive(Debug)] +struct DummyTlsVerifier; + +#[cfg(feature = "tls-rustls")] +impl ServerCertVerifier for DummyTlsVerifier { + fn verify_server_cert( + &self, + _end_entity: &CertificateDer<'_>, + _intermediates: &[CertificateDer<'_>], + _server_name: &ServerName<'_>, + _ocsp_response: &[u8], + _now: UnixTime, + ) -> std::result::Result { + Ok(ServerCertVerified::assertion()) + } + + fn verify_tls12_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> std::result::Result { + verify_tls12_signature( + message, + cert, + dss, + &rustls::crypto::ring::default_provider().signature_verification_algorithms, + ) + } + + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> std::result::Result { + verify_tls13_signature( + message, + cert, + dss, + &rustls::crypto::ring::default_provider().signature_verification_algorithms, + ) + } + + fn supported_verify_schemes(&self) -> Vec { + rustls::crypto::ring::default_provider() + .signature_verification_algorithms + .supported_schemes() + } +} + impl ConnectingStream { #[allow(unused_variables)] pub(crate) fn new(addr: &Url, options: &Options) -> Self { @@ -154,7 +218,7 @@ impl ConnectingStream { } } - #[cfg(feature = "tls")] + #[cfg(feature = "tls-native-tls")] fn new_tls_connection( addr: &Url, socket: SelectOk>, @@ -185,6 +249,63 @@ impl ConnectingStream { } } } + + #[cfg(feature = "tls-rustls")] + fn new_tls_connection( + addr: &Url, + socket: SelectOk>, + options: &Options, + ) -> Self { + match addr.host_str().map(|host| host.to_owned()) { + None => Self { + state: State::tls_host_err(), + }, + Some(host) => { + let config = if options.skip_verify { + ClientConfig::builder() + .dangerous() + .with_custom_certificate_verifier(Arc::new(DummyTlsVerifier)) + .with_no_client_auth() + } else { + let mut cert_store = RootCertStore::empty(); + // TODO: add webpki_roots::TLS_SERVER_ROOTS + if let Some(certificates) = options.certificate.clone() { + for certificate in + Into::>>::into( + certificates, + ) + { + match cert_store.add(certificate) { + Ok(_) => {}, + Err(err) => { + let err = io::Error::new( + io::ErrorKind::InvalidInput, + format!("Could not load certificate: {}.", err), + ); + return Self { state: State::tcp_err(err) }; + }, + } + } + } + ClientConfig::builder() + .with_root_certificates(cert_store) + .with_no_client_auth() + }; + Self { + state: State::tls_wait(Box::pin(async move { + let (s, _) = socket.await?; + let cx = TlsConnector::from(Arc::new(config)); + let host = ServerName::try_from(host) + .map_err(|_| ConnectionError::TlsHostNotProvided)?; + Ok(cx + .connect(host, s) + .await + .map_err(|e| ConnectionError::IoError(e))?) + })), + } + } + } + } } impl Future for ConnectingStream { diff --git a/src/errors/mod.rs b/src/errors/mod.rs index 5d59fb0..0cff697 100644 --- a/src/errors/mod.rs +++ b/src/errors/mod.rs @@ -5,6 +5,11 @@ use thiserror::Error; use tokio::time::error::Elapsed; use url::ParseError; +#[cfg(feature = "tls-native-tls")] +use native_tls::Error as TlsError; +#[cfg(feature = "tls-rustls")] +use rustls::Error as TlsError; + /// Clickhouse error codes pub mod codes; @@ -57,7 +62,7 @@ pub enum ConnectionError { #[cfg(feature = "tls")] #[error("TLS connection error: `{}`", _0)] - TlsError(#[source] native_tls::Error), + TlsError(#[source] TlsError), #[error("Connection broken")] Broken, @@ -138,8 +143,8 @@ impl From for Error { } #[cfg(feature = "tls")] -impl From for ConnectionError { - fn from(error: native_tls::Error) -> Self { +impl From for ConnectionError { + fn from(error: TlsError) -> Self { ConnectionError::TlsError(error) } } diff --git a/src/io/stream.rs b/src/io/stream.rs index 14a4d45..f0bd98b 100644 --- a/src/io/stream.rs +++ b/src/io/stream.rs @@ -7,8 +7,10 @@ use std::{ #[cfg(feature = "tokio_io")] use tokio::{io::ReadBuf, net::TcpStream}; -#[cfg(feature = "tls")] +#[cfg(feature = "tls-native-tls")] use tokio_native_tls::TlsStream; +#[cfg(feature = "tls-rustls")] +use tokio_rustls::client::TlsStream; #[cfg(feature = "async_std")] use async_std::io::prelude::*; @@ -68,10 +70,12 @@ impl Stream { pub(crate) fn set_nodelay(&mut self, nodelay: bool) -> io::Result<()> { match *self { Self::Plain(ref mut stream) => stream.set_nodelay(nodelay), - #[cfg(feature = "tls")] + #[cfg(feature = "tls-native-tls")] Self::Secure(ref mut stream) => { stream.get_mut().get_mut().get_mut().set_nodelay(nodelay) } + #[cfg(feature = "tls-rustls")] + Self::Secure(ref mut stream) => stream.get_mut().0.set_nodelay(nodelay), } .map_err(|err| io::Error::new(err.kind(), format!("set_nodelay error: {err}"))) } diff --git a/src/lib.rs b/src/lib.rs index c475868..1ecd896 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,7 +66,8 @@ //! //! - `tokio_io` *(enabled by default)* — I/O based on [Tokio](https://tokio.rs/). //! - `async_std` — I/O based on [async-std](https://async.rs/) (doesn't work together with `tokio_io`). -//! - `tls` — TLS support (allowed only with `tokio_io`). +//! - `tls-native-tls` — TLS support with native-tls (allowed only with `tokio_io`). +//! - `tls-rustls` — TLS support with rustls (allowed only with `tokio_io`). //! //! ### Example //! @@ -108,6 +109,9 @@ #![recursion_limit = "1024"] +#[cfg(all(feature = "tls-native-tls", feature = "tls-rustls"))] +compile_error!("tls-native-tls and tls-rustls are mutually exclusive and cannot be enabled together"); + use std::{fmt, future::Future, time::Duration}; use futures_util::{ diff --git a/src/types/options.rs b/src/types/options.rs index 5f02fb2..26ce51a 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -8,8 +8,6 @@ use std::{ }; use crate::errors::{Error, Result, UrlError}; -#[cfg(feature = "tls")] -use native_tls; use percent_encoding::percent_decode; use url::Url; @@ -93,12 +91,11 @@ impl IntoOptions for String { } } -/// An X509 certificate. -#[cfg(feature = "tls")] +/// An X509 certificate for native-tls. +#[cfg(feature = "tls-native-tls")] #[derive(Clone)] pub struct Certificate(Arc); - -#[cfg(feature = "tls")] +#[cfg(feature = "tls-native-tls")] impl Certificate { /// Parses a DER-formatted X509 certificate. pub fn from_der(der: &[u8]) -> Result { @@ -118,6 +115,43 @@ impl Certificate { Ok(Certificate(Arc::new(inner))) } } +#[cfg(feature = "tls-native-tls")] +impl From for native_tls::Certificate { + fn from(value: Certificate) -> Self { + value.0.as_ref().clone() + } +} + +/// An X509 certificate for rustls. +#[cfg(feature = "tls-rustls")] +#[derive(Clone)] +pub struct Certificate(Arc>>); +#[cfg(feature = "tls-rustls")] +impl Certificate { + /// Parses a DER-formatted X509 certificate. + pub fn from_der(der: &[u8]) -> Result { + let der = der.to_vec(); + let inner = match rustls::pki_types::CertificateDer::try_from(der) { + Ok(certificate) => certificate, + Err(err) => return Err(Error::Other(err.to_string().into())), + }; + Ok(Certificate(Arc::new(vec![inner]))) + } + + /// Parses a PEM-formatted X509 certificate. + pub fn from_pem(der: &[u8]) -> Result { + let certs = rustls_pemfile::certs(&mut der.as_ref()) + .map(|result| result.unwrap()) + .collect(); + Ok(Certificate(Arc::new(certs))) + } +} +#[cfg(feature = "tls-rustls")] +impl From for Vec> { + fn from(value: Certificate) -> Self { + value.0.as_ref().clone() + } +} #[cfg(feature = "tls")] impl fmt::Debug for Certificate { @@ -125,7 +159,6 @@ impl fmt::Debug for Certificate { write!(f, "[Certificate]") } } - #[cfg(feature = "tls")] impl PartialEq for Certificate { fn eq(&self, _other: &Self) -> bool { @@ -133,13 +166,6 @@ impl PartialEq for Certificate { } } -#[cfg(feature = "tls")] -impl From for native_tls::Certificate { - fn from(value: Certificate) -> Self { - value.0.as_ref().clone() - } -} - #[derive(Clone, PartialEq, Debug)] pub enum SettingType { String(String), From 0fefc0d7206f6cc6919d7d5efb161568e8e55576 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 16:21:29 +0100 Subject: [PATCH 02/13] ci: use image from clickhouse/clickhouse-server (over yandex) --- .github/workflows/test.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fecdbb0..728e37a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,15 +13,12 @@ env: jobs: build: - runs-on: ubuntu-latest - services: clickhouse: - image: yandex/clickhouse-server + image: clickhouse/clickhouse-server ports: - 9000:9000 - steps: - uses: actions/checkout@v3 - name: Build From 68569c5d74584403f4d1391df70f16a186acf805 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 16:27:36 +0100 Subject: [PATCH 03/13] ci: add configuration with native-tls --- .github/workflows/test.yml | 20 ++++++++++++++++++++ extras/ci/generate_certs.sh | 7 +++++++ extras/ci/overrides.xml | 18 ++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100755 extras/ci/generate_certs.sh create mode 100644 extras/ci/overrides.xml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 728e37a..d71b2f8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,3 +25,23 @@ jobs: run: cargo build --verbose - name: Run tests run: cargo test --verbose + + build-native-tls: + runs-on: ubuntu-latest + services: + clickhouse: + image: clickhouse/clickhouse-server + env: + CH_SSL_CERTIFICATE: /etc/clickhouse-server/config.d/server.crt + CH_SSL_PRIVATE_KEY: /etc/clickhouse-server/config.d/server.key + volumes: + - ./extras/ci/generate_certs.sh:/docker-entrypoint-initdb.d/generate_certs.sh "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" + - ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml + ports: + - 9440:9440 + steps: + - uses: actions/checkout@v3 + - name: Build + run: cargo build --features tls-native-tls --verbose + - name: Run tests + run: cargo test --features tls-native-tls --verbose diff --git a/extras/ci/generate_certs.sh b/extras/ci/generate_certs.sh new file mode 100755 index 0000000..3e2caa1 --- /dev/null +++ b/extras/ci/generate_certs.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +crt=$CH_SSL_CERTIFICATE +key=$CH_SSL_PRIVATE_KEY + +openssl req -subj "/CN=localhost" -new -newkey rsa:2048 -days 365 -nodes -x509 -keyout "$key" -out "$crt" +chown clickhouse:clickhouse "$crt" "$key" diff --git a/extras/ci/overrides.xml b/extras/ci/overrides.xml new file mode 100644 index 0000000..0947420 --- /dev/null +++ b/extras/ci/overrides.xml @@ -0,0 +1,18 @@ + + + + + + none + true + true + sslv2,sslv3 + true + + + 9440 + + + 1 + + From b6109f780aef47ede6ab7eba8772c172e5a8bfb4 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 17:17:37 +0100 Subject: [PATCH 04/13] ci: start docker manually without services "services" cannot be used right now because now container depends on the repository already checkedout, while this is not true for "services". Fix this by manually starting docker as a separate step. --- .github/workflows/test.yml | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d71b2f8..9f8c754 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,19 +28,25 @@ jobs: build-native-tls: runs-on: ubuntu-latest - services: - clickhouse: - image: clickhouse/clickhouse-server - env: - CH_SSL_CERTIFICATE: /etc/clickhouse-server/config.d/server.crt - CH_SSL_PRIVATE_KEY: /etc/clickhouse-server/config.d/server.key - volumes: - - ./extras/ci/generate_certs.sh:/docker-entrypoint-initdb.d/generate_certs.sh "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" - - ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml - ports: - - 9440:9440 steps: - uses: actions/checkout@v3 + # NOTE: + # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. + # - "job.container.network" is empty, hence "host" + # - not all tests "secure" aware, so both ports are exported + # - github actions does not support YAML anchors (sigh) + - name: Run clickhouse-server + run: docker run + -v ./extras/ci/generate_certs.sh:/docker-entrypoint-initdb.d/generate_certs.sh + -v ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml + -e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt + -e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key + --network host + --rm + --detach + --publish 9440:9440 + --publish 9000:9000 + clickhouse/clickhouse-server - name: Build run: cargo build --features tls-native-tls --verbose - name: Run tests From 85fa00376a6c5f408e813d12a81973ab9a427d8b Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 17:33:36 +0100 Subject: [PATCH 05/13] ci: add rustls build --- .github/workflows/test.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9f8c754..29ce12e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -51,3 +51,29 @@ jobs: run: cargo build --features tls-native-tls --verbose - name: Run tests run: cargo test --features tls-native-tls --verbose + + build-rustls: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + # NOTE: + # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. + # - "job.container.network" is empty, hence "host" + # - not all tests "secure" aware, so both ports are exported + # - github actions does not support YAML anchors (sigh) + - name: Run clickhouse-server + run: docker run + -v ./extras/ci/generate_certs.sh:/docker-entrypoint-initdb.d/generate_certs.sh + -v ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml + -e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt + -e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key + --network host + --rm + --detach + --publish 9440:9440 + --publish 9000:9000 + clickhouse/clickhouse-server + - name: Build + run: cargo build --features tls-rustls --verbose + - name: Run tests + run: cargo test --features tls-rustls --verbose From 5eb98195ec985cab68dfff5da58f8a9e0ff22fae Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 18:02:24 +0100 Subject: [PATCH 06/13] ci: run all tests over secure connection for TLS builds --- .github/workflows/test.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 29ce12e..19afbde 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,12 +28,14 @@ jobs: build-native-tls: runs-on: ubuntu-latest + env: + # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly + DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true" steps: - uses: actions/checkout@v3 # NOTE: # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. # - "job.container.network" is empty, hence "host" - # - not all tests "secure" aware, so both ports are exported # - github actions does not support YAML anchors (sigh) - name: Run clickhouse-server run: docker run @@ -45,7 +47,6 @@ jobs: --rm --detach --publish 9440:9440 - --publish 9000:9000 clickhouse/clickhouse-server - name: Build run: cargo build --features tls-native-tls --verbose @@ -54,12 +55,14 @@ jobs: build-rustls: runs-on: ubuntu-latest + env: + # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly + DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true" steps: - uses: actions/checkout@v3 # NOTE: # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. # - "job.container.network" is empty, hence "host" - # - not all tests "secure" aware, so both ports are exported # - github actions does not support YAML anchors (sigh) - name: Run clickhouse-server run: docker run @@ -71,7 +74,6 @@ jobs: --rm --detach --publish 9440:9440 - --publish 9000:9000 clickhouse/clickhouse-server - name: Build run: cargo build --features tls-rustls --verbose From dd827bd561722a1a67e4c3f7ef4dfc616ae87aeb Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 18:08:29 +0100 Subject: [PATCH 07/13] ci: increase timeout for native-tls (sometimes 500ms is not enough) --- .github/workflows/test.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 19afbde..75edb1c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,7 +30,8 @@ jobs: runs-on: ubuntu-latest env: # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly - DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true" + # NOTE: sometimes for native-tls default connection_timeout (500ms) is not enough, interestingly that for rustls it is OK. + DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true&connection_timeout=2s" steps: - uses: actions/checkout@v3 # NOTE: From 5750ab6868829af1357c415e14fb8900114e72bc Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 18:12:22 +0100 Subject: [PATCH 08/13] Fix test_many_connection for TLS CI reports [1]: ---- pool::test::test_many_connection stdout ---- thread 'pool::test::test_many_connection' panicked at src/pool/mod.rs:380:9: assertion failed: spent < Duration::from_millis(2500) [1]: https://github.com/azat-rust/clickhouse-rs/actions/runs/7323232432/job/19945622315?pr=1 --- src/pool/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pool/mod.rs b/src/pool/mod.rs index 5d94fa0..8398214 100644 --- a/src/pool/mod.rs +++ b/src/pool/mod.rs @@ -377,6 +377,9 @@ mod test { let spent = start.elapsed(); assert!(spent >= Duration::from_millis(2000)); + #[cfg(feature = "tls")] + assert!(spent < Duration::from_millis(5000)); // slow connect + #[cfg(not(feature = "tls"))] assert!(spent < Duration::from_millis(2500)); assert_eq!(pool.info().idle_len, 6); From c9c11b7c61ebbb007adefc0fee5dd2c87254d583 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 18:15:54 +0100 Subject: [PATCH 09/13] ci: enable full stacktraces on CI (to sched some light on timeout problems) --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 75edb1c..2a1bdee 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,7 @@ on: env: CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 jobs: build: From f7ef07566789a33e5728727f4bdf20f73110d11b Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 18:22:22 +0100 Subject: [PATCH 10/13] ci: increase timeout for native-tls one more time (attempt to fix test_concurrent_queries) --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2a1bdee..89a91cd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: env: # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly # NOTE: sometimes for native-tls default connection_timeout (500ms) is not enough, interestingly that for rustls it is OK. - DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true&connection_timeout=2s" + DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true&connection_timeout=5s" steps: - uses: actions/checkout@v3 # NOTE: From 9fb0d8bd97fa62c0ca2a3f35b0336de37cdfe3e3 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 19:52:38 +0100 Subject: [PATCH 11/13] Rename tls feature into _tls --- Cargo.toml | 6 +++--- examples/simple.rs | 4 ++-- src/connecting_stream.rs | 16 ++++++++-------- src/errors/mod.rs | 4 ++-- src/io/stream.rs | 14 +++++++------- src/pool/mod.rs | 4 ++-- src/types/options.rs | 28 ++++++++++++++-------------- tests/clickhouse.rs | 4 ++-- 8 files changed, 40 insertions(+), 40 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4155bcf..92bba3a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,9 +15,9 @@ exclude = ["tests/*", "examples/*"] [features] default = ["tokio_io"] -tls = [] # meta feature for the clickhouse-rs generic TLS code -tls-native-tls = ["tokio-native-tls", "native-tls", "tls"] -tls-rustls = ["tokio-rustls", "rustls", "rustls-pemfile", "tls"] +_tls = [] # meta feature for the clickhouse-rs generic TLS code +tls-native-tls = ["tokio-native-tls", "native-tls", "_tls"] +tls-rustls = ["tokio-rustls", "rustls", "rustls-pemfile", "_tls"] async_std = ["async-std"] tokio_io = ["tokio"] diff --git a/examples/simple.rs b/examples/simple.rs index f472995..bbfa46b 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -38,7 +38,7 @@ async fn execute(database_url: String) -> Result<(), Box> { Ok(()) } -#[cfg(all(feature = "tokio_io", not(feature = "tls")))] +#[cfg(all(feature = "tokio_io", not(feature = "_tls")))] #[tokio::main] async fn main() -> Result<(), Box> { let database_url = @@ -46,7 +46,7 @@ async fn main() -> Result<(), Box> { execute(database_url).await } -#[cfg(all(feature = "tokio_io", feature = "tls"))] +#[cfg(all(feature = "tokio_io", feature = "_tls"))] #[tokio::main] async fn main() -> Result<(), Box> { let database_url = env::var("DATABASE_URL") diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index 78c23cb..3963a48 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -6,7 +6,7 @@ use std::{ }; use futures_util::future::{select_ok, BoxFuture, SelectOk, TryFutureExt}; -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] use futures_util::FutureExt; #[cfg(feature = "async_std")] @@ -46,7 +46,7 @@ enum TcpState { Fail(Option), } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] #[pin_project(project = TlsStateProj)] enum TlsState { Wait(#[pin] ConnectingFuture>), @@ -56,7 +56,7 @@ enum TlsState { #[pin_project(project = StateProj)] enum State { Tcp(#[pin] TcpState), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] Tls(#[pin] TlsState), } @@ -73,7 +73,7 @@ impl TcpState { } } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] impl TlsState { fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { match self.project() { @@ -94,7 +94,7 @@ impl State { fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { match self.project() { StateProj::Tcp(inner) => inner.poll(cx), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] StateProj::Tls(inner) => inner.poll(cx), } } @@ -104,7 +104,7 @@ impl State { State::Tcp(TcpState::Fail(Some(conn_error))) } - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] fn tls_host_err() -> Self { State::Tls(TlsState::Fail(Some(ConnectionError::TlsHostNotProvided))) } @@ -113,7 +113,7 @@ impl State { State::Tcp(TcpState::Wait(socket)) } - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] fn tls_wait(s: ConnectingFuture>) -> Self { State::Tls(TlsState::Wait(s)) } @@ -201,7 +201,7 @@ impl ConnectingStream { let socket = select_ok(streams); - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] { if options.secure { return ConnectingStream::new_tls_connection(addr, socket, options); diff --git a/src/errors/mod.rs b/src/errors/mod.rs index 0cff697..4af62b5 100644 --- a/src/errors/mod.rs +++ b/src/errors/mod.rs @@ -60,7 +60,7 @@ pub enum ConnectionError { #[error("Input/output error: `{}`", _0)] IoError(#[source] io::Error), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] #[error("TLS connection error: `{}`", _0)] TlsError(#[source] TlsError), @@ -142,7 +142,7 @@ impl From for Error { } } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] impl From for ConnectionError { fn from(error: TlsError) -> Self { ConnectionError::TlsError(error) diff --git a/src/io/stream.rs b/src/io/stream.rs index f0bd98b..0ef4ec4 100644 --- a/src/io/stream.rs +++ b/src/io/stream.rs @@ -20,13 +20,13 @@ use pin_project::pin_project; #[cfg(feature = "tokio_io")] use tokio::io::{AsyncRead, AsyncWrite}; -#[cfg(all(feature = "tls", feature = "tokio_io"))] +#[cfg(all(feature = "_tls", feature = "tokio_io"))] type SecureTcpStream = TlsStream; #[pin_project(project = StreamProj)] pub(crate) enum Stream { Plain(#[pin] TcpStream), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] Secure(#[pin] SecureTcpStream), } @@ -36,7 +36,7 @@ impl From for Stream { } } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] impl From for Stream { fn from(stream: SecureTcpStream) -> Stream { Self::Secure(stream) @@ -57,7 +57,7 @@ impl Stream { pub(crate) fn set_keepalive(&mut self, keepalive: Option) -> io::Result<()> { // match *self { // Self::Plain(ref mut stream) => stream.set_keepalive(keepalive), - // #[cfg(feature = "tls")] + // #[cfg(feature = "_tls")] // Self::Secure(ref mut stream) => stream.get_mut().set_keepalive(keepalive), // }.map_err(|err| io::Error::new(err.kind(), format!("set_keepalive error: {}", err))) if keepalive.is_some() { @@ -88,7 +88,7 @@ impl Stream { ) -> Poll> { match self.project() { StreamProj::Plain(stream) => stream.poll_read(cx, buf), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] StreamProj::Secure(stream) => stream.poll_read(cx, buf), } } @@ -103,7 +103,7 @@ impl Stream { let result = match self.project() { StreamProj::Plain(stream) => stream.poll_read(cx, &mut read_buf), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] StreamProj::Secure(stream) => stream.poll_read(cx, &mut read_buf), }; @@ -121,7 +121,7 @@ impl Stream { ) -> Poll> { match self.project() { StreamProj::Plain(stream) => stream.poll_write(cx, buf), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] StreamProj::Secure(stream) => stream.poll_write(cx, buf), } } diff --git a/src/pool/mod.rs b/src/pool/mod.rs index 8398214..a4e2f1f 100644 --- a/src/pool/mod.rs +++ b/src/pool/mod.rs @@ -377,9 +377,9 @@ mod test { let spent = start.elapsed(); assert!(spent >= Duration::from_millis(2000)); - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] assert!(spent < Duration::from_millis(5000)); // slow connect - #[cfg(not(feature = "tls"))] + #[cfg(not(feature = "_tls"))] assert!(spent < Duration::from_millis(2500)); assert_eq!(pool.info().idle_len, 6); diff --git a/src/types/options.rs b/src/types/options.rs index 26ce51a..dd3d810 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -153,13 +153,13 @@ impl From for Vec> { } } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] impl fmt::Debug for Certificate { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "[Certificate]") } } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] impl PartialEq for Certificate { fn eq(&self, _other: &Self) -> bool { true @@ -280,15 +280,15 @@ pub struct Options { pub(crate) execute_timeout: Option, /// Enable TLS encryption (defaults to `false`) - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] pub(crate) secure: bool, /// Skip certificate verification (default is `false`). - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] pub(crate) skip_verify: bool, /// An X509 certificate. - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] pub(crate) certificate: Option, /// Query settings @@ -339,11 +339,11 @@ impl Default for Options { query_timeout: Duration::from_secs(180), insert_timeout: Some(Duration::from_secs(180)), execute_timeout: Some(Duration::from_secs(180)), - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] secure: false, - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] skip_verify: false, - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] certificate: None, settings: HashMap::new(), alt_hosts: Vec::new(), @@ -481,19 +481,19 @@ impl Options { => execute_timeout: Option } - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] property! { /// Establish secure connection (default is `false`). => secure: bool } - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] property! { /// Skip certificate verification (default is `false`). => skip_verify: bool } - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] property! { /// An X509 certificate. => certificate: Option @@ -586,9 +586,9 @@ where options.execute_timeout = parse_param(key, value, parse_opt_duration)? } "compression" => options.compression = parse_param(key, value, parse_compression)?, - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] "secure" => options.secure = parse_param(key, value, bool::from_str)?, - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] "skip_verify" => options.skip_verify = parse_param(key, value, bool::from_str)?, "alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?, _ => { @@ -727,7 +727,7 @@ mod test { } #[test] - #[cfg(feature = "tls")] + #[cfg(feature = "_tls")] fn test_parse_secure_options() { let url = "tcp://username:password@host1:9001/database?ping_timeout=42ms&keepalive=99s&compression=lz4&connection_timeout=10s&secure=true&skip_verify=true"; assert_eq!( diff --git a/tests/clickhouse.rs b/tests/clickhouse.rs index e72a1ad..e276f9b 100644 --- a/tests/clickhouse.rs +++ b/tests/clickhouse.rs @@ -31,14 +31,14 @@ use std::{ use uuid::Uuid; use Tz::{Asia__Istanbul as IST, UTC}; -#[cfg(not(feature = "tls"))] +#[cfg(not(feature = "_tls"))] fn database_url() -> String { env::var("DATABASE_URL").unwrap_or_else(|_| { "tcp://localhost:9000?compression=lz4&ping_timeout=2s&retry_timeout=3s".into() }) } -#[cfg(feature = "tls")] +#[cfg(feature = "_tls")] fn database_url() -> String { env::var("DATABASE_URL").unwrap_or_else(|_| { "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&skip_verify=true".into() From 0b48f7f89851c9f526ce81144d5ce527005c30ca Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 25 Dec 2023 19:53:02 +0100 Subject: [PATCH 12/13] Add backward compatiblity for tls feature (alias for native-tls now) --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 92bba3a..fa111ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ exclude = ["tests/*", "examples/*"] [features] default = ["tokio_io"] _tls = [] # meta feature for the clickhouse-rs generic TLS code +tls = ["tls-native-tls"] # backward compatibility tls-native-tls = ["tokio-native-tls", "native-tls", "_tls"] tls-rustls = ["tokio-rustls", "rustls", "rustls-pemfile", "_tls"] async_std = ["async-std"] From 464b886a3ff5b9d7e1043fa134444e29e8da68b6 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 23 Jan 2024 08:07:43 +0100 Subject: [PATCH 13/13] Load root certificates for rustls --- Cargo.toml | 6 +++++- src/connecting_stream.rs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fa111ba..3a9c259 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ default = ["tokio_io"] _tls = [] # meta feature for the clickhouse-rs generic TLS code tls = ["tls-native-tls"] # backward compatibility tls-native-tls = ["tokio-native-tls", "native-tls", "_tls"] -tls-rustls = ["tokio-rustls", "rustls", "rustls-pemfile", "_tls"] +tls-rustls = ["tokio-rustls", "rustls", "rustls-pemfile", "webpki-roots", "_tls"] async_std = ["async-std"] tokio_io = ["tokio"] @@ -82,6 +82,10 @@ optional = true version = "0.25.0" optional = true +[dependencies.webpki-roots] +version = "*" +optional = true + [dependencies.chrono] version = "^0.4" default-features = false diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index 3963a48..05b69e5 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -268,7 +268,11 @@ impl ConnectingStream { .with_no_client_auth() } else { let mut cert_store = RootCertStore::empty(); - // TODO: add webpki_roots::TLS_SERVER_ROOTS + cert_store.extend( + webpki_roots::TLS_SERVER_ROOTS + .iter() + .cloned() + ); if let Some(certificates) = options.certificate.clone() { for certificate in Into::>>::into(