Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove broken mtls code to fix CI #2218

Merged
merged 2 commits into from
May 21, 2024
Merged

Remove broken mtls code to fix CI #2218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c402372
Remove broken mtls code
djc May 21, 2024
8f6e4af
Update dependencies
djc May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
371 changes: 183 additions & 188 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions bin/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@ dns-over-tls = []
tls-openssl = ["dns-over-openssl"]
tls = ["dns-over-openssl"]

# WARNING: there is a bug in the mutual tls auth code at the moment see issue #100
# mtls = ["hickory-client/mtls"]

webpki-roots = ["hickory-client/webpki-roots", "hickory-server/webpki-roots"]
native-certs = ["hickory-client/native-certs", "hickory-server/native-certs"]

Expand Down
3 changes: 0 additions & 3 deletions crates/proto/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ serde-config = ["serde", "url/serde"]
# enables experimental the mDNS (multicast) feature
mdns = ["socket2/all"]

# WARNING: there is a bug in the mutual tls auth code at the moment see issue #100
# mtls = ["tls"]

wasm-bindgen = ["wasm-bindgen-crate", "js-sys"]

backtrace = ["dep:backtrace"]
Expand Down
56 changes: 2 additions & 54 deletions crates/proto/src/native_tls/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,7 @@ use crate::{iocompat::AsyncIoTokioAsStd, DnsStreamHandle};
#[test]
#[cfg_attr(target_os = "macos", ignore)] // TODO: add back once https://github.com/sfackler/rust-native-tls/issues/143 is fixed
fn test_tls_client_stream_ipv4() {
tls_client_stream_test(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), false)
}

// FIXME: mtls is disabled at the moment, it causes a hang on Linux, and is currently not supported on macOS
#[cfg(feature = "mtls")]
#[test]
#[cfg(not(target_os = "macos"))]
fn test_tls_client_stream_ipv4_mtls() {
tls_client_stream_test(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), true)
tls_client_stream_test(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)))
}

#[test]
Expand All @@ -74,7 +66,7 @@ fn read_file(path: &str) -> Vec<u8> {
}

#[allow(unused, unused_mut)]
fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
fn tls_client_stream_test(server_addr: IpAddr) {
let succeeded = Arc::new(atomic::AtomicBool::new(false));
let succeeded_clone = succeeded.clone();
thread::Builder::new()
Expand Down Expand Up @@ -117,28 +109,6 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
.spawn(move || {
let mut tls = TlsAcceptor::builder(identity);

// #[cfg(target_os = "linux")]
// {
// let mut openssl_builder = tls.builder_mut();
// let mut openssl_ctx_builder = openssl_builder.builder_mut();

// let mut mode = openssl::ssl::SslVerifyMode::empty();

// // TODO: mtls tests hang on Linux...
// if mtls {
// // mode = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;

// // let mut store = X509StoreBuilder::new().unwrap();
// // let root_ca = X509::from_der(&root_cert_der_copy).unwrap();
// // store.add_cert(root_ca).unwrap();
// // openssl_ctx_builder.set_verify_cert_store(store.build()).unwrap();
// } else {
// mode.insert(SSL_VERIFY_NONE);
// }

// openssl_ctx_builder.set_verify(mode);
// }

// TODO: add CA on macOS

let tls = tls.build().expect("tls build failed");
Expand Down Expand Up @@ -199,11 +169,6 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
let mut builder = TlsStreamBuilder::<AsyncIoTokioAsStd<TokioTcpStream>>::new();
builder.add_ca(trust_chain);

// fix MTLS
// if mtls {
// config_mtls(&root_pkey, &root_name, &root_cert, &mut builder);
// }

let (stream, mut sender) = builder.build(server_addr, dns_name.to_string());

// TODO: there is a race failure here... a race with the server thread most likely...
Expand All @@ -226,20 +191,3 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
succeeded.store(true, std::sync::atomic::Ordering::Relaxed);
server_handle.join().expect("server thread failed");
}

// TODO: fix MTLS
// #[allow(unused_variables)]
// fn config_mtls(root_pkey: &PKey,
// root_name: &X509Name,
// root_cert: &X509,
// builder: &mut TlsStreamBuilder) {
// // signed by the same root cert
// let client_name = "resolv.example.com";
// let (_ /*client_pkey*/, _ /*client_cert*/, client_identity) =
// cert(client_name, root_pkey, root_name, root_cert);
// let client_identity =
// native_tls::Pkcs12::from_der(&client_identity.to_der().unwrap(), "mypass").unwrap();

// #[cfg(feature = "mtls")]
// builder.identity(client_identity);
// }
8 changes: 0 additions & 8 deletions crates/proto/src/native_tls/tls_client_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ use std::pin::Pin;

use futures_util::TryFutureExt;
use native_tls::Certificate;
#[cfg(feature = "mtls")]
use native_tls::Pkcs12;
use tokio_native_tls::TlsStream as TokioTlsStream;

use crate::error::ProtoError;
Expand Down Expand Up @@ -46,12 +44,6 @@ impl<S: DnsTcpStream> TlsClientStreamBuilder<S> {
self.0.add_ca(ca);
}

/// Client side identity for client auth in TLS (aka mutual TLS auth)
#[cfg(feature = "mtls")]
pub fn identity(&mut self, pkcs12: Pkcs12) {
self.0.identity(pkcs12);
}

/// Sets the address to connect from.
pub fn bind_addr(&mut self, bind_addr: SocketAddr) {
self.0.bind_addr(bind_addr);
Expand Down
6 changes: 0 additions & 6 deletions crates/proto/src/native_tls/tls_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,6 @@ impl<S: DnsTcpStream> TlsStreamBuilder<S> {
self.ca_chain.push(ca);
}

/// Client side identity for client auth in TLS (aka mutual TLS auth)
#[cfg(feature = "mtls")]
pub fn identity(&mut self, identity: Identity) {
self.identity = Some(identity);
}

/// Sets the address to connect from.
pub fn bind_addr(&mut self, bind_addr: SocketAddr) {
self.bind_addr = Some(bind_addr);
Expand Down
8 changes: 0 additions & 8 deletions crates/proto/src/openssl/tls_client_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ use std::net::SocketAddr;
use std::pin::Pin;

use futures_util::TryFutureExt;
#[cfg(feature = "mtls")]
use openssl::pkcs12::Pkcs12;
use openssl::x509::X509;
use tokio_openssl::SslStream as TokioTlsStream;

Expand Down Expand Up @@ -54,12 +52,6 @@ impl<S: DnsTcpStream> TlsClientStreamBuilder<S> {
Ok(())
}

/// Client side identity for client auth in TLS (aka mutual TLS auth)
#[cfg(feature = "mtls")]
pub fn identity(&mut self, pkcs12: Pkcs12) {
self.0.identity(pkcs12);
}

/// Sets the address to connect from.
pub fn bind_addr(&mut self, bind_addr: SocketAddr) {
self.0.bind_addr(bind_addr);
Expand Down
6 changes: 0 additions & 6 deletions crates/proto/src/openssl/tls_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,6 @@ impl<S: DnsTcpStream> TlsStreamBuilder<S> {
self.ca_chain.push(ca);
}

/// Client side identity for client auth in TLS (aka mutual TLS auth)
#[cfg(feature = "mtls")]
pub fn identity(&mut self, pkcs12: ParsedPkcs12) {
self.identity = Some(pkcs12);
}

/// Sets the address to connect from.
pub fn bind_addr(&mut self, bind_addr: SocketAddr) {
self.bind_addr = Some(bind_addr);
Expand Down
56 changes: 4 additions & 52 deletions crates/proto/src/rustls/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use std::{thread, time};

use openssl::pkey::PKey;
use openssl::ssl::*;
use openssl::x509::store::X509StoreBuilder;
use openssl::x509::*;

use futures_util::stream::StreamExt;
Expand All @@ -36,20 +35,12 @@ use crate::{iocompat::AsyncIoTokioAsStd, DnsStreamHandle};
// #[cfg(not(target_os = "linux"))]
#[test]
fn test_tls_client_stream_ipv4() {
tls_client_stream_test(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), false)
}

// FIXME: mtls is disabled at the moment, it causes a hang on Linux, and is currently not supported on macOS
#[cfg(feature = "mtls")]
#[test]
#[cfg(not(target_os = "macos"))] // ignored until Travis-CI fixes IPv6
fn test_tls_client_stream_ipv4_mtls() {
tls_client_stream_test(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), true)
tls_client_stream_test(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)))
}

#[test]
fn test_tls_client_stream_ipv6() {
tls_client_stream_test(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1)), false)
tls_client_stream_test(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1)))
}

const TEST_BYTES: &[u8; 8] = b"DEADBEEF";
Expand All @@ -65,7 +56,7 @@ fn read_file(path: &str) -> Vec<u8> {
}

#[allow(unused_mut)]
fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
fn tls_client_stream_test(server_addr: IpAddr) {
let succeeded = Arc::new(atomic::AtomicBool::new(false));
let succeeded_clone = succeeded.clone();
thread::Builder::new()
Expand All @@ -88,7 +79,6 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
println!("using server src path: {server_path}");

let root_cert_der = read_file(&format!("{server_path}/tests/test-data/ca.der"));
let root_cert_der_copy = root_cert_der.clone();

// Generate X509 certificate
let ca = X509::from_der(&root_cert_der).expect("could not read CA");
Expand Down Expand Up @@ -124,23 +114,8 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {

{
let mut openssl_ctx_builder = &mut tls;

let mut mode = SslVerifyMode::empty();

// FIXME: mtls tests hang on Linux...
if mtls {
mode = SslVerifyMode::PEER | SslVerifyMode::FAIL_IF_NO_PEER_CERT;

let mut store = X509StoreBuilder::new().unwrap();
let root_ca = X509::from_der(&root_cert_der_copy).unwrap();
store.add_cert(root_ca).unwrap();
openssl_ctx_builder
.set_verify_cert_store(store.build())
.unwrap();
} else {
mode.insert(SslVerifyMode::NONE);
}

mode.insert(SslVerifyMode::NONE);
openssl_ctx_builder.set_verify(mode);
}

Expand Down Expand Up @@ -206,12 +181,6 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
.with_root_certificates(roots)
.with_no_client_auth();

// barrier.wait();
// fix MTLS
// if mtls {
// config_mtls(&root_pkey, &root_name, &root_cert, &mut builder);
// }

let (stream, mut sender) = tls_connect::<AsyncIoTokioAsStd<TokioTcpStream>>(
server_addr,
dns_name.to_string(),
Expand All @@ -237,20 +206,3 @@ fn tls_client_stream_test(server_addr: IpAddr, mtls: bool) {
succeeded.store(true, std::sync::atomic::Ordering::Relaxed);
server_handle.join().expect("server thread failed");
}

// TODO: fix MTLS
// #[allow(unused_variables)]
// fn config_mtls(root_pkey: &PKey,
// root_name: &X509Name,
// root_cert: &X509,
// builder: &mut TlsStreamBuilder) {
// // signed by the same root cert
// let client_name = "resolv.example.com";
// let (_ /*client_pkey*/, _ /*client_cert*/, client_identity) =
// cert(client_name, root_pkey, root_name, root_cert);
// let client_identity =
// native_tls::server_cert::from_der(&client_identity.to_der().unwrap(), "mypass").unwrap();

// #[cfg(feature = "mtls")]
// builder.identity(client_identity);
// }
Loading
Loading