From 98f09117f7fd8d36cc6040b80e8f52ba2bd0b580 Mon Sep 17 00:00:00 2001 From: Alexis Mousset Date: Thu, 27 Aug 2020 17:30:37 +0200 Subject: [PATCH] fix(transport-smtp): Use 127.0.0.1 literal as EHLO parameter when we have no hostname Also fix formatting of address literals Comes from https://github.com/async-email/async-smtp/commit/2275fd8d13e39b2c58d6605c786ff06ff9e05708 with a different approach for default value. --- CHANGELOG.md | 3 + src/transport/smtp/client/async_connection.rs | 25 ++++---- src/transport/smtp/client/connection.rs | 31 +++++----- src/transport/smtp/client/mod.rs | 8 ++- src/transport/smtp/client/net.rs | 8 ++- src/transport/smtp/extension.rs | 57 +++++++++++-------- src/transport/smtp/mod.rs | 4 +- 7 files changed, 74 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed54df4..f3ae7b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,9 @@ Several breaking changes were made between 0.9 and 0.10, but changes should be s * Change website url schemes to https ([6014f5c](https://github.com/lettre/lettre/commit/6014f5c)) * Use serde's `derive` feature instead of the `serde_derive` crate ([4fbe700](https://github.com/lettre/lettre/commit/4fbe700)) * Merge `Email` and `SendableEmail` into `lettre::Email` ([ce37464](https://github.com/lettre/lettre/commit/ce37464)) +* When the hostname feature is disabled or hostname cannot be fetched, `127.0.0.1` is used instead of `localhost` as + EHLO parameter (for better RFC compliance and mail server compatibility) +* The `new` method of `ClientId` is renamed to `new_domain` #### Bug Fixes diff --git a/src/transport/smtp/client/async_connection.rs b/src/transport/smtp/client/async_connection.rs index a014d05..43d176b 100644 --- a/src/transport/smtp/client/async_connection.rs +++ b/src/transport/smtp/client/async_connection.rs @@ -1,5 +1,4 @@ -use std::fmt::Display; -use std::io; +use std::{fmt::Display, io}; use futures_util::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; @@ -7,14 +6,16 @@ use futures_util::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use log::debug; use super::{AsyncNetworkStream, ClientCodec, TlsParameters}; -use crate::transport::smtp::authentication::{Credentials, Mechanism}; -use crate::transport::smtp::commands::*; -use crate::transport::smtp::error::Error; -use crate::transport::smtp::extension::{ - ClientId, Extension, MailBodyParameter, MailParameter, ServerInfo, +use crate::{ + transport::smtp::{ + authentication::{Credentials, Mechanism}, + commands::*, + error::Error, + extension::{ClientId, Extension, MailBodyParameter, MailParameter, ServerInfo}, + response::{parse_response, Response}, + }, + Envelope, }; -use crate::transport::smtp::response::{parse_response, Response}; -use crate::Envelope; #[cfg(feature = "log")] use super::escape_crlf; @@ -144,11 +145,7 @@ impl AsyncSmtpConnection { /// Send EHLO and update server info async fn ehlo(&mut self, hello_name: &ClientId) -> Result<(), Error> { - let ehlo_response = try_smtp!( - self.command(Ehlo::new(ClientId::new(hello_name.to_string()))) - .await, - self - ); + let ehlo_response = try_smtp!(self.command(Ehlo::new(hello_name.clone())).await, self); self.server_info = try_smtp!(ServerInfo::from_response(&ehlo_response), self); Ok(()) } diff --git a/src/transport/smtp/client/connection.rs b/src/transport/smtp/client/connection.rs index 681e88e..d5c3268 100644 --- a/src/transport/smtp/client/connection.rs +++ b/src/transport/smtp/client/connection.rs @@ -1,20 +1,24 @@ -use std::fmt::Display; -use std::io::{self, BufRead, BufReader, Write}; -use std::net::ToSocketAddrs; -use std::time::Duration; +use std::{ + fmt::Display, + io::{self, BufRead, BufReader, Write}, + net::ToSocketAddrs, + time::Duration, +}; #[cfg(feature = "log")] use log::debug; use super::{ClientCodec, NetworkStream, TlsParameters}; -use crate::transport::smtp::authentication::{Credentials, Mechanism}; -use crate::transport::smtp::commands::*; -use crate::transport::smtp::error::Error; -use crate::transport::smtp::extension::{ - ClientId, Extension, MailBodyParameter, MailParameter, ServerInfo, +use crate::{ + transport::smtp::{ + authentication::{Credentials, Mechanism}, + commands::*, + error::Error, + extension::{ClientId, Extension, MailBodyParameter, MailParameter, ServerInfo}, + response::{parse_response, Response}, + }, + Envelope, }; -use crate::transport::smtp::response::{parse_response, Response}; -use crate::Envelope; #[cfg(feature = "log")] use super::escape_crlf; @@ -138,10 +142,7 @@ impl SmtpConnection { /// Send EHLO and update server info fn ehlo(&mut self, hello_name: &ClientId) -> Result<(), Error> { - let ehlo_response = try_smtp!( - self.command(Ehlo::new(ClientId::new(hello_name.to_string()))), - self - ); + let ehlo_response = try_smtp!(self.command(Ehlo::new(hello_name.clone())), self); self.server_info = try_smtp!(ServerInfo::from_response(&ehlo_response), self); Ok(()) } diff --git a/src/transport/smtp/client/mod.rs b/src/transport/smtp/client/mod.rs index 4582a69..b80e8c0 100644 --- a/src/transport/smtp/client/mod.rs +++ b/src/transport/smtp/client/mod.rs @@ -7,12 +7,14 @@ use std::fmt::Debug; pub(crate) use self::async_connection::AsyncSmtpConnection; #[cfg(feature = "tokio02")] pub(crate) use self::async_net::AsyncNetworkStream; -pub use self::connection::SmtpConnection; -pub use self::mock::MockStream; use self::net::NetworkStream; #[cfg(any(feature = "native-tls", feature = "rustls-tls"))] pub(super) use self::tls::InnerTlsParameters; -pub use self::tls::{Tls, TlsParameters}; +pub use self::{ + connection::SmtpConnection, + mock::MockStream, + tls::{Tls, TlsParameters}, +}; #[cfg(feature = "tokio02")] mod async_connection; diff --git a/src/transport/smtp/client/net.rs b/src/transport/smtp/client/net.rs index 8dccc65..8aeb4b8 100644 --- a/src/transport/smtp/client/net.rs +++ b/src/transport/smtp/client/net.rs @@ -1,8 +1,10 @@ -use std::io::{self, Read, Write}; -use std::net::{Ipv4Addr, Shutdown, SocketAddr, SocketAddrV4, TcpStream, ToSocketAddrs}; #[cfg(feature = "rustls-tls")] use std::sync::Arc; -use std::time::Duration; +use std::{ + io::{self, Read, Write}, + net::{Ipv4Addr, Shutdown, SocketAddr, SocketAddrV4, TcpStream, ToSocketAddrs}, + time::Duration, +}; #[cfg(feature = "native-tls")] use native_tls::TlsStream; diff --git a/src/transport/smtp/extension.rs b/src/transport/smtp/extension.rs index f9206c3..014f400 100644 --- a/src/transport/smtp/extension.rs +++ b/src/transport/smtp/extension.rs @@ -10,9 +10,6 @@ use std::{ result::Result, }; -/// Default client id -const DEFAULT_DOMAIN_CLIENT_ID: &str = "localhost"; - /// Client identifier, the parameter to `EHLO` #[derive(PartialEq, Eq, Clone, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -25,35 +22,44 @@ pub enum ClientId { Ipv6(Ipv6Addr), } +const LOCALHOST_CLIENT: ClientId = ClientId::Ipv4(Ipv4Addr::new(127, 0, 0, 1)); + +impl Default for ClientId { + fn default() -> Self { + // https://tools.ietf.org/html/rfc5321#section-4.1.4 + // + // The SMTP client MUST, if possible, ensure that the domain parameter + // to the EHLO command is a primary host name as specified for this + // command in Section 2.3.5. If this is not possible (e.g., when the + // client's address is dynamically assigned and the client does not have + // an obvious name), an address literal SHOULD be substituted for the + // domain name. + #[cfg(feature = "hostname")] + { + hostname::get() + .ok() + .and_then(|s| s.into_string().map(Self::Domain).ok()) + .unwrap_or(LOCALHOST_CLIENT) + } + #[cfg(not(feature = "hostname"))] + LOCALHOST_CLIENT + } +} + impl Display for ClientId { fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { - ClientId::Domain(ref value) => f.write_str(value), - ClientId::Ipv4(ref value) => write!(f, "{}", value), - ClientId::Ipv6(ref value) => write!(f, "{}", value), + Self::Domain(ref value) => f.write_str(value), + Self::Ipv4(ref value) => write!(f, "[{}]", value), + Self::Ipv6(ref value) => write!(f, "[IPv6:{}]", value), } } } impl ClientId { /// Creates a new `ClientId` from a fully qualified domain name - pub fn new(domain: String) -> ClientId { - ClientId::Domain(domain) - } - - /// Defines a `ClientId` with the current hostname, of `localhost` if hostname could not be - /// found - pub fn hostname() -> ClientId { - #[cfg(feature = "hostname")] - return ClientId::Domain( - hostname::get() - .ok() - .and_then(|s| s.into_string().ok()) - .unwrap_or_else(|| DEFAULT_DOMAIN_CLIENT_ID.to_string()), - ); - - #[cfg(not(feature = "hostname"))] - return ClientId::Domain(DEFAULT_DOMAIN_CLIENT_ID.to_string()); + pub fn new_domain(domain: String) -> Self { + Self::Domain(domain) } } @@ -274,7 +280,7 @@ impl Display for RcptParameter { #[cfg(test)] mod test { - use super::{ClientId, Extension, ServerInfo}; + use super::*; use crate::transport::smtp::{ authentication::Mechanism, response::{Category, Code, Detail, Response, Severity}, @@ -284,9 +290,10 @@ mod test { #[test] fn test_clientid_fmt() { assert_eq!( - format!("{}", ClientId::new("test".to_string())), + format!("{}", ClientId::new_domain("test".to_string())), "test".to_string() ); + assert_eq!(format!("{}", LOCALHOST_CLIENT), "[127.0.0.1]".to_string()); } #[test] diff --git a/src/transport/smtp/mod.rs b/src/transport/smtp/mod.rs index 42ebb42..c4d2c76 100644 --- a/src/transport/smtp/mod.rs +++ b/src/transport/smtp/mod.rs @@ -162,7 +162,7 @@ //! # { //! use lettre::transport::smtp::{SMTP_PORT, extension::ClientId, commands::*, client::SmtpConnection}; //! -//! let hello = ClientId::new("my_hostname".to_string()); +//! let hello = ClientId::new_domain("my_hostname".to_string()); //! let mut client = SmtpConnection::connect(&("localhost", SMTP_PORT), None, &hello, None).unwrap(); //! client.command( //! Mail::new(Some("user@example.com".parse().unwrap()), vec![]) @@ -249,7 +249,7 @@ impl Default for SmtpInfo { Self { server: "localhost".to_string(), port: SMTP_PORT, - hello_name: ClientId::hostname(), + hello_name: ClientId::default(), credentials: None, authentication: DEFAULT_MECHANISMS.into(), timeout: Some(DEFAULT_TIMEOUT),