Repository: lettre/lettre (language: rust)

Issue:
Explore allowing only one authentication_mechanism on SmtpTransportBuilder
Currently, the SmtpTransportBuilder has a field called authentication_mechanisms that allows for an Vec of mechanisms.

This caught my eye because it seemed like an unnecessary wasteful Vec allocation, but then I notice that you allow for multiple assignments to check them in priority order as seen [here](https://github.com/lettre/lettre/blob/master/src/transport/smtp/mod.rs#L328).

I cannot think of a scenario in which I would be targeting an email relay without being sure of how to authenticate. For example, if the relay server expects CramMd5 and that fails, it seems weird that I would also want to check for Plain, just in case it would magically go through. But that is what is allowed if I specify multiple mechanisms in the Vec. If Plain went through, even though the server was expecting CramMd5, it would actually be dangerous, because it would hide the authentication problem from me, since the emails would be sent successfully during the Plain fallback.



Discussion / hints:

Fix patch:
diff --git a/src/email/mod.rs b/src/email/mod.rs
index 0d1a179c0..e404c03b3 100644
--- a/src/email/mod.rs
+++ b/src/email/mod.rs
@@ -72,7 +72,7 @@ pub struct EmailBuilder {
     date_issued: bool,
 }
 
-/// todo
+/// Simple email enveloppe representation
 #[derive(PartialEq,Eq,Clone,Debug)]
 pub struct Envelope {
     /// The envelope recipients' addresses
diff --git a/src/lib.rs b/src/lib.rs
index 212b4bb7a..25ed52b44 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -144,8 +144,8 @@
 //!     .security_level(SecurityLevel::AlwaysEncrypt)
 //!     // Enable SMTPUTF8 if the server supports it
 //!     .smtp_utf8(true)
-//!     // Configure accepted authentication mechanisms
-//!     .authentication_mechanisms(vec![Mechanism::CramMd5])
+//!     // Configure expected authentication mechanism
+//!     .authentication_mechanism(Mechanism::CramMd5)
 //!     // Enable connection reuse
 //!     .connection_reuse(true).build();
 //!
diff --git a/src/transport/smtp/client/mod.rs b/src/transport/smtp/client/mod.rs
index cd28019da..710b24773 100644
--- a/src/transport/smtp/client/mod.rs
+++ b/src/transport/smtp/client/mod.rs
@@ -82,7 +82,15 @@ impl<S: Connector + Write + Read + Debug + Clone> Client<S> {
     pub fn upgrade_tls_stream(&mut self, ssl_context: &SslContext) -> io::Result<()> {
         match self.stream {
             Some(ref mut stream) => stream.get_mut().upgrade_tls(ssl_context),
-            None => Ok(())
+            None => Ok(()),
+        }
+    }
+
+    /// Tells if the underlying stream is currently encrypted
+    pub fn is_encrypted(&self) -> bool {
+        match self.stream {
+            Some(ref stream) => stream.get_ref().is_encrypted(),
+            None => false,
         }
     }
 
diff --git a/src/transport/smtp/client/net.rs b/src/transport/smtp/client/net.rs
index a7b129846..15472a195 100644
--- a/src/transport/smtp/client/net.rs
+++ b/src/transport/smtp/client/net.rs
@@ -14,6 +14,8 @@ pub trait Connector: Sized {
     fn connect(addr: &SocketAddr, ssl_context: Option<&SslContext>) -> io::Result<Self>;
     /// Upgrades to TLS connection
     fn upgrade_tls(&mut self, ssl_context: &SslContext) -> io::Result<()>;
+    /// Is the NetworkStream encrypted
+    fn is_encrypted(&self) -> bool;
 }
 
 impl Connector for NetworkStream {
@@ -43,6 +45,13 @@ impl Connector for NetworkStream {
         };
         Ok(())
     }
+    
+    fn is_encrypted(&self) -> bool {
+        match *self {
+            NetworkStream::Plain(_) => false,
+            NetworkStream::Ssl(_) => true,
+        }
+    }
 }
 
 
diff --git a/src/transport/smtp/mod.rs b/src/transport/smtp/mod.rs
index 01a5650b1..a194b6410 100644
--- a/src/transport/smtp/mod.rs
+++ b/src/transport/smtp/mod.rs
@@ -83,8 +83,8 @@ pub struct SmtpTransportBuilder {
     security_level: SecurityLevel,
     /// Enable UTF8 mailboxes in envelope or headers
     smtp_utf8: bool,
-    /// List of authentication mechanisms, sorted by priority
-    authentication_mechanisms: Vec<Mechanism>,
+    /// Optionnal enforced authentication mechanism
+    authentication_mechanism: Option<Mechanism>,
 }
 
 /// Builder for the SMTP SmtpTransport
@@ -104,7 +104,7 @@ impl SmtpTransportBuilder {
                     connection_reuse_count_limit: 100,
                     connection_reuse: false,
                     hello_name: "localhost".to_string(),
-                    authentication_mechanisms: vec![Mechanism::CramMd5, Mechanism::Plain],
+                    authentication_mechanism: None,
                 })
             }
             None => Err(From::from("Could nor resolve hostname")),
@@ -175,8 +175,8 @@ impl SmtpTransportBuilder {
     }
 
     /// Set the authentication mechanisms
-    pub fn authentication_mechanisms(mut self, mechanisms: Vec<Mechanism>) -> SmtpTransportBuilder {
-        self.authentication_mechanisms = mechanisms;
+    pub fn authentication_mechanism(mut self, mechanism: Mechanism) -> SmtpTransportBuilder {
+        self.authentication_mechanism = Some(mechanism);
         self
     }
 
@@ -325,7 +325,20 @@ impl EmailTransport for SmtpTransport {
 
                 let mut found = false;
 
-                for mechanism in self.client_info.authentication_mechanisms.clone() {
+                // Compute accepted mechnism
+                let accepted_mechanisms = match self.client_info.authentication_mechanism {
+                    Some(mechanism) => vec![mechanism],
+                    None => {
+                        match self.client.is_encrypted() {
+                            // If encrypted, allow all mechanisms, with a preference for the simplest
+                            true => vec![Mechanism::Plain, Mechanism::CramMd5],
+                            // If not encrypted, do not all clear-text passwords
+                            false => vec![Mechanism::CramMd5],
+                        }
+                    }
+                };
+
+                for mechanism in accepted_mechanisms {
                     if self.server_info.as_ref().unwrap().supports_auth_mechanism(mechanism) {
                         found = true;
                         try_smtp!(self.client.auth(mechanism, &username, &password), self);


Test patch: