From c352efcb8690a3850951f28a66b8faf416272c3c Mon Sep 17 00:00:00 2001 From: Alexis Mousset Date: Sat, 2 May 2020 21:47:03 +0200 Subject: [PATCH] feat(transport-smtp): Retry over DNS connection issues (fixes #391) --- .github/workflows/test.yml | 50 +++++++++++++------------- Cargo.toml | 4 +-- src/transport/smtp/client/mod.rs | 4 +-- src/transport/smtp/client/net.rs | 60 ++++++++++++++++++++------------ src/transport/smtp/error.rs | 1 + src/transport/smtp/mod.rs | 46 +++++++++++++----------- 6 files changed, 93 insertions(+), 72 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 31fa4b5..ce4f282 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -83,28 +83,28 @@ jobs: command: clippy args: -- -D warnings - coverage: - name: Coverage - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v1 - - uses: actions-rs/toolchain@v1 - with: - toolchain: nightly - override: true - - run: sudo DEBIAN_FRONTEND=noninteractive apt-get -y install postfix - - run: smtp-sink 2525 1000& - - uses: actions-rs/cargo@v1 - with: - command: test - args: --no-fail-fast - env: - CARGO_INCREMENTAL: "0" - RUSTFLAGS: "-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Zno-landing-pads" - - id: coverage - uses: actions-rs/grcov@v0.1 - - name: Coveralls upload - uses: coverallsapp/github-action@master - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - path-to-lcov: ${{ steps.coverage.outputs.report }} +# coverage: +# name: Coverage +# runs-on: ubuntu-latest +# steps: +# - uses: actions/checkout@v1 +# - uses: actions-rs/toolchain@v1 +# with: +# toolchain: nightly +# override: true +# - run: sudo DEBIAN_FRONTEND=noninteractive apt-get -y install postfix +# - run: smtp-sink 2525 1000& +# - uses: actions-rs/cargo@v1 +# with: +# command: test +# args: --no-fail-fast +# env: +# CARGO_INCREMENTAL: "0" +# RUSTFLAGS: "-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Zno-landing-pads" +# - id: coverage +# uses: actions-rs/grcov@v0.1 +# - name: Coveralls upload +# uses: coverallsapp/github-action@master +# with: +# github-token: ${{ secrets.GITHUB_TOKEN }} +# path-to-lcov: ${{ steps.coverage.outputs.report }} diff --git a/Cargo.toml b/Cargo.toml index 04b1f7c..8cbb963 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ hyperx = { version = "1", optional = true, features = ["headers"] } idna = "0.2" line-wrap = "0.1" log = "0.4" -uuid = { version = "0.8", features = ["v4"] } mime = { version = "0.3", optional = true } native-tls = { version = "0.2", optional = true } nom = { version = "5", optional = true } @@ -36,6 +35,7 @@ rustls = { version = "0.17", optional = true } serde = { version = "1", optional = true, features = ["derive"] } serde_json = { version = "1", optional = true } textnonce = { version = "0.7", optional = true } +uuid = { version = "0.8", features = ["v4"] } webpki = { version = "0.21", optional = true } webpki-roots = { version = "0.19", optional = true } @@ -51,7 +51,7 @@ name = "transport_smtp" [features] builder = ["mime", "base64", "hyperx", "textnonce", "quoted_printable"] -default = ["file-transport", "smtp-transport", "hostname", "sendmail-transport", "builder", "r2d2"] +default = ["file-transport", "smtp-transport", "rustls-tls", "hostname", "sendmail-transport", "builder"] file-transport = ["serde", "serde_json"] rustls-tls = ["webpki", "webpki-roots", "rustls"] sendmail-transport = [] diff --git a/src/transport/smtp/client/mod.rs b/src/transport/smtp/client/mod.rs index 5af1c9c..d32ca84 100644 --- a/src/transport/smtp/client/mod.rs +++ b/src/transport/smtp/client/mod.rs @@ -193,7 +193,7 @@ impl SmtpConnection { hello_name: &ClientId, ) -> Result<(), Error> { if self.server_info.supports_feature(Extension::StartTls) { - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] { try_smtp!(self.command(Starttls), self); try_smtp!(self.stream.get_mut().upgrade_tls(tls_parameters), self); @@ -202,7 +202,7 @@ impl SmtpConnection { try_smtp!(self.ehlo(hello_name), self); Ok(()) } - #[cfg(not(any(feature = "native-tls", feature = "rustls")))] + #[cfg(not(any(feature = "native-tls", feature = "rustls-tls")))] // This should never happen as `Tls` can only be created // when a TLS library is enabled unreachable!("TLS support required but not supported"); diff --git a/src/transport/smtp/client/net.rs b/src/transport/smtp/client/net.rs index e0c20d6..5dcf553 100644 --- a/src/transport/smtp/client/net.rs +++ b/src/transport/smtp/client/net.rs @@ -3,15 +3,15 @@ use crate::transport::smtp::{client::mock::MockStream, error::Error}; #[cfg(feature = "native-tls")] use native_tls::{TlsConnector, TlsStream}; -#[cfg(feature = "rustls")] +#[cfg(feature = "rustls-tls")] use rustls::{ClientConfig, ClientSession}; #[cfg(feature = "native-tls")] use std::io::ErrorKind; -#[cfg(feature = "rustls")] +#[cfg(feature = "rustls-tls")] use std::sync::Arc; use std::{ io::{self, Read, Write}, - net::{Ipv4Addr, Shutdown, SocketAddr, SocketAddrV4, TcpStream}, + net::{Ipv4Addr, Shutdown, SocketAddr, SocketAddrV4, TcpStream, ToSocketAddrs}, time::Duration, }; @@ -23,7 +23,7 @@ pub struct TlsParameters { #[cfg(feature = "native-tls")] connector: TlsConnector, /// A client from `rustls` - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] // TODO use the same in all transports of the client connector: Box, /// The domain name which is expected in the TLS certificate from the server @@ -38,7 +38,7 @@ impl TlsParameters { } /// Creates a `TlsParameters` - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] pub fn new(domain: String, connector: ClientConfig) -> Self { Self { connector: Box::new(connector), @@ -54,7 +54,7 @@ pub enum NetworkStream { /// Encrypted TCP stream #[cfg(feature = "native-tls")] Tls(Box>), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] Tls(Box>), /// Mock stream Mock(MockStream), @@ -67,7 +67,7 @@ impl NetworkStream { NetworkStream::Tcp(ref s) => s.peer_addr(), #[cfg(feature = "native-tls")] NetworkStream::Tls(ref s) => s.get_ref().peer_addr(), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] NetworkStream::Tls(ref s) => s.get_ref().peer_addr(), NetworkStream::Mock(_) => Ok(SocketAddr::V4(SocketAddrV4::new( Ipv4Addr::new(127, 0, 0, 1), @@ -82,20 +82,34 @@ impl NetworkStream { NetworkStream::Tcp(ref s) => s.shutdown(how), #[cfg(feature = "native-tls")] NetworkStream::Tls(ref s) => s.get_ref().shutdown(how), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] NetworkStream::Tls(ref s) => s.get_ref().shutdown(how), NetworkStream::Mock(_) => Ok(()), } } - pub fn connect( - addr: &SocketAddr, + pub fn connect( + server: T, timeout: Option, tls_parameters: Option<&TlsParameters>, ) -> Result { + fn try_connect_timeout( + server: T, + timeout: Duration, + ) -> Result { + let addrs = server.to_socket_addrs()?; + for addr in addrs { + let result = TcpStream::connect_timeout(&addr, timeout); + if result.is_ok() { + return result.map_err(|e| e.into()); + } + } + Err(Error::Client("Could not connect")) + } + let tcp_stream = match timeout { - Some(t) => TcpStream::connect_timeout(addr, t)?, - None => TcpStream::connect(addr)?, + Some(t) => try_connect_timeout(server, t)?, + None => TcpStream::connect(server)?, }; match tls_parameters { @@ -105,7 +119,7 @@ impl NetworkStream { .connect(context.domain.as_ref(), tcp_stream) .map(|tls| NetworkStream::Tls(Box::new(tls))) .map_err(|e| Error::Io(io::Error::new(ErrorKind::Other, e))), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] Some(context) => { let domain = webpki::DNSNameRef::try_from_ascii_str(&context.domain)?; @@ -114,7 +128,7 @@ impl NetworkStream { tcp_stream, )))) } - #[cfg(not(any(feature = "native-tls", feature = "rustls")))] + #[cfg(not(any(feature = "native-tls", feature = "rustls-tls")))] Some(_) => panic!("TLS configuration without support"), None => Ok(NetworkStream::Tcp(tcp_stream)), } @@ -131,7 +145,7 @@ impl NetworkStream { Ok(tls_stream) => NetworkStream::Tls(Box::new(tls_stream)), Err(err) => return Err(Error::Io(io::Error::new(ErrorKind::Other, err))), }, - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] NetworkStream::Tcp(ref mut stream) => { let domain = webpki::DNSNameRef::try_from_ascii_str(&tls_parameters.domain)?; @@ -140,9 +154,9 @@ impl NetworkStream { stream.try_clone().unwrap(), ))) } - #[cfg(not(any(feature = "native-tls", feature = "rustls")))] + #[cfg(not(any(feature = "native-tls", feature = "rustls-tls")))] NetworkStream::Tcp(_) => panic!("STARTTLS without TLS support"), - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] NetworkStream::Tls(_) => return Ok(()), NetworkStream::Mock(_) => return Ok(()), }; @@ -153,7 +167,7 @@ impl NetworkStream { pub fn is_encrypted(&self) -> bool { match *self { NetworkStream::Tcp(_) | NetworkStream::Mock(_) => false, - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] NetworkStream::Tls(_) => true, } } @@ -161,7 +175,7 @@ impl NetworkStream { pub fn set_read_timeout(&mut self, duration: Option) -> io::Result<()> { match *self { NetworkStream::Tcp(ref mut stream) => stream.set_read_timeout(duration), - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] NetworkStream::Tls(ref mut stream) => stream.get_ref().set_read_timeout(duration), NetworkStream::Mock(_) => Ok(()), } @@ -171,7 +185,7 @@ impl NetworkStream { pub fn set_write_timeout(&mut self, duration: Option) -> io::Result<()> { match *self { NetworkStream::Tcp(ref mut stream) => stream.set_write_timeout(duration), - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] NetworkStream::Tls(ref mut stream) => stream.get_ref().set_write_timeout(duration), NetworkStream::Mock(_) => Ok(()), } @@ -184,7 +198,7 @@ impl Read for NetworkStream { NetworkStream::Tcp(ref mut s) => s.read(buf), #[cfg(feature = "native-tls")] NetworkStream::Tls(ref mut s) => s.read(buf), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] NetworkStream::Tls(ref mut s) => s.read(buf), NetworkStream::Mock(ref mut s) => s.read(buf), } @@ -197,7 +211,7 @@ impl Write for NetworkStream { NetworkStream::Tcp(ref mut s) => s.write(buf), #[cfg(feature = "native-tls")] NetworkStream::Tls(ref mut s) => s.write(buf), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] NetworkStream::Tls(ref mut s) => s.write(buf), NetworkStream::Mock(ref mut s) => s.write(buf), } @@ -208,7 +222,7 @@ impl Write for NetworkStream { NetworkStream::Tcp(ref mut s) => s.flush(), #[cfg(feature = "native-tls")] NetworkStream::Tls(ref mut s) => s.flush(), - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] NetworkStream::Tls(ref mut s) => s.flush(), NetworkStream::Mock(ref mut s) => s.flush(), } diff --git a/src/transport/smtp/error.rs b/src/transport/smtp/error.rs index 4fad19a..0652f11 100644 --- a/src/transport/smtp/error.rs +++ b/src/transport/smtp/error.rs @@ -69,6 +69,7 @@ impl Display for Error { Parsing(ref err) => fmt.write_str(err.description()), #[cfg(feature = "rustls-tls")] InvalidDNSName(ref err) => err.fmt(fmt), + #[cfg(feature = "r2d2")] Pool(ref err) => err.fmt(fmt), } } diff --git a/src/transport/smtp/mod.rs b/src/transport/smtp/mod.rs index c6766f7..5936003 100644 --- a/src/transport/smtp/mod.rs +++ b/src/transport/smtp/mod.rs @@ -12,8 +12,8 @@ //! * STARTTLS ([RFC 2487](http://tools.ietf.org/html/rfc2487)) //! -#[cfg(any(feature = "native-tls", feature = "rustls"))] -use crate::transport::smtp::net::TlsParameters; +#[cfg(any(feature = "native-tls", feature = "rustls-tls"))] +use crate::transport::smtp::client::net::TlsParameters; use crate::{ transport::smtp::{ authentication::{Credentials, Mechanism, DEFAULT_MECHANISMS}, @@ -27,11 +27,12 @@ use crate::{ use native_tls::{Protocol, TlsConnector}; #[cfg(feature = "r2d2")] use r2d2::Pool; -#[cfg(feature = "rustls")] +#[cfg(feature = "rustls-tls")] use rustls::ClientConfig; +#[cfg(feature = "r2d2")] use std::ops::DerefMut; use std::time::Duration; -#[cfg(feature = "rustls")] +#[cfg(feature = "rustls-tls")] use webpki_roots::TLS_SERVER_ROOTS; pub mod authentication; @@ -68,13 +69,13 @@ pub enum Tls { /// Insecure connection only (for testing purposes) None, /// Start with insecure connection and use `STARTTLS` when available - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] Opportunistic(TlsParameters), /// Start with insecure connection and require `STARTTLS` - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] Required(TlsParameters), /// Use TLS wrapped connection - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] Wrapper(TlsParameters), } @@ -130,7 +131,7 @@ impl SmtpTransport { /// Simple and secure transport, should be used when possible. /// Creates an encrypted transport over submissions port, using the provided domain /// to validate TLS certificates. - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] pub fn relay(relay: &str) -> Result { #[cfg(feature = "native-tls")] let mut tls_builder = TlsConnector::builder(); @@ -139,22 +140,25 @@ impl SmtpTransport { #[cfg(feature = "native-tls")] let tls_parameters = TlsParameters::new(relay.to_string(), tls_builder.build().unwrap()); - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] let mut tls = ClientConfig::new(); - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] tls.root_store.add_server_trust_anchors(&TLS_SERVER_ROOTS); - #[cfg(feature = "rustls")] + #[cfg(feature = "rustls-tls")] let tls_parameters = TlsParameters::new(relay.to_string(), tls); - let new = Self::new(relay) + #[allow(unused_mut)] + let mut new = Self::new(relay) .port(SUBMISSIONS_PORT) .tls(Tls::Wrapper(tls_parameters)); #[cfg(feature = "r2d2")] - // Pool with default configuration - // FIXME avoid clone - let tpool = new.clone(); - let new = new.pool(Pool::new(tpool)?); + { + // Pool with default configuration + // FIXME avoid clone + let tpool = new.clone(); + new = new.pool(Pool::new(tpool)?); + } Ok(new) } @@ -196,7 +200,7 @@ impl SmtpTransport { } /// Set the TLS settings to use - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] pub fn tls(mut self, tls: Tls) -> Self { self.tls = tls; self @@ -217,21 +221,23 @@ impl SmtpTransport { (self.server.as_ref(), self.port), self.timeout, &self.hello_name, + #[allow(clippy::match_single_binding)] match self.tls { - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] Tls::Wrapper(ref tls_parameters) => Some(tls_parameters), _ => None, }, )?; + #[allow(clippy::match_single_binding)] match self.tls { - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] Tls::Opportunistic(ref tls_parameters) => { if conn.can_starttls() { conn.starttls(tls_parameters, &self.hello_name)?; } } - #[cfg(any(feature = "native-tls", feature = "rustls"))] + #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] Tls::Required(ref tls_parameters) => { conn.starttls(tls_parameters, &self.hello_name)?; }