Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lettre panics when there is an invalid certificate or invalid hostname match #678

Closed
BlackDex opened this issue Oct 6, 2021 · 2 comments · Fixed by #679
Closed

Lettre panics when there is an invalid certificate or invalid hostname match #678

BlackDex opened this issue Oct 6, 2021 · 2 comments · Fixed by #679

Comments

@BlackDex
Copy link

BlackDex commented Oct 6, 2021

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 }
Backtrace
[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
@paolobarbolini
Copy link
Member

Very strange. When compiling with --release it instead seems to return the correct error and not panic.

The InnerNetworkStream::None panic is caused by

let tcp_stream = mem::replace(&mut self.inner, InnerNetworkStream::None);

let tcp_stream = mem::replace(&mut self.inner, InnerAsyncNetworkStream::None);

let tcp_stream = mem::replace(&mut self.inner, InnerAsyncNetworkStream::None);

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.

@BlackDex
Copy link
Author

BlackDex commented Oct 6, 2021

Quick fix people!
Works like a charm.

BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Oct 7, 2021
- Fixed a bug in JavaScript which caused no messages to be shown to the
user in-case of an error send by the server.
- Changed mail error handling for better error messages
- Changed user/org actions from a to buttons, this should prevent
strange issues in-case of javascript issues and the page does re-load.
- Added Alpine and Debian info for the running docker image

During the mail error testing i encountered a bug which caused lettre to
panic. This panic only happens on debug builds and not release builds,
so no need to update anything on that part. This bug is also already
fixed. See lettre/lettre#678 and lettre/lettre#679

Resolves dani-garcia#2021
Could also fix the issue reported here dani-garcia#2022, or at least no hash `#` in
the url.
BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Nov 5, 2021
- Decreased `recursion_limit` from 512 to 87
  Mainly done by optimizing the config macro's.
  This fixes an issue with the rust-analyzer which doesn't go beyond 128
- Removed Regex for masking sensitive values and replaced it with a map()
  This is much faster then using a Regex.
- Refactored the get_support_json macro's
- All items above also lowered the binary size and possibly compile-time
- Removed `_conn: DbConn` from several functions, these caused unnecessary database connections for functions who didn't used that at all
- Decreased json response for `/plans`
- Updated libraries and where needed some code changes
  This also fixes some rare issues with SMTP lettre/lettre#678
- Using Rust 2021 instead of 2018
- Updated rust nightly
BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Nov 6, 2021
- Decreased `recursion_limit` from 512 to 87
  Mainly done by optimizing the config macro's.
  This fixes an issue with the rust-analyzer which doesn't go beyond 128
- Removed Regex for masking sensitive values and replaced it with a map()
  This is much faster then using a Regex.
- Refactored the get_support_json macro's
- All items above also lowered the binary size and possibly compile-time
- Removed `_conn: DbConn` from several functions, these caused unnecessary database connections for functions who didn't used that at all
- Decreased json response for `/plans`
- Updated libraries and where needed some code changes
  This also fixes some rare issues with SMTP lettre/lettre#678
- Using Rust 2021 instead of 2018
- Updated rust nightly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants