diff --git a/openssl/src/ssl/connector.rs b/openssl/src/ssl/connector.rs index a730cc49..a91b46a1 100644 --- a/openssl/src/ssl/connector.rs +++ b/openssl/src/ssl/connector.rs @@ -132,13 +132,9 @@ impl SslConnector { self.configure()?.connect(domain, stream) } - /// Initiates a client-side TLS session on a stream without performing hostname verification. - /// - /// # Warning - /// - /// You should think very carefully before you use this method. If hostname verification is not - /// used, *any* valid certificate for *any* site will be trusted for use from any other. This - /// introduces a significant vulnerability to man-in-the-middle attacks. + #[deprecated( + since = "0.9.24", + note = "use `ConnectConfiguration::verify_hostname` and `ConnectConfiguration::use_server_name_indication` instead")] pub fn danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication< S, >( @@ -149,53 +145,80 @@ impl SslConnector { S: Read + Write, { self.configure()? - .danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(stream) + .use_server_name_indication(false) + .verify_hostname(false) + .connect("", stream) } /// Returns a structure allowing for configuration of a single TLS session before connection. pub fn configure(&self) -> Result { - Ssl::new(&self.0).map(ConnectConfiguration) + Ssl::new(&self.0).map(|ssl| ConnectConfiguration { ssl, sni: true, verify_hostname: true }) } } /// A type which allows for configuration of a client-side TLS session before connection. -pub struct ConnectConfiguration(Ssl); +pub struct ConnectConfiguration { + ssl: Ssl, + sni: bool, + verify_hostname: bool, +} impl ConnectConfiguration { #[deprecated(since = "0.9.23", note = "ConnectConfiguration now implements Deref")] pub fn ssl(&self) -> &Ssl { - &self.0 + &self.ssl } #[deprecated(since = "0.9.23", note = "ConnectConfiguration now implements DerefMut")] pub fn ssl_mut(&mut self) -> &mut Ssl { - &mut self.0 + &mut self.ssl } - /// Initiates a client-side TLS session on a stream. + /// Configures the use of Server Name Indication (SNI) when connecting. /// - /// The domain is used for SNI and hostname verification. - pub fn connect(mut self, domain: &str, stream: S) -> Result, HandshakeError> - where - S: Read + Write, - { - self.0.set_hostname(domain)?; - setup_verify_hostname(&mut self.0, domain)?; - - self.0.connect(stream) + /// Defaults to `true`. + pub fn use_server_name_indication(mut self, use_sni: bool) -> ConnectConfiguration { + self.sni = use_sni; + self } - /// Initiates a client-side TLS session on a stream without performing hostname verification. + /// Configures the use of hostname verification when connecting. /// - /// The verification configuration of the connector's `SslContext` is not overridden. + /// Defaults to `true`. /// /// # Warning /// /// You should think very carefully before you use this method. If hostname verification is not /// used, *any* valid certificate for *any* site will be trusted for use from any other. This /// introduces a significant vulnerability to man-in-the-middle attacks. + pub fn verify_hostname(mut self, verify_hostname: bool) -> ConnectConfiguration { + self.verify_hostname = verify_hostname; + self + } + + /// Initiates a client-side TLS session on a stream. + /// + /// The domain is used for SNI and hostname verification if enabled. + pub fn connect(mut self, domain: &str, stream: S) -> Result, HandshakeError> + where + S: Read + Write, + { + if self.sni { + self.ssl.set_hostname(domain)?; + } + + if self.verify_hostname { + setup_verify_hostname(&mut self.ssl, domain)?; + } + + self.ssl.connect(stream) + } + + #[deprecated( + since = "0.9.24", + note = "use `ConnectConfiguration::verify_hostname` and `ConnectConfiguration::use_server_name_indication` instead")] pub fn danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication< S, >( @@ -205,7 +228,7 @@ impl ConnectConfiguration { where S: Read + Write, { - self.0.connect(stream) + self.use_server_name_indication(false).verify_hostname(false).connect("", stream) } } @@ -213,13 +236,13 @@ impl Deref for ConnectConfiguration { type Target = SslRef; fn deref(&self) -> &SslRef { - &self.0 + &self.ssl } } impl DerefMut for ConnectConfiguration { fn deref_mut(&mut self) -> &mut SslRef { - &mut self.0 + &mut self.ssl } } diff --git a/openssl/src/ssl/tests/mod.rs b/openssl/src/ssl/tests/mod.rs index 1cc36c7f..ff1d1c86 100644 --- a/openssl/src/ssl/tests/mod.rs +++ b/openssl/src/ssl/tests/mod.rs @@ -6,10 +6,10 @@ use std::io::prelude::*; use std::io::{self, BufReader}; use std::iter; use std::mem; -use std::net::{TcpStream, TcpListener, SocketAddr}; +use std::net::{SocketAddr, TcpListener, TcpStream}; use std::path::Path; -use std::process::{Command, Child, Stdio, ChildStdin}; -use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering}; +use std::process::{Child, ChildStdin, Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering, ATOMIC_BOOL_INIT}; use std::thread; use std::time::Duration; use tempdir::TempDir; @@ -18,10 +18,9 @@ use dh::Dh; use hash::MessageDigest; use ocsp::{OcspResponse, RESPONSE_STATUS_UNAUTHORIZED}; use ssl; -use ssl::{SslMethod, HandshakeError, SslContext, SslStream, Ssl, ShutdownResult, - SslConnectorBuilder, SslAcceptorBuilder, Error, SSL_VERIFY_PEER, SSL_VERIFY_NONE, - STATUS_TYPE_OCSP}; -use x509::{X509StoreContext, X509, X509Name, X509_FILETYPE_PEM}; +use ssl::{Error, HandshakeError, ShutdownResult, Ssl, SslAcceptorBuilder, SslConnectorBuilder, + SslContext, SslMethod, SslStream, SSL_VERIFY_NONE, SSL_VERIFY_PEER, STATUS_TYPE_OCSP}; +use x509::{X509, X509Name, X509StoreContext, X509_FILETYPE_PEM}; #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))] use x509::verify::X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS; use pkey::PKey; @@ -35,7 +34,7 @@ static CERT: &'static [u8] = include_bytes!("../../../test/cert.pem"); static KEY: &'static [u8] = include_bytes!("../../../test/key.pem"); fn next_addr() -> SocketAddr { - use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; + use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; static PORT: AtomicUsize = ATOMIC_USIZE_INIT; let port = 15411 + PORT.fetch_add(1, Ordering::SeqCst); @@ -104,15 +103,13 @@ impl Server { #[allow(dead_code)] fn new_alpn() -> (Server, TcpStream) { - Server::new_tcp( - &[ - "-www", - "-nextprotoneg", - "http/1.1,spdy/3.1", - "-alpn", - "http/1.1,spdy/3.1", - ], - ) + Server::new_tcp(&[ + "-www", + "-nextprotoneg", + "http/1.1,spdy/3.1", + "-alpn", + "http/1.1,spdy/3.1", + ]) } } @@ -157,10 +154,9 @@ macro_rules! run_test( ); ); -run_test!( - new_ctx, - |method, _| { SslContext::builder(method).unwrap(); } -); +run_test!(new_ctx, |method, _| { + SslContext::builder(method).unwrap(); +}); run_test!(verify_untrusted, |method, stream| { let mut ctx = SslContext::builder(method).unwrap(); @@ -309,7 +305,7 @@ run_test!(verify_callback_data, |method, stream| { }); run_test!(ssl_verify_callback, |method, stream| { - use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; + use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; static CHECKED: AtomicUsize = ATOMIC_USIZE_INIT; @@ -437,9 +433,9 @@ fn test_read() { let mut stream = Ssl::new(&ctx.build()).unwrap().connect(tcp).unwrap(); stream.write_all("GET /\r\n\r\n".as_bytes()).unwrap(); stream.flush().unwrap(); - io::copy(&mut stream, &mut io::sink()).ok().expect( - "read error", - ); + io::copy(&mut stream, &mut io::sink()) + .ok() + .expect("read error"); } #[test] @@ -994,9 +990,8 @@ fn verify_valid_hostname() { ctx.set_verify(SSL_VERIFY_PEER); let mut ssl = Ssl::new(&ctx.build()).unwrap(); - ssl.param_mut().set_hostflags( - X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS, - ); + ssl.param_mut() + .set_hostflags(X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); ssl.param_mut().set_host("google.com").unwrap(); let s = TcpStream::connect("google.com:443").unwrap(); @@ -1019,9 +1014,8 @@ fn verify_invalid_hostname() { ctx.set_verify(SSL_VERIFY_PEER); let mut ssl = Ssl::new(&ctx.build()).unwrap(); - ssl.param_mut().set_hostflags( - X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS, - ); + ssl.param_mut() + .set_hostflags(X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); ssl.param_mut().set_host("foobar.com").unwrap(); let s = TcpStream::connect("google.com:443").unwrap(); @@ -1057,7 +1051,12 @@ fn connector_invalid_no_hostname_verification() { let connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap().build(); let s = TcpStream::connect("google.com:443").unwrap(); - connector.danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(s) + connector + .configure() + .unwrap() + .use_server_name_indication(false) + .verify_hostname(false) + .connect("foobar.com", s) .unwrap(); } @@ -1067,8 +1066,14 @@ fn connector_no_hostname_still_verifies() { let connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap().build(); - assert!(connector.danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(tcp) - .is_err()); + assert!( + connector + .configure() + .unwrap() + .verify_hostname(false) + .connect("fizzbuzz.com", tcp) + .is_err() + ); } #[test] @@ -1079,7 +1084,12 @@ fn connector_no_hostname_can_disable_verify() { connector.set_verify(SSL_VERIFY_NONE); let connector = connector.build(); - connector.danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(tcp).unwrap(); + connector + .configure() + .unwrap() + .verify_hostname(false) + .connect("foobar.com", tcp) + .unwrap(); } #[test] @@ -1101,9 +1111,7 @@ fn connector_client_server_mozilla_intermediate() { }); let mut connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap(); - connector - .set_ca_file("test/root-ca.pem") - .unwrap(); + connector.set_ca_file("test/root-ca.pem").unwrap(); let connector = connector.build(); let stream = TcpStream::connect(("127.0.0.1", port)).unwrap(); @@ -1135,9 +1143,7 @@ fn connector_client_server_mozilla_modern() { }); let mut connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap(); - connector - .set_ca_file("test/root-ca.pem") - .unwrap(); + connector.set_ca_file("test/root-ca.pem").unwrap(); let connector = connector.build(); let stream = TcpStream::connect(("127.0.0.1", port)).unwrap(); @@ -1239,7 +1245,8 @@ fn tmp_dh_callback() { } #[test] -#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))), all(feature = "v102", ossl102)))] +#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))), + all(feature = "v102", ossl102)))] fn tmp_ecdh_callback() { use ec::EcKey; use nid; @@ -1306,7 +1313,8 @@ fn tmp_dh_callback_ssl() { } #[test] -#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))), all(feature = "v102", ossl102)))] +#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))), + all(feature = "v102", ossl102)))] fn tmp_ecdh_callback_ssl() { use ec::EcKey; use nid;