v0.89.0: app-email plaintext-only listener (:2526) + split-From mapping

Gap 1: third shim listener :2526, plaintext, does NOT advertise STARTTLS (TLSConfig nil) — for opportunistic-STARTTLS clients with no cert-skip (cal.com, nextcloud). Gap 2: SMTPMapping tls_mode (picks port 2525/2526/2465) + from_domain_var (split local-part + domain for nextcloud's MAIL_FROM_ADDRESS/MAIL_DOMAIN). Default keeps existing apps on 2525. Hub untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 13:13:40 +02:00
parent 6692a2f631
commit a405505e81
8 changed files with 252 additions and 47 deletions
@@ -40,11 +40,12 @@ type Config struct {
 type MailRelayConfig struct {
 	// Enabled is a hard kill-switch: false disables the shim regardless of the runtime
 	// toggle. nil/true → the runtime app-email toggle decides. (Operational override only.)
-	Enabled     *bool    `yaml:"enabled"`
-	PlainListen string   `yaml:"plain_listen"` // plaintext+STARTTLS, default ":2525"
-	TLSListen   string   `yaml:"tls_listen"`   // implicit-TLS, default ":2465"
-	ShimHost    string   `yaml:"shim_host"`    // app-network DNS name of the controller; default "felhom-controller"
-	FromDomains []string `yaml:"from_domains"` // From-header allowlist; default ["felhom.eu"]
+	Enabled          *bool    `yaml:"enabled"`
+	PlainListen      string   `yaml:"plain_listen"`        // plaintext+STARTTLS, default ":2525"
+	TLSListen        string   `yaml:"tls_listen"`          // implicit-TLS, default ":2465"
+	PlainNoTLSListen string   `yaml:"plain_no_tls_listen"` // plaintext-only, no STARTTLS, default ":2526"
+	ShimHost         string   `yaml:"shim_host"`           // app-network DNS name of the controller; default "felhom-controller"
+	FromDomains      []string `yaml:"from_domains"`        // From-header allowlist; default ["felhom.eu"]
 }

 // HardEnabled reports the operational kill-switch (default on unless explicitly false).
@@ -66,7 +67,7 @@ type LocalAPIConfig struct {
 // backup. Runs only when the local API is configured (a provisioned guest). MaxQuiesce bounds the
 // app downtime — the controller unquiesces no matter what once it elapses.
 type QuiesceConfig struct {
-	Enabled    *bool  `yaml:"enabled"`      // nil/true → on when local API configured; false → off
+	Enabled      *bool  `yaml:"enabled"`              // nil/true → on when local API configured; false → off
 	PollInterval string `yaml:"poll_interval"`        // /backup/due check cadence (default "5m")
 	StatusPoll   string `yaml:"status_poll_interval"` // /backup/status poll while quiesced (default "10s")
 	MaxQuiesce   string `yaml:"max_quiesce_duration"` // hard downtime bound (default "30m")
@@ -103,7 +104,7 @@ type PathsConfig struct {

 type WebConfig struct {
 	Listen        string `yaml:"listen"`
-	SetupListen   string `yaml:"setup_listen"`   // Plain HTTP listener for setup wizard (only active during setup mode)
+	SetupListen   string `yaml:"setup_listen"` // Plain HTTP listener for setup wizard (only active during setup mode)
 	PasswordHash  string `yaml:"password_hash"`
 	SessionSecret string `yaml:"session_secret"`
 }
@@ -185,9 +186,9 @@ type LoggingConfig struct {
 }

 type AssetsConfig struct {
-	SourceURL    string `yaml:"source_url"`     // Only used during build, not runtime
-	SyncEnabled  bool   `yaml:"sync_enabled"`   // Download assets from Hub API
-	SyncSchedule string `yaml:"sync_schedule"`  // Daily sync time (HH:MM), default "05:00"
+	SourceURL    string `yaml:"source_url"`    // Only used during build, not runtime
+	SyncEnabled  bool   `yaml:"sync_enabled"`  // Download assets from Hub API
+	SyncSchedule string `yaml:"sync_schedule"` // Daily sync time (HH:MM), default "05:00"
 }

 type HubConfig struct {
@@ -320,6 +321,7 @@ func applyDefaults(cfg *Config) {
 	d(&cfg.Quiesce.MaxQuiesce, "30m")
 	d(&cfg.MailRelay.PlainListen, ":2525")
 	d(&cfg.MailRelay.TLSListen, ":2465")
+	d(&cfg.MailRelay.PlainNoTLSListen, ":2526")
 	d(&cfg.MailRelay.ShimHost, "felhom-controller")
 	if len(cfg.MailRelay.FromDomains) == 0 {
 		cfg.MailRelay.FromDomains = []string{"felhom.eu"}
@@ -212,11 +212,12 @@ func TestSanitizeReason(t *testing.T) {
 func TestLifecycle_StartStopIdempotent(t *testing.T) {
 	fwd := &fakeForwarder{status: 200}
 	lc := NewLifecycle(Options{
-		PlainAddr: "127.0.0.1:0",
-		TLSAddr:   "127.0.0.1:0",
-		Policy:    NewPolicy([]string{"felhom.eu"}),
-		Forwarder: fwd,
-		Logger:    quietLogger(),
+		PlainAddr:      "127.0.0.1:0",
+		TLSAddr:        "127.0.0.1:0",
+		PlainNoTLSAddr: "127.0.0.1:0",
+		Policy:         NewPolicy([]string{"felhom.eu"}),
+		Forwarder:      fwd,
+		Logger:         quietLogger(),
 	})
 	if lc.Running() {
 		t.Fatal("should not be running before Apply")
@@ -257,14 +258,63 @@ func TestLifecycle_StartStopIdempotent(t *testing.T) {
 	}
 }

+// Gap-1: the :2526 listener must NOT advertise STARTTLS (TLSConfig nil), while :2525 must.
+// Proven both at the config level and over a real EHLO.
+func TestServer_PlainNoTLSListener_NoSTARTTLS(t *testing.T) {
+	fwd := &fakeForwarder{status: 200}
+	s, err := New(Options{
+		PlainAddr:      "127.0.0.1:0",
+		TLSAddr:        "127.0.0.1:0",
+		PlainNoTLSAddr: "127.0.0.1:0",
+		Policy:         NewPolicy([]string{"felhom.eu"}),
+		Forwarder:      fwd,
+		Logger:         quietLogger(),
+	})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	// Config-level: STARTTLS is advertised iff TLSConfig != nil.
+	if s.plainNoTLSSrv.TLSConfig != nil {
+		t.Fatal(":2526 server must have TLSConfig==nil (so STARTTLS is not advertised)")
+	}
+	if s.plainSrv.TLSConfig == nil {
+		t.Fatal(":2525 server must keep TLSConfig (STARTTLS advertised)")
+	}
+	if err := s.Start(); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	defer s.Close()
+
+	// Real EHLO: :2526 must NOT offer STARTTLS; :2525 must.
+	advertises := func(addr string) bool {
+		c, err := netsmtp.Dial(addr)
+		if err != nil {
+			t.Fatalf("dial %s: %v", addr, err)
+		}
+		defer c.Close()
+		if err := c.Hello("test.local"); err != nil {
+			t.Fatalf("EHLO %s: %v", addr, err)
+		}
+		ok, _ := c.Extension("STARTTLS")
+		return ok
+	}
+	if advertises(s.PlainNoTLSAddr()) {
+		t.Error(":2526 must NOT advertise STARTTLS over EHLO")
+	}
+	if !advertises(s.PlainAddr()) {
+		t.Error(":2525 must advertise STARTTLS over EHLO")
+	}
+}
+
 func TestServer_EndToEnd_STARTTLS(t *testing.T) {
 	fwd := &fakeForwarder{status: 200}
 	s, err := New(Options{
-		PlainAddr: "127.0.0.1:0",
-		TLSAddr:   "127.0.0.1:0",
-		Policy:    NewPolicy([]string{"felhom.eu"}),
-		Forwarder: fwd,
-		Logger:    quietLogger(),
+		PlainAddr:      "127.0.0.1:0",
+		TLSAddr:        "127.0.0.1:0",
+		PlainNoTLSAddr: "127.0.0.1:0",
+		Policy:         NewPolicy([]string{"felhom.eu"}),
+		Forwarder:      fwd,
+		Logger:         quietLogger(),
 	})
 	if err != nil {
 		t.Fatalf("New: %v", err)
@@ -39,22 +39,25 @@ const forwardTimeout = 30 * time.Second

 // Options configures the shim.
 type Options struct {
-	PlainAddr   string    // plaintext + STARTTLS listener (default ":2525")
-	TLSAddr     string    // implicit-TLS listener (default ":2465")
-	ServiceName string    // CN/SAN of the self-signed cert + SMTP greeting (e.g. "felhom-controller")
-	Policy      *Policy   // From-domain allowlist (required)
-	Forwarder   Forwarder // hub forwarder (required)
-	Logger      *log.Logger
+	PlainAddr      string    // plaintext + STARTTLS listener (default ":2525")
+	TLSAddr        string    // implicit-TLS listener (default ":2465")
+	PlainNoTLSAddr string    // plaintext-only listener, STARTTLS NOT advertised (default ":2526")
+	ServiceName    string    // CN/SAN of the self-signed cert + SMTP greeting (e.g. "felhom-controller")
+	Policy         *Policy   // From-domain allowlist (required)
+	Forwarder      Forwarder // hub forwarder (required)
+	Logger         *log.Logger
 }

-// Server runs the two SMTP listeners.
+// Server runs the three SMTP listeners.
 type Server struct {
-	opts     Options
-	tlsConf  *tls.Config
-	plainSrv *smtp.Server
-	tlsSrv   *smtp.Server
-	plainLn  net.Listener
-	tlsLn    net.Listener
+	opts          Options
+	tlsConf       *tls.Config
+	plainSrv      *smtp.Server
+	tlsSrv        *smtp.Server
+	plainNoTLSSrv *smtp.Server
+	plainLn       net.Listener
+	tlsLn         net.Listener
+	plainNoTLSLn  net.Listener
 }

 // New builds the shim and its self-signed cert. It does not bind sockets — call Start.
@@ -71,6 +74,9 @@ func New(opts Options) (*Server, error) {
 	if opts.TLSAddr == "" {
 		opts.TLSAddr = ":2465"
 	}
+	if opts.PlainNoTLSAddr == "" {
+		opts.PlainNoTLSAddr = ":2526"
+	}
 	if opts.ServiceName == "" {
 		opts.ServiceName = "felhom-controller"
 	}
@@ -107,6 +113,19 @@ func New(opts Options) (*Server, error) {
 	s.tlsSrv.ReadTimeout = 60 * time.Second
 	s.tlsSrv.WriteTimeout = 60 * time.Second

+	// :2526 — plaintext, STARTTLS NOT advertised (TLSConfig stays nil ⇒ go-smtp omits the
+	// STARTTLS capability from EHLO). For clients that opportunistically upgrade to STARTTLS
+	// whenever it's offered AND then validate the cert with no skip-verify knob (cal.com /
+	// Nodemailer, nextcloud / Symfony Mailer): with no offer, they never attempt TLS. Accepted
+	// posture: plaintext on the single-tenant app Docker bridge only (never host/internet).
+	s.plainNoTLSSrv = smtp.NewServer(be)
+	s.plainNoTLSSrv.Addr = opts.PlainNoTLSAddr
+	s.plainNoTLSSrv.Domain = opts.ServiceName
+	s.plainNoTLSSrv.AllowInsecureAuth = true
+	s.plainNoTLSSrv.MaxMessageBytes = maxMessageBytes
+	s.plainNoTLSSrv.ReadTimeout = 60 * time.Second
+	s.plainNoTLSSrv.WriteTimeout = 60 * time.Second
+
 	return s, nil
 }

@@ -123,10 +142,17 @@ func (s *Server) Start() error {
 		pl.Close()
 		return fmt.Errorf("mailrelay: listen TLS %s: %w", s.opts.TLSAddr, err)
 	}
-	s.plainLn, s.tlsLn = pl, tl
+	pn, err := net.Listen("tcp", s.opts.PlainNoTLSAddr)
+	if err != nil {
+		pl.Close()
+		tl.Close()
+		return fmt.Errorf("mailrelay: listen %s: %w", s.opts.PlainNoTLSAddr, err)
+	}
+	s.plainLn, s.tlsLn, s.plainNoTLSLn = pl, tl, pn

 	s.opts.Logger.Printf("[INFO] [mailrelay] plaintext+STARTTLS listener on %s", pl.Addr())
 	s.opts.Logger.Printf("[INFO] [mailrelay] implicit-TLS listener on %s", tl.Addr())
+	s.opts.Logger.Printf("[INFO] [mailrelay] plaintext-only (no STARTTLS) listener on %s", pn.Addr())

 	go func() {
 		if err := s.plainSrv.Serve(pl); err != nil && !isClosedErr(err) {
@@ -138,6 +164,11 @@ func (s *Server) Start() error {
 			s.opts.Logger.Printf("[ERROR] [mailrelay] implicit-TLS listener stopped: %v", err)
 		}
 	}()
+	go func() {
+		if err := s.plainNoTLSSrv.Serve(pn); err != nil && !isClosedErr(err) {
+			s.opts.Logger.Printf("[ERROR] [mailrelay] plaintext-only listener stopped: %v", err)
+		}
+	}()
 	return nil
 }

@@ -157,7 +188,15 @@ func (s *Server) TLSAddr() string {
 	return s.opts.TLSAddr
 }

-// Close stops both listeners.
+// PlainNoTLSAddr returns the bound plaintext-only (no-STARTTLS) address.
+func (s *Server) PlainNoTLSAddr() string {
+	if s.plainNoTLSLn != nil {
+		return s.plainNoTLSLn.Addr().String()
+	}
+	return s.opts.PlainNoTLSAddr
+}
+
+// Close stops all listeners.
 func (s *Server) Close() error {
 	var err error
 	if s.plainSrv != nil {
@@ -170,6 +209,11 @@ func (s *Server) Close() error {
 			err = e
 		}
 	}
+	if s.plainNoTLSSrv != nil {
+		if e := s.plainNoTLSSrv.Close(); e != nil {
+			err = e
+		}
+	}
 	return err
 }

@@ -47,11 +47,20 @@ func (m *Manager) smtpEnv(meta *Metadata, appEmailEnabled bool) []string {
 	if security == "" {
 		security = "starttls"
 	}
+	port := shimPortForTLSMode(sm.TLSMode)

 	out := []string{
 		fmt.Sprintf("%s=%s", sm.HostVar, host),
-		fmt.Sprintf("%s=%s", sm.PortVar, "2525"),
-		fmt.Sprintf("%s=%s@%s", sm.FromVar, local, fromDomain),
+		fmt.Sprintf("%s=%s", sm.PortVar, port),
+	}
+	// From: single full-address var (default) or split local-part + domain (nextcloud-style).
+	if sm.FromDomainVar != "" {
+		out = append(out,
+			fmt.Sprintf("%s=%s", sm.FromVar, local),
+			fmt.Sprintf("%s=%s", sm.FromDomainVar, fromDomain),
+		)
+	} else {
+		out = append(out, fmt.Sprintf("%s=%s@%s", sm.FromVar, local, fromDomain))
 	}
 	if sm.SecurityVar != "" {
 		out = append(out, fmt.Sprintf("%s=%s", sm.SecurityVar, security))
@@ -67,6 +76,20 @@ func (m *Manager) smtpEnv(meta *Metadata, appEmailEnabled bool) []string {
 	return out
 }

+// shimPortForTLSMode maps a mapping's tls_mode to the shim listener port. Default (empty/"starttls")
+// keeps existing apps on :2525 (plaintext + STARTTLS); "plaintext" → :2526 (STARTTLS NOT advertised,
+// for opportunistic-upgrade clients with no cert-skip); "implicit-tls" → :2465.
+func shimPortForTLSMode(mode string) string {
+	switch mode {
+	case "plaintext":
+		return "2526"
+	case "implicit-tls":
+		return "2465"
+	default: // "" or "starttls"
+		return "2525"
+	}
+}
+
 // fromDisplayName picks the From display name: the household-set name (settings) wins,
 // else the app's display name.
 func (m *Manager) fromDisplayName(meta *Metadata) string {
@@ -143,6 +143,61 @@ func TestSMTPEnv_MealieMapping(t *testing.T) {
 	}
 }

+// §10: port chosen by tls_mode. plaintext→2526, starttls/empty→2525, implicit-tls→2465.
+func TestSMTPEnv_PortByTLSMode(t *testing.T) {
+	m := newMailManager(t, true, "")
+	cases := map[string]string{"": "2525", "starttls": "2525", "plaintext": "2526", "implicit-tls": "2465"}
+	for mode, wantPort := range cases {
+		meta := vaultwardenMeta()
+		meta.SMTPMapping.TLSMode = mode
+		got := envMap(m.smtpEnv(meta, true))
+		if got["SMTP_PORT"] != wantPort {
+			t.Errorf("tls_mode=%q → SMTP_PORT=%q, want %q", mode, got["SMTP_PORT"], wantPort)
+		}
+	}
+}
+
+// §10 companion: shimPortForTLSMode must actually branch on the mode (not always 2525).
+func TestShimPortForTLSMode(t *testing.T) {
+	if shimPortForTLSMode("plaintext") == shimPortForTLSMode("starttls") {
+		t.Fatal("companion: plaintext and starttls must map to DIFFERENT ports (gap-1 fix)")
+	}
+	if shimPortForTLSMode("plaintext") != "2526" {
+		t.Fatalf("plaintext must be 2526, got %q", shimPortForTLSMode("plaintext"))
+	}
+}
+
+// §10: split-From. from_domain_var set → two keys (local + domain); unset → single <local>@<domain>.
+func TestSMTPEnv_SplitFrom(t *testing.T) {
+	m := newMailManager(t, true, "")
+	meta := &Metadata{
+		Slug: "nextcloud", DisplayName: "Nextcloud",
+		SMTPMapping: &SMTPMapping{
+			HostVar: "SMTP_HOST", PortVar: "SMTP_PORT",
+			SecurityVar: "SMTP_SECURE", SecurityValue: "",
+			FromVar: "MAIL_FROM_ADDRESS", FromDomainVar: "MAIL_DOMAIN", FromLocal: "nextcloud",
+			TLSMode: "plaintext",
+		},
+	}
+	got := envMap(m.smtpEnv(meta, true))
+	if got["MAIL_FROM_ADDRESS"] != "nextcloud" {
+		t.Errorf("split From local = %q, want bare 'nextcloud'", got["MAIL_FROM_ADDRESS"])
+	}
+	if got["MAIL_DOMAIN"] != "felhom.eu" {
+		t.Errorf("split From domain = %q, want 'felhom.eu'", got["MAIL_DOMAIN"])
+	}
+	if got["SMTP_PORT"] != "2526" {
+		t.Errorf("nextcloud SMTP_PORT = %q, want 2526 (plaintext)", got["SMTP_PORT"])
+	}
+
+	// Companion: a mapping WITHOUT from_domain_var must produce the single full address (not split).
+	single := vaultwardenMeta()
+	gotS := envMap(m.smtpEnv(single, true))
+	if gotS["SMTP_FROM"] != "vaultwarden@felhom.eu" {
+		t.Errorf("single-From mapping = %q, want 'vaultwarden@felhom.eu'", gotS["SMTP_FROM"])
+	}
+}
+
 func TestMetadata_SMTPMappingParse(t *testing.T) {
 	dir := t.TempDir()
 	yml := `display_name: Vaultwarden
@@ -11,11 +11,11 @@ import (

 // Metadata holds app information parsed from .felhom.yml.
 type Metadata struct {
-	DisplayName    string                `yaml:"display_name" json:"display_name"`
-	Description    string                `yaml:"description" json:"description"`
-	Category       string                `yaml:"category" json:"category"`
-	Subdomain      string                `yaml:"subdomain" json:"subdomain"`
-	Slug           string                `yaml:"slug" json:"slug"`
+	DisplayName string `yaml:"display_name" json:"display_name"`
+	Description string `yaml:"description" json:"description"`
+	Category    string `yaml:"category" json:"category"`
+	Subdomain   string `yaml:"subdomain" json:"subdomain"`
+	Slug        string `yaml:"slug" json:"slug"`
 	// OpenPath is appended to the app's public URL for the "Megnyitás" (open) link, for apps whose UI
 	// isn't at "/" (e.g. Gokapi → "/admin"). Empty = bare root. Must start with "/".
 	OpenPath       string                `yaml:"open_path,omitempty" json:"open_path,omitempty"`
@@ -50,6 +50,16 @@ type SMTPMapping struct {
 	FromNameVar   string            `yaml:"from_name_var" json:"from_name_var"`   // env key for the From display name (optional)
 	FromLocal     string            `yaml:"from_local" json:"from_local"`         // From local-part (defaults to the app slug)
 	Extra         map[string]string `yaml:"extra" json:"extra"`                   // fixed extra env (accept-invalid-cert flags, etc.)
+	// TLSMode selects which shim listener the app is pointed at (which fixes the port):
+	//   "" / "starttls" → :2525 (plaintext + STARTTLS advertised) — the default; existing apps unchanged.
+	//   "plaintext"     → :2526 (plaintext, STARTTLS NOT advertised) — for clients that opportunistically
+	//                     upgrade to STARTTLS and can't skip cert verification (cal.com, nextcloud).
+	//   "implicit-tls"  → :2465 (whole connection TLS).
+	TLSMode string `yaml:"tls_mode" json:"tls_mode"`
+	// FromDomainVar: for apps that SPLIT the From into local-part + domain env vars (nextcloud:
+	// MAIL_FROM_ADDRESS + MAIL_DOMAIN). When set, the controller injects FromVar=<local> and
+	// FromDomainVar=<allowlisted-domain> separately instead of FromVar=<local>@<domain>.
+	FromDomainVar string `yaml:"from_domain_var" json:"from_domain_var"`
 }

 // HasSMTPMapping reports whether this app declares a usable email mapping.
@@ -61,8 +71,8 @@ func (m *Metadata) HasSMTPMapping() bool {
 // credential from a file inside the running container. The file path is catalog-defined (trusted).
 // Verification stays catalog-driven so any future self-seeding app can reuse the mechanism.
 type InitialCredentials struct {
-	File   string `yaml:"file" json:"file"`                 // path INSIDE the container
-	Format string `yaml:"format" json:"format"`             // "json" | "regex" | "plain"
+	File   string `yaml:"file" json:"file"`     // path INSIDE the container
+	Format string `yaml:"format" json:"format"` // "json" | "regex" | "plain"
 	// Container overrides which container to read from; empty → the stack's main container.
 	Container string `yaml:"container,omitempty" json:"container,omitempty"`
 	// json format: which keys hold the username/password (password_key required; username optional).