Use MidiSmtpServer for incoming SMTP Service - Enables AUTH and STARTTLS #386

TomFreudenberg · 2018-11-17T00:31:33Z

allows optional TLS and AUTH for SMTP Clients
SSL cert will be generated temporarily
all auth credentials will be accepted

I hope this will make it in your project. It helps to test and work also with clients that send only via SSL/TLS or needs an authentification.

Thanks for any comments
Tom

- allows optional TLS and AUTH for SMTP Clients - SSL cert will be generated temporarily - all auth credentials will be accepted

TomFreudenberg · 2018-11-18T00:24:44Z

Hi,

I recognized today that even the above checks had passed, the daemonized mode didn't worked anymore for SMTP Server. I have changed Process.daemon into Process.detach to make it work again. Now checks and my tests worked fine.

Thanks for your feedback
Tom

TomFreudenberg · 2018-11-18T00:27:20Z

lib/mail_catcher.rb

@@ -219,7 +227,10 @@ def run! options=nil
          else
            puts "*** MailCatcher is now running as a daemon that cannot be quit."
          end
-          Process.daemon
+          # when daemonize redirect standard input, standard output and standard error to /dev/null
+          $stdin.reopen "/dev/null"


re-routing stdin, stdout and stderr was former handled by Process.daemon.
Read at https://ruby-doc.org/core-2.1.0/Process.html#method-c-daemon
Second paramter (noclose = nil)

TomFreudenberg · 2018-11-18T00:29:27Z

lib/mail_catcher.rb

+    if options[:daemon] && pid = fork
+      Process.detach(pid)
+      # wait a second to make sure that all output is send before exit
+      sleep 2


If not sleeping a second the output from command line is printed too fast so that the output of starting mailcatcher will follow later. That confuses the user.

TomFreudenberg · 2018-11-25T17:25:29Z

Hi @sj26 Samuel,

a short check about this PR, is there anything missing from my side or some open tasks?

Thanks for your feedback
Tom

sj26 · 2018-11-27T01:13:36Z

Sorry for the delay, @TomFreudenberg — this looks amazing!

I've seen MidiSmtpServer before and didn't quite feel comfortable subbing it in, but it does look like this fixes a whole host of issues. I haven't had time to dig in and give this a good test to see if it's all working right, but it does read well.

I'm not sure I understand why daemonization has been changed?

TomFreudenberg · 2018-11-27T01:36:01Z

Hi @sj26 Samuel,

Thanks for your feedback.

About testing: As far as I have made a number of checks, mailcatcher worked as expected (Foreground and Background). For MidiSmtpServer I have created a lot of specs and integration tests (https://github.com/4commerce-technologies-AG/midi-smtp-server/tree/master/test). Currently there is no bug issue addressed to MidiSmtpServer.

How may I support you on that task to make sure that stabilty of mailcatcher is not affected?

TomFreudenberg · 2018-11-27T02:01:30Z

Hi @sj26 Samuel,

I'm not sure I understand why daemonization has been changed?

During my updates from 1st commit I had only run mailcatcher in foreground mode and everything worked as expected. After pushing the PR, I realized that running mailcatcher in background mode will stop Input/Output of MidiSmtpServer.

I am not sure for 100% if this is only due process.daemon or also cased by loop of EventMachine.run. But while checking the internet and looking for the issue, I read a number of threads about daemonizing ruby TCPServer instances. They all did it by process.detach. So I changed that to seen best practise approach.

The problem was located at (https://github.com/4commerce-technologies-AG/midi-smtp-server/blob/master/lib/midi-smtp-server.rb#L387) tcp_server.accept method which will hang when using process.daemon but not process.detach.

After 2nd commit, mailcatcher worked smoothly in foreground and background mode.

sj26 · 2018-11-27T02:10:56Z

Okay, I'll look into that. I'd prefer not to introduce a sleep into the code, too. Probably moving the manual detach down to where it was before should solve that — by then the output should have been printed, stdout is already sync, the ports would have been bound, and it should be safe to detach immediately.

sj26 · 2018-11-27T02:11:23Z

This seems to break catchmail:

$ bundle exec catchmail < examples/htmlmail
OpenSSL::SSL::SSLError: hostname "127.0.0.1" does not match the server certificate
  /usr/local/rbenv/versions/2.5.3/lib/ruby/2.5.0/openssl/ssl.rb:394:in `post_connection_check'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/2.5.0/net/smtp.rb:586:in `tlsconnect'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/2.5.0/net/smtp.rb:561:in `do_start'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/2.5.0/net/smtp.rb:518:in `start'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/mail-2.7.1/lib/mail/network/delivery_methods/smtp.rb:109:in `start_smtp_session'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/mail-2.7.1/lib/mail/network/delivery_methods/smtp.rb:100:in `deliver!'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/mail-2.7.1/lib/mail/message.rb:2159:in `do_delivery'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/mail-2.7.1/lib/mail/message.rb:262:in `deliver'
  /Users/sj26/Projects/mailcatcher/bin/catchmail:71:in `<top (required)>'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/bin/catchmail:23:in `load'
  /usr/local/rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/bin/catchmail:23:in `<top (required)>'

sj26 · 2018-11-27T02:11:45Z

I'm concerned self-signed starttls is going to break a lot of clients for the same reason.

sj26 · 2018-11-27T02:21:50Z

Ah, midi-smtp-server is not eventmachine?

TomFreudenberg · 2018-11-27T02:55:51Z

I'm concerned self-signed starttls is going to break a lot of clients for the same reason.

Wondering that ... normally that isn't a concern ... I will check what happened

TomFreudenberg · 2018-11-27T02:56:30Z

Ah, midi-smtp-server is not eventmachine?

Yes, it is not

TomFreudenberg · 2018-11-27T03:15:20Z

I'm concerned self-signed starttls is going to break a lot of clients for the same reason.

Hi Samuel, I got that. Just based naming. In case of creating the the cert CN, I just use localhost.local as CN.

catchmail will connect to 127.0.0.1 and expect that as CN.

OpenSSL::X509::Name.new [['CN', 'localhost.local'], ['CN', 'localhost'], ['CN', '127.0.0.1']]

does fix that. I will check how to sign a CN for all given IP addresses or hosts.

Will comeback soon.

Thanks for feedback
Tom

sj26 · 2018-11-27T03:17:43Z

I'm more concerned about CA validation — self-signed certs aren't going to pass unless they're explicitly added to the system's certificate store. And many clients which are working now because they don't attempt to starttls will break on an upgrade to this version if they do automatically try to starttls but don't accept self-signed certs.

If we want to add starttls then I think it should be optional, and off by default. At least for now.

TomFreudenberg · 2018-11-27T03:41:57Z

CA validation is as far as I know not a concern, because Mail-Clients do not check the CA. The just check the CN for called address.

About nature of STARTTLS

Opportunistic encryption

STARTTLS is opportunistic encryption.

Opportunistic encryption just changes the attack requirement from simple passive observation to risky active interception.

So, bring it on with STARTTLS, just don't bother with getting a CA issued certificate and know that a targeted MitM–capable attacker can totally intercept your mail on the wire.

By the way, keep in mind that SMTP encryption is just protection on transit and it does not do anything to authenticate the sender.

From exim.org

A self-signed certificate made in this way is sufficient for testing, and may be adequate for all your requirements if you are mainly interested in encrypting transfers, and not in secure identification.

https://www.exim.org/exim-html-current/doc/html/spec_html/ch-encrypted_smtp_connections_using_tlsssl.html

But as you like, I can disable by default the optional STARTTLS

opts[:tls_mode] = :TLS_OPTIONAL

into

opts[:tls_mode] = :TLS_FORBIDDEN

What do think about an additional runtime parameter like --startssl (e.g.) to enable STARTTLS

TomFreudenberg · 2018-11-27T03:44:18Z

If we want to add starttls then I think it should be optional

Be aware: currently it is already optional. It will shown as a capabilty by EHLO command and the client already may decide how to send

In that case: catchmail is aware of optionally STARTTLS but does enable it by default.

sj26 · 2018-11-27T03:53:32Z

No I mean we should require an option to advertise it

TomFreudenberg · 2018-11-27T03:56:00Z

No I mean we should require an option to advertise it

Allright, so by additional command line option? --startssl ?

TomFreudenberg · 2018-11-27T03:56:55Z

I'd prefer not to introduce a sleep into the code, too. Probably moving the manual detach down to where it was before should solve that

Allright, I checked that - old position - does work without sleep 👍

TomFreudenberg · 2018-11-29T11:58:24Z

Hi @sj26 Samuel,

I have update the PR as discussed.

No sleep statement anymore
STARTTLS optionally feature with option --smtp-tls

I will also make an improvement to midi-smtp-server self signed certifactes to auto-include all necessary CNs like on 'localhost' also to sign for '127.0.0.1' and '::1' (if IPv6). This PR does not depend on that.

Hope all is fine for your by now.

Cheers
Tom

jackcasey · 2019-12-06T07:35:16Z

This looks good!
We need LOGIN auth and TLS to use mailcatcher with Auth0 :) Looking at just running this branch for the time being...

TomFreudenberg · 2019-12-06T11:00:20Z

Hi Jack @jackcasey

This branch should work again, I have made an upstream pull to get latest mailcatcher/master branch.

TomFreudenberg · 2019-12-06T11:01:58Z

Hi Samuel @sj26

The integration checks fail for Ruby 2.2 but it looks in Travis log that this does not depend of MidiSmtpServer but MailCatcher itself. Is this correct or do I have to change something else?

Thanks for your feedback
Tom

Update: --------------------------

This is from Travis log / looks same on your master branch build log

$ gem install bundler

ERROR: Error installing bundler:

bundler requires Ruby version >= 2.3.0.

The command "gem install bundler" failed and exited with 1 during .

TomFreudenberg · 2019-12-06T11:18:13Z

Hi Samuel @sj26

in file https://github.com/sj26/mailcatcher/blob/master/lib/mail_catcher/smtp.rb you state out: allow multiple mail-from commands

class MailCatcher::Smtp < EventMachine::Protocols::SmtpServer
  # We override EM's mail from processing to allow multiple mail-from commands
  # per [RFC 2821](http://tools.ietf.org/html/rfc2821#section-4.1.1.2)
  def process_mail_from sender
    if @state.include? :mail_from
      @state -= [:mail_from, :rcpt, :data]
      receive_reset
    end

    super
  end

But as far as I know and I always have and see this implemented, this NOT allowed.

When looking at referenced RFC section http://tools.ietf.org/html/rfc2821#section-4.1.1.2 it says:

This command (MAIL) is used to initiate a mail transaction
In general, the MAIL command may be sent only when no mail transaction is in progress

IMHO the changes to smtp.rb are not conform to RFC when changing EM standards.

sj26 · 2019-12-08T00:35:14Z

@TomFreudenberg iirc the problem was that EM's SMTP wasn't allowing multiple MAIL commands even after the first mail transaction had finished — it's state machine wasn't sophisticated enough. This was a hack to make multiple mail transactions work. But it looks like this was fixed.

sj26 · 2019-12-08T01:05:47Z

Honestly, I'm not convinced by removing Process.daemon, and I'd prefer to stay within EventMachine instead of starting to deal with multiple threads and adding more gems. EM's SMTP is capable of AUTH and STARTTLS.

jackcasey · 2019-12-09T09:05:13Z

Thanks for your quick help Tom!

We got this up and running in AWS but it turns out that Auth0 need for the cert used to not be self signed 🙁 I guess because they don't really differentiate between using an SMTP server for testing vs for production.

We can run this at one of our subdomains and get a cert for it, but is being able to supply the cert to mailcatcher to use for the TLS complicated?

TomFreudenberg · 2019-12-09T13:54:29Z

Honestly, I'm not convinced by removing Process.daemon, and I'd prefer to stay within EventMachine instead of starting to deal with multiple threads and adding more gems. EM's SMTP is capable of AUTH and STARTTLS.

Hi Samuel @sj26

does this mean we will close up here unmerged and drop that PR off?

Wondering why issue #142 still open when EM has already SSL support?

Anyway - its your project and you decide ...

Cheers
Tom

TomFreudenberg · 2019-12-09T14:06:49Z

Hi Jack @jackcasey

there is no really effort to apply any valid cert to MidiSmtpServer.

As written in section https://github.com/4commerce-technologies-AG/midi-smtp-server#certificates you may specify any valid certificates by options like

server = MySmtpd.new(2525, '127.0.0.1', 4, { tls_mode: :TLS_OPTIONAL, tls_cert_path: 'cert.pem', tls_key_path: 'key.pem' })

So just append two lines in file lib/mail_catcher.rb for initialization after line 208 (https://github.com/sj26/mailcatcher/pull/386/files#diff-7a1db141a354d739ef15c7a782305a4aR208)

midi_smtp_server_opts[:tls_cert_path] = 'your-cert.pem'
midi_smtp_server_opts[:tls_key_path] = 'your-key.pem'

That will use your own certificate.

TomFreudenberg · 2019-12-09T14:12:22Z

@jackcasey @sj26

Hi Jack,

After Samuels decision about that PR, maybe I will fork this project until features are also established using EM. For then, I will append command line options to specifiy certs and keys.

My above comment to you is just a quick tip to check wether its fit for you.

Cheers
Tom

TomFreudenberg · 2020-01-12T22:04:26Z

Hi @jackcasey

any news about your Auth0 question?

TomFreudenberg · 2020-01-12T22:05:49Z

Hi @sj26

Samuel, would you be so kind and leave just a feedback to my last comment #386 (comment)

Thanks in advance
Tom

sj26 · 2020-01-13T00:48:03Z

does this mean we will close up here unmerged and drop that PR off?

Yes, sorry.

Wondering why issue #142 still open when EM has already SSL support?

EM's had SSL support since that feature request was added. I haven't had time/interest in implementing it, and nobody else has tried to my knowledge. A PR adding basic self-signed tls support using EM's SSL would be accepted.

jackcasey · 2020-01-13T00:54:31Z

@TomFreudenberg Thankyou for your guidance.
Sorry about the radio silence over the xmas and new year time. FYI we were spending too long fiddling with provisioning our own mailcatcher server, and having to now re-sign a new cert and ship it along with the server we just decided to bite the bullet and pay for Mailtrap which was very easy to use and is currently working well for us.

sj26 · 2020-01-13T01:08:02Z

If you're hosting mailcatcher somewhere in a production-like scenario, which it wasn't designed for, then you might like to investigate mailhog:

https://github.com/mailhog/MailHog

akostadinov · 2022-01-04T23:05:09Z

FYI em implementation in #494

Use MidiSmtpServer for incoming SMTP Service

240e829

- allows optional TLS and AUTH for SMTP Clients - SSL cert will be generated temporarily - all auth credentials will be accepted

This was referenced Nov 17, 2018

SSL/TLS connection #142

Open

SMTP Transcript Tab #128

Open

Reject 8bit messages #327

Closed

Change Process.daemon with Process.detach

9434417

TomFreudenberg commented Nov 18, 2018

View reviewed changes

TomFreudenberg added 2 commits November 27, 2018 05:13

Remove sleep but replace process.daemon by process.detach

f840d4b

Allow optional STARTTLS with option --smtp-tls

43fd4f2

TomFreudenberg mentioned this pull request Nov 29, 2018

STARTTLS Enhance CN generator for self signed certificate 4commerce-technologies-AG/midi-smtp-server#25

Closed

TomFreudenberg added 2 commits December 6, 2019 11:40

Enable internationalization options from MidiSmtpServer by default

af09d89

Forward with changes from upstream master branch

064cac4

Forward with changes from upstream master branch

ce8e73d

sj26 closed this Jan 13, 2020

Use MidiSmtpServer for incoming SMTP Service - Enables AUTH and STARTTLS #386

Use MidiSmtpServer for incoming SMTP Service - Enables AUTH and STARTTLS #386

Conversation

TomFreudenberg commented Nov 17, 2018

TomFreudenberg commented Nov 18, 2018 • edited Loading

TomFreudenberg Nov 18, 2018

Choose a reason for hiding this comment

TomFreudenberg Nov 18, 2018

Choose a reason for hiding this comment

TomFreudenberg commented Nov 25, 2018

sj26 commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018 • edited Loading

TomFreudenberg commented Nov 27, 2018 • edited Loading

sj26 commented Nov 27, 2018

sj26 commented Nov 27, 2018

sj26 commented Nov 27, 2018

sj26 commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018

sj26 commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018

Opportunistic encryption

From exim.org

TomFreudenberg commented Nov 27, 2018

sj26 commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018

TomFreudenberg commented Nov 27, 2018

TomFreudenberg commented Nov 29, 2018

jackcasey commented Dec 6, 2019

TomFreudenberg commented Dec 6, 2019

TomFreudenberg commented Dec 6, 2019 • edited Loading

TomFreudenberg commented Dec 6, 2019

sj26 commented Dec 8, 2019

sj26 commented Dec 8, 2019

jackcasey commented Dec 9, 2019

TomFreudenberg commented Dec 9, 2019 • edited Loading

TomFreudenberg commented Dec 9, 2019

TomFreudenberg commented Dec 9, 2019 • edited Loading

TomFreudenberg commented Jan 12, 2020

TomFreudenberg commented Jan 12, 2020

sj26 commented Jan 13, 2020

jackcasey commented Jan 13, 2020

sj26 commented Jan 13, 2020

akostadinov commented Jan 4, 2022

TomFreudenberg commented Nov 18, 2018 •

edited

Loading

TomFreudenberg commented Nov 27, 2018 •

edited

Loading

TomFreudenberg commented Nov 27, 2018 •

edited

Loading

TomFreudenberg commented Dec 6, 2019 •

edited

Loading

TomFreudenberg commented Dec 9, 2019 •

edited

Loading

TomFreudenberg commented Dec 9, 2019 •

edited

Loading