mail icon indicating copy to clipboard operation
mail copied to clipboard

SMTP: allow disabling starttls_auto since it's now true by default in Ruby 3

Open jeremy opened this issue 4 years ago • 8 comments

Passing enable_tls/starttls/starttls_auto: false now explicitly disables these options. They used to be all false by default, so they could only be turned ON. So we ignored turning them OFF!

We now disable_tls/starttls/starttls_auto when they're set to false. Leaving them set to nil inherits the upstream default.

Fixes #1434.

jeremy avatar Mar 17 '21 22:03 jeremy

@jeremy this change worked for us at GitHub with net-smtp version 0.2.1.

Thanks for finding this!

HParker avatar Sep 29 '21 21:09 HParker

@jeremy any update? this doesn't affect just Ruby 3, but even older Rubies with a recent net-smtp gem

without this patch, there's no way to disable starttls_auto, even with explicit settings:

default:
  email_delivery:
    delivery_method: :smtp
    smtp_settings:
      tls: false
      enable_starttls_auto: false
      ....

ahorek avatar Feb 18 '22 17:02 ahorek

This broke our rails app when upgrading. We need to send mail through a local mailer with Ubuntu's self-signed snake oil certificate. It was working for a long time with enable_starttls_auto: false until it broke.

As a workaround, removing the enable_starttls_auto setting at all (thus keeping TLS transmission) but just disabling the certificate validation with openssl_verify_mode: OpenSSL::SSL::VERIFY_NONE was helping us (but will not help if you have other reasons to avoid TLS than a bogus certificate).

However, this was quite hard to discover :-(

When I use the settings enable_starttls_auto: false, openssl_verify_mode: OpenSSL::SSL::VERIFY_NONE, it does perform TLS transmission and fails with state=error: certificate verify failed (self signed certificate) (OpenSSL::SSL::SSLError), which is clearly wrong in my eyes.

You can simply build an SMTP server with unverifiable cert to test against using this Dockerfile:

FROM ubuntu:bionic

RUN apt-get update && apt-get install -y postfix && rm -rf /var/lib/apt/lists/*
RUN echo 'mynetworks = 0.0.0.0/8, 127.0.0.0/8' >> /etc/postfix/main.cf
RUN echo 'smtp_use_tls = yes' >> /etc/postfix/main.cf
CMD ["postfix", "start-fg"]

hoeni avatar Mar 14 '22 10:03 hoeni

+1

seb-sykio avatar Mar 16 '22 11:03 seb-sykio

with rails 7 on ruby 2.7 i'm using this patch to get around the issue:

module DisableStarttls
  def build_smtp_session
    super.tap do |smtp|
      unless settings[:enable_starttls_auto]
        if smtp.respond_to?(:disable_starttls)
          smtp.disable_starttls
        end
      end
    end
  end
end

Mail::SMTP.prepend DisableStarttls

source

glaszig avatar Mar 16 '22 22:03 glaszig

@mikel just a reminder, could we include this bug-fix into the 2.8 release? I can rebase if necessary, thanks!

ahorek avatar Apr 21 '22 13:04 ahorek

Thanks @ahorek . Bumping this. It’s having a cascading effect on products which are using this - like discourse.

rboy1 avatar May 27 '22 15:05 rboy1

@hoeni's workaround helped us two. This is how it looks like in production code:

diff --git a/config/environments/production.rb b/config/environments/production.rb
index c1c2f442b..f215f49ce 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -43,10 +43,10 @@ Fairfood::Application.configure do
 
   config.action_mailer.raise_delivery_errors = true
 
-  # Disable unnecessary TLS for local connection
+  # Disable TLS verification for local connection
   config.action_mailer.smtp_settings = {
     address: 'localhost',
-    enable_starttls_auto: false,
+    openssl_verify_mode: OpenSSL::SSL::VERIFY_NONE,
   }
 
   # Enable threaded mode

It avoids hostname "localhost" does not match the server certificate (OpenSSL::SSL::SSLError).

mkllnk avatar Jun 30 '22 06:06 mkllnk

I forked 2-8-stable, added jeremy's patch and fixed "disable_starttls" flag. You can clone it from here: https://github.com/GChuf/mail/

After that, build and install the gem locally: _cd mail gem build mail.gemspec gem install mail -v2.8.0.pat_ch

GChuf avatar Oct 07 '22 09:10 GChuf

Hey there @ahorek please go ahead and do a new PR with this working against master and I'll release 2.9 shortly.

mikel avatar Dec 03 '22 09:12 mikel

Fixed by https://github.com/mikel/mail/pull/1536

eval avatar Jan 03 '24 14:01 eval