Fix and provide sane defaults for SMTP #686

Oneric · 2024-02-12T21:46:52Z

Oneric commented

2024-02-12 21:46:52 +00:00

OTP’s default SSL/TLS settings are rather restricitive and in particular do not use sysem CA certs. This lead to #660 once verification failures became an error by default in OTP 26.

This root-cause and some somehwat-working workaround were already identified in the Swoosh issue linked from #660. Acutally choosing the right config or getting this to work with in-database config isn’t immediately obvious though.
Now that some time passed it seems like Swoosh does (atm) not intend to provide their own defaults over OTP’s and it appears neither does gen_smtp, therefore lets set defaults appropiate for “sending emails via an external SMTP server” outselves.

Additionally complexity comes from gen_smtp using its tls_options exclusively for STARTTLS upgrades and not for direct TLS connections (and appending TLS-specific options to sockopts when using plain TCP being a badarg error).

This change deals with all of that and should make things simple again for admins.

I tested this with a direct SSL mailserver and verfied that if trying to connect via plain/STARTTLS the connection times out as expected rather then running into errors early (the mail server drops plain traffic coming to the SMTP port). I.e. a config like this:

config :pleroma, Pleroma.Emails.Mailer,
  enabled: true,
  adapter: Swoosh.Adapters.SMTP,
  relay: "some.domain",
  username: "user",
  password: "thebestpassword",
  port: 456,
  ssl: true,
  tls: :if_available, # <-- not required and anything but :always works
  auth: :always

It should also work for servers limited to STARTTLS (or a theoretical plain-TCP-only server) but i can’t personally test this. If someone could check and report back that it works, it’d be great ^^ (i.e. a server which needs or at least accepts: ssl: false, tls: :always)

OTP’s default SSL/TLS settings are rather restricitive and in particular do not use sysem CA certs. This lead to https://akkoma.dev/AkkomaGang/akkoma/issues/660 once verification failures became an error by default in OTP 26. This root-cause and some somehwat-working workaround were already identified in the Swoosh issue linked from #660. Acutally choosing the right config or getting this to work with in-database config isn’t immediately obvious though. Now that some time passed it seems like Swoosh does (atm) not intend to provide their own defaults over OTP’s and it appears neither does gen_smtp, therefore lets set defaults appropiate for “sending emails via an external SMTP server” outselves. Additionally complexity comes from gen_smtp using its `tls_options` exclusively for `STARTTLS` upgrades and not for direct TLS connections *(and appending TLS-specific options to `sockopts` when using plain TCP being a `badarg` error)*. This change deals with all of that and should make things simple again for admins. --- I tested this with a direct SSL mailserver and verfied that if trying to connect via plain/`STARTTLS` the connection times out as expected rather then running into errors early (the mail server drops plain traffic coming to the SMTP port). I.e. a config like this: ```exs config :pleroma, Pleroma.Emails.Mailer, enabled: true, adapter: Swoosh.Adapters.SMTP, relay: "some.domain", username: "user", password: "thebestpassword", port: 456, ssl: true, tls: :if_available, # <-- not required and anything but :always works auth: :always ``` It _should_ also work for servers limited to `STARTTLS` *(or a theoretical plain-TCP-only server)* but i can’t personally test this. If someone could check and report back that it works, it’d be great ^^ (i.e. a server which needs or at least accepts: `ssl: false, tls: :always`)

👍 1

Oneric added 1 commit 2024-02-12 21:46:52 +00:00

Provide sane defaults for SMTP

ci/woodpecker/pr/lint Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

ci/woodpecker/pr/build-arm64 unknown status

Details

ci/woodpecker/pr/build-amd64 unknown status

Details

ci/woodpecker/pr/docs unknown status

Details

192480093c

OTP’s default SSL/TLS settings are rather restricitive
and in particular do not use system CA certs.
In our case using system CA certs is virtually always desired
and the lack of it leads to non-obvious errors. Manually configuring
system CA certs from in-database config also isn’t straightforward.

Furthermore, gen_smtp uses a different set of connection options
for direct SSL/TLS and a later TLS upgrade providing additional
confusion and complexity in how to configure this.

Thus provide some suitable defaults for sending SMTP emails.
Everything can still be overriden by admins if necessary.

Note: defaults are not appended when validating the config
in hopes of improving the error message (as the required relay key
is already accessed to generate defaults for optional fields)

Fixes: #660

norm commented

2024-02-17 20:30:50 +00:00

This fixed the issue on akko.wtf.

👀 1

floatingghost reviewed 2024-02-19 13:33:45 +00:00

lib/pleroma/emails/mailer.ex

					
				@ -62,3 +62,2 @@

				  defp parse_config(config) do

				    Swoosh.Mailer.parse_config(@otp_app, __MODULE__, @mailer_config, config)

				  defp ensure_charlist(input) do

floatingghost commented

2024-02-19 13:33:45 +00:00

minor stylistic thing that i'm not going to make you change

this would probably have been cleaner as two function definitions

defp ensure_charlist(input) when is_binary(i), do: String.to_charlist(input)
defp ensure_charlist(input) when is_list(i), do: i

something to bear in mind for future

minor stylistic thing that i'm not going to make you change this would probably have been cleaner as two function definitions ```elixir defp ensure_charlist(input) when is_binary(i), do: String.to_charlist(input) defp ensure_charlist(input) when is_list(i), do: i ``` something to bear in mind for future

floatingghost commented

2024-02-19 13:39:12 +00:00

tests pass, fixes for someone that had the issue and does not break my previously working setup

all good, thanks!

tests pass, fixes for someone that had the issue and does not break my previously working setup all good, thanks!

floatingghost merged commit 551ae69541 into develop

2024-02-19 13:39:16 +00:00

floatingghost deleted branch smtp-defaults

2024-02-19 13:39:16 +00:00

floatingghost referenced this pull request from a commit

2024-02-19 13:39:17 +00:00

Merge pull request 'Fix and provide sane defaults for SMTP' (#686) from Oneric/akkoma:smtp-defaults into develop

Sign in to join this conversation.

No reviewers

No milestone

No project

No assignees

3 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma#686