Skip to content

Commit 1aea56a

Browse files
committed
Refactor mTLS support
1 parent 7272327 commit 1aea56a

File tree

6 files changed

+98
-151
lines changed

6 files changed

+98
-151
lines changed

.github/workflows/test.yml

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -41,31 +41,21 @@ jobs:
4141
# for TLS we need skip_verify for self-signed certificate
4242
- tcp://localhost:9440?skip_verify=true
4343
# we don't need skip_verify when we pass CA cert
44-
- tcp://localhost:9440?ca_certificate=/tmp/ca.pem
45-
# mTLS
46-
include:
47-
- feature: tls-native-tls
48-
database_url: tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&tls_identity=/tmp/client.p12
49-
- feature: tls-rustls
50-
database_url: tcp://tls@localhost:9440?ca_certificate=/tmp/ca.pem&certificate_file=/tmp/client.crt&private_key_file=/tmp/client.key
44+
- tcp://localhost:9440?ca_certificate=tls/ca.pem
45+
# mTLS
46+
- tcp://tls@localhost:9440?ca_certificate=tls/ca.pem&client_certificate=tls/client.crt&client_private_key=tls/client.key
5147
runs-on: ubuntu-latest
5248
env:
5349
# NOTE: not all tests "secure" aware, so let's define DATABASE_URL explicitly
5450
# NOTE: sometimes for native-tls default connection_timeout (500ms) is not enough, interestingly that for rustls it is OK.
5551
DATABASE_URL: ${{ matrix.database_url }}&compression=lz4&ping_timeout=2s&retry_timeout=3s&secure=true&connection_timeout=5s
56-
CH_SSL_CA_CERTIFICATE: /tmp/ca.pem
57-
CH_SSL_CERTIFICATE: /tmp/server.crt
58-
CH_SSL_PRIVATE_KEY: /tmp/server.key
59-
CH_SSL_CLIENT_PRIVATE_KEY: /tmp/client.key
60-
CH_SSL_CLIENT_CERTIFICATE: /tmp/client.crt
61-
CH_SSL_CLIENT_P12: /tmp/client.p12
6252
steps:
6353
- uses: actions/checkout@v3
6454
- name: Generate TLS certificates
6555
run: |
66-
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"
67-
# server needs access to those
68-
chmod 644 "$CH_SSL_CERTIFICATE" "$CH_SSL_PRIVATE_KEY" "$CH_SSL_CA_CERTIFICATE"
56+
mkdir -p tls
57+
cd tls
58+
extras/ci/generate_certs.sh
6959
# NOTE:
7060
# - we cannot use "services" because they are executed before the steps, i.e. repository checkout.
7161
# - "job.container.network" is empty, hence "host"
@@ -74,12 +64,7 @@ jobs:
7464
run: docker run
7565
-v ./extras/ci/overrides.xml:/etc/clickhouse-server/config.d/overrides.xml
7666
-v ./extras/ci/users-overrides.yaml:/etc/clickhouse-server/users.d/overrides.yaml
77-
-v $CH_SSL_CERTIFICATE:/etc/clickhouse-server/config.d/server.crt
78-
-v $CH_SSL_PRIVATE_KEY:/etc/clickhouse-server/config.d/server.key
79-
-v $CH_SSL_CA_CERTIFICATE:/etc/clickhouse-server/config.d/ca.pem
80-
-e CH_SSL_CERTIFICATE=/etc/clickhouse-server/config.d/server.crt
81-
-e CH_SSL_PRIVATE_KEY=/etc/clickhouse-server/config.d/server.key
82-
-e CH_SSL_CA_CERTIFICATE=/etc/clickhouse-server/config.d/ca.pem
67+
-v ./tls:/etc/clickhouse-server/tls
8368
-e CLICKHOUSE_SKIP_USER_SETUP=1
8469
--network host
8570
--name clickhouse

extras/ci/generate_certs.sh

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,18 @@
11
#!/usr/bin/env bash
22

3-
ca_crt=${1:-"$CH_SSL_CA_CERTIFICATE"} && shift
4-
crt=${1-:"$CH_SSL_CERTIFICATE"} && shift
5-
key=${1-:"$CH_SSL_PRIVATE_KEY"} && shift
6-
7-
client_crt=${1-:"$CH_SSL_CLIENT_CERTIFICATE"} && shift
8-
client_key=${1-:"$CH_SSL_CLIENT_PRIVATE_KEY"} && shift
9-
client_p12=${1-:"$CH_SSL_CLIENT_P12"} && shift
10-
11-
ca_key=${ca_crt/.pem/.key}
12-
csr=${key/.key/.csr}
13-
ext=${key/.key/.ext}
14-
15-
openssl genrsa -out "$ca_key" 4096
16-
openssl req -x509 -new -nodes -key "$ca_key" -sha256 -days 3650 -out "$ca_crt" -subj "/C=US/ST=DevState/O=DevOrg/CN=MyDevCA"
17-
18-
openssl genrsa -out "$key" 2048
19-
openssl req -new -key "$key" -out "$csr" -subj "/C=US/ST=DevState/O=DevOrg/CN=localhost"
20-
21-
cat > "$ext" <<EOL
3+
#
4+
# CA
5+
#
6+
openssl genrsa -out ca.key 4096
7+
openssl req -x509 -new -nodes -key ca.key -sha256 -days 3650 -out ca.pem -subj "/C=US/ST=DevState/O=DevOrg/CN=MyDevCA"
8+
9+
#
10+
# server
11+
#
12+
openssl genrsa -out server.key 2048
13+
openssl req -new -key server.key -out server.csr -subj "/C=US/ST=DevState/O=DevOrg/CN=localhost"
14+
15+
cat > server.ext <<EOL
2216
authorityKeyIdentifier=keyid,issuer
2317
basicConstraints=CA:FALSE
2418
keyUsage = digitalSignature, keyEncipherment
@@ -29,12 +23,13 @@ subjectAltName = @alt_names
2923
DNS.1 = localhost
3024
EOL
3125

32-
openssl x509 -req -in "$csr" -CA "$ca_crt" -CAkey "$ca_key" -CAcreateserial -out "$crt" -days 825 -sha256 -extfile "$ext"
33-
openssl verify -CAfile "$ca_crt" "$crt"
26+
openssl x509 -req -in server.csr -CA ca.pem -CAkey ca.key -CAcreateserial -out server.crt -days 825 -sha256 -extfile server.ext
27+
openssl verify -CAfile ca.pem server.crt
3428

35-
client_csr=${client_key/.key/.csr}
36-
client_ext=${client_key/.key/.ext}
37-
cat > "$client_ext" <<EOL
29+
#
30+
# client
31+
#
32+
cat > client.ext <<EOL
3833
basicConstraints=CA:FALSE
3934
keyUsage = digitalSignature
4035
extendedKeyUsage = clientAuth
@@ -44,9 +39,10 @@ subjectAltName = @alt_names
4439
DNS.1 = localhost
4540
EOL
4641

47-
openssl genrsa -out "$client_key" 2048
48-
openssl req -new -key "$client_key" -out "$client_csr" -subj "/C=US/ST=DevState/O=DevOrg/CN=MyClient"
49-
openssl x509 -req -in "$client_csr" -CA "$ca_crt" -CAkey "$ca_key" -CAcreateserial -out "$client_crt" -days 3650 -sha256 -extfile "$client_ext"
50-
openssl verify -CAfile "$ca_crt" "$client_crt"
42+
openssl genrsa -out client.key 2048
43+
openssl req -new -key client.key -out client.csr -subj "/C=US/ST=DevState/O=DevOrg/CN=MyClient"
44+
openssl x509 -req -in client.csr -CA ca.pem -CAkey ca.key -CAcreateserial -out client.crt -days 3650 -sha256 -extfile client.ext
45+
openssl verify -CAfile ca.pem client.crt
5146

52-
openssl pkcs12 -export -inkey "$client_key" -in "$client_crt" -certfile "$ca_crt" -out "$client_p12" -passout pass:
47+
# server needs access to those
48+
chmod 644 ca.pem server.key server.crt

extras/ci/overrides.xml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
<clickhouse>
22
<openSSL>
33
<server>
4-
<certificateFile from_env="CH_SSL_CERTIFICATE" replace="1"></certificateFile>
5-
<privateKeyFile from_env="CH_SSL_PRIVATE_KEY" replace="1"></privateKeyFile>
6-
<caConfig from_env="CH_SSL_CA_CERTIFICATE" replace="1"></caConfig>
4+
<certificateFile>/etc/clickhouse-server/tls/server.crt</certificateFile>
5+
<privateKeyFile>/etc/clickhouse-server/tls/server.key</privateKeyFile>
6+
<caConfig>/etc/clickhouse-server/tls/ca.pem</caConfig>
77
<verificationMode>relaxed</verificationMode>
88
<loadDefaultCAFile>true</loadDefaultCAFile>
99
<cacheSessions>true</cacheSessions>

src/connecting_stream.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ use crate::{errors::ConnectionError, io::Stream as InnerStream, Options};
3535
use tokio_native_tls::TlsStream;
3636
#[cfg(feature = "tls-rustls")]
3737
use tokio_rustls::client::TlsStream;
38+
#[cfg(feature = "_tls")]
39+
use crate::types::ClientTlsIdentity;
3840

3941
type Result<T> = std::result::Result<T, ConnectionError>;
4042

@@ -240,8 +242,10 @@ impl ConnectingStream {
240242
let native_cert = native_tls::Certificate::from(certificate);
241243
builder.add_root_certificate(native_cert);
242244
}
243-
if let Some(identity) = options.tls_identity.clone() {
244-
builder.identity(identity.into());
245+
if let Some(identity) = &options.client_tls_identity {
246+
if let ClientTlsIdentity::Pkcs(pkcs) = identity {
247+
builder.identity(pkcs.clone());
248+
}
245249
}
246250

247251
Self {
@@ -301,12 +305,9 @@ impl ConnectingStream {
301305
ClientConfig::builder()
302306
.with_root_certificates(cert_store)
303307
};
304-
let config = if let Some(certificate_file) = options.certificate_file.clone() {
305-
if let Some(private_key_file) = options.private_key_file.clone() {
306-
builder.with_client_auth_cert(certificate_file.into(), private_key_file.into())
307-
} else {
308-
Ok(builder.with_no_client_auth())
309-
}
308+
let config = if let Some(identity) = &options.client_tls_identity {
309+
let ClientTlsIdentity::Pem { key, certs } = identity;
310+
builder.with_client_auth_cert(certs.clone().into(), key.clone_key())
310311
} else {
311312
Ok(builder.with_no_client_auth())
312313
};

src/types/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub use self::{
1616
from_sql::{FromSql, FromSqlResult},
1717
options::Options,
1818
options::{SettingType, SettingValue},
19+
options::ClientTlsIdentity,
1920
query::Query,
2021
query_result::QueryResult,
2122
value::Value,

src/types/options.rs

Lines changed: 53 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -180,75 +180,51 @@ pub fn load_certificate(file: &str) -> Result<Certificate> {
180180
}
181181
}
182182

183-
/// Private key for rustls.
184-
#[cfg(feature = "tls-rustls")]
183+
#[cfg(feature = "_tls")]
185184
#[derive(Clone)]
186-
pub struct PrivateKey(Arc<rustls::pki_types::PrivateKeyDer<'static>>);
187-
#[cfg(feature = "tls-rustls")]
188-
impl PrivateKey {
189-
pub fn from_pem(pem: &[u8]) -> Result<Self> {
190-
let key = rustls::pki_types::PrivateKeyDer::from_pem_slice(&mut pem.as_ref()).map_err(|_| UrlError::Invalid)?;
191-
Ok(Self(Arc::new(key)))
192-
}
193-
}
194-
#[cfg(feature = "tls-rustls")]
195-
impl From<PrivateKey> for rustls::pki_types::PrivateKeyDer<'static> {
196-
fn from(value: PrivateKey) -> Self {
197-
rustls::pki_types::PrivateKeyDer::clone_key(value.0.as_ref())
198-
}
199-
}
200-
#[cfg(feature = "tls-rustls")]
201-
impl fmt::Debug for PrivateKey {
202-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
203-
write!(f, "[Private Key]")
204-
}
205-
}
206-
#[cfg(feature = "tls-rustls")]
207-
impl PartialEq for PrivateKey {
208-
fn eq(&self, _other: &Self) -> bool {
209-
true
210-
}
211-
}
212-
#[cfg(feature = "tls-rustls")]
213-
pub fn load_private_key(file: &str) -> Result<PrivateKey> {
214-
let data = fs::read(file)?;
215-
PrivateKey::from_pem(&data)
185+
pub enum ClientTlsIdentity {
186+
#[cfg(feature = "tls-rustls")]
187+
Pem {
188+
key: Arc<rustls::pki_types::PrivateKeyDer<'static>>,
189+
certs: Certificate,
190+
},
191+
#[cfg(feature = "tls-native-tls")]
192+
Pkcs(native_tls::Identity),
216193
}
217194

218-
/// Identity for mTLS for native-tls
219-
#[cfg(feature = "tls-native-tls")]
220-
#[derive(Clone)]
221-
pub struct TlsIdentity(Arc<native_tls::Identity>);
222-
#[cfg(feature = "tls-native-tls")]
223-
impl TlsIdentity {
224-
pub fn from_pkcs12_der(der: &[u8]) -> Result<Self> {
225-
// FIXME: support password
226-
Ok(Self(Arc::new(native_tls::Identity::from_pkcs12(der, "").map_err(|_| UrlError::Invalid)?)))
195+
#[cfg(feature = "_tls")]
196+
impl ClientTlsIdentity {
197+
#[cfg(feature = "tls-rustls")]
198+
pub fn load(cert_path: &str, key_path: &str) -> Result<Self> {
199+
let key = rustls::pki_types::PrivateKeyDer::from_pem_slice(fs::read(key_path)?.as_ref())
200+
.map_err(|e| format!("Cannot read private key from {}: {}", key_path, e))?;
201+
let key = Arc::new(key);
202+
let certs = load_certificate(cert_path)?;
203+
return Ok(Self::Pem{ key, certs });
227204
}
228-
}
229-
#[cfg(feature = "tls-native-tls")]
230-
impl From<TlsIdentity> for native_tls::Identity {
231-
fn from(value: TlsIdentity) -> Self {
232-
value.0.as_ref().clone()
205+
206+
#[cfg(feature = "tls-native-tls")]
207+
pub fn load(cert_path: &str, key_path: &str) -> Result<Self> {
208+
let identity = native_tls::Identity::from_pkcs8(
209+
fs::read(cert_path)?.as_ref(),
210+
fs::read(key_path)?.as_ref(),
211+
).map_err(|e| format!("Cannot load identity from {} and {}: {}", cert_path, key_path, e))?;
212+
return Ok(Self::Pkcs(identity));
233213
}
234214
}
235-
#[cfg(feature = "tls-native-tls")]
236-
impl fmt::Debug for TlsIdentity {
215+
216+
#[cfg(feature = "_tls")]
217+
impl fmt::Debug for ClientTlsIdentity {
237218
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
238-
write!(f, "[Private Key]")
219+
write!(f, "[Client Certificate]")
239220
}
240221
}
241-
#[cfg(feature = "tls-native-tls")]
242-
impl PartialEq for TlsIdentity {
222+
#[cfg(feature = "_tls")]
223+
impl PartialEq for ClientTlsIdentity {
243224
fn eq(&self, _other: &Self) -> bool {
244225
true
245226
}
246227
}
247-
#[cfg(feature = "tls-native-tls")]
248-
pub fn load_tls_identity(file: &str) -> Result<TlsIdentity> {
249-
let data = fs::read(file)?;
250-
TlsIdentity::from_pkcs12_der(&data)
251-
}
252228

253229
#[derive(Clone, PartialEq, Debug)]
254230
pub enum SettingType {
@@ -375,16 +351,9 @@ pub struct Options {
375351
#[cfg(feature = "_tls")]
376352
pub(crate) ca_certificate: Option<Certificate>,
377353

378-
/// Certificate for authorization.
354+
/// Authorization with certificate (mTLS).
379355
#[cfg(feature = "_tls")]
380-
pub(crate) certificate_file: Option<Certificate>,
381-
382-
/// Private key for certificate.
383-
/// TODO: merge private_key_file + certificate_file into client_tls_identity
384-
#[cfg(feature = "tls-rustls")]
385-
pub(crate) private_key_file: Option<PrivateKey>,
386-
#[cfg(feature = "tls-native-tls")]
387-
pub(crate) tls_identity: Option<TlsIdentity>,
356+
pub(crate) client_tls_identity: Option<ClientTlsIdentity>,
388357

389358
/// Query settings
390359
pub(crate) settings: HashMap<String, SettingValue>,
@@ -442,11 +411,7 @@ impl Default for Options {
442411
#[cfg(feature = "_tls")]
443412
ca_certificate: None,
444413
#[cfg(feature = "_tls")]
445-
certificate_file: None,
446-
#[cfg(feature = "tls-rustls")]
447-
private_key_file: None,
448-
#[cfg(feature = "tls-native-tls")]
449-
tls_identity: None,
414+
client_tls_identity: None,
450415
settings: HashMap::new(),
451416
alt_hosts: Vec::new(),
452417
}
@@ -603,20 +568,8 @@ impl Options {
603568

604569
#[cfg(feature = "_tls")]
605570
property! {
606-
/// Certificate for authorization.
607-
=> certificate_file: Option<Certificate>
608-
}
609-
610-
#[cfg(feature = "tls-rustls")]
611-
property! {
612-
/// Private key for certificate.
613-
=> private_key_file: Option<PrivateKey>
614-
}
615-
616-
#[cfg(feature = "tls-native-tls")]
617-
property! {
618-
/// Identity for mTLS.
619-
=> tls_identity: Option<TlsIdentity>
571+
/// Authorization with certificate (mTLS).
572+
=> client_tls_identity: Option<ClientTlsIdentity>
620573
}
621574

622575
property! {
@@ -683,6 +636,9 @@ fn set_params<'a, I>(options: &mut Options, iter: I) -> std::result::Result<(),
683636
where
684637
I: Iterator<Item = (Cow<'a, str>, Cow<'a, str>)>,
685638
{
639+
let mut client_certificate = None;
640+
let mut client_private_key = None;
641+
686642
for (key, value) in iter {
687643
match key.as_ref() {
688644
"pool_min" => options.pool_min = parse_param(key, value, usize::from_str)?,
@@ -713,11 +669,9 @@ where
713669
#[cfg(feature = "_tls")]
714670
"ca_certificate" => options.ca_certificate = Some(parse_param(key, value, load_certificate)?),
715671
#[cfg(feature = "_tls")]
716-
"certificate_file" => options.certificate_file = Some(parse_param(key, value, load_certificate)?),
717-
#[cfg(feature = "tls-rustls")]
718-
"private_key_file" => options.private_key_file = Some(parse_param(key, value, load_private_key)?),
719-
#[cfg(feature = "tls-native-tls")]
720-
"tls_identity" => options.tls_identity = Some(parse_param(key, value, load_tls_identity)?),
672+
"client_certificate" => client_certificate = Some(value),
673+
#[cfg(feature = "_tls")]
674+
"client_private_key" => client_private_key = Some(value),
721675
"alt_hosts" => options.alt_hosts = parse_param(key, value, parse_hosts)?,
722676
_ => {
723677
let value = SettingType::String(value.to_string());
@@ -732,6 +686,16 @@ where
732686
};
733687
}
734688

689+
match (client_certificate, client_private_key) {
690+
(Some(cert), Some(key)) => {
691+
options.client_tls_identity = Some(ClientTlsIdentity::load(&cert, &key).map_err(|_| UrlError::Invalid)?);
692+
}
693+
(None, None) => {}
694+
_ => {
695+
return Err(UrlError::Invalid);
696+
},
697+
}
698+
735699
Ok(())
736700
}
737701

0 commit comments

Comments
 (0)