Fix TLS config #473

Merged
simon merged 3 commits from bugfix/fix-tls-config into main 2026-03-16 15:04:35 +01:00
6 changed files with 162 additions and 54 deletions
Showing only changes of commit e8f27690a1 - Show all commits

View file

@ -130,6 +130,8 @@ lib/
│ ├── constants.ex # Application constants (member_fields, custom_field_prefix, vereinfacht_required_member_fields) │ ├── constants.ex # Application constants (member_fields, custom_field_prefix, vereinfacht_required_member_fields)
│ ├── application.ex # OTP application │ ├── application.ex # OTP application
│ ├── mailer.ex # Email mailer │ ├── mailer.ex # Email mailer
│ ├── smtp/
│ │ └── config_builder.ex # SMTP adapter opts (TLS/sockopts); used by runtime.exs and Mailer
│ ├── release.ex # Release tasks │ ├── release.ex # Release tasks
│ ├── repo.ex # Database repository │ ├── repo.ex # Database repository
│ ├── secrets.ex # Secret management │ ├── secrets.ex # Secret management

View file

@ -226,11 +226,7 @@ if config_env() == :prod do
# SMTP configuration from environment variables (overrides base adapter in prod). # SMTP configuration from environment variables (overrides base adapter in prod).
# When SMTP_HOST is set, configure Swoosh to use the SMTP adapter at boot time. # When SMTP_HOST is set, configure Swoosh to use the SMTP adapter at boot time.
# If SMTP is configured only via Settings (Admin UI), the mailer builds the config # If SMTP is configured only via Settings (Admin UI), the mailer builds the config
# per-send at runtime using Mv.Config.smtp_*() helpers. # per-send at runtime using Mv.Mailer.smtp_config/0 (which uses the same Mv.Smtp.ConfigBuilder).
#
# TLS/SSL options (tls_options, sockopts) are duplicated here and in Mv.Mailer.smtp_config/0
# because boot config must be set in this file; the Mailer uses the same logic for
# Settings-only config. Keep verify behaviour in sync (see SMTP_VERIFY_PEER below).
smtp_host_env = System.get_env("SMTP_HOST") smtp_host_env = System.get_env("SMTP_HOST")
if smtp_host_env && String.trim(smtp_host_env) != "" do if smtp_host_env && String.trim(smtp_host_env) != "" do
@ -263,26 +259,15 @@ if config_env() == :prod do
verify_mode = if smtp_verify_peer, do: :verify_peer, else: :verify_none verify_mode = if smtp_verify_peer, do: :verify_peer, else: :verify_none
base_smtp_opts = [
adapter: Swoosh.Adapters.SMTP,
relay: String.trim(smtp_host_env),
port: smtp_port_env,
username: System.get_env("SMTP_USERNAME"),
password: smtp_password_env,
ssl: smtp_ssl_mode == "ssl",
tls: if(smtp_ssl_mode == "tls", do: :always, else: :never),
auth: :always,
tls_options: [verify: verify_mode]
]
# Port 465: pass verify in sockopts for initial ssl:connect. Port 587: do not (gen_tcp rejects it).
smtp_opts = smtp_opts =
if smtp_ssl_mode == "ssl" do Mv.Smtp.ConfigBuilder.build_opts(
Keyword.put(base_smtp_opts, :sockopts, verify: verify_mode) host: String.trim(smtp_host_env),
else port: smtp_port_env,
base_smtp_opts username: System.get_env("SMTP_USERNAME"),
end password: smtp_password_env,
|> Enum.reject(fn {_k, v} -> is_nil(v) end) ssl_mode: smtp_ssl_mode,
verify_mode: verify_mode
)
config :mv, Mv.Mailer, smtp_opts config :mv, Mv.Mailer, smtp_opts
end end

View file

@ -105,7 +105,7 @@ By default, TLS certificate verification is relaxed (`verify_none`) so self-sign
- **ENV (prod):** Set `SMTP_VERIFY_PEER=true` (or `1`/`yes`) when configuring SMTP via environment variables in `config/runtime.exs`. This sets `config :mv, :smtp_verify_peer` and is used for both boot-time and per-send config. - **ENV (prod):** Set `SMTP_VERIFY_PEER=true` (or `1`/`yes`) when configuring SMTP via environment variables in `config/runtime.exs`. This sets `config :mv, :smtp_verify_peer` and is used for both boot-time and per-send config.
- **Default:** `false` (verify_none) for backward compatibility and internal/self-signed certs. - **Default:** `false` (verify_none) for backward compatibility and internal/self-signed certs.
Verify mode is set in `tls_options` for port 587 (STARTTLS). For port 465 (implicit SSL), the initial connection is `ssl:connect`, so we also pass `sockopts: [verify: verify_mode]` so the SSL handshake uses the same mode. For 587 we must not pass `verify` in sockopts—gen_tcp is used first and rejects it (ArgumentError). The logic is in `config/runtime.exs` (boot) and `Mv.Mailer.smtp_config/0` (Settings-only); keep in sync. Verify mode is set in `tls_options` for port 587 (STARTTLS). For port 465 (implicit SSL), the initial connection is `ssl:connect`, so we also pass `sockopts: [verify: verify_mode]` so the SSL handshake uses the same mode. For 587 we must not pass `verify` in sockopts—gen_tcp is used first and rejects it (ArgumentError). The logic lives in `Mv.Smtp.ConfigBuilder.build_opts/1` (single source of truth), used by `config/runtime.exs` (boot) and `Mv.Mailer.smtp_config/0` (Settings-only).
--- ---

View file

@ -31,6 +31,7 @@ defmodule Mv.Mailer do
import Swoosh.Email import Swoosh.Email
use Gettext, backend: MvWeb.Gettext, otp_app: :mv use Gettext, backend: MvWeb.Gettext, otp_app: :mv
alias Mv.Smtp.ConfigBuilder
require Logger require Logger
# Simple format check for test-email recipient only (e.g. allows a@b.c). Not for strict RFC validation. # Simple format check for test-email recipient only (e.g. allows a@b.c). Not for strict RFC validation.
@ -100,40 +101,19 @@ defmodule Mv.Mailer do
@spec smtp_config() :: keyword() @spec smtp_config() :: keyword()
def smtp_config do def smtp_config do
if Mv.Config.smtp_configured?() and not boot_smtp_configured?() do if Mv.Config.smtp_configured?() and not boot_smtp_configured?() do
host = Mv.Config.smtp_host()
port = Mv.Config.smtp_port() || 587
username = Mv.Config.smtp_username()
password = Mv.Config.smtp_password()
ssl_mode = Mv.Config.smtp_ssl() || "tls"
verify_mode = verify_mode =
if Application.get_env(:mv, :smtp_verify_peer, false), if Application.get_env(:mv, :smtp_verify_peer, false),
do: :verify_peer, do: :verify_peer,
else: :verify_none else: :verify_none
base_opts = [ ConfigBuilder.build_opts(
adapter: Swoosh.Adapters.SMTP, host: Mv.Config.smtp_host(),
relay: host, port: Mv.Config.smtp_port() || 587,
port: port, username: Mv.Config.smtp_username(),
ssl: ssl_mode == "ssl", password: Mv.Config.smtp_password(),
tls: if(ssl_mode == "tls", do: :always, else: :never), ssl_mode: Mv.Config.smtp_ssl() || "tls",
auth: :always, verify_mode: verify_mode
username: username, )
password: password,
# tls_options: used for STARTTLS (587). For 465, gen_smtp uses sockopts for initial ssl:connect.
tls_options: [verify: verify_mode]
]
# Port 465: initial connection is ssl:connect; pass verify in sockopts so server cert is not required.
# Port 587: initial connection is gen_tcp; sockopts must NOT contain verify (gen_tcp rejects it).
opts =
if ssl_mode == "ssl" do
Keyword.put(base_opts, :sockopts, verify: verify_mode)
else
base_opts
end
opts |> Enum.reject(fn {_k, v} -> is_nil(v) end)
else else
[] []
end end

View file

@ -0,0 +1,58 @@
defmodule Mv.Smtp.ConfigBuilder do
@moduledoc """
Builds Swoosh/gen_smtp SMTP adapter options from connection parameters.
Single source of truth for TLS/sockopts logic (port 587 vs 465):
- Port 587 (STARTTLS): `gen_tcp` is used first; `sockopts` must NOT contain `:verify`.
- Port 465 (implicit SSL): initial connection is `ssl:connect`; `sockopts` must contain `:verify`.
Used by `config/runtime.exs` (boot-time ENV) and `Mv.Mailer.smtp_config/0` (Settings-only).
"""
@doc """
Builds the keyword list of Swoosh SMTP adapter options.
Options (keyword list):
- `:host` (required) relay hostname
- `:port` (required) port number (e.g. 587 or 465)
- `:ssl_mode` (required) `"tls"` or `"ssl"`
- `:verify_mode` (required) `:verify_peer` or `:verify_none`
- `:username` (optional)
- `:password` (optional)
Nil values are stripped from the result.
"""
@spec build_opts(keyword()) :: keyword()
def build_opts(opts) do
host = Keyword.fetch!(opts, :host)
port = Keyword.fetch!(opts, :port)
username = Keyword.get(opts, :username)
password = Keyword.get(opts, :password)
ssl_mode = Keyword.fetch!(opts, :ssl_mode)
verify_mode = Keyword.fetch!(opts, :verify_mode)
base_opts = [
adapter: Swoosh.Adapters.SMTP,
relay: host,
port: port,
username: username,
password: password,
ssl: ssl_mode == "ssl",
tls: if(ssl_mode == "tls", do: :always, else: :never),
auth: :always,
# tls_options: used for STARTTLS (587). For 465, gen_smtp uses sockopts for initial ssl:connect.
tls_options: [verify: verify_mode]
]
# Port 465: initial connection is ssl:connect; pass verify in sockopts.
# Port 587: initial connection is gen_tcp; sockopts must NOT contain verify (gen_tcp rejects it).
opts =
if ssl_mode == "ssl" do
Keyword.put(base_opts, :sockopts, verify: verify_mode)
else
base_opts
end
Enum.reject(opts, fn {_k, v} -> is_nil(v) end)
end
end

View file

@ -0,0 +1,83 @@
defmodule Mv.Smtp.ConfigBuilderTest do
@moduledoc """
Unit tests for Mv.Smtp.ConfigBuilder.build_opts/1.
Ensures the single source of truth for SMTP opts correctly handles:
- Port 587 (TLS): sockopts must NOT contain :verify (gen_tcp rejects it).
- Port 465 (SSL): sockopts MUST contain :verify for initial ssl:connect.
"""
use ExUnit.Case, async: true
alias Mv.Smtp.ConfigBuilder
describe "build_opts/1" do
test "port 587 (TLS): does not include :verify in sockopts" do
opts =
ConfigBuilder.build_opts(
host: "smtp.example.com",
port: 587,
username: "user",
password: "secret",
ssl_mode: "tls",
verify_mode: :verify_none
)
sockopts = Keyword.get(opts, :sockopts, [])
refute Keyword.has_key?(sockopts, :verify), "for 587 sockopts must not contain :verify"
assert Keyword.get(opts, :tls) == :always
assert Keyword.get(opts, :ssl) == false
end
test "port 465 (SSL): includes :verify in sockopts" do
opts =
ConfigBuilder.build_opts(
host: "smtp.example.com",
port: 465,
username: "user",
password: "secret",
ssl_mode: "ssl",
verify_mode: :verify_peer
)
sockopts = Keyword.get(opts, :sockopts, [])
assert Keyword.has_key?(sockopts, :verify), "for 465 sockopts must contain :verify"
assert Keyword.get(sockopts, :verify) == :verify_peer
assert Keyword.get(opts, :ssl) == true
assert Keyword.get(opts, :tls) == :never
end
test "strips nil values" do
opts =
ConfigBuilder.build_opts(
host: "smtp.example.com",
port: 587,
username: nil,
password: nil,
ssl_mode: "tls",
verify_mode: :verify_none
)
refute Keyword.has_key?(opts, :username)
refute Keyword.has_key?(opts, :password)
assert Keyword.get(opts, :relay) == "smtp.example.com"
end
test "includes adapter and required keys" do
opts =
ConfigBuilder.build_opts(
host: "mail.example.com",
port: 587,
username: "u",
password: "p",
ssl_mode: "tls",
verify_mode: :verify_none
)
assert Keyword.get(opts, :adapter) == Swoosh.Adapters.SMTP
assert Keyword.get(opts, :relay) == "mail.example.com"
assert Keyword.get(opts, :port) == 587
assert Keyword.get(opts, :auth) == :always
assert Keyword.get(opts, :tls_options) == [verify: :verify_none]
end
end
end