From dceebe1e80d623b19d334c90f3500acdf8aa567b Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 15 Apr 2025 22:53:46 +0200 Subject: [PATCH 01/18] Ensure that Hello packet correctly received --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 1ecd896..385fea5 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -328,7 +328,7 @@ impl ClientHandle { } self.inner = h; - self.context.server_info = info.unwrap(); + self.context.server_info = info.ok_or(Error::Other("Missing Hello/Exception packet".into()))?; Ok(()) } From 0e67ce274d33a9b1dc48327dced411081ac41ec1 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 15 Apr 2025 22:53:46 +0200 Subject: [PATCH 02/18] Properly handle terminated connection by the server Previously ClickhouseTransport::poll_next did not tries to process packets if the server closed the connection, however there can be some packet (likely Exception), that we need to pass to user. Also this will avoid panic in case of missing Hello packet, i.e. in case of credentials mismatch. --- src/io/transport.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/io/transport.rs b/src/io/transport.rs index 7ecfdfd..e4eff04 100644 --- a/src/io/transport.rs +++ b/src/io/transport.rs @@ -287,6 +287,15 @@ impl Stream for ClickhouseTransport { } if *this.done { + // We still have may have something in buffer, since the client may read something + // first, and only after the server will close the connection, likely in case of + // exception, let's try to parse it here + if !this.rd.is_empty() { + if let Poll::Ready(ret) = this.try_parse_msg()? { + return Poll::Ready(ret.map(Ok)); + } + } + return Poll::Ready(None); } From 6d42f4105fbd809d9d1981bf35d8068b57e8ac29 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 15 Apr 2025 23:06:21 +0200 Subject: [PATCH 03/18] Fix test_size_of expectation (the size should be 64 with alignment) --- src/types/value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/value.rs b/src/types/value.rs index 9b38b41..e86ef9c 100644 --- a/src/types/value.rs +++ b/src/types/value.rs @@ -806,7 +806,7 @@ mod test { #[test] fn test_size_of() { use std::mem; - assert_eq!(56, mem::size_of::<[Value; 1]>()); + assert_eq!(64, mem::size_of::<[Value; 1]>()); } #[test] From af660a84c7fbb1ea0498bd564c7a385281083920 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 15 Apr 2025 23:12:37 +0200 Subject: [PATCH 04/18] ci: set CLICKHOUSE_SKIP_USER_SETUP to avoid restricting default user --- .github/workflows/test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 89a91cd..1ccde1d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,6 +20,8 @@ jobs: image: clickhouse/clickhouse-server ports: - 9000:9000 + env: + CLICKHOUSE_SKIP_USER_SETUP: 1 steps: - uses: actions/checkout@v3 - name: Build @@ -45,6 +47,7 @@ jobs: -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 + -e CLICKHOUSE_SKIP_USER_SETUP=1 --network host --rm --detach @@ -72,6 +75,7 @@ jobs: -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 + -e CLICKHOUSE_SKIP_USER_SETUP=1 --network host --rm --detach From 829f595458985675d43bda1eaecc94826d96802f Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 05/18] ci: simplify TLS jobs --- .github/workflows/test.yml | 39 ++++++++------------------------------ 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1ccde1d..cc5864b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,7 +29,12 @@ jobs: - name: Run tests run: cargo test --verbose - build-native-tls: + build-tls: + strategy: + matrix: + feature: + - tls-native-tls + - tls-rustls runs-on: ubuntu-latest env: # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly @@ -54,34 +59,6 @@ jobs: --publish 9440:9440 clickhouse/clickhouse-server - name: Build - run: cargo build --features tls-native-tls --verbose + run: cargo build --features ${{ matrix.feature }} --verbose - name: Run tests - run: cargo test --features tls-native-tls --verbose - - 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" - # - 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 - -e CLICKHOUSE_SKIP_USER_SETUP=1 - --network host - --rm - --detach - --publish 9440:9440 - clickhouse/clickhouse-server - - name: Build - run: cargo build --features tls-rustls --verbose - - name: Run tests - run: cargo test --features tls-rustls --verbose + run: cargo test --features ${{ matrix.feature }} --verbose From f27a36de41647c8d1344d1db8c1bae298e387580 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 01:01:48 +0200 Subject: [PATCH 06/18] ci: disable fail-fast for matrix (plus it is how it works before) --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cc5864b..641ddf1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,6 +31,7 @@ jobs: build-tls: strategy: + fail-fast: false matrix: feature: - tls-native-tls From f7395a3486d50a4c6e2294480d12eeaf304bf956 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 07/18] ci: temporary enable CI for next branch --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 641ddf1..fbe64a7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,6 +7,7 @@ on: pull_request: branches: - async-await + - next env: CARGO_TERM_COLOR: always From 525644dbb431fbffbeb94fd45b6996e97de38453 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 08/18] Fix typo (s/DER/PEM/) --- src/types/options.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/types/options.rs b/src/types/options.rs index dd3d810..2a39026 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -107,8 +107,8 @@ impl Certificate { } /// Parses a PEM-formatted X509 certificate. - pub fn from_pem(der: &[u8]) -> Result { - let inner = match native_tls::Certificate::from_pem(der) { + pub fn from_pem(pem: &[u8]) -> Result { + let inner = match native_tls::Certificate::from_pem(pem) { Ok(certificate) => certificate, Err(err) => return Err(Error::Other(err.to_string().into())), }; @@ -139,8 +139,8 @@ impl Certificate { } /// Parses a PEM-formatted X509 certificate. - pub fn from_pem(der: &[u8]) -> Result { - let certs = rustls_pemfile::certs(&mut der.as_ref()) + pub fn from_pem(pem: &[u8]) -> Result { + let certs = rustls_pemfile::certs(&mut pem.as_ref()) .map(|result| result.unwrap()) .collect(); Ok(Certificate(Arc::new(certs))) From 4a19933d4dbed9987af1ddd9b7b989b7cfbc0090 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 09/18] Add ability to specify certificate --- .github/workflows/test.yml | 5 ++++- src/types/options.rs | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fbe64a7..ff5ed89 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,11 +37,14 @@ jobs: feature: - tls-native-tls - tls-rustls + options: + - skip_verify=true + - certificate=$CH_SSL_CERTIFICATE runs-on: ubuntu-latest 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=5s" + DATABASE_URL: "tcp://localhost:9440?compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&${{ matrix.options }}&connection_timeout=5s" steps: - uses: actions/checkout@v3 # NOTE: diff --git a/src/types/options.rs b/src/types/options.rs index 2a39026..f61e947 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -2,6 +2,7 @@ use std::{ borrow::Cow, collections::HashMap, fmt, + fs, str::FromStr, sync::{Arc, Mutex}, time::Duration, @@ -166,6 +167,16 @@ impl PartialEq for Certificate { } } +#[cfg(feature = "_tls")] +pub fn load_certificate(file: &str) -> Result { + let data = fs::read(file)?; + if file.ends_with(".der") || file.ends_with(".cer") { + Certificate::from_der(&data) + } else { + Certificate::from_pem(&data) + } +} + #[derive(Clone, PartialEq, Debug)] pub enum SettingType { String(String), @@ -315,6 +326,7 @@ impl fmt::Debug for Options { .field("connection_timeout", &self.connection_timeout) .field("settings", &self.settings) .field("alt_hosts", &self.alt_hosts) + .field("certificate", &self.certificate) .finish() } } @@ -590,6 +602,8 @@ where "secure" => options.secure = parse_param(key, value, bool::from_str)?, #[cfg(feature = "_tls")] "skip_verify" => options.skip_verify = parse_param(key, value, bool::from_str)?, + #[cfg(feature = "_tls")] + "certificate" => options.certificate = Some(parse_param(key, value, load_certificate)?), "alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?, _ => { let value = SettingType::String(value.to_string()); From 0f913fbc2510d4a0840c81c5c4fad70b521f5bd2 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 10/18] Fix CaUsedAsEndEntity error --- .github/workflows/test.yml | 3 ++- extras/ci/generate_certs.sh | 28 ++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ff5ed89..b953d03 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,7 +39,7 @@ jobs: - tls-rustls options: - skip_verify=true - - certificate=$CH_SSL_CERTIFICATE + - certificate=/etc/clickhouse-server/config.d/ca.pem runs-on: ubuntu-latest env: # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly @@ -55,6 +55,7 @@ jobs: 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_CA_CERTIFICATE=/etc/clickhouse-server/config.d/ca.pem -e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt -e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key -e CLICKHOUSE_SKIP_USER_SETUP=1 diff --git a/extras/ci/generate_certs.sh b/extras/ci/generate_certs.sh index 3e2caa1..dd21179 100755 --- a/extras/ci/generate_certs.sh +++ b/extras/ci/generate_certs.sh @@ -2,6 +2,30 @@ crt=$CH_SSL_CERTIFICATE key=$CH_SSL_PRIVATE_KEY +ca_crt=$CH_SSL_CA_CERTIFICATE -openssl req -subj "/CN=localhost" -new -newkey rsa:2048 -days 365 -nodes -x509 -keyout "$key" -out "$crt" -chown clickhouse:clickhouse "$crt" "$key" +ca_key=${CH_SSL_CA_CERTIFICATE/.pem/.key} +csr=${key/.key/.csr} +ext=${key/.key/.ext} + +openssl genrsa -out "$ca_key" 4096 +openssl req -x509 -new -nodes -key "$ca_key" -sha256 -days 3650 -out "$ca_crt" -subj "/C=US/ST=DevState/O=DevOrg/CN=MyDevCA" + +openssl genrsa -out "$key" 2048 +openssl req -new -key "$key" -out "$csr" -subj "/C=US/ST=DevState/O=DevOrg/CN=localhost" + +cat > "$ext" < Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 11/18] ci: generate TLS certificates on the host for client --- .github/workflows/test.yml | 14 +++++++++++--- extras/ci/generate_certs.sh | 10 ++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b953d03..12b07fa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,27 +39,35 @@ jobs: - tls-rustls options: - skip_verify=true - - certificate=/etc/clickhouse-server/config.d/ca.pem + - certificate=/tmp/ca.pem runs-on: ubuntu-latest 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&${{ matrix.options }}&connection_timeout=5s" + CH_SSL_CA_CERTIFICATE: /tmp/ca.pem + CH_SSL_CERTIFICATE: /tmp/server.crt + CH_SSL_PRIVATE_KEY: /tmp/server.key steps: - uses: actions/checkout@v3 + - name: Generate TLS certificates + run: | + extras/ci/generate_certs.sh "$CH_SSL_CA_CERTIFICATE" "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" + chmod 644 "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" # NOTE: # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. # - "job.container.network" is empty, hence "host" # - 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_CA_CERTIFICATE=/etc/clickhouse-server/config.d/ca.pem + -v $CH_SSL_CERTIFICATE:/etc/clickhouse-server/config.d/server.crt + -v $CH_SSL_PRIVATE_KEY:/etc/clickhouse-server/config.d/server.key -e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt -e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key -e CLICKHOUSE_SKIP_USER_SETUP=1 --network host + --name clickhouse --rm --detach --publish 9440:9440 diff --git a/extras/ci/generate_certs.sh b/extras/ci/generate_certs.sh index dd21179..94abb4e 100755 --- a/extras/ci/generate_certs.sh +++ b/extras/ci/generate_certs.sh @@ -1,10 +1,10 @@ #!/usr/bin/env bash -crt=$CH_SSL_CERTIFICATE -key=$CH_SSL_PRIVATE_KEY -ca_crt=$CH_SSL_CA_CERTIFICATE +ca_crt=${1:-"$CH_SSL_CA_CERTIFICATE"} && shift +crt=${1-:"$CH_SSL_CERTIFICATE"} && shift +key=${1-:"$CH_SSL_PRIVATE_KEY"} && shift -ca_key=${CH_SSL_CA_CERTIFICATE/.pem/.key} +ca_key=${ca_crt/.pem/.key} csr=${key/.key/.csr} ext=${key/.key/.ext} @@ -27,5 +27,3 @@ EOL openssl x509 -req -in "$csr" -CA "$ca_crt" -CAkey "$ca_key" -CAcreateserial -out "$crt" -days 825 -sha256 -extfile "$ext" openssl verify -CAfile "$ca_crt" "$crt" - -chown clickhouse:clickhouse "$crt" "$key" "$ca_crt" "$ca_key" "$csr" "$ext" From 32051ad3a180061b07eb2499f59814d57c0e763f Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 12/18] Rename certificate to ca_certificate --- .github/workflows/test.yml | 2 +- src/connecting_stream.rs | 4 ++-- src/types/options.rs | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 12b07fa..a36451d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,7 +39,7 @@ jobs: - tls-rustls options: - skip_verify=true - - certificate=/tmp/ca.pem + - ca_certificate=/tmp/ca.pem runs-on: ubuntu-latest env: # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index 05b69e5..e537b2e 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -231,7 +231,7 @@ impl ConnectingStream { Some(host) => { let mut builder = TlsConnector::builder(); builder.danger_accept_invalid_certs(options.skip_verify); - if let Some(certificate) = options.certificate.clone() { + if let Some(certificate) = options.ca_certificate.clone() { let native_cert = native_tls::Certificate::from(certificate); builder.add_root_certificate(native_cert); } @@ -273,7 +273,7 @@ impl ConnectingStream { .iter() .cloned() ); - if let Some(certificates) = options.certificate.clone() { + if let Some(certificates) = options.ca_certificate.clone() { for certificate in Into::>>::into( certificates, diff --git a/src/types/options.rs b/src/types/options.rs index f61e947..d7c17c6 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -298,9 +298,9 @@ pub struct Options { #[cfg(feature = "_tls")] pub(crate) skip_verify: bool, - /// An X509 certificate. + /// CA certificate. #[cfg(feature = "_tls")] - pub(crate) certificate: Option, + pub(crate) ca_certificate: Option, /// Query settings pub(crate) settings: HashMap, @@ -326,7 +326,7 @@ impl fmt::Debug for Options { .field("connection_timeout", &self.connection_timeout) .field("settings", &self.settings) .field("alt_hosts", &self.alt_hosts) - .field("certificate", &self.certificate) + .field("ca_certificate", &self.ca_certificate) .finish() } } @@ -356,7 +356,7 @@ impl Default for Options { #[cfg(feature = "_tls")] skip_verify: false, #[cfg(feature = "_tls")] - certificate: None, + ca_certificate: None, settings: HashMap::new(), alt_hosts: Vec::new(), } @@ -507,8 +507,8 @@ impl Options { #[cfg(feature = "_tls")] property! { - /// An X509 certificate. - => certificate: Option + /// CA certificate. + => ca_certificate: Option } property! { @@ -603,7 +603,7 @@ where #[cfg(feature = "_tls")] "skip_verify" => options.skip_verify = parse_param(key, value, bool::from_str)?, #[cfg(feature = "_tls")] - "certificate" => options.certificate = Some(parse_param(key, value, load_certificate)?), + "ca_certificate" => options.ca_certificate = Some(parse_param(key, value, load_certificate)?), "alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?, _ => { let value = SettingType::String(value.to_string()); From 0ed5046cf4238e78f6874cbd529e45621ca09bc2 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 00:57:16 +0200 Subject: [PATCH 13/18] Implement mTLS for rustls --- src/connecting_stream.rs | 24 ++++++++++++-- src/types/options.rs | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index e537b2e..35e158b 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -104,6 +104,11 @@ impl State { State::Tcp(TcpState::Fail(Some(conn_error))) } + #[cfg(feature = "_tls")] + fn tls_err(e: TlsError) -> Self { + State::Tls(TlsState::Fail(Some(ConnectionError::TlsError(e)))) + } + #[cfg(feature = "_tls")] fn tls_host_err() -> Self { State::Tls(TlsState::Fail(Some(ConnectionError::TlsHostNotProvided))) @@ -261,11 +266,10 @@ impl ConnectingStream { state: State::tls_host_err(), }, Some(host) => { - let config = if options.skip_verify { + let builder = 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(); cert_store.extend( @@ -293,7 +297,21 @@ impl ConnectingStream { } ClientConfig::builder() .with_root_certificates(cert_store) - .with_no_client_auth() + }; + let config = if let Some(certificate_file) = options.certificate_file.clone() { + if let Some(private_key_file) = options.private_key_file.clone() { + builder.with_client_auth_cert(certificate_file.into(), private_key_file.into()) + } else { + Ok(builder.with_no_client_auth()) + } + } else { + Ok(builder.with_no_client_auth()) + }; + let config = match config { + Ok(config) => config, + Err(err) => { + return Self { state: State::tls_err(err) }; + }, }; Self { state: State::tls_wait(Box::pin(async move { diff --git a/src/types/options.rs b/src/types/options.rs index d7c17c6..4287636 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -12,6 +12,9 @@ use crate::errors::{Error, Result, UrlError}; use percent_encoding::percent_decode; use url::Url; +#[cfg(feature = "tls-rustls")] +use rustls::pki_types::pem::PemObject; + const DEFAULT_MIN_CONNS: usize = 10; const DEFAULT_MAX_CONNS: usize = 20; @@ -177,6 +180,44 @@ pub fn load_certificate(file: &str) -> Result { } } +/// Private key for rustls. +#[cfg(feature = "tls-rustls")] +#[derive(Clone)] +pub struct PrivateKey(Arc>); +#[cfg(feature = "tls-rustls")] +impl PrivateKey { + pub fn from_pem(pem: &[u8]) -> Result { + let key = rustls::pki_types::PrivateKeyDer::from_pem_slice(&mut pem.as_ref()).map_err(|_| UrlError::Invalid)?; + Ok(Self(Arc::new(key))) + } +} +#[cfg(feature = "tls-rustls")] +impl From for rustls::pki_types::PrivateKeyDer<'static> { + fn from(value: PrivateKey) -> Self { + rustls::pki_types::PrivateKeyDer::clone_key(value.0.as_ref()) + } +} + +#[cfg(feature = "_tls")] +impl fmt::Debug for PrivateKey { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "[Private Key]") + } +} + +#[cfg(feature = "_tls")] +impl PartialEq for PrivateKey { + fn eq(&self, _other: &Self) -> bool { + true + } +} + +#[cfg(feature = "_tls")] +pub fn load_private_key(file: &str) -> Result { + let data = fs::read(file)?; + PrivateKey::from_pem(&data) +} + #[derive(Clone, PartialEq, Debug)] pub enum SettingType { String(String), @@ -302,6 +343,14 @@ pub struct Options { #[cfg(feature = "_tls")] pub(crate) ca_certificate: Option, + /// Certificate for authorization. + #[cfg(feature = "_tls")] + pub(crate) certificate_file: Option, + + /// Private key for certificate. + #[cfg(feature = "_tls")] + pub(crate) private_key_file: Option, + /// Query settings pub(crate) settings: HashMap, @@ -357,6 +406,10 @@ impl Default for Options { skip_verify: false, #[cfg(feature = "_tls")] ca_certificate: None, + #[cfg(feature = "_tls")] + certificate_file: None, + #[cfg(feature = "_tls")] + private_key_file: None, settings: HashMap::new(), alt_hosts: Vec::new(), } @@ -511,6 +564,18 @@ impl Options { => ca_certificate: Option } + #[cfg(feature = "_tls")] + property! { + /// Certificate for authorization. + => certificate_file: Option + } + + #[cfg(feature = "_tls")] + property! { + /// Private key for certificate. + => private_key_file: Option + } + property! { /// Query settings => settings: HashMap @@ -604,6 +669,10 @@ where "skip_verify" => options.skip_verify = parse_param(key, value, bool::from_str)?, #[cfg(feature = "_tls")] "ca_certificate" => options.ca_certificate = Some(parse_param(key, value, load_certificate)?), + #[cfg(feature = "_tls")] + "certificate_file" => options.certificate_file = Some(parse_param(key, value, load_certificate)?), + #[cfg(feature = "_tls")] + "private_key_file" => options.private_key_file = Some(parse_param(key, value, load_private_key)?), "alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?, _ => { let value = SettingType::String(value.to_string()); From 0dfa42be268217c0a53fa3eb39f3b7696d4c86cb Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 01:03:56 +0200 Subject: [PATCH 14/18] ci: add mTLS --- .github/workflows/test.yml | 22 ++++++++++++++++------ extras/ci/generate_certs.sh | 20 ++++++++++++++++++++ extras/ci/overrides.xml | 3 ++- extras/ci/users-overrides.yaml | 6 ++++++ 4 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 extras/ci/users-overrides.yaml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a36451d..b0d769c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,23 +37,30 @@ jobs: feature: - tls-native-tls - tls-rustls - options: - - skip_verify=true - - ca_certificate=/tmp/ca.pem + database_url: + # for TLS we need skip_verify for self-signed certificate + - tcp://localhost:9440?skip_verify=true + # we don't need skip_verify when we pass CA cert + - tcp://localhost:9440?ca_certificate=/tmp/ca.pem + # auth via mTLS + - tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&certificate_file=/tmp/client.crt&private_key_file=/tmp/client.key runs-on: ubuntu-latest 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&${{ matrix.options }}&connection_timeout=5s" + DATABASE_URL: ${{ matrix.database_url }}&compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&connection_timeout=5s CH_SSL_CA_CERTIFICATE: /tmp/ca.pem CH_SSL_CERTIFICATE: /tmp/server.crt CH_SSL_PRIVATE_KEY: /tmp/server.key + CH_SSL_CLIENT_PRIVATE_KEY: /tmp/client.key + CH_SSL_CLIENT_CERTIFICATE: /tmp/client.crt steps: - uses: actions/checkout@v3 - name: Generate TLS certificates run: | - extras/ci/generate_certs.sh "$CH_SSL_CA_CERTIFICATE" "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" - chmod 644 "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" + extras/ci/generate_certs.sh "$CH_SSL_CA_CERTIFICATE" "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CLIENT_CERTIFICATE" "$CH_SSL_CLIENT_PRIVATE_KEY" + # server needs access to those + chmod 644 "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CA_CERTIFICATE" # NOTE: # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. # - "job.container.network" is empty, hence "host" @@ -61,10 +68,13 @@ jobs: - name: Run clickhouse-server run: docker run -v ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml + -v ./extras/ci/users-overrides.yaml:/etc/clickhouse-server/users.d/overrides.yaml -v $CH_SSL_CERTIFICATE:/etc/clickhouse-server/config.d/server.crt -v $CH_SSL_PRIVATE_KEY:/etc/clickhouse-server/config.d/server.key + -v $CH_SSL_CA_CERTIFICATE:/etc/clickhouse-server/config.d/ca.pem -e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt -e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key + -e CH_SSL_CA_CERTIFICATE=/etc/clickhouse-server/config.d/ca.pem -e CLICKHOUSE_SKIP_USER_SETUP=1 --network host --name clickhouse diff --git a/extras/ci/generate_certs.sh b/extras/ci/generate_certs.sh index 94abb4e..f74a07a 100755 --- a/extras/ci/generate_certs.sh +++ b/extras/ci/generate_certs.sh @@ -4,6 +4,9 @@ ca_crt=${1:-"$CH_SSL_CA_CERTIFICATE"} && shift crt=${1-:"$CH_SSL_CERTIFICATE"} && shift key=${1-:"$CH_SSL_PRIVATE_KEY"} && shift +client_crt=${1-:"$CH_SSL_CLIENT_CERTIFICATE"} && shift +client_key=${1-:"$CH_SSL_CLIENT_PRIVATE_KEY"} && shift + ca_key=${ca_crt/.pem/.key} csr=${key/.key/.csr} ext=${key/.key/.ext} @@ -27,3 +30,20 @@ EOL openssl x509 -req -in "$csr" -CA "$ca_crt" -CAkey "$ca_key" -CAcreateserial -out "$crt" -days 825 -sha256 -extfile "$ext" openssl verify -CAfile "$ca_crt" "$crt" + +client_csr=${client_key/.key/.csr} +client_ext=${client_key/.key/.ext} +cat > "$client_ext" < - none + + relaxed true true sslv2,sslv3 diff --git a/extras/ci/users-overrides.yaml b/extras/ci/users-overrides.yaml new file mode 100644 index 0000000..16f3e4d --- /dev/null +++ b/extras/ci/users-overrides.yaml @@ -0,0 +1,6 @@ +--- +users: + tls: + ssl_certificates: + subject_alt_name: + - DNS:localhost From 7272327168a569a0e3dcb80b14cd8dd4afe01077 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 01:26:17 +0200 Subject: [PATCH 15/18] Add mTLS support for native-tls --- .github/workflows/test.yml | 11 +++++-- extras/ci/generate_certs.sh | 3 ++ src/connecting_stream.rs | 5 ++- src/types/options.rs | 65 +++++++++++++++++++++++++++++++------ 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b0d769c..00318fb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,8 +42,12 @@ jobs: - tcp://localhost:9440?skip_verify=true # we don't need skip_verify when we pass CA cert - tcp://localhost:9440?ca_certificate=/tmp/ca.pem - # auth via mTLS - - tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&certificate_file=/tmp/client.crt&private_key_file=/tmp/client.key + # mTLS + include: + - feature: tls-native-tls + database_url: tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&tls_identity=/tmp/client.p12 + - feature: tls-rustls + database_url: tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&certificate_file=/tmp/client.crt&private_key_file=/tmp/client.key runs-on: ubuntu-latest env: # NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly @@ -54,11 +58,12 @@ jobs: CH_SSL_PRIVATE_KEY: /tmp/server.key CH_SSL_CLIENT_PRIVATE_KEY: /tmp/client.key CH_SSL_CLIENT_CERTIFICATE: /tmp/client.crt + CH_SSL_CLIENT_P12: /tmp/client.p12 steps: - uses: actions/checkout@v3 - name: Generate TLS certificates run: | - extras/ci/generate_certs.sh "$CH_SSL_CA_CERTIFICATE" "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CLIENT_CERTIFICATE" "$CH_SSL_CLIENT_PRIVATE_KEY" + extras/ci/generate_certs.sh "$CH_SSL_CA_CERTIFICATE" "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CLIENT_CERTIFICATE" "$CH_SSL_CLIENT_PRIVATE_KEY" "$CH_SSL_CLIENT_P12" # server needs access to those chmod 644 "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CA_CERTIFICATE" # NOTE: diff --git a/extras/ci/generate_certs.sh b/extras/ci/generate_certs.sh index f74a07a..1b29ce3 100755 --- a/extras/ci/generate_certs.sh +++ b/extras/ci/generate_certs.sh @@ -6,6 +6,7 @@ key=${1-:"$CH_SSL_PRIVATE_KEY"} && shift client_crt=${1-:"$CH_SSL_CLIENT_CERTIFICATE"} && shift client_key=${1-:"$CH_SSL_CLIENT_PRIVATE_KEY"} && shift +client_p12=${1-:"$CH_SSL_CLIENT_P12"} && shift ca_key=${ca_crt/.pem/.key} csr=${key/.key/.csr} @@ -47,3 +48,5 @@ openssl genrsa -out "$client_key" 2048 openssl req -new -key "$client_key" -out "$client_csr" -subj "/C=US/ST=DevState/O=DevOrg/CN=MyClient" openssl x509 -req -in "$client_csr" -CA "$ca_crt" -CAkey "$ca_key" -CAcreateserial -out "$client_crt" -days 3650 -sha256 -extfile "$client_ext" openssl verify -CAfile "$ca_crt" "$client_crt" + +openssl pkcs12 -export -inkey "$client_key" -in "$client_crt" -certfile "$ca_crt" -out "$client_p12" -passout pass: diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index 35e158b..b67561b 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -104,7 +104,7 @@ impl State { State::Tcp(TcpState::Fail(Some(conn_error))) } - #[cfg(feature = "_tls")] + #[cfg(feature = "tls-rustls")] fn tls_err(e: TlsError) -> Self { State::Tls(TlsState::Fail(Some(ConnectionError::TlsError(e)))) } @@ -240,6 +240,9 @@ impl ConnectingStream { let native_cert = native_tls::Certificate::from(certificate); builder.add_root_certificate(native_cert); } + if let Some(identity) = options.tls_identity.clone() { + builder.identity(identity.into()); + } Self { state: State::tls_wait(Box::pin(async move { diff --git a/src/types/options.rs b/src/types/options.rs index 4287636..cf63601 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -197,27 +197,59 @@ impl From for rustls::pki_types::PrivateKeyDer<'static> { rustls::pki_types::PrivateKeyDer::clone_key(value.0.as_ref()) } } - -#[cfg(feature = "_tls")] +#[cfg(feature = "tls-rustls")] impl fmt::Debug for PrivateKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "[Private Key]") } } - -#[cfg(feature = "_tls")] +#[cfg(feature = "tls-rustls")] impl PartialEq for PrivateKey { fn eq(&self, _other: &Self) -> bool { true } } - -#[cfg(feature = "_tls")] +#[cfg(feature = "tls-rustls")] pub fn load_private_key(file: &str) -> Result { let data = fs::read(file)?; PrivateKey::from_pem(&data) } +/// Identity for mTLS for native-tls +#[cfg(feature = "tls-native-tls")] +#[derive(Clone)] +pub struct TlsIdentity(Arc); +#[cfg(feature = "tls-native-tls")] +impl TlsIdentity { + pub fn from_pkcs12_der(der: &[u8]) -> Result { + // FIXME: support password + Ok(Self(Arc::new(native_tls::Identity::from_pkcs12(der, "").map_err(|_| UrlError::Invalid)?))) + } +} +#[cfg(feature = "tls-native-tls")] +impl From for native_tls::Identity { + fn from(value: TlsIdentity) -> Self { + value.0.as_ref().clone() + } +} +#[cfg(feature = "tls-native-tls")] +impl fmt::Debug for TlsIdentity { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "[Private Key]") + } +} +#[cfg(feature = "tls-native-tls")] +impl PartialEq for TlsIdentity { + fn eq(&self, _other: &Self) -> bool { + true + } +} +#[cfg(feature = "tls-native-tls")] +pub fn load_tls_identity(file: &str) -> Result { + let data = fs::read(file)?; + TlsIdentity::from_pkcs12_der(&data) +} + #[derive(Clone, PartialEq, Debug)] pub enum SettingType { String(String), @@ -348,8 +380,11 @@ pub struct Options { pub(crate) certificate_file: Option, /// Private key for certificate. - #[cfg(feature = "_tls")] + /// TODO: merge private_key_file + certificate_file into client_tls_identity + #[cfg(feature = "tls-rustls")] pub(crate) private_key_file: Option, + #[cfg(feature = "tls-native-tls")] + pub(crate) tls_identity: Option, /// Query settings pub(crate) settings: HashMap, @@ -408,8 +443,10 @@ impl Default for Options { ca_certificate: None, #[cfg(feature = "_tls")] certificate_file: None, - #[cfg(feature = "_tls")] + #[cfg(feature = "tls-rustls")] private_key_file: None, + #[cfg(feature = "tls-native-tls")] + tls_identity: None, settings: HashMap::new(), alt_hosts: Vec::new(), } @@ -570,12 +607,18 @@ impl Options { => certificate_file: Option } - #[cfg(feature = "_tls")] + #[cfg(feature = "tls-rustls")] property! { /// Private key for certificate. => private_key_file: Option } + #[cfg(feature = "tls-native-tls")] + property! { + /// Identity for mTLS. + => tls_identity: Option + } + property! { /// Query settings => settings: HashMap @@ -671,8 +714,10 @@ where "ca_certificate" => options.ca_certificate = Some(parse_param(key, value, load_certificate)?), #[cfg(feature = "_tls")] "certificate_file" => options.certificate_file = Some(parse_param(key, value, load_certificate)?), - #[cfg(feature = "_tls")] + #[cfg(feature = "tls-rustls")] "private_key_file" => options.private_key_file = Some(parse_param(key, value, load_private_key)?), + #[cfg(feature = "tls-native-tls")] + "tls_identity" => options.tls_identity = Some(parse_param(key, value, load_tls_identity)?), "alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?, _ => { let value = SettingType::String(value.to_string()); From 6059960d67ab8f577a36483415b269ee78fc7d81 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 17:06:22 +0200 Subject: [PATCH 16/18] Refactor mTLS support --- .github/workflows/test.yml | 27 ++---- extras/ci/generate_certs.sh | 58 ++++++------- extras/ci/overrides.xml | 6 +- src/connecting_stream.rs | 17 ++-- src/types/mod.rs | 2 + src/types/options.rs | 161 +++++++++++++++--------------------- 6 files changed, 116 insertions(+), 155 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 00318fb..e2d1a0d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -41,31 +41,19 @@ jobs: # for TLS we need skip_verify for self-signed certificate - tcp://localhost:9440?skip_verify=true # we don't need skip_verify when we pass CA cert - - tcp://localhost:9440?ca_certificate=/tmp/ca.pem - # mTLS - include: - - feature: tls-native-tls - database_url: tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&tls_identity=/tmp/client.p12 - - feature: tls-rustls - database_url: tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&certificate_file=/tmp/client.crt&private_key_file=/tmp/client.key + - tcp://localhost:9440?ca_certificate=tls/ca.pem + # mTLS + - tcp://tls@localhost:9440?ca_certificate=tls/ca.pem&client_certificate=tls/client.crt&client_private_key=tls/client.key runs-on: ubuntu-latest 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: ${{ matrix.database_url }}&compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&connection_timeout=5s - CH_SSL_CA_CERTIFICATE: /tmp/ca.pem - CH_SSL_CERTIFICATE: /tmp/server.crt - CH_SSL_PRIVATE_KEY: /tmp/server.key - CH_SSL_CLIENT_PRIVATE_KEY: /tmp/client.key - CH_SSL_CLIENT_CERTIFICATE: /tmp/client.crt - CH_SSL_CLIENT_P12: /tmp/client.p12 steps: - uses: actions/checkout@v3 - name: Generate TLS certificates run: | - extras/ci/generate_certs.sh "$CH_SSL_CA_CERTIFICATE" "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CLIENT_CERTIFICATE" "$CH_SSL_CLIENT_PRIVATE_KEY" "$CH_SSL_CLIENT_P12" - # server needs access to those - chmod 644 "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CA_CERTIFICATE" + extras/ci/generate_certs.sh tls # NOTE: # - we cannot use "services" because they are executed before the steps, i.e. repository checkout. # - "job.container.network" is empty, hence "host" @@ -74,12 +62,7 @@ jobs: run: docker run -v ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml -v ./extras/ci/users-overrides.yaml:/etc/clickhouse-server/users.d/overrides.yaml - -v $CH_SSL_CERTIFICATE:/etc/clickhouse-server/config.d/server.crt - -v $CH_SSL_PRIVATE_KEY:/etc/clickhouse-server/config.d/server.key - -v $CH_SSL_CA_CERTIFICATE:/etc/clickhouse-server/config.d/ca.pem - -e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt - -e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key - -e CH_SSL_CA_CERTIFICATE=/etc/clickhouse-server/config.d/ca.pem + -v ./tls:/etc/clickhouse-server/tls -e CLICKHOUSE_SKIP_USER_SETUP=1 --network host --name clickhouse diff --git a/extras/ci/generate_certs.sh b/extras/ci/generate_certs.sh index 1b29ce3..ed41da3 100755 --- a/extras/ci/generate_certs.sh +++ b/extras/ci/generate_certs.sh @@ -1,24 +1,22 @@ #!/usr/bin/env bash -ca_crt=${1:-"$CH_SSL_CA_CERTIFICATE"} && shift -crt=${1-:"$CH_SSL_CERTIFICATE"} && shift -key=${1-:"$CH_SSL_PRIVATE_KEY"} && shift - -client_crt=${1-:"$CH_SSL_CLIENT_CERTIFICATE"} && shift -client_key=${1-:"$CH_SSL_CLIENT_PRIVATE_KEY"} && shift -client_p12=${1-:"$CH_SSL_CLIENT_P12"} && shift - -ca_key=${ca_crt/.pem/.key} -csr=${key/.key/.csr} -ext=${key/.key/.ext} - -openssl genrsa -out "$ca_key" 4096 -openssl req -x509 -new -nodes -key "$ca_key" -sha256 -days 3650 -out "$ca_crt" -subj "/C=US/ST=DevState/O=DevOrg/CN=MyDevCA" - -openssl genrsa -out "$key" 2048 -openssl req -new -key "$key" -out "$csr" -subj "/C=US/ST=DevState/O=DevOrg/CN=localhost" - -cat > "$ext" < server.ext < "$client_ext" < client.ext < - - - + /etc/clickhouse-server/tls/server.crt + /etc/clickhouse-server/tls/server.key + /etc/clickhouse-server/tls/ca.pem relaxed true true diff --git a/src/connecting_stream.rs b/src/connecting_stream.rs index b67561b..de06b0b 100644 --- a/src/connecting_stream.rs +++ b/src/connecting_stream.rs @@ -35,6 +35,8 @@ use crate::{errors::ConnectionError, io::Stream as InnerStream, Options}; use tokio_native_tls::TlsStream; #[cfg(feature = "tls-rustls")] use tokio_rustls::client::TlsStream; +#[cfg(feature = "_tls")] +use crate::types::ClientTlsIdentity; type Result = std::result::Result; @@ -130,6 +132,7 @@ pub(crate) struct ConnectingStream { state: State, } +#[cfg(feature = "tls-rustls")] #[derive(Debug)] struct DummyTlsVerifier; @@ -240,8 +243,9 @@ impl ConnectingStream { let native_cert = native_tls::Certificate::from(certificate); builder.add_root_certificate(native_cert); } - if let Some(identity) = options.tls_identity.clone() { - builder.identity(identity.into()); + if let Some(identity) = &options.client_tls_identity { + let ClientTlsIdentity::Pkcs(pkcs) = identity; + builder.identity(pkcs.clone()); } Self { @@ -301,12 +305,9 @@ impl ConnectingStream { ClientConfig::builder() .with_root_certificates(cert_store) }; - let config = if let Some(certificate_file) = options.certificate_file.clone() { - if let Some(private_key_file) = options.private_key_file.clone() { - builder.with_client_auth_cert(certificate_file.into(), private_key_file.into()) - } else { - Ok(builder.with_no_client_auth()) - } + let config = if let Some(identity) = &options.client_tls_identity { + let ClientTlsIdentity::Pem { key, certs } = identity; + builder.with_client_auth_cert(certs.clone().into(), key.clone_key()) } else { Ok(builder.with_no_client_auth()) }; diff --git a/src/types/mod.rs b/src/types/mod.rs index 83ff796..aaba44c 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -21,6 +21,8 @@ pub use self::{ value::Value, value_ref::ValueRef, }; +#[cfg(feature = "_tls")] +pub use self::options::ClientTlsIdentity; pub(crate) use self::{ cmd::Cmd, diff --git a/src/types/options.rs b/src/types/options.rs index cf63601..94979c3 100644 --- a/src/types/options.rs +++ b/src/types/options.rs @@ -2,11 +2,12 @@ use std::{ borrow::Cow, collections::HashMap, fmt, - fs, str::FromStr, sync::{Arc, Mutex}, time::Duration, }; +#[cfg(feature = "_tls")] +use std::fs; use crate::errors::{Error, Result, UrlError}; use percent_encoding::percent_decode; @@ -180,75 +181,51 @@ pub fn load_certificate(file: &str) -> Result { } } -/// Private key for rustls. -#[cfg(feature = "tls-rustls")] +#[cfg(feature = "_tls")] #[derive(Clone)] -pub struct PrivateKey(Arc>); -#[cfg(feature = "tls-rustls")] -impl PrivateKey { - pub fn from_pem(pem: &[u8]) -> Result { - let key = rustls::pki_types::PrivateKeyDer::from_pem_slice(&mut pem.as_ref()).map_err(|_| UrlError::Invalid)?; - Ok(Self(Arc::new(key))) - } -} -#[cfg(feature = "tls-rustls")] -impl From for rustls::pki_types::PrivateKeyDer<'static> { - fn from(value: PrivateKey) -> Self { - rustls::pki_types::PrivateKeyDer::clone_key(value.0.as_ref()) - } -} -#[cfg(feature = "tls-rustls")] -impl fmt::Debug for PrivateKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "[Private Key]") - } -} -#[cfg(feature = "tls-rustls")] -impl PartialEq for PrivateKey { - fn eq(&self, _other: &Self) -> bool { - true - } -} -#[cfg(feature = "tls-rustls")] -pub fn load_private_key(file: &str) -> Result { - let data = fs::read(file)?; - PrivateKey::from_pem(&data) +pub enum ClientTlsIdentity { + #[cfg(feature = "tls-rustls")] + Pem { + key: Arc>, + certs: Certificate, + }, + #[cfg(feature = "tls-native-tls")] + Pkcs(native_tls::Identity), } -/// Identity for mTLS for native-tls -#[cfg(feature = "tls-native-tls")] -#[derive(Clone)] -pub struct TlsIdentity(Arc); -#[cfg(feature = "tls-native-tls")] -impl TlsIdentity { - pub fn from_pkcs12_der(der: &[u8]) -> Result { - // FIXME: support password - Ok(Self(Arc::new(native_tls::Identity::from_pkcs12(der, "").map_err(|_| UrlError::Invalid)?))) +#[cfg(feature = "_tls")] +impl ClientTlsIdentity { + #[cfg(feature = "tls-rustls")] + pub fn load(cert_path: &str, key_path: &str) -> Result { + let key = rustls::pki_types::PrivateKeyDer::from_pem_slice(fs::read(key_path)?.as_ref()) + .map_err(|e| format!("Cannot read private key from {}: {}", key_path, e))?; + let key = Arc::new(key); + let certs = load_certificate(cert_path)?; + return Ok(Self::Pem{ key, certs }); } -} -#[cfg(feature = "tls-native-tls")] -impl From for native_tls::Identity { - fn from(value: TlsIdentity) -> Self { - value.0.as_ref().clone() + + #[cfg(feature = "tls-native-tls")] + pub fn load(cert_path: &str, key_path: &str) -> Result { + let identity = native_tls::Identity::from_pkcs8( + fs::read(cert_path)?.as_ref(), + fs::read(key_path)?.as_ref(), + ).map_err(|e| format!("Cannot load identity from {} and {}: {}", cert_path, key_path, e))?; + return Ok(Self::Pkcs(identity)); } } -#[cfg(feature = "tls-native-tls")] -impl fmt::Debug for TlsIdentity { + +#[cfg(feature = "_tls")] +impl fmt::Debug for ClientTlsIdentity { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "[Private Key]") + write!(f, "[Client Certificate]") } } -#[cfg(feature = "tls-native-tls")] -impl PartialEq for TlsIdentity { +#[cfg(feature = "_tls")] +impl PartialEq for ClientTlsIdentity { fn eq(&self, _other: &Self) -> bool { true } } -#[cfg(feature = "tls-native-tls")] -pub fn load_tls_identity(file: &str) -> Result { - let data = fs::read(file)?; - TlsIdentity::from_pkcs12_der(&data) -} #[derive(Clone, PartialEq, Debug)] pub enum SettingType { @@ -375,16 +352,9 @@ pub struct Options { #[cfg(feature = "_tls")] pub(crate) ca_certificate: Option, - /// Certificate for authorization. + /// Authorization with certificate (mTLS). #[cfg(feature = "_tls")] - pub(crate) certificate_file: Option, - - /// Private key for certificate. - /// TODO: merge private_key_file + certificate_file into client_tls_identity - #[cfg(feature = "tls-rustls")] - pub(crate) private_key_file: Option, - #[cfg(feature = "tls-native-tls")] - pub(crate) tls_identity: Option, + pub(crate) client_tls_identity: Option, /// Query settings pub(crate) settings: HashMap, @@ -395,7 +365,8 @@ pub struct Options { impl fmt::Debug for Options { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Options") + let mut debug = f.debug_struct("Options"); + let res = debug .field("addr", &self.addr) .field("database", &self.database) .field("compression", &self.compression) @@ -409,9 +380,15 @@ impl fmt::Debug for Options { .field("ping_timeout", &self.ping_timeout) .field("connection_timeout", &self.connection_timeout) .field("settings", &self.settings) - .field("alt_hosts", &self.alt_hosts) + .field("alt_hosts", &self.alt_hosts); + + #[cfg(feature = "_tls")] + res + .field("secure", &self.secure) .field("ca_certificate", &self.ca_certificate) - .finish() + .field("client_tls_identity", &self.client_tls_identity); + + res.finish() } } @@ -442,11 +419,7 @@ impl Default for Options { #[cfg(feature = "_tls")] ca_certificate: None, #[cfg(feature = "_tls")] - certificate_file: None, - #[cfg(feature = "tls-rustls")] - private_key_file: None, - #[cfg(feature = "tls-native-tls")] - tls_identity: None, + client_tls_identity: None, settings: HashMap::new(), alt_hosts: Vec::new(), } @@ -603,20 +576,8 @@ impl Options { #[cfg(feature = "_tls")] property! { - /// Certificate for authorization. - => certificate_file: Option - } - - #[cfg(feature = "tls-rustls")] - property! { - /// Private key for certificate. - => private_key_file: Option - } - - #[cfg(feature = "tls-native-tls")] - property! { - /// Identity for mTLS. - => tls_identity: Option + /// Authorization with certificate (mTLS). + => client_tls_identity: Option } property! { @@ -683,6 +644,11 @@ fn set_params<'a, I>(options: &mut Options, iter: I) -> std::result::Result<(), where I: Iterator, Cow<'a, str>)>, { + #[cfg(feature = "_tls")] + let mut client_certificate = None; + #[cfg(feature = "_tls")] + let mut client_private_key = None; + for (key, value) in iter { match key.as_ref() { "pool_min" => options.pool_min = parse_param(key, value, usize::from_str)?, @@ -713,11 +679,9 @@ where #[cfg(feature = "_tls")] "ca_certificate" => options.ca_certificate = Some(parse_param(key, value, load_certificate)?), #[cfg(feature = "_tls")] - "certificate_file" => options.certificate_file = Some(parse_param(key, value, load_certificate)?), - #[cfg(feature = "tls-rustls")] - "private_key_file" => options.private_key_file = Some(parse_param(key, value, load_private_key)?), - #[cfg(feature = "tls-native-tls")] - "tls_identity" => options.tls_identity = Some(parse_param(key, value, load_tls_identity)?), + "client_certificate" => client_certificate = Some(value), + #[cfg(feature = "_tls")] + "client_private_key" => client_private_key = Some(value), "alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?, _ => { let value = SettingType::String(value.to_string()); @@ -732,6 +696,17 @@ where }; } + #[cfg(feature = "_tls")] + match (client_certificate, client_private_key) { + (Some(cert), Some(key)) => { + options.client_tls_identity = Some(ClientTlsIdentity::load(&cert, &key).map_err(|_| UrlError::Invalid)?); + } + (None, None) => {} + _ => { + return Err(UrlError::Invalid); + }, + } + Ok(()) } From 1463a3b003d9359d0f4a863669ad3b592a875fcb Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 18:41:42 +0200 Subject: [PATCH 17/18] Update README about TLS features --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index eedb4e4..badbdfb 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,13 @@ for the most common use cases. The following features are available. - `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` — TLS support (allowed only with `tokio_io` and one of TLS libraries, under `tls-rustls` or `tls-native-tls` features). + +### TLS + +- `skip_verify` - do not verify the server certificate (**insecure**) +- `ca_certificate` - instead of `skip_verify` it is better to pass CA certificate explicitly (in case of self-signed certificates). +- `client_certificate`/`client_private_key` - authentication using TLS certificates (mTLS) (see [ClickHouse documentation](https://clickhouse.com/docs/operations/external-authenticators/ssl-x509) for more info) ## Example From 32ca7b6e7c292eb597a30a1e472f0fe6fff69d2a Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 19 Apr 2025 18:44:24 +0200 Subject: [PATCH 18/18] Revert "ci: temporary enable CI for next branch" This reverts commit f7395a3486d50a4c6e2294480d12eeaf304bf956. --- .github/workflows/test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e2d1a0d..cfb56db 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,6 @@ on: pull_request: branches: - async-await - - next env: CARGO_TERM_COLOR: always