Repository: lettre/lettre (language: rust)

Issue:
Lettre panics when there is an invalid certificate or invalid hostname match
When using STARTTLS and having a wrong host (or by using an IP) which doesn't match the one in the certificate a panic is generated.

**To Reproduce**
Make a connection to a STARTTLS enabled mail server, but instead of using the DNS/Hostname use it's IP instead.

**Expected behavior**
No panic, but a nice error which can be caught.
I know in the past this worked fine.

**Environment (please complete the following information):**
- Lettre version: `0.10.0-rc.3` and `af157c5f2663d52aed1f3680ba72dc1c7f9fe6ec` (Also `0.10.0-rc.2` and `0.10.0-rc.1`)
- OS: Linux Ubuntu 20.04
- Cargo.toml: `lettre = { version = "0.10.0-rc.3", features = ["smtp-transport", "builder", "serde", "native-tls", "hostname", "tracing"], default-features = false }`

<details><summary>Backtrace</summary>

```
[panic][ERROR] thread 'unnamed' panicked at 'InnerNetworkStream::None must never be built': /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/client/net.rs:242
   0: vaultwarden::init_logging::{{closure}}
             at src/main.rs:179:25
   1: std::panicking::rust_panic_with_hook
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/panicking.rs:628:17
   2: std::panicking::begin_panic::{{closure}}
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/panicking.rs:544:9
   3: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/sys_common/backtrace.rs:141:18
   4: std::panicking::begin_panic
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/panicking.rs:543:12
   5: <lettre::transport::smtp::client::net::NetworkStream as std::io::Write>::write
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/client/net.rs:242:17
   6: std::io::Write::write_all
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/io/mod.rs:1527:19
   7: lettre::transport::smtp::client::connection::SmtpConnection::write
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/client/connection.rs:256:9
   8: lettre::transport::smtp::client::connection::SmtpConnection::command
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/client/connection.rs:250:9
   9: lettre::transport::smtp::client::connection::SmtpConnection::abort
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/client/connection.rs:179:21
  10: lettre::transport::smtp::client::connection::SmtpConnection::starttls
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/client/connection.rs:148:17
  11: lettre::transport::smtp::transport::SmtpClient::connection
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/transport.rs:222:17
  12: <lettre::transport::smtp::transport::SmtpTransport as lettre::transport::Transport>::send_raw
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/smtp/transport.rs:32:24
  13: lettre::transport::Transport::send
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/lettre-0.10.0-rc.3/src/transport/mod.rs:132:9
  14: vaultwarden::mail::send_email
             at src/mail.rs:471:11
  15: vaultwarden::mail::send_test
             at src/mail.rs:435:5
  16: vaultwarden::api::admin::test_smtp
             at src/api/admin.rs:288:9
  17: vaultwarden::api::admin::rocket_route_fn_test_smtp
             at src/api/admin.rs:284:4
  18: core::ops::function::Fn::call
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/core/src/ops/function.rs:70:5
  19: <F as rocket::handler::Handler>::handle
             at /home/mathijs/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/263e39b/core/lib/src/handler.rs:177:9
  20: rocket::rocket::Rocket::route
             at /home/mathijs/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/263e39b/core/lib/src/rocket.rs:297:27
  21: rocket::rocket::Rocket::route_and_process
             at /home/mathijs/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/263e39b/core/lib/src/rocket.rs:243:34
  22: rocket::rocket::Rocket::dispatch
             at /home/mathijs/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/263e39b/core/lib/src/rocket.rs:218:28
  23: <rocket::rocket::Rocket as hyper::server::Handler>::handle
             at /home/mathijs/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/263e39b/core/lib/src/rocket.rs:83:24
  24: hyper::server::Worker<H>::keep_alive_loop
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.16/src/server/mod.rs:340:13
  25: hyper::server::Worker<H>::handle_connection
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.16/src/server/mod.rs:282:15
  26: hyper::server::handle::{{closure}}
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.16/src/server/mod.rs:242:34
  27: hyper::server::listener::spawn_with::{{closure}}
             at /home/mathijs/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.16/src/server/listener.rs:50:31
  28: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/sys_common/backtrace.rs:125:18
  29: std::thread::Builder::spawn_unchecked::{{closure}}::{{closure}}
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/thread/mod.rs:481:17
  30: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/core/src/panic/unwind_safe.rs:271:9
  31: std::panicking::try::do_call
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/panicking.rs:403:40
  32: __rust_try
  33: std::panicking::try
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/panicking.rs:367:19
  34: std::panic::catch_unwind
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/panic.rs:133:14
  35: std::thread::Builder::spawn_unchecked::{{closure}}
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/thread/mod.rs:480:30
  36: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/core/src/ops/function.rs:227:5
  37: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/alloc/src/boxed.rs:1636:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/alloc/src/boxed.rs:1636:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/ac2d9fc509e36d1b32513744adf58c34bcc4f43c/library/std/src/sys/unix/thread.rs:106:17
  38: start_thread
             at /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
  39: clone
             at /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
```

</details>


Discussion / hints:
Very strange. When compiling with `--release` it instead seems to return the correct error and not panic.

The `InnerNetworkStream::None` panic is caused by 

https://github.com/lettre/lettre/blob/af157c5f2663d52aed1f3680ba72dc1c7f9fe6ec/src/transport/smtp/client/net.rs#L132

https://github.com/lettre/lettre/blob/af157c5f2663d52aed1f3680ba72dc1c7f9fe6ec/src/transport/smtp/client/async_net.rs#L222

https://github.com/lettre/lettre/blob/af157c5f2663d52aed1f3680ba72dc1c7f9fe6ec/src/transport/smtp/client/async_net.rs#L245

taking the connection out and not caring to put the connection back into `self.inner` if the TLS handshake fails, as propagating the error should be enough to cause the entire attempt to send the email to be dropped.


Fix patch:
diff --git a/src/transport/smtp/client/async_connection.rs b/src/transport/smtp/client/async_connection.rs
index e341726c5..88c7eb5d3 100644
--- a/src/transport/smtp/client/async_connection.rs
+++ b/src/transport/smtp/client/async_connection.rs
@@ -161,10 +161,7 @@ impl AsyncSmtpConnection {
     ) -> Result<(), Error> {
         if self.server_info.supports_feature(Extension::StartTls) {
             try_smtp!(self.command(Starttls).await, self);
-            try_smtp!(
-                self.stream.get_mut().upgrade_tls(tls_parameters).await,
-                self
-            );
+            self.stream.get_mut().upgrade_tls(tls_parameters).await?;
             #[cfg(feature = "tracing")]
             tracing::debug!("connection encrypted");
             // Send EHLO again
diff --git a/src/transport/smtp/client/connection.rs b/src/transport/smtp/client/connection.rs
index e73b50cec..4de1339d5 100644
--- a/src/transport/smtp/client/connection.rs
+++ b/src/transport/smtp/client/connection.rs
@@ -145,7 +145,7 @@ impl SmtpConnection {
             #[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);
+                self.stream.get_mut().upgrade_tls(tls_parameters)?;
                 #[cfg(feature = "tracing")]
                 tracing::debug!("connection encrypted");
                 // Send EHLO again


Test patch: