Skip to content

Commit 546a6de

Browse files
committed
feat: unsafe option to disable tls hostname verification
Closes #27.
1 parent 4539678 commit 546a6de

File tree

4 files changed

+201
-94
lines changed

4 files changed

+201
-94
lines changed

src/client/mod.rs

+2-82
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ use std::time::Duration;
99
use const_format::formatcp;
1010
use either::{Either, Left, Right};
1111
use ignore_result::Ignore;
12-
use rustls::pki_types::{CertificateDer, PrivateKeyDer};
13-
use rustls::{ClientConfig, RootCertStore};
1412
use thiserror::Error;
1513
use tokio::sync::{mpsc, watch};
1614

@@ -46,9 +44,10 @@ pub use crate::proto::{EnsembleUpdate, Stat};
4644
use crate::record::{self, Record, StaticRecord};
4745
use crate::session::StateReceiver;
4846
pub use crate::session::{EventType, SessionId, SessionState, WatchedEvent};
47+
use crate::tls::TlsOptions;
4948
use crate::util::{self, Ref as _};
5049

51-
type Result<T> = std::result::Result<T, Error>;
50+
pub(crate) type Result<T, E = Error> = std::result::Result<T, E>;
5251

5352
/// CreateMode specifies ZooKeeper znode type. It covers all znode types with help from
5453
/// [CreateOptions::with_ttl].
@@ -1533,85 +1532,6 @@ impl Drop for OwnedLockClient {
15331532
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
15341533
pub(crate) struct Version(u32, u32, u32);
15351534

1536-
/// Options for tls connection.
1537-
#[derive(Debug)]
1538-
pub struct TlsOptions {
1539-
identity: Option<(Vec<CertificateDer<'static>>, PrivateKeyDer<'static>)>,
1540-
ca_certs: RootCertStore,
1541-
}
1542-
1543-
impl Clone for TlsOptions {
1544-
fn clone(&self) -> Self {
1545-
Self {
1546-
identity: self.identity.as_ref().map(|id| (id.0.clone(), id.1.clone_key())),
1547-
ca_certs: self.ca_certs.clone(),
1548-
}
1549-
}
1550-
}
1551-
1552-
impl Default for TlsOptions {
1553-
/// Tls options with well-known ca roots.
1554-
fn default() -> Self {
1555-
let mut options = Self::no_ca();
1556-
options.ca_certs.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned());
1557-
options
1558-
}
1559-
}
1560-
1561-
impl TlsOptions {
1562-
/// Tls options with no ca certificates. Use [TlsOptions::default] if well-known ca roots is
1563-
/// desirable.
1564-
pub fn no_ca() -> Self {
1565-
Self { ca_certs: RootCertStore::empty(), identity: None }
1566-
}
1567-
1568-
/// Adds new ca certificates.
1569-
pub fn with_pem_ca_certs(mut self, certs: &str) -> Result<Self> {
1570-
for r in rustls_pemfile::certs(&mut certs.as_bytes()) {
1571-
let cert = match r {
1572-
Ok(cert) => cert,
1573-
Err(err) => return Err(Error::other(format!("fail to read cert {}", err), err)),
1574-
};
1575-
if let Err(err) = self.ca_certs.add(cert) {
1576-
return Err(Error::other(format!("fail to add cert {}", err), err));
1577-
}
1578-
}
1579-
Ok(self)
1580-
}
1581-
1582-
/// Specifies client identity for server to authenticate.
1583-
pub fn with_pem_identity(mut self, cert: &str, key: &str) -> Result<Self> {
1584-
let r: std::result::Result<Vec<_>, _> = rustls_pemfile::certs(&mut cert.as_bytes()).collect();
1585-
let certs = match r {
1586-
Err(err) => return Err(Error::other(format!("fail to read cert {}", err), err)),
1587-
Ok(certs) => certs,
1588-
};
1589-
let key = match rustls_pemfile::private_key(&mut key.as_bytes()) {
1590-
Err(err) => return Err(Error::other(format!("fail to read client private key {err}"), err)),
1591-
Ok(None) => return Err(Error::BadArguments(&"no client private key")),
1592-
Ok(Some(key)) => key,
1593-
};
1594-
self.identity = Some((certs, key));
1595-
Ok(self)
1596-
}
1597-
1598-
fn take_roots(&mut self) -> RootCertStore {
1599-
std::mem::replace(&mut self.ca_certs, RootCertStore::empty())
1600-
}
1601-
1602-
fn into_config(mut self) -> Result<ClientConfig> {
1603-
let builder = ClientConfig::builder().with_root_certificates(self.take_roots());
1604-
if let Some((client_cert, client_key)) = self.identity.take() {
1605-
match builder.with_client_auth_cert(client_cert, client_key) {
1606-
Ok(config) => Ok(config),
1607-
Err(err) => Err(Error::other(format!("invalid client private key {err}"), err)),
1608-
}
1609-
} else {
1610-
Ok(builder.with_no_client_auth())
1611-
}
1612-
}
1613-
}
1614-
16151535
/// A builder for [Client].
16161536
#[derive(Clone, Debug)]
16171537
pub struct Connector {

src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ mod error;
55
mod proto;
66
mod record;
77
mod session;
8+
mod tls;
89
mod util;
910

1011
pub use self::acl::{Acl, Acls, AuthId, AuthUser, Permission};
1112
pub use self::error::Error;
13+
pub use self::tls::TlsOptions;
1214
pub use crate::client::*;

src/tls.rs

+172
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
use std::sync::Arc;
2+
3+
use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier};
4+
use rustls::crypto::{CryptoProvider, WebPkiSupportedAlgorithms};
5+
use rustls::pki_types::{CertificateDer, PrivateKeyDer, ServerName, UnixTime};
6+
use rustls::server::ParsedCertificate;
7+
use rustls::{ClientConfig, DigitallySignedStruct, Error as TlsError, RootCertStore, SignatureScheme};
8+
9+
use crate::client::Result;
10+
use crate::Error;
11+
12+
/// Options for tls connection.
13+
#[derive(Debug)]
14+
pub struct TlsOptions {
15+
identity: Option<(Vec<CertificateDer<'static>>, PrivateKeyDer<'static>)>,
16+
ca_certs: RootCertStore,
17+
hostname_verification: bool,
18+
}
19+
20+
impl Clone for TlsOptions {
21+
fn clone(&self) -> Self {
22+
Self {
23+
identity: self.identity.as_ref().map(|id| (id.0.clone(), id.1.clone_key())),
24+
ca_certs: self.ca_certs.clone(),
25+
hostname_verification: self.hostname_verification,
26+
}
27+
}
28+
}
29+
30+
impl Default for TlsOptions {
31+
/// Tls options with well-known ca roots.
32+
fn default() -> Self {
33+
let mut options = Self::no_ca();
34+
options.ca_certs.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned());
35+
options
36+
}
37+
}
38+
39+
// Rustls tends to make disable of hostname verification verbose since it exposes man-in-the-middle
40+
// attacks. Though, there are still attempts to disable hostname verification in rustls, but no got
41+
// merged until now.
42+
// * Allow disabling Hostname Verification: https://github.com/rustls/rustls/issues/578
43+
// * Dangerous verifiers API proposal: https://github.com/rustls/rustls/pull/1197
44+
#[derive(Debug)]
45+
struct TlsServerCertVerifier {
46+
roots: RootCertStore,
47+
supported: WebPkiSupportedAlgorithms,
48+
hostname_verification: bool,
49+
}
50+
51+
impl TlsServerCertVerifier {
52+
fn new(roots: RootCertStore, hostname_verification: bool) -> Self {
53+
Self {
54+
roots,
55+
supported: CryptoProvider::get_default().unwrap().signature_verification_algorithms,
56+
hostname_verification,
57+
}
58+
}
59+
}
60+
61+
impl ServerCertVerifier for TlsServerCertVerifier {
62+
fn verify_server_cert(
63+
&self,
64+
end_entity: &CertificateDer<'_>,
65+
intermediates: &[CertificateDer<'_>],
66+
server_name: &ServerName<'_>,
67+
_ocsp_response: &[u8],
68+
now: UnixTime,
69+
) -> Result<ServerCertVerified, TlsError> {
70+
let cert = ParsedCertificate::try_from(end_entity)?;
71+
rustls::client::verify_server_cert_signed_by_trust_anchor(
72+
&cert,
73+
&self.roots,
74+
intermediates,
75+
now,
76+
self.supported.all,
77+
)?;
78+
79+
if self.hostname_verification {
80+
rustls::client::verify_server_name(&cert, server_name)?;
81+
}
82+
Ok(ServerCertVerified::assertion())
83+
}
84+
85+
fn verify_tls12_signature(
86+
&self,
87+
message: &[u8],
88+
cert: &CertificateDer<'_>,
89+
dss: &DigitallySignedStruct,
90+
) -> Result<HandshakeSignatureValid, TlsError> {
91+
rustls::crypto::verify_tls12_signature(message, cert, dss, &self.supported)
92+
}
93+
94+
fn verify_tls13_signature(
95+
&self,
96+
message: &[u8],
97+
cert: &CertificateDer<'_>,
98+
dss: &DigitallySignedStruct,
99+
) -> Result<HandshakeSignatureValid, TlsError> {
100+
rustls::crypto::verify_tls12_signature(message, cert, dss, &self.supported)
101+
}
102+
103+
fn supported_verify_schemes(&self) -> Vec<SignatureScheme> {
104+
self.supported.supported_schemes()
105+
}
106+
}
107+
108+
impl TlsOptions {
109+
/// Tls options with no ca certificates. Use [TlsOptions::default] if well-known ca roots is
110+
/// desirable.
111+
pub fn no_ca() -> Self {
112+
Self { ca_certs: RootCertStore::empty(), identity: None, hostname_verification: true }
113+
}
114+
115+
/// Disables hostname verification in tls handshake.
116+
///
117+
/// # Safety
118+
/// This exposes risk to man-in-the-middle attacks.
119+
pub unsafe fn with_no_hostname_verification(mut self) -> Self {
120+
self.hostname_verification = false;
121+
self
122+
}
123+
124+
/// Adds new ca certificates.
125+
pub fn with_pem_ca_certs(mut self, certs: &str) -> Result<Self> {
126+
for r in rustls_pemfile::certs(&mut certs.as_bytes()) {
127+
let cert = match r {
128+
Ok(cert) => cert,
129+
Err(err) => return Err(Error::other(format!("fail to read cert {}", err), err)),
130+
};
131+
if let Err(err) = self.ca_certs.add(cert) {
132+
return Err(Error::other(format!("fail to add cert {}", err), err));
133+
}
134+
}
135+
Ok(self)
136+
}
137+
138+
/// Specifies client identity for server to authenticate.
139+
pub fn with_pem_identity(mut self, cert: &str, key: &str) -> Result<Self> {
140+
let r: std::result::Result<Vec<_>, _> = rustls_pemfile::certs(&mut cert.as_bytes()).collect();
141+
let certs = match r {
142+
Err(err) => return Err(Error::other(format!("fail to read cert {}", err), err)),
143+
Ok(certs) => certs,
144+
};
145+
let key = match rustls_pemfile::private_key(&mut key.as_bytes()) {
146+
Err(err) => return Err(Error::other(format!("fail to read client private key {err}"), err)),
147+
Ok(None) => return Err(Error::BadArguments(&"no client private key")),
148+
Ok(Some(key)) => key,
149+
};
150+
self.identity = Some((certs, key));
151+
Ok(self)
152+
}
153+
154+
fn take_roots(&mut self) -> RootCertStore {
155+
std::mem::replace(&mut self.ca_certs, RootCertStore::empty())
156+
}
157+
158+
pub(crate) fn into_config(mut self) -> Result<ClientConfig> {
159+
// This has to be called before server cert verifier to install default crypto provider.
160+
let builder = ClientConfig::builder();
161+
let verifier = TlsServerCertVerifier::new(self.take_roots(), self.hostname_verification);
162+
let builder = builder.dangerous().with_custom_certificate_verifier(Arc::new(verifier));
163+
if let Some((client_cert, client_key)) = self.identity.take() {
164+
match builder.with_client_auth_cert(client_cert, client_key) {
165+
Ok(config) => Ok(config),
166+
Err(err) => Err(Error::other(format!("invalid client private key {err}"), err)),
167+
}
168+
} else {
169+
Ok(builder.with_no_client_auth())
170+
}
171+
}
172+
}

tests/zookeeper.rs

+25-12
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ static ZK_IMAGE_TAG: &'static str = "3.9.0";
2222
static PERSISTENT_OPEN: &zk::CreateOptions<'static> = &zk::CreateMode::Persistent.with_acls(zk::Acls::anyone_all());
2323
static CONTAINER_OPEN: &zk::CreateOptions<'static> = &zk::CreateMode::Container.with_acls(zk::Acls::anyone_all());
2424

25+
fn env_toggle(name: &str) -> bool {
26+
match std::env::var(name) {
27+
Ok(v) if v == "true" => true,
28+
Ok(v) if v == "false" => false,
29+
_ => rand::random(),
30+
}
31+
}
32+
2533
fn random_data() -> Vec<u8> {
2634
let rng = rand::thread_rng();
2735
rng.sample_iter(Standard).take(32).collect()
@@ -163,6 +171,7 @@ struct Tls {
163171
ca: String,
164172
cert: String,
165173
key: String,
174+
hostname_verification: bool,
166175
}
167176

168177
struct Server {
@@ -221,16 +230,10 @@ impl Server {
221230
}
222231

223232
pub fn with_options(options: ServerOptions<'_>) -> Self {
224-
let tls = options.tls.unwrap_or_else(|| match std::env::var("ZK_TEST_TLS") {
225-
Ok(v) if v == "true" => true,
226-
Ok(v) if v == "false" => false,
227-
_ => rand::random(),
228-
});
233+
let tls = options.tls.unwrap_or_else(|| env_toggle("ZK_TEST_TLS"));
229234
if tls {
230-
println!("starting tls zookeeper server ...");
231235
Self::tls(options)
232236
} else {
233-
println!("starting plaintext zookeeper server ...");
234237
Self::plaintext(options)
235238
}
236239
}
@@ -241,9 +244,14 @@ impl Server {
241244

242245
fn tls(options: ServerOptions<'_>) -> Self {
243246
let dir = tempdir().unwrap();
247+
let hostname_verification = env_toggle("ZK_TLS_HOSTNAME_VERIFICATION");
248+
println!(
249+
"starting tls zookeeper server with hostname verification {} ...",
250+
if hostname_verification { "enabled" } else { "disabled" }
251+
);
244252

245253
let (ca_cert, ca_cert_pem) = generate_ca_cert();
246-
let server_cert = generate_server_cert();
254+
let server_cert = generate_server_cert(hostname_verification);
247255
let signed_server_cert = server_cert.serialize_pem_with_signer(&ca_cert).unwrap();
248256

249257
// ZooKeeper needs a keystore with both key and signed cert.
@@ -304,14 +312,15 @@ serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory
304312
let container = unsafe { std::mem::transmute(docker.run(image)) };
305313

306314
Self {
307-
tls: Some(Tls { ca: ca_cert_pem, cert: signed_client_cert, key: client_key }),
315+
tls: Some(Tls { ca: ca_cert_pem, cert: signed_client_cert, key: client_key, hostname_verification }),
308316
_dir: Some(dir),
309317
_docker: docker,
310318
container,
311319
}
312320
}
313321

314322
fn plaintext(options: ServerOptions<'_>) -> Self {
323+
println!("starting plaintext zookeeper server ...");
315324
let docker = Box::new(DockerCli::default());
316325
let image = zookeeper_image(options.into());
317326
let container = unsafe { std::mem::transmute(docker.run(image)) };
@@ -337,11 +346,14 @@ serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory
337346
let Some(tls) = self.tls.as_ref() else {
338347
return connector;
339348
};
340-
let tls_options = zk::TlsOptions::default()
349+
let mut tls_options = zk::TlsOptions::default()
341350
.with_pem_ca_certs(&tls.ca)
342351
.unwrap()
343352
.with_pem_identity(&tls.cert, &tls.key)
344353
.unwrap();
354+
if !tls.hostname_verification {
355+
tls_options = unsafe { tls_options.with_no_hostname_verification() };
356+
}
345357
connector.tls(tls_options);
346358
connector
347359
}
@@ -1595,8 +1607,9 @@ fn generate_ca_cert() -> (Certificate, String) {
15951607
(Certificate::from_params(ca_cert_params).unwrap(), ca_cert_pem)
15961608
}
15971609

1598-
fn generate_server_cert() -> Certificate {
1599-
let mut params = CertificateParams::new(vec!["127.0.0.1".to_string()]);
1610+
fn generate_server_cert(hostname_verification: bool) -> Certificate {
1611+
let san = if hostname_verification { vec!["127.0.0.1".to_string()] } else { vec![] };
1612+
let mut params = CertificateParams::new(san);
16001613
params.key_usages = vec![rcgen::KeyUsagePurpose::DigitalSignature, rcgen::KeyUsagePurpose::KeyEncipherment];
16011614
params.extended_key_usages = vec![rcgen::ExtendedKeyUsagePurpose::ServerAuth];
16021615
params.distinguished_name.push(rcgen::DnType::CommonName, "server");

0 commit comments

Comments
 (0)