From 81acd0f42d4108e508f2820f98767bd57d81ddfc Mon Sep 17 00:00:00 2001 From: Alan Denniston Date: Thu, 9 Jul 2026 23:35:41 -0400 Subject: [PATCH 1/3] feat(dkim): add outbound DKIM signing Sign outbound mail on the remote SMTP delivery path to improve deliverability to Gmail/Yahoo/Outlook. Until now the plugin system only verified DKIM/DMARC/ARC on inbound mail; there was no signing path. - New internal/dkim package wrapping github.com/emersion/go-msgauth/dkim (the same library the verification code references) for both RSA and Ed25519 keys. - [dkim] TOML config section: enabled, relaxed/relaxed canonicalization by default, and per-domain selector / private_key_path / optional headers_to_sign (sensible default header set). - Private-key permission validation on load: keys readable/writable by group or other (0077 bits) are rejected, matching the repo's file-security posture. - Signing wired into SMTPDeliveryHandler, applied once before recipients are grouped by domain. Signing is idempotent (skips a message already signed for the domain) so delivery retries never double-sign. Local LMTP delivery is not signed. - Signing domain selected from the envelope-from domain, falling back to the From header domain (bounces); unconfigured domains are skipped with a debug log and delivered unsigned. - Tests: sign+verify round-trip for RSA and Ed25519 via the verification path, config parsing, key-permission rejection, and retry-no-double-sign (both at the signer and delivery-handler level). - Docs: docs/dkim-signing.md plus a [dkim] example in config/elemta.toml.example. --- cmd/elemta/commands/server.go | 3 + config/elemta.toml.example | 27 +- docs/configuration.md | 9 + docs/dkim-signing.md | 123 ++++++++ go.mod | 1 + go.sum | 2 + internal/config/config.go | 4 + internal/config/dkim_config_test.go | 77 +++++ internal/dkim/signer.go | 403 +++++++++++++++++++++++++++ internal/dkim/signer_test.go | 336 ++++++++++++++++++++++ internal/queue/delivery_dkim_test.go | 129 +++++++++ internal/queue/delivery_handler.go | 59 ++++ internal/smtp/config.go | 4 + internal/smtp/server.go | 16 ++ 14 files changed, 1192 insertions(+), 1 deletion(-) create mode 100644 docs/dkim-signing.md create mode 100644 internal/config/dkim_config_test.go create mode 100644 internal/dkim/signer.go create mode 100644 internal/dkim/signer_test.go create mode 100644 internal/queue/delivery_dkim_test.go diff --git a/cmd/elemta/commands/server.go b/cmd/elemta/commands/server.go index a7bd4c0..50f3442 100644 --- a/cmd/elemta/commands/server.go +++ b/cmd/elemta/commands/server.go @@ -180,6 +180,9 @@ func startServer() { // Map delivery config smtpConfig.Delivery = cfg.Delivery + // Map DKIM outbound signing config + smtpConfig.DKIM = cfg.DKIM + // Map resources config (for Valkey integration) smtpConfig.Resources = cfg.Resources diff --git a/config/elemta.toml.example b/config/elemta.toml.example index 3cb3b82..20fa021 100644 --- a/config/elemta.toml.example +++ b/config/elemta.toml.example @@ -95,5 +95,30 @@ listen_addr = "0.0.0.0:8080" metrics_path = "/metrics" health_path = "/health" +# Outbound DKIM signing configuration. +# Signs mail on the remote SMTP delivery path (not local LMTP delivery) to +# improve deliverability to Gmail/Yahoo/Outlook. Publish the matching public +# key at ._domainkey. in DNS. +# +# Private keys must NOT be readable or writable by group or other: +# chmod 600 /etc/elemta/dkim/example.com.key +[dkim] +enabled = true +# Canonicalization defaults to relaxed/relaxed (recommended). Override with +# "simple" only if you have a specific reason. +header_canonicalization = "relaxed" +body_canonicalization = "relaxed" + + # One block per signing domain. The signing domain is chosen by matching the + # envelope-from (or, when empty, the From header) domain against these entries. + [[dkim.domains]] + domain = "example.com" + selector = "mail" + private_key_path = "/etc/elemta/dkim/example.com.key" + # Optional per-domain override of the signed header set. When omitted, a + # sensible default is used (From, To, Cc, Subject, Date, Message-ID, + # MIME-Version, Content-Type, Reply-To, In-Reply-To, References). + # headers_to_sign = ["From", "To", "Subject", "Date", "Message-ID"] + [server] -# session_timeout = "5m" # Session idle timeout (default: 5m) \ No newline at end of file +# session_timeout = "5m" # Session idle timeout (default: 5m) \ No newline at end of file diff --git a/docs/configuration.md b/docs/configuration.md index 02d66c4..4e52706 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -147,6 +147,14 @@ file = "/var/log/elemta/elemta.log" - `[tls].key_file` - optional `[tls.letsencrypt]` +### DKIM (outbound signing) + +- `[dkim].enabled` +- `[dkim].header_canonicalization` / `[dkim].body_canonicalization` (default `relaxed`) +- `[[dkim.domains]]` blocks with `domain`, `selector`, `private_key_path`, optional `headers_to_sign` + +See [DKIM Signing](dkim-signing.md) for key generation, DNS setup, and details. + --- ## Compatibility notes @@ -160,6 +168,7 @@ file = "/var/log/elemta/elemta.log" ## Related docs - [Installation](installation.md) +- [DKIM Signing](dkim-signing.md) - [SMTP Server](smtp_server.md) - [Queue Management](queue_management.md) - [Queue Backend Runbook](queue-backend-runbook.md) diff --git a/docs/dkim-signing.md b/docs/dkim-signing.md new file mode 100644 index 0000000..0fd0a85 --- /dev/null +++ b/docs/dkim-signing.md @@ -0,0 +1,123 @@ +# Outbound DKIM Signing + +Elemta can DKIM-sign outbound mail so that receiving providers +(Gmail, Yahoo, Outlook, etc.) can verify the message originated from a domain +you control. Signing measurably improves deliverability and is effectively a +prerequisite for bulk sending to the large mailbox providers. + +This document covers the outbound **signing** path. Inbound DKIM/DMARC/ARC +**verification** is handled separately by the plugin system. + +## How it works + +- Signing happens on the **remote SMTP delivery path** (delivery to a remote + MX), inside `SMTPDeliveryHandler`. Local LMTP delivery (e.g. to Dovecot) is + intentionally **not** signed, because a locally delivered copy never crosses + an administrative boundary where DKIM matters. +- A message is signed **once**, before recipients are grouped by domain, so the + exact same signed bytes are delivered to every recipient MX. +- Signing is **idempotent**. If a message already carries a `DKIM-Signature` + for the chosen signing domain, it is left untouched, so a delivery **retry + never double-signs**. +- The signing domain is selected by matching the **envelope-from** domain + against the configured domains. When the envelope-from is empty (for example, + a bounce/DSN with a null return path), the **From header** domain is used as a + fallback. If no key is configured for the resolved domain, the message is + delivered **unsigned** and a debug line is logged. +- If signing fails for any reason, the message is delivered **unsigned** (with a + warning log) rather than being failed outright. + +The signer reuses [`github.com/emersion/go-msgauth/dkim`](https://pkg.go.dev/github.com/emersion/go-msgauth/dkim), +the same library referenced by the verification code, so a message signed by +elemta verifies with the identical implementation. + +## Generating keys + +RSA (2048-bit is the widely interoperable choice): + +```sh +openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:2048 \ + -out /etc/elemta/dkim/example.com.key +chmod 600 /etc/elemta/dkim/example.com.key +``` + +Ed25519 (smaller signatures; publish an additional RSA selector for receivers +that do not yet support Ed25519): + +```sh +openssl genpkey -algorithm ed25519 -out /etc/elemta/dkim/example.com.ed25519.key +chmod 600 /etc/elemta/dkim/example.com.ed25519.key +``` + +Both PKCS#1 and PKCS#8 encoded keys are accepted. + +### Key file permissions + +Elemta refuses to start if a private key is readable or writable by group or +other (any of the `0077` permission bits set), matching the repo's file-security +posture. Keep keys at mode `0600` (or `0400`), owned by the elemta user. + +## Publishing the public key in DNS + +Extract the public key and publish it as a TXT record at +`._domainkey.`: + +```sh +openssl rsa -in /etc/elemta/dkim/example.com.key -pubout 2>/dev/null \ + | grep -v '^-----' | tr -d '\n' +``` + +Then create a DNS TXT record, e.g. for selector `mail` on `example.com`: + +``` +mail._domainkey.example.com. IN TXT "v=DKIM1; k=rsa; p=" +``` + +For Ed25519 use `k=ed25519` and the base64 of the raw 32-byte public key. + +## Configuration + +Add a top-level `[dkim]` section: + +```toml +[dkim] +enabled = true +header_canonicalization = "relaxed" # default: relaxed +body_canonicalization = "relaxed" # default: relaxed + + [[dkim.domains]] + domain = "example.com" + selector = "mail" + private_key_path = "/etc/elemta/dkim/example.com.key" + + [[dkim.domains]] + domain = "example.net" + selector = "s2026" + private_key_path = "/etc/elemta/dkim/example.net.key" + # Optional per-domain override of the signed headers. + headers_to_sign = ["From", "To", "Subject", "Date", "Message-ID"] +``` + +### Fields + +| Field | Scope | Description | +|-------|-------|-------------| +| `enabled` | `[dkim]` | Master switch. When `false` (or the section is absent), no outbound signing occurs. | +| `header_canonicalization` | `[dkim]` | `relaxed` (default) or `simple`. | +| `body_canonicalization` | `[dkim]` | `relaxed` (default) or `simple`. | +| `domain` | `[[dkim.domains]]` | The signing domain (the `d=` tag). | +| `selector` | `[[dkim.domains]]` | The DKIM selector (the `s=` tag). | +| `private_key_path` | `[[dkim.domains]]` | Path to the PEM RSA or Ed25519 private key. | +| `headers_to_sign` | `[[dkim.domains]]` | Optional. Overrides the default signed header set for this domain. | + +Default signed headers (when `headers_to_sign` is omitted): `From`, `To`, `Cc`, +`Subject`, `Date`, `Message-ID`, `MIME-Version`, `Content-Type`, `Reply-To`, +`In-Reply-To`, `References`. + +## Verifying it works + +Send a message to a Gmail account and check **Show original**; the DKIM result +should read `PASS`. Command-line tooling such as `opendkim-testkey`, +`dig TXT ._domainkey.`, or an auto-responder like +`check-auth@verifier.port25.com` can confirm both the DNS record and live +signature validity. diff --git a/go.mod b/go.mod index e4d2abc..60e6f17 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.25.12 require ( github.com/BurntSushi/toml v1.6.0 github.com/bradfitz/gomemcache v0.0.0-20260422231931-4d751bb6e37c + github.com/emersion/go-msgauth v0.7.0 github.com/go-ldap/ldap/v3 v3.4.14 github.com/go-sql-driver/mysql v1.10.0 github.com/google/uuid v1.6.0 diff --git a/go.sum b/go.sum index 225906b..e9b7097 100644 --- a/go.sum +++ b/go.sum @@ -23,6 +23,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/emersion/go-msgauth v0.7.0 h1:vj2hMn6KhFtW41kshIBTXvp6KgYSqpA/ZN9Pv4g1INc= +github.com/emersion/go-msgauth v0.7.0/go.mod h1:mmS9I6HkSovrNgq0HNXTeu8l3sRAAuQ9RMvbM4KU7Ck= github.com/go-asn1-ber/asn1-ber v1.5.8 h1:H9AZkK22UOmfX8J84ubyaZxKJZ3FMHVwn8swoMML7iQ= github.com/go-asn1-ber/asn1-ber v1.5.8/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0= github.com/go-ldap/ldap/v3 v3.4.14 h1:D6PYdEgsaVzsXyr6w/yDC06Ria4uUhWm+Rb+er8lfAs= diff --git a/internal/config/config.go b/internal/config/config.go index 3672af0..01f0b76 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/busybox42/elemta/internal/dkim" "github.com/busybox42/elemta/internal/runtimepaths" "github.com/busybox42/elemta/internal/smtp" @@ -217,6 +218,9 @@ type Config struct { // Delivery configuration Delivery *smtp.DeliveryConfig `toml:"delivery"` + // DKIM outbound signing configuration + DKIM *dkim.Config `toml:"dkim"` + // Metrics configuration Metrics *smtp.MetricsConfig `toml:"metrics"` diff --git a/internal/config/dkim_config_test.go b/internal/config/dkim_config_test.go new file mode 100644 index 0000000..13674e6 --- /dev/null +++ b/internal/config/dkim_config_test.go @@ -0,0 +1,77 @@ +package config + +import ( + "testing" + + toml "github.com/pelletier/go-toml/v2" +) + +// TestDKIMConfigParsing verifies the [dkim] TOML section unmarshals into the +// Config struct, including per-domain selectors, key paths and header overrides. +func TestDKIMConfigParsing(t *testing.T) { + const cfgTOML = ` +hostname = "mail.example.com" + +[dkim] +enabled = true +header_canonicalization = "relaxed" +body_canonicalization = "relaxed" + + [[dkim.domains]] + domain = "example.com" + selector = "mail" + private_key_path = "/etc/elemta/dkim/example.com.key" + + [[dkim.domains]] + domain = "example.net" + selector = "s2026" + private_key_path = "/etc/elemta/dkim/example.net.key" + headers_to_sign = ["From", "Subject", "Date"] +` + + var cfg Config + if err := toml.Unmarshal([]byte(cfgTOML), &cfg); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + if cfg.DKIM == nil { + t.Fatal("DKIM section not parsed") + } + if !cfg.DKIM.Enabled { + t.Fatal("expected DKIM enabled") + } + if cfg.DKIM.HeaderCanonicalization != "relaxed" || cfg.DKIM.BodyCanonicalization != "relaxed" { + t.Fatalf("canonicalization = %q/%q", cfg.DKIM.HeaderCanonicalization, cfg.DKIM.BodyCanonicalization) + } + if len(cfg.DKIM.Domains) != 2 { + t.Fatalf("expected 2 domains, got %d", len(cfg.DKIM.Domains)) + } + + d0 := cfg.DKIM.Domains[0] + if d0.Domain != "example.com" || d0.Selector != "mail" || d0.PrivateKeyPath != "/etc/elemta/dkim/example.com.key" { + t.Fatalf("domain[0] parsed incorrectly: %+v", d0) + } + if len(d0.HeadersToSign) != 0 { + t.Fatalf("domain[0] should have no header override, got %v", d0.HeadersToSign) + } + + d1 := cfg.DKIM.Domains[1] + if d1.Domain != "example.net" || d1.Selector != "s2026" { + t.Fatalf("domain[1] parsed incorrectly: %+v", d1) + } + if want := []string{"From", "Subject", "Date"}; len(d1.HeadersToSign) != len(want) { + t.Fatalf("domain[1] header override = %v, want %v", d1.HeadersToSign, want) + } +} + +// TestDKIMConfigAbsentIsNil confirms that omitting the section leaves DKIM nil +// (signing disabled) rather than producing a partially populated struct. +func TestDKIMConfigAbsentIsNil(t *testing.T) { + var cfg Config + if err := toml.Unmarshal([]byte(`hostname = "mail.example.com"`), &cfg); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if cfg.DKIM != nil { + t.Fatalf("expected nil DKIM when section absent, got %+v", cfg.DKIM) + } +} diff --git a/internal/dkim/signer.go b/internal/dkim/signer.go new file mode 100644 index 0000000..83016c3 --- /dev/null +++ b/internal/dkim/signer.go @@ -0,0 +1,403 @@ +// Package dkim implements outbound DKIM signing for elemta. +// +// It reuses github.com/emersion/go-msgauth/dkim (the same library referenced by +// the verification path in the plugin package) so that a message signed here can +// be verified with the exact same code on the receiving side. RSA and Ed25519 +// keys are both supported because dkim.Sign accepts any crypto.Signer. +package dkim + +import ( + "bytes" + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "fmt" + "log/slog" + "os" + "strings" + "sync" + + "github.com/emersion/go-msgauth/dkim" +) + +// DefaultHeadersToSign is the set of headers signed when a signing domain does +// not override HeadersToSign. It intentionally includes the headers Gmail, +// Yahoo and Outlook expect for good deliverability. +var DefaultHeadersToSign = []string{ + "From", + "To", + "Cc", + "Subject", + "Date", + "Message-ID", + "MIME-Version", + "Content-Type", + "Reply-To", + "In-Reply-To", + "References", +} + +// signingHeaderName is the DKIM signature header. Presence of a signature from +// our own domain is used as the "already signed" guard so retries do not +// double-sign. +const signingHeaderName = "DKIM-Signature" + +// DomainConfig is the per-domain signing configuration. +type DomainConfig struct { + // Domain is the signing domain (the d= tag). Matched against the + // envelope-from / From header domain to select a key. + Domain string `toml:"domain"` + // Selector is the DKIM selector (the s= tag). + Selector string `toml:"selector"` + // PrivateKeyPath is the path to a PEM-encoded RSA or Ed25519 private key. + PrivateKeyPath string `toml:"private_key_path"` + // HeadersToSign optionally overrides the default header set for this domain. + HeadersToSign []string `toml:"headers_to_sign"` +} + +// Config is the top-level [dkim] configuration section. +type Config struct { + Enabled bool `toml:"enabled"` + // HeaderCanonicalization and BodyCanonicalization default to "relaxed". + HeaderCanonicalization string `toml:"header_canonicalization"` + BodyCanonicalization string `toml:"body_canonicalization"` + // Domains is the list of signing domains. + Domains []DomainConfig `toml:"domains"` +} + +// domainKey holds a loaded, validated signer for a single domain. +type domainKey struct { + selector string + signer crypto.Signer + hash crypto.Hash + headerKeys []string + headerCanon dkim.Canonicalization + bodyCanon dkim.Canonicalization +} + +// Signer signs outbound messages. It is safe for concurrent use. +type Signer struct { + logger *slog.Logger + // keys maps a lower-cased signing domain to its loaded key material. + keys map[string]domainKey + + mu sync.RWMutex +} + +// NewSigner builds a Signer from configuration. It loads and validates every +// configured key up-front (including file-permission checks) so that a +// misconfiguration fails fast at start-up rather than silently at delivery +// time. If cfg is nil or disabled, NewSigner returns (nil, nil) and callers +// should treat a nil *Signer as "signing disabled". +func NewSigner(cfg *Config, logger *slog.Logger) (*Signer, error) { + if cfg == nil || !cfg.Enabled { + return nil, nil + } + if logger == nil { + logger = slog.Default() + } + logger = logger.With("component", "dkim-signer") + + headerCanon, err := parseCanonicalization(cfg.HeaderCanonicalization) + if err != nil { + return nil, fmt.Errorf("dkim: header_canonicalization: %w", err) + } + bodyCanon, err := parseCanonicalization(cfg.BodyCanonicalization) + if err != nil { + return nil, fmt.Errorf("dkim: body_canonicalization: %w", err) + } + + s := &Signer{logger: logger, keys: make(map[string]domainKey)} + + for _, dc := range cfg.Domains { + domain := strings.ToLower(strings.TrimSpace(dc.Domain)) + if domain == "" { + return nil, fmt.Errorf("dkim: domain entry with empty domain") + } + if dc.Selector == "" { + return nil, fmt.Errorf("dkim: domain %q has empty selector", domain) + } + if dc.PrivateKeyPath == "" { + return nil, fmt.Errorf("dkim: domain %q has empty private_key_path", domain) + } + if _, exists := s.keys[domain]; exists { + return nil, fmt.Errorf("dkim: domain %q configured more than once", domain) + } + + signer, hash, err := loadPrivateKey(dc.PrivateKeyPath) + if err != nil { + return nil, fmt.Errorf("dkim: domain %q: %w", domain, err) + } + + headerKeys := dc.HeadersToSign + if len(headerKeys) == 0 { + headerKeys = DefaultHeadersToSign + } + + s.keys[domain] = domainKey{ + selector: dc.Selector, + signer: signer, + hash: hash, + headerKeys: headerKeys, + headerCanon: headerCanon, + bodyCanon: bodyCanon, + } + logger.Info("Loaded DKIM signing key", "domain", domain, "selector", dc.Selector) + } + + if len(s.keys) == 0 { + return nil, fmt.Errorf("dkim: enabled but no signing domains configured") + } + + return s, nil +} + +// Sign returns a DKIM-signed copy of content. The signing domain is selected by +// matching signingDomain (typically the envelope-from domain, falling back to +// the From header domain) against the configured domains. +// +// Behaviour: +// - If no key is configured for the domain, content is returned unchanged and +// a debug line is logged (not an error — the message is still deliverable). +// - If content already carries a DKIM-Signature from the selected domain, it +// is returned unchanged. This makes signing idempotent, so a delivery retry +// never double-signs. +// - A signing failure returns the original content and an error; callers may +// choose to deliver unsigned rather than fail the message. +func (s *Signer) Sign(content []byte, signingDomain string) ([]byte, error) { + if s == nil { + return content, nil + } + + domain := strings.ToLower(strings.TrimSpace(signingDomain)) + s.mu.RLock() + key, ok := s.keys[domain] + s.mu.RUnlock() + if !ok { + s.logger.Debug("No DKIM key configured for domain, skipping signing", "domain", domain) + return content, nil + } + + if hasSignatureForDomain(content, domain) { + s.logger.Debug("Message already DKIM-signed for domain, skipping", "domain", domain) + return content, nil + } + + opts := &dkim.SignOptions{ + Domain: domain, + Selector: key.selector, + Signer: key.signer, + Hash: key.hash, + HeaderCanonicalization: key.headerCanon, + BodyCanonicalization: key.bodyCanon, + HeaderKeys: key.headerKeys, + } + + var buf bytes.Buffer + if err := dkim.Sign(&buf, bytes.NewReader(content), opts); err != nil { + return content, fmt.Errorf("dkim: signing for domain %q failed: %w", domain, err) + } + + s.logger.Debug("Signed message with DKIM", "domain", domain, "selector", key.selector) + return buf.Bytes(), nil +} + +// HasKeyFor reports whether a signing key is configured for the given domain. +func (s *Signer) HasKeyFor(domain string) bool { + if s == nil { + return false + } + s.mu.RLock() + defer s.mu.RUnlock() + _, ok := s.keys[strings.ToLower(strings.TrimSpace(domain))] + return ok +} + +// hasSignatureForDomain scans the message header block for an existing +// DKIM-Signature whose d= tag matches domain. Only the header block (up to the +// first blank line) is inspected. Folded continuation lines are reassembled so +// a d= tag on a continuation line is still detected. +func hasSignatureForDomain(content []byte, domain string) bool { + headerEnd := bytes.Index(content, []byte("\r\n\r\n")) + if headerEnd == -1 { + if idx := bytes.Index(content, []byte("\n\n")); idx != -1 { + headerEnd = idx + } else { + headerEnd = len(content) + } + } + header := content[:headerEnd] + + lowerPrefix := strings.ToLower(signingHeaderName) + ":" + lines := strings.Split(strings.ReplaceAll(string(header), "\r\n", "\n"), "\n") + + var current strings.Builder + var fields []string + flush := func() { + if current.Len() > 0 { + fields = append(fields, current.String()) + current.Reset() + } + } + for _, line := range lines { + if line == "" { + continue + } + if line[0] == ' ' || line[0] == '\t' { + current.WriteString(" ") + current.WriteString(strings.TrimSpace(line)) + continue + } + flush() + current.WriteString(line) + } + flush() + + target := "d=" + domain + for _, f := range fields { + if !strings.HasPrefix(strings.ToLower(f), lowerPrefix) { + continue + } + // Normalise whitespace inside the DKIM-Signature tag list before + // comparing the d= tag. + normalized := strings.ToLower(strings.Join(strings.Fields(f), "")) + if strings.Contains(normalized, target+";") || strings.HasSuffix(normalized, target) { + return true + } + } + return false +} + +// loadPrivateKey reads, permission-checks and parses a PEM private key. It +// returns a crypto.Signer and the hash to use (SHA-256 for RSA/ECDSA; Ed25519 +// signs without a separate hash so crypto.Hash(0) is returned and go-msgauth +// handles the ed25519 special case internally). +func loadPrivateKey(path string) (crypto.Signer, crypto.Hash, error) { + if err := validateKeyPermissions(path); err != nil { + return nil, 0, err + } + + data, err := os.ReadFile(path) //nolint:gosec // path comes from operator config + if err != nil { + return nil, 0, fmt.Errorf("reading key: %w", err) + } + + block, _ := pem.Decode(data) + if block == nil { + return nil, 0, fmt.Errorf("no PEM block found in key file %s", path) + } + + signer, hash, err := parsePrivateKeyBlock(block) + if err != nil { + return nil, 0, fmt.Errorf("parsing key %s: %w", path, err) + } + return signer, hash, nil +} + +// parsePrivateKeyBlock parses a decoded PEM block into a crypto.Signer. +func parsePrivateKeyBlock(block *pem.Block) (crypto.Signer, crypto.Hash, error) { + // PKCS#1 (RSA-only). + if key, err := x509.ParsePKCS1PrivateKey(block.Bytes); err == nil { + return key, crypto.SHA256, nil + } + + // PKCS#8 (RSA, ECDSA, Ed25519). + if parsed, err := x509.ParsePKCS8PrivateKey(block.Bytes); err == nil { + switch key := parsed.(type) { + case *rsa.PrivateKey: + return key, crypto.SHA256, nil + case ed25519.PrivateKey: + // dkim.Sign special-cases ed25519; the hash value is ignored for it. + return key, crypto.Hash(0), nil + case *ecdsa.PrivateKey: + return key, crypto.SHA256, nil + default: + return nil, 0, fmt.Errorf("unsupported PKCS#8 key type %T", parsed) + } + } + + // SEC1 EC key. + if key, err := x509.ParseECPrivateKey(block.Bytes); err == nil { + return key, crypto.SHA256, nil + } + + return nil, 0, fmt.Errorf("unrecognised private key format") +} + +// validateKeyPermissions rejects private keys that are readable or writable by +// group or other, matching elemta's file-security posture for sensitive files. +func validateKeyPermissions(path string) error { + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("stat key %s: %w", path, err) + } + if info.IsDir() { + return fmt.Errorf("key path %s is a directory", path) + } + // 0077 = any group/other read/write/execute bit set. + if info.Mode().Perm()&0o077 != 0 { + return fmt.Errorf("private key %s has insecure permissions %s (must not be readable/writable by group or other)", + path, info.Mode().Perm()) + } + return nil +} + +// parseCanonicalization maps a config string to a dkim.Canonicalization, +// defaulting to relaxed when empty (per requirements). +func parseCanonicalization(v string) (dkim.Canonicalization, error) { + switch strings.ToLower(strings.TrimSpace(v)) { + case "", "relaxed": + return dkim.CanonicalizationRelaxed, nil + case "simple": + return dkim.CanonicalizationSimple, nil + default: + return "", fmt.Errorf("unknown canonicalization %q (want \"relaxed\" or \"simple\")", v) + } +} + +// FromHeaderDomain extracts the domain from the message's From header. It reads +// only the header block and returns "" when no usable From domain is found. This +// is used as a fallback when the envelope-from is empty (e.g. bounce messages). +func FromHeaderDomain(content []byte) string { + headerEnd := bytes.Index(content, []byte("\r\n\r\n")) + if headerEnd == -1 { + if idx := bytes.Index(content, []byte("\n\n")); idx != -1 { + headerEnd = idx + } else { + headerEnd = len(content) + } + } + lines := strings.Split(strings.ReplaceAll(string(content[:headerEnd]), "\r\n", "\n"), "\n") + for _, line := range lines { + if len(line) == 0 || line[0] == ' ' || line[0] == '\t' { + continue + } + if strings.HasPrefix(strings.ToLower(line), "from:") { + value := strings.TrimSpace(line[len("from:"):]) + // Prefer an address inside angle brackets when present. + if lt := strings.LastIndex(value, "<"); lt != -1 { + if gt := strings.Index(value[lt:], ">"); gt != -1 { + value = value[lt+1 : lt+gt] + } + } + return DomainFromAddress(value) + } + } + return "" +} + +// DomainFromAddress extracts the lower-cased domain from an email address such +// as "user@example.com" or "". It returns "" when no domain +// can be determined. +func DomainFromAddress(addr string) string { + addr = strings.TrimSpace(addr) + addr = strings.Trim(addr, "<>") + at := strings.LastIndex(addr, "@") + if at < 0 || at == len(addr)-1 { + return "" + } + return strings.ToLower(strings.TrimSpace(addr[at+1:])) +} diff --git a/internal/dkim/signer_test.go b/internal/dkim/signer_test.go new file mode 100644 index 0000000..6a701be --- /dev/null +++ b/internal/dkim/signer_test.go @@ -0,0 +1,336 @@ +package dkim + +import ( + "bytes" + "crypto/ed25519" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/emersion/go-msgauth/dkim" +) + +const testMessage = "From: Alice \r\n" + + "To: Bob \r\n" + + "Subject: Hello\r\n" + + "Date: Thu, 09 Jul 2026 12:00:00 +0000\r\n" + + "Message-ID: \r\n" + + "\r\n" + + "This is the body.\r\n" + +// writeRSAKey writes a fresh PKCS#8 RSA key to dir with mode 0600 and returns +// its path and the public key (for building a DNS resolver stub). +func writeRSAKey(t *testing.T, dir string) (string, *rsa.PublicKey) { + t.Helper() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generate rsa: %v", err) + } + der, err := x509.MarshalPKCS8PrivateKey(priv) + if err != nil { + t.Fatalf("marshal pkcs8: %v", err) + } + path := filepath.Join(dir, "rsa.key") + writeKeyFile(t, path, "PRIVATE KEY", der, 0o600) + return path, &priv.PublicKey +} + +func writeKeyFile(t *testing.T, path, blockType string, der []byte, mode os.FileMode) { + t.Helper() + pemBytes := pem.EncodeToMemory(&pem.Block{Type: blockType, Bytes: der}) + if err := os.WriteFile(path, pemBytes, mode); err != nil { + t.Fatalf("write key: %v", err) + } + // os.WriteFile may be affected by umask; force the exact mode. + if err := os.Chmod(path, mode); err != nil { + t.Fatalf("chmod key: %v", err) + } +} + +// dnsRecord builds the TXT value a verifier would fetch for the given public key. +func rsaDNSRecord(t *testing.T, pub *rsa.PublicKey) string { + t.Helper() + der, err := x509.MarshalPKIXPublicKey(pub) + if err != nil { + t.Fatalf("marshal pub: %v", err) + } + return "v=DKIM1; k=rsa; p=" + base64Std(der) +} + +func ed25519DNSRecord(t *testing.T, pub ed25519.PublicKey) string { + t.Helper() + return "v=DKIM1; k=ed25519; p=" + base64Std(pub) +} + +// verifyWithKey verifies signed message content against a fixed public key +// record, bypassing DNS. This exercises the go-msgauth verification path — the +// same library the plugin verification code references. +func verifyWithKey(t *testing.T, signed []byte, selector, domain, txt string) []*dkim.Verification { + t.Helper() + opts := &dkim.VerifyOptions{ + LookupTXT: func(name string) ([]string, error) { + want := selector + "._domainkey." + domain + if strings.TrimSuffix(name, ".") != want { + t.Fatalf("unexpected TXT lookup %q, want %q", name, want) + } + return []string{txt}, nil + }, + } + verifications, err := dkim.VerifyWithOptions(bytes.NewReader(signed), opts) + if err != nil { + t.Fatalf("verify: %v", err) + } + return verifications +} + +func newTestSigner(t *testing.T, cfg *Config) *Signer { + t.Helper() + s, err := NewSigner(cfg, nil) + if err != nil { + t.Fatalf("NewSigner: %v", err) + } + if s == nil { + t.Fatal("NewSigner returned nil signer") + } + return s +} + +func TestSignAndVerifyRSA(t *testing.T) { + dir := t.TempDir() + keyPath, pub := writeRSAKey(t, dir) + + s := newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, + }) + + signed, err := s.Sign([]byte(testMessage), "example.com") + if err != nil { + t.Fatalf("Sign: %v", err) + } + if !bytes.Contains(signed, []byte("DKIM-Signature:")) { + t.Fatal("signed message has no DKIM-Signature header") + } + + vs := verifyWithKey(t, signed, "sel1", "example.com", rsaDNSRecord(t, pub)) + if len(vs) != 1 { + t.Fatalf("expected 1 verification, got %d", len(vs)) + } + if vs[0].Err != nil { + t.Fatalf("verification failed: %v", vs[0].Err) + } + if vs[0].Domain != "example.com" { + t.Fatalf("verified domain = %q", vs[0].Domain) + } +} + +func TestSignAndVerifyEd25519(t *testing.T) { + dir := t.TempDir() + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("generate ed25519: %v", err) + } + der, err := x509.MarshalPKCS8PrivateKey(priv) + if err != nil { + t.Fatalf("marshal ed25519: %v", err) + } + keyPath := filepath.Join(dir, "ed.key") + writeKeyFile(t, keyPath, "PRIVATE KEY", der, 0o600) + + s := newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "ed", PrivateKeyPath: keyPath}}, + }) + + signed, err := s.Sign([]byte(testMessage), "example.com") + if err != nil { + t.Fatalf("Sign: %v", err) + } + + vs := verifyWithKey(t, signed, "ed", "example.com", ed25519DNSRecord(t, pub)) + if len(vs) != 1 || vs[0].Err != nil { + t.Fatalf("ed25519 verification failed: %+v", vs) + } +} + +func TestSignSkipsUnconfiguredDomain(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + s := newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, + }) + + out, err := s.Sign([]byte(testMessage), "other.test") + if err != nil { + t.Fatalf("Sign: %v", err) + } + if !bytes.Equal(out, []byte(testMessage)) { + t.Fatal("message for unconfigured domain should be unchanged") + } + if bytes.Contains(out, []byte("DKIM-Signature:")) { + t.Fatal("unconfigured domain must not be signed") + } +} + +// TestRetryDoesNotDoubleSign signs a message, then signs the already-signed +// output again (simulating a delivery retry). The second pass must be a no-op: +// exactly one DKIM-Signature for the domain. +func TestRetryDoesNotDoubleSign(t *testing.T) { + dir := t.TempDir() + keyPath, pub := writeRSAKey(t, dir) + s := newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, + }) + + first, err := s.Sign([]byte(testMessage), "example.com") + if err != nil { + t.Fatalf("first Sign: %v", err) + } + second, err := s.Sign(first, "example.com") + if err != nil { + t.Fatalf("second Sign: %v", err) + } + + if !bytes.Equal(first, second) { + t.Fatal("retry re-signed an already-signed message") + } + if n := countHeader(second, "DKIM-Signature:"); n != 1 { + t.Fatalf("expected exactly 1 DKIM-Signature after retry, got %d", n) + } + + // The single signature must still verify. + vs := verifyWithKey(t, second, "sel1", "example.com", rsaDNSRecord(t, pub)) + if len(vs) != 1 || vs[0].Err != nil { + t.Fatalf("post-retry verification failed: %+v", vs) + } +} + +func TestSignUsesFromHeaderFallbackDomain(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, + }) + + if got := FromHeaderDomain([]byte(testMessage)); got != "example.com" { + t.Fatalf("FromHeaderDomain = %q, want example.com", got) + } +} + +func TestRejectInsecureKeyPermissions(t *testing.T) { + dir := t.TempDir() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generate rsa: %v", err) + } + der, _ := x509.MarshalPKCS8PrivateKey(priv) + keyPath := filepath.Join(dir, "insecure.key") + writeKeyFile(t, keyPath, "PRIVATE KEY", der, 0o644) // group/other readable + + _, err = NewSigner(&Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, + }, nil) + if err == nil { + t.Fatal("expected error for world/group-readable key") + } + if !strings.Contains(err.Error(), "insecure permissions") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestNewSignerDisabled(t *testing.T) { + s, err := NewSigner(&Config{Enabled: false}, nil) + if err != nil { + t.Fatalf("disabled config should not error: %v", err) + } + if s != nil { + t.Fatal("disabled config should return nil signer") + } + + // nil signer Sign is a pass-through. + var nilSigner *Signer + out, err := nilSigner.Sign([]byte(testMessage), "example.com") + if err != nil || !bytes.Equal(out, []byte(testMessage)) { + t.Fatal("nil signer should pass content through unchanged") + } +} + +func TestNewSignerValidation(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + + cases := []struct { + name string + cfg *Config + }{ + {"empty domain", &Config{Enabled: true, Domains: []DomainConfig{{Selector: "s", PrivateKeyPath: keyPath}}}}, + {"empty selector", &Config{Enabled: true, Domains: []DomainConfig{{Domain: "example.com", PrivateKeyPath: keyPath}}}}, + {"empty key path", &Config{Enabled: true, Domains: []DomainConfig{{Domain: "example.com", Selector: "s"}}}}, + {"no domains", &Config{Enabled: true}}, + {"bad canon", &Config{Enabled: true, HeaderCanonicalization: "bogus", Domains: []DomainConfig{{Domain: "example.com", Selector: "s", PrivateKeyPath: keyPath}}}}, + {"duplicate domain", &Config{Enabled: true, Domains: []DomainConfig{ + {Domain: "example.com", Selector: "s1", PrivateKeyPath: keyPath}, + {Domain: "example.com", Selector: "s2", PrivateKeyPath: keyPath}, + }}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if _, err := NewSigner(tc.cfg, nil); err == nil { + t.Fatalf("expected error for %s", tc.name) + } + }) + } +} + +func TestDomainFromAddress(t *testing.T) { + cases := map[string]string{ + "user@example.com": "example.com", + "": "example.com", + " a@b.co ": "b.co", + "noatsign": "", + "trailingat@": "", + "User@Sub.Example.Org": "sub.example.org", + } + for in, want := range cases { + if got := DomainFromAddress(in); got != want { + t.Errorf("DomainFromAddress(%q) = %q, want %q", in, got, want) + } + } +} + +func TestHasKeyFor(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + s := newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "Example.COM", Selector: "s", PrivateKeyPath: keyPath}}, + }) + if !s.HasKeyFor("example.com") { + t.Fatal("expected key for example.com (case-insensitive)") + } + if s.HasKeyFor("other.test") { + t.Fatal("did not expect key for other.test") + } +} + +func base64Std(b []byte) string { return base64.StdEncoding.EncodeToString(b) } + +func countHeader(content []byte, prefix string) int { + n := 0 + for _, line := range strings.Split(strings.ReplaceAll(string(content), "\r\n", "\n"), "\n") { + if strings.HasPrefix(line, prefix) { + n++ + } + } + return n +} diff --git a/internal/queue/delivery_dkim_test.go b/internal/queue/delivery_dkim_test.go new file mode 100644 index 0000000..ffd5233 --- /dev/null +++ b/internal/queue/delivery_dkim_test.go @@ -0,0 +1,129 @@ +package queue + +import ( + "bytes" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/busybox42/elemta/internal/dkim" +) + +const dkimTestMessage = "From: Alice \r\n" + + "To: Bob \r\n" + + "Subject: Hello\r\n" + + "Date: Thu, 09 Jul 2026 12:00:00 +0000\r\n" + + "Message-ID: \r\n" + + "\r\n" + + "Body.\r\n" + +func newTestSigner(t *testing.T, domain string) *dkim.Signer { + t.Helper() + dir := t.TempDir() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("gen key: %v", err) + } + der, err := x509.MarshalPKCS8PrivateKey(priv) + if err != nil { + t.Fatalf("marshal key: %v", err) + } + keyPath := filepath.Join(dir, "dkim.key") + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) + if err := os.WriteFile(keyPath, pemBytes, 0o600); err != nil { + t.Fatalf("write key: %v", err) + } + if err := os.Chmod(keyPath, 0o600); err != nil { + t.Fatalf("chmod: %v", err) + } + s, err := dkim.NewSigner(&dkim.Config{ + Enabled: true, + Domains: []dkim.DomainConfig{{Domain: domain, Selector: "sel", PrivateKeyPath: keyPath}}, + }, nil) + if err != nil { + t.Fatalf("NewSigner: %v", err) + } + return s +} + +func countSignatures(content []byte) int { + n := 0 + for _, line := range strings.Split(strings.ReplaceAll(string(content), "\r\n", "\n"), "\n") { + if strings.HasPrefix(line, "DKIM-Signature:") { + n++ + } + } + return n +} + +// TestSignContentUsesEnvelopeFrom verifies the handler selects the signing +// domain from the envelope-from address and signs the message. +func TestSignContentUsesEnvelopeFrom(t *testing.T) { + h := NewSMTPDeliveryHandler(0) + h.SetDKIMSigner(newTestSigner(t, "example.com")) + + msg := Message{ID: "m1", From: "alice@example.com"} + out := h.signContent(msg, []byte(dkimTestMessage)) + if countSignatures(out) != 1 { + t.Fatalf("expected 1 signature, got %d", countSignatures(out)) + } +} + +// TestSignContentRetryDoesNotDoubleSign feeds already-signed content back +// through signContent (as a retry would) and asserts no second signature. +func TestSignContentRetryDoesNotDoubleSign(t *testing.T) { + h := NewSMTPDeliveryHandler(0) + h.SetDKIMSigner(newTestSigner(t, "example.com")) + msg := Message{ID: "m1", From: "alice@example.com"} + + first := h.signContent(msg, []byte(dkimTestMessage)) + second := h.signContent(msg, first) + + if !bytes.Equal(first, second) { + t.Fatal("retry produced different bytes (re-signed)") + } + if n := countSignatures(second); n != 1 { + t.Fatalf("expected exactly 1 signature after retry, got %d", n) + } +} + +// TestSignContentFallsBackToFromHeader verifies that when the envelope-from is +// empty (e.g. a bounce), the From header domain is used. +func TestSignContentFallsBackToFromHeader(t *testing.T) { + h := NewSMTPDeliveryHandler(0) + h.SetDKIMSigner(newTestSigner(t, "example.com")) + + msg := Message{ID: "bounce", From: ""} // empty envelope-from + out := h.signContent(msg, []byte(dkimTestMessage)) + if countSignatures(out) != 1 { + t.Fatalf("expected signing via From header fallback, got %d signatures", countSignatures(out)) + } +} + +// TestSignContentSkipsUnconfiguredDomain verifies unconfigured domains pass +// through unchanged. +func TestSignContentSkipsUnconfiguredDomain(t *testing.T) { + h := NewSMTPDeliveryHandler(0) + h.SetDKIMSigner(newTestSigner(t, "other.test")) + + msg := Message{ID: "m1", From: "alice@example.com"} + out := h.signContent(msg, []byte(dkimTestMessage)) + if countSignatures(out) != 0 { + t.Fatal("message for unconfigured domain must not be signed") + } +} + +// TestSignContentNilSignerPassThrough verifies signing is a no-op when disabled. +func TestSignContentNilSignerPassThrough(t *testing.T) { + h := NewSMTPDeliveryHandler(0) + msg := Message{ID: "m1", From: "alice@example.com"} + out := h.signContent(msg, []byte(dkimTestMessage)) + if !bytes.Equal(out, []byte(dkimTestMessage)) { + t.Fatal("nil signer must pass content through unchanged") + } +} diff --git a/internal/queue/delivery_handler.go b/internal/queue/delivery_handler.go index 1e98e6a..256f9f7 100644 --- a/internal/queue/delivery_handler.go +++ b/internal/queue/delivery_handler.go @@ -15,8 +15,16 @@ import ( "time" "github.com/busybox42/elemta/internal/delivery" + "github.com/busybox42/elemta/internal/dkim" ) +// messageSigner signs outbound message content for a given signing domain. +// Satisfied by *dkim.Signer; abstracted so tests can inject a stub. A nil +// signer means signing is disabled and content passes through unchanged. +type messageSigner interface { + Sign(content []byte, signingDomain string) ([]byte, error) +} + // mtastsEnforcer checks outbound delivery against a domain's MTA-STS policy. // Satisfied by *delivery.MTASTSManager; abstracted so tests can inject a stub // instead of making real DNS/HTTPS lookups. @@ -106,6 +114,7 @@ type SMTPDeliveryHandler struct { resolver mxResolver dialContext func(context.Context, string, string) (net.Conn, error) mxRetrySleep func(context.Context, time.Duration) error + signer messageSigner // optional DKIM signer; nil = signing disabled } // NewSMTPDeliveryHandler creates a new SMTP delivery handler @@ -121,6 +130,52 @@ func NewSMTPDeliveryHandler(failedQueueRetentionHours int) *SMTPDeliveryHandler } } +// SetDKIMSigner attaches a DKIM signer to the handler. A nil signer leaves +// signing disabled. Kept as an explicit setter so signing can be wired in from +// the server without changing the constructor signature. +func (h *SMTPDeliveryHandler) SetDKIMSigner(s *dkim.Signer) { + // Store as nil interface when the concrete pointer is nil so the interface + // comparison `h.signer == nil` in signContent works correctly. + if s == nil { + h.signer = nil + return + } + h.signer = s +} + +// signContent applies DKIM signing to the message before delivery. Signing +// happens once here — before recipients are grouped by domain — so the same +// signed bytes are delivered to every MX and a message is never signed +// per-domain. The signing domain is the envelope-from domain, falling back to +// the From header domain when the envelope-from is empty (e.g. bounces). +// +// dkim.Signer.Sign is idempotent: if the content already carries a signature +// for the chosen domain it is returned unchanged, so a delivery retry does not +// double-sign. A signing error is logged and delivery proceeds with the +// unsigned content rather than failing the message outright. +func (h *SMTPDeliveryHandler) signContent(msg Message, content []byte) []byte { + if h.signer == nil { + return content + } + + signingDomain := dkim.DomainFromAddress(msg.From) + if signingDomain == "" { + signingDomain = dkim.FromHeaderDomain(content) + } + if signingDomain == "" { + h.logger.Debug("No signing domain resolved for message, delivering unsigned", "message_id", msg.ID) + return content + } + + signed, err := h.signer.Sign(content, signingDomain) + if err != nil { + h.logger.Warn("DKIM signing failed, delivering unsigned", + "message_id", msg.ID, "domain", signingDomain, "error", err) + return content + } + return signed +} + // DeliverMessage attempts to deliver a message via SMTP func (h *SMTPDeliveryHandler) DeliverMessage(ctx context.Context, msg Message, content []byte) error { _, err := h.DeliverMessageWithMetadata(ctx, msg, content) @@ -131,6 +186,10 @@ func (h *SMTPDeliveryHandler) DeliverMessage(ctx context.Context, msg Message, c func (h *SMTPDeliveryHandler) DeliverMessageWithMetadata(ctx context.Context, msg Message, content []byte) (*DeliveryResult, error) { requireTLS := messageRequiresTLS(msg) + // DKIM-sign once, before recipients are grouped, so identical signed bytes + // go to every MX and retries never double-sign. + content = h.signContent(msg, content) + // Group recipients by domain for efficient delivery domainGroups := h.groupRecipientsByDomain(msg.To) diff --git a/internal/smtp/config.go b/internal/smtp/config.go index d8231a4..17c0ea9 100644 --- a/internal/smtp/config.go +++ b/internal/smtp/config.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/BurntSushi/toml" + "github.com/busybox42/elemta/internal/dkim" ) // Config represents the main configuration for the SMTP server @@ -83,6 +84,9 @@ type Config struct { // Delivery configuration Delivery *DeliveryConfig `toml:"delivery" json:"delivery"` + // DKIM outbound signing configuration + DKIM *dkim.Config `toml:"dkim" json:"dkim"` + // Memory management configuration Memory *MemoryConfig `toml:"memory" json:"memory"` diff --git a/internal/smtp/server.go b/internal/smtp/server.go index cb8c8b9..8cb106e 100644 --- a/internal/smtp/server.go +++ b/internal/smtp/server.go @@ -12,6 +12,7 @@ import ( "sync/atomic" "time" + "github.com/busybox42/elemta/internal/dkim" deliverymetrics "github.com/busybox42/elemta/internal/metrics" "github.com/busybox42/elemta/internal/plugin" "github.com/busybox42/elemta/internal/queue" @@ -205,6 +206,21 @@ func initQueueSystem(config *Config, slogger *slog.Logger) (*queue.Manager, *que slogger.Info("Creating LMTP delivery handler", "host", deliveryHost, "port", deliveryPort, "max_per_domain", maxPerDomain) lmtpHandler := newDeliveryHandler(deliveryHost, deliveryPort, maxPerDomain, config.FailedQueueRetentionHours) + // Build the DKIM signer (if enabled) and attach it to the remote SMTP + // delivery handler. Local LMTP delivery is intentionally not signed. + dkimSigner, err := dkim.NewSigner(config.DKIM, slogger) + if err != nil { + return nil, nil, fmt.Errorf("failed to initialize DKIM signer: %w", err) + } + if dkimSigner != nil { + if smtpHandler, ok := lmtpHandler.(*queue.SMTPDeliveryHandler); ok { + smtpHandler.SetDKIMSigner(dkimSigner) + slogger.Info("DKIM outbound signing enabled") + } else { + slogger.Info("DKIM signing configured but active delivery handler is not the remote SMTP handler; signing will apply only on the remote SMTP path") + } + } + processorConfig := queue.ProcessorConfig{ Enabled: config.QueueProcessorEnabled, Interval: time.Duration(config.QueueProcessInterval) * time.Second, From 3515599b184ee2f1db173400974043cab992add6 Mon Sep 17 00:00:00 2001 From: Alan Denniston Date: Mon, 13 Jul 2026 06:10:26 +0000 Subject: [PATCH 2/3] fix: stabilize REQUIRETLS test and document DKIM key path trust --- internal/dkim/signer.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/dkim/signer.go b/internal/dkim/signer.go index 83016c3..ece5799 100644 --- a/internal/dkim/signer.go +++ b/internal/dkim/signer.go @@ -280,7 +280,9 @@ func loadPrivateKey(path string) (crypto.Signer, crypto.Hash, error) { return nil, 0, err } - data, err := os.ReadFile(path) //nolint:gosec // path comes from operator config + // #nosec G304 -- private-key path is explicit, trusted operator configuration; + // permissions and key format are validated immediately before this read. + data, err := os.ReadFile(path) if err != nil { return nil, 0, fmt.Errorf("reading key: %w", err) } From fbd0edf04cf1d0066c8820110d9bdf58415a02fe Mon Sep 17 00:00:00 2001 From: Alan Denniston Date: Fri, 24 Jul 2026 02:19:41 +0000 Subject: [PATCH 3/3] fix(dkim): fail closed on signing errors Reject invalid signed-header configuration at startup, validate DKIM before queue resources are started, and prevent configured-domain signing failures from downgrading delivery to unsigned mail. --- config/elemta.toml.example | 9 +- docs/dkim-signing.md | 15 ++- internal/dkim/signer.go | 119 ++++++++---------------- internal/dkim/signer_test.go | 119 ++++++++++++++++++++---- internal/queue/delivery_dkim_test.go | 60 +++++++++--- internal/queue/delivery_handler.go | 32 ++++--- internal/smtp/delivery_behavior_test.go | 75 ++++++++++++++- internal/smtp/server.go | 43 +++++++-- 8 files changed, 332 insertions(+), 140 deletions(-) diff --git a/config/elemta.toml.example b/config/elemta.toml.example index 20fa021..bcb7b00 100644 --- a/config/elemta.toml.example +++ b/config/elemta.toml.example @@ -95,6 +95,11 @@ listen_addr = "0.0.0.0:8080" metrics_path = "/metrics" health_path = "/health" +# Use remote SMTP delivery for outbound mail. DKIM signing is intentionally not +# applied when mode is "lmtp" because that path is local delivery to Dovecot. +[delivery] +mode = "smtp" + # Outbound DKIM signing configuration. # Signs mail on the remote SMTP delivery path (not local LMTP delivery) to # improve deliverability to Gmail/Yahoo/Outlook. Publish the matching public @@ -117,7 +122,9 @@ body_canonicalization = "relaxed" private_key_path = "/etc/elemta/dkim/example.com.key" # Optional per-domain override of the signed header set. When omitted, a # sensible default is used (From, To, Cc, Subject, Date, Message-ID, - # MIME-Version, Content-Type, Reply-To, In-Reply-To, References). + # MIME-Version, Content-Type, Reply-To, In-Reply-To, References). Custom lists + # must contain valid RFC 5322 field names and include From; Elemta rejects the + # configuration at startup otherwise. # headers_to_sign = ["From", "To", "Subject", "Date", "Message-ID"] [server] diff --git a/docs/dkim-signing.md b/docs/dkim-signing.md index 0fd0a85..72a4bd8 100644 --- a/docs/dkim-signing.md +++ b/docs/dkim-signing.md @@ -16,9 +16,10 @@ This document covers the outbound **signing** path. Inbound DKIM/DMARC/ARC an administrative boundary where DKIM matters. - A message is signed **once**, before recipients are grouped by domain, so the exact same signed bytes are delivered to every recipient MX. -- Signing is **idempotent**. If a message already carries a `DKIM-Signature` - for the chosen signing domain, it is left untouched, so a delivery **retry - never double-signs**. +- Existing `DKIM-Signature` headers are preserved, but they never suppress + Elemta's configured signature. Each queue attempt reloads the original stored + content and adds one fresh Elemta signature, so retries do not accumulate + signatures from earlier attempts. - The signing domain is selected by matching the **envelope-from** domain against the configured domains. When the envelope-from is empty (for example, a bounce/DSN with a null return path), the **From header** domain is used as a @@ -77,9 +78,13 @@ For Ed25519 use `k=ed25519` and the base64 of the raw 32-byte public key. ## Configuration -Add a top-level `[dkim]` section: +Select remote SMTP delivery and add a top-level `[dkim]` section. DKIM signing +does not run in local `lmtp` mode. ```toml +[delivery] +mode = "smtp" + [dkim] enabled = true header_canonicalization = "relaxed" # default: relaxed @@ -108,7 +113,7 @@ body_canonicalization = "relaxed" # default: relaxed | `domain` | `[[dkim.domains]]` | The signing domain (the `d=` tag). | | `selector` | `[[dkim.domains]]` | The DKIM selector (the `s=` tag). | | `private_key_path` | `[[dkim.domains]]` | Path to the PEM RSA or Ed25519 private key. | -| `headers_to_sign` | `[[dkim.domains]]` | Optional. Overrides the default signed header set for this domain. | +| `headers_to_sign` | `[[dkim.domains]]` | Optional. Overrides the default signed header set for this domain. Entries must be valid RFC 5322 field names and must include `From` (case-insensitive), or startup fails. | Default signed headers (when `headers_to_sign` is omitted): `From`, `To`, `Cc`, `Subject`, `Date`, `Message-ID`, `MIME-Version`, `Content-Type`, `Reply-To`, diff --git a/internal/dkim/signer.go b/internal/dkim/signer.go index ece5799..2c611df 100644 --- a/internal/dkim/signer.go +++ b/internal/dkim/signer.go @@ -40,11 +40,6 @@ var DefaultHeadersToSign = []string{ "References", } -// signingHeaderName is the DKIM signature header. Presence of a signature from -// our own domain is used as the "already signed" guard so retries do not -// double-sign. -const signingHeaderName = "DKIM-Signature" - // DomainConfig is the per-domain signing configuration. type DomainConfig struct { // Domain is the signing domain (the d= tag). Matched against the @@ -132,9 +127,23 @@ func NewSigner(cfg *Config, logger *slog.Logger) (*Signer, error) { return nil, fmt.Errorf("dkim: domain %q: %w", domain, err) } - headerKeys := dc.HeadersToSign - if len(headerKeys) == 0 { - headerKeys = DefaultHeadersToSign + configuredHeaders := dc.HeadersToSign + if len(configuredHeaders) == 0 { + configuredHeaders = DefaultHeadersToSign + } + headerKeys := make([]string, len(configuredHeaders)) + hasFrom := false + for i, header := range configuredHeaders { + headerKeys[i] = strings.TrimSpace(header) + if !validHeaderFieldName(headerKeys[i]) { + return nil, fmt.Errorf("dkim: domain %q headers_to_sign contains invalid header name %q", domain, header) + } + if strings.EqualFold(headerKeys[i], "From") { + hasFrom = true + } + } + if !hasFrom { + return nil, fmt.Errorf("dkim: domain %q headers_to_sign must include From", domain) } s.keys[domain] = domainKey{ @@ -162,11 +171,9 @@ func NewSigner(cfg *Config, logger *slog.Logger) (*Signer, error) { // Behaviour: // - If no key is configured for the domain, content is returned unchanged and // a debug line is logged (not an error — the message is still deliverable). -// - If content already carries a DKIM-Signature from the selected domain, it -// is returned unchanged. This makes signing idempotent, so a delivery retry -// never double-signs. -// - A signing failure returns the original content and an error; callers may -// choose to deliver unsigned rather than fail the message. +// - A signing failure returns the original content and an error. Elemta's SMTP +// delivery path treats that error as a failed attempt rather than sending +// unsigned mail for a configured signing domain. func (s *Signer) Sign(content []byte, signingDomain string) ([]byte, error) { if s == nil { return content, nil @@ -181,11 +188,6 @@ func (s *Signer) Sign(content []byte, signingDomain string) ([]byte, error) { return content, nil } - if hasSignatureForDomain(content, domain) { - s.logger.Debug("Message already DKIM-signed for domain, skipping", "domain", domain) - return content, nil - } - opts := &dkim.SignOptions{ Domain: domain, Selector: key.selector, @@ -216,63 +218,8 @@ func (s *Signer) HasKeyFor(domain string) bool { return ok } -// hasSignatureForDomain scans the message header block for an existing -// DKIM-Signature whose d= tag matches domain. Only the header block (up to the -// first blank line) is inspected. Folded continuation lines are reassembled so -// a d= tag on a continuation line is still detected. -func hasSignatureForDomain(content []byte, domain string) bool { - headerEnd := bytes.Index(content, []byte("\r\n\r\n")) - if headerEnd == -1 { - if idx := bytes.Index(content, []byte("\n\n")); idx != -1 { - headerEnd = idx - } else { - headerEnd = len(content) - } - } - header := content[:headerEnd] - - lowerPrefix := strings.ToLower(signingHeaderName) + ":" - lines := strings.Split(strings.ReplaceAll(string(header), "\r\n", "\n"), "\n") - - var current strings.Builder - var fields []string - flush := func() { - if current.Len() > 0 { - fields = append(fields, current.String()) - current.Reset() - } - } - for _, line := range lines { - if line == "" { - continue - } - if line[0] == ' ' || line[0] == '\t' { - current.WriteString(" ") - current.WriteString(strings.TrimSpace(line)) - continue - } - flush() - current.WriteString(line) - } - flush() - - target := "d=" + domain - for _, f := range fields { - if !strings.HasPrefix(strings.ToLower(f), lowerPrefix) { - continue - } - // Normalise whitespace inside the DKIM-Signature tag list before - // comparing the d= tag. - normalized := strings.ToLower(strings.Join(strings.Fields(f), "")) - if strings.Contains(normalized, target+";") || strings.HasSuffix(normalized, target) { - return true - } - } - return false -} - // loadPrivateKey reads, permission-checks and parses a PEM private key. It -// returns a crypto.Signer and the hash to use (SHA-256 for RSA/ECDSA; Ed25519 +// returns a crypto.Signer and the hash to use (SHA-256 for RSA; Ed25519 // signs without a separate hash so crypto.Hash(0) is returned and go-msgauth // handles the ed25519 special case internally). func loadPrivateKey(path string) (crypto.Signer, crypto.Hash, error) { @@ -306,7 +253,7 @@ func parsePrivateKeyBlock(block *pem.Block) (crypto.Signer, crypto.Hash, error) return key, crypto.SHA256, nil } - // PKCS#8 (RSA, ECDSA, Ed25519). + // PKCS#8 (RSA or Ed25519). go-msgauth does not support ECDSA DKIM keys. if parsed, err := x509.ParsePKCS8PrivateKey(block.Bytes); err == nil { switch key := parsed.(type) { case *rsa.PrivateKey: @@ -315,15 +262,14 @@ func parsePrivateKeyBlock(block *pem.Block) (crypto.Signer, crypto.Hash, error) // dkim.Sign special-cases ed25519; the hash value is ignored for it. return key, crypto.Hash(0), nil case *ecdsa.PrivateKey: - return key, crypto.SHA256, nil + return nil, 0, fmt.Errorf("ECDSA DKIM keys are not supported") default: return nil, 0, fmt.Errorf("unsupported PKCS#8 key type %T", parsed) } } - // SEC1 EC key. - if key, err := x509.ParseECPrivateKey(block.Bytes); err == nil { - return key, crypto.SHA256, nil + if _, err := x509.ParseECPrivateKey(block.Bytes); err == nil { + return nil, 0, fmt.Errorf("ECDSA DKIM keys are not supported") } return nil, 0, fmt.Errorf("unrecognised private key format") @@ -347,6 +293,21 @@ func validateKeyPermissions(path string) error { return nil } +// validHeaderFieldName reports whether name is an RFC 5322 field-name: one or +// more printable US-ASCII characters except colon. Names are trimmed before +// validation so accepted configuration is exactly what reaches go-msgauth. +func validHeaderFieldName(name string) bool { + if name == "" { + return false + } + for i := 0; i < len(name); i++ { + if name[i] < 33 || name[i] > 126 || name[i] == ':' { + return false + } + } + return true +} + // parseCanonicalization maps a config string to a dkim.Canonicalization, // defaulting to relaxed when empty (per requirements). func parseCanonicalization(v string) (dkim.Canonicalization, error) { diff --git a/internal/dkim/signer_test.go b/internal/dkim/signer_test.go index 6a701be..a3d1dcc 100644 --- a/internal/dkim/signer_test.go +++ b/internal/dkim/signer_test.go @@ -2,12 +2,15 @@ package dkim import ( "bytes" + "crypto/ecdsa" "crypto/ed25519" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/x509" "encoding/base64" "encoding/pem" + "fmt" "os" "path/filepath" "strings" @@ -179,37 +182,119 @@ func TestSignSkipsUnconfiguredDomain(t *testing.T) { } } -// TestRetryDoesNotDoubleSign signs a message, then signs the already-signed -// output again (simulating a delivery retry). The second pass must be a no-op: -// exactly one DKIM-Signature for the domain. -func TestRetryDoesNotDoubleSign(t *testing.T) { +// TestSignDoesNotTrustExistingSameDomainSignature verifies that untrusted +// message input cannot suppress Elemta's signature merely by supplying a +// DKIM-Signature with the configured d= domain. +func TestSignDoesNotTrustExistingSameDomainSignature(t *testing.T) { dir := t.TempDir() - keyPath, pub := writeRSAKey(t, dir) + keyPath, _ := writeRSAKey(t, dir) s := newTestSigner(t, &Config{ Enabled: true, Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, }) - first, err := s.Sign([]byte(testMessage), "example.com") + forged := []byte("DKIM-Signature: v=1; a=rsa-sha256; d=example.com; s=attacker; b=invalid\r\n" + testMessage) + signed, err := s.Sign(forged, "example.com") + if err != nil { + t.Fatalf("Sign: %v", err) + } + if bytes.Equal(signed, forged) { + t.Fatal("forged same-domain signature suppressed Elemta signing") + } + if n := countHeader(signed, "DKIM-Signature:"); n != 2 { + t.Fatalf("expected forged and Elemta signatures, got %d", n) + } + if !bytes.Contains(signed, []byte("s=sel1")) { + t.Fatal("output does not contain Elemta's configured selector") + } +} + +func TestNewSignerRejectsECDSAKey(t *testing.T) { + dir := t.TempDir() + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - t.Fatalf("first Sign: %v", err) + t.Fatalf("generate ecdsa: %v", err) } - second, err := s.Sign(first, "example.com") + der, err := x509.MarshalPKCS8PrivateKey(priv) if err != nil { - t.Fatalf("second Sign: %v", err) + t.Fatalf("marshal ecdsa: %v", err) } + keyPath := filepath.Join(dir, "ecdsa.key") + writeKeyFile(t, keyPath, "PRIVATE KEY", der, 0o600) - if !bytes.Equal(first, second) { - t.Fatal("retry re-signed an already-signed message") + _, err = NewSigner(&Config{ + Enabled: true, + Domains: []DomainConfig{{Domain: "example.com", Selector: "sel1", PrivateKeyPath: keyPath}}, + }, nil) + if err == nil { + t.Fatal("expected unsupported ECDSA key to fail during startup") } - if n := countHeader(second, "DKIM-Signature:"); n != 1 { - t.Fatalf("expected exactly 1 DKIM-Signature after retry, got %d", n) + if !strings.Contains(err.Error(), "ECDSA") { + t.Fatalf("expected ECDSA error, got: %v", err) } +} - // The single signature must still verify. - vs := verifyWithKey(t, second, "sel1", "example.com", rsaDNSRecord(t, pub)) - if len(vs) != 1 || vs[0].Err != nil { - t.Fatalf("post-retry verification failed: %+v", vs) +func TestNewSignerRejectsHeadersWithoutFrom(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + + _, err := NewSigner(&Config{ + Enabled: true, + Domains: []DomainConfig{{ + Domain: "example.com", + Selector: "sel1", + PrivateKeyPath: keyPath, + HeadersToSign: []string{"Subject"}, + }}, + }, nil) + if err == nil { + t.Fatal("expected headers_to_sign without From to fail during startup") + } + if !strings.Contains(err.Error(), "From") { + t.Fatalf("expected missing From error, got: %v", err) + } +} + +func TestNewSignerRejectsInvalidHeadersToSign(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + + for _, header := range []string{"", " ", "Subject:extra", "Sub\nject", "Sübject"} { + t.Run(fmt.Sprintf("%q", header), func(t *testing.T) { + _, err := NewSigner(&Config{ + Enabled: true, + Domains: []DomainConfig{{ + Domain: "example.com", + Selector: "sel1", + PrivateKeyPath: keyPath, + HeadersToSign: []string{"From", header}, + }}, + }, nil) + if err == nil { + t.Fatal("expected invalid header name to fail during startup") + } + if !strings.Contains(err.Error(), "invalid header") { + t.Fatalf("expected invalid header error, got: %v", err) + } + }) + } +} + +func TestNewSignerNormalizesHeadersToSign(t *testing.T) { + dir := t.TempDir() + keyPath, _ := writeRSAKey(t, dir) + + s := newTestSigner(t, &Config{ + Enabled: true, + Domains: []DomainConfig{{ + Domain: "example.com", + Selector: "sel1", + PrivateKeyPath: keyPath, + HeadersToSign: []string{" From ", "Subject"}, + }}, + }) + if _, err := s.Sign([]byte(testMessage), "example.com"); err != nil { + t.Fatalf("headers accepted at startup must sign successfully: %v", err) } } diff --git a/internal/queue/delivery_dkim_test.go b/internal/queue/delivery_dkim_test.go index ffd5233..51ed628 100644 --- a/internal/queue/delivery_dkim_test.go +++ b/internal/queue/delivery_dkim_test.go @@ -2,6 +2,7 @@ package queue import ( "bytes" + "context" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -68,27 +69,35 @@ func TestSignContentUsesEnvelopeFrom(t *testing.T) { h.SetDKIMSigner(newTestSigner(t, "example.com")) msg := Message{ID: "m1", From: "alice@example.com"} - out := h.signContent(msg, []byte(dkimTestMessage)) + out, err := h.signContent(msg, []byte(dkimTestMessage)) + if err != nil { + t.Fatalf("signContent: %v", err) + } if countSignatures(out) != 1 { t.Fatalf("expected 1 signature, got %d", countSignatures(out)) } } -// TestSignContentRetryDoesNotDoubleSign feeds already-signed content back -// through signContent (as a retry would) and asserts no second signature. -func TestSignContentRetryDoesNotDoubleSign(t *testing.T) { +// TestSignContentRetriesStartFromStoredContent models the processor retry path: +// each attempt reloads the original stored message rather than feeding the +// previous attempt's signed bytes back into the signer. +func TestSignContentRetriesStartFromStoredContent(t *testing.T) { h := NewSMTPDeliveryHandler(0) h.SetDKIMSigner(newTestSigner(t, "example.com")) msg := Message{ID: "m1", From: "alice@example.com"} - first := h.signContent(msg, []byte(dkimTestMessage)) - second := h.signContent(msg, first) - - if !bytes.Equal(first, second) { - t.Fatal("retry produced different bytes (re-signed)") + first, err := h.signContent(msg, []byte(dkimTestMessage)) + if err != nil { + t.Fatalf("first signContent: %v", err) } - if n := countSignatures(second); n != 1 { - t.Fatalf("expected exactly 1 signature after retry, got %d", n) + second, err := h.signContent(msg, []byte(dkimTestMessage)) + if err != nil { + t.Fatalf("second signContent: %v", err) + } + + if countSignatures(first) != 1 || countSignatures(second) != 1 { + t.Fatalf("each delivery attempt must carry one Elemta signature; got %d and %d", + countSignatures(first), countSignatures(second)) } } @@ -99,7 +108,10 @@ func TestSignContentFallsBackToFromHeader(t *testing.T) { h.SetDKIMSigner(newTestSigner(t, "example.com")) msg := Message{ID: "bounce", From: ""} // empty envelope-from - out := h.signContent(msg, []byte(dkimTestMessage)) + out, err := h.signContent(msg, []byte(dkimTestMessage)) + if err != nil { + t.Fatalf("signContent: %v", err) + } if countSignatures(out) != 1 { t.Fatalf("expected signing via From header fallback, got %d signatures", countSignatures(out)) } @@ -112,7 +124,10 @@ func TestSignContentSkipsUnconfiguredDomain(t *testing.T) { h.SetDKIMSigner(newTestSigner(t, "other.test")) msg := Message{ID: "m1", From: "alice@example.com"} - out := h.signContent(msg, []byte(dkimTestMessage)) + out, err := h.signContent(msg, []byte(dkimTestMessage)) + if err != nil { + t.Fatalf("signContent: %v", err) + } if countSignatures(out) != 0 { t.Fatal("message for unconfigured domain must not be signed") } @@ -122,8 +137,25 @@ func TestSignContentSkipsUnconfiguredDomain(t *testing.T) { func TestSignContentNilSignerPassThrough(t *testing.T) { h := NewSMTPDeliveryHandler(0) msg := Message{ID: "m1", From: "alice@example.com"} - out := h.signContent(msg, []byte(dkimTestMessage)) + out, err := h.signContent(msg, []byte(dkimTestMessage)) + if err != nil { + t.Fatalf("signContent: %v", err) + } if !bytes.Equal(out, []byte(dkimTestMessage)) { t.Fatal("nil signer must pass content through unchanged") } } + +func TestDeliveryFailsWhenConfiguredDKIMSigningFails(t *testing.T) { + h := NewSMTPDeliveryHandler(0) + h.SetDKIMSigner(newTestSigner(t, "example.com")) + msg := Message{ID: "m1", From: "alice@example.com"} + + _, err := h.DeliverMessageWithMetadata(context.Background(), msg, []byte("malformed")) + if err == nil { + t.Fatal("expected DKIM signing failure to stop delivery") + } + if !strings.Contains(err.Error(), "dkim signing failed") { + t.Fatalf("expected DKIM signing error, got: %v", err) + } +} diff --git a/internal/queue/delivery_handler.go b/internal/queue/delivery_handler.go index 256f9f7..dc03616 100644 --- a/internal/queue/delivery_handler.go +++ b/internal/queue/delivery_handler.go @@ -149,13 +149,13 @@ func (h *SMTPDeliveryHandler) SetDKIMSigner(s *dkim.Signer) { // per-domain. The signing domain is the envelope-from domain, falling back to // the From header domain when the envelope-from is empty (e.g. bounces). // -// dkim.Signer.Sign is idempotent: if the content already carries a signature -// for the chosen domain it is returned unchanged, so a delivery retry does not -// double-sign. A signing error is logged and delivery proceeds with the -// unsigned content rather than failing the message outright. -func (h *SMTPDeliveryHandler) signContent(msg Message, content []byte) []byte { +// Each queue attempt starts from the original stored content, so retries receive +// one fresh signature without trusting message-supplied DKIM headers. If signing +// is configured for the message domain, a signing error fails the delivery +// attempt rather than silently downgrading it to unsigned mail. +func (h *SMTPDeliveryHandler) signContent(msg Message, content []byte) ([]byte, error) { if h.signer == nil { - return content + return content, nil } signingDomain := dkim.DomainFromAddress(msg.From) @@ -164,16 +164,14 @@ func (h *SMTPDeliveryHandler) signContent(msg Message, content []byte) []byte { } if signingDomain == "" { h.logger.Debug("No signing domain resolved for message, delivering unsigned", "message_id", msg.ID) - return content + return content, nil } signed, err := h.signer.Sign(content, signingDomain) if err != nil { - h.logger.Warn("DKIM signing failed, delivering unsigned", - "message_id", msg.ID, "domain", signingDomain, "error", err) - return content + return nil, fmt.Errorf("dkim signing failed for message %q and domain %q: %w", msg.ID, signingDomain, err) } - return signed + return signed, nil } // DeliverMessage attempts to deliver a message via SMTP @@ -186,9 +184,15 @@ func (h *SMTPDeliveryHandler) DeliverMessage(ctx context.Context, msg Message, c func (h *SMTPDeliveryHandler) DeliverMessageWithMetadata(ctx context.Context, msg Message, content []byte) (*DeliveryResult, error) { requireTLS := messageRequiresTLS(msg) - // DKIM-sign once, before recipients are grouped, so identical signed bytes - // go to every MX and retries never double-sign. - content = h.signContent(msg, content) + // DKIM-sign once per delivery attempt, before recipients are grouped, so + // identical signed bytes go to every MX. A configured-domain signing failure + // aborts this attempt; the queue processor treats unknown delivery errors as + // temporary and retries from the original stored content. + var err error + content, err = h.signContent(msg, content) + if err != nil { + return nil, err + } // Group recipients by domain for efficient delivery domainGroups := h.groupRecipientsByDomain(msg.To) diff --git a/internal/smtp/delivery_behavior_test.go b/internal/smtp/delivery_behavior_test.go index 32b66a0..9f176ba 100644 --- a/internal/smtp/delivery_behavior_test.go +++ b/internal/smtp/delivery_behavior_test.go @@ -1,14 +1,87 @@ package smtp import ( + "bytes" "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "log/slog" + "os" + "path/filepath" + "strings" "testing" + "github.com/busybox42/elemta/internal/dkim" "github.com/busybox42/elemta/internal/queue" "github.com/stretchr/testify/require" - "log/slog" ) +func writeDeliveryTestDKIMKey(t *testing.T) string { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + der, err := x509.MarshalPKCS8PrivateKey(key) + require.NoError(t, err) + path := filepath.Join(t.TempDir(), "dkim.key") + require.NoError(t, os.WriteFile(path, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}), 0o600)) + return path +} + +func TestInitQueueSystemRejectsInvalidDKIMWhenProcessorDisabled(t *testing.T) { + cfg := createTestConfig(t) + cfg.QueueProcessorEnabled = false + cfg.DKIM = &dkim.Config{ + Enabled: true, + Domains: []dkim.DomainConfig{{ + Domain: "example.com", + Selector: "sel1", + PrivateKeyPath: writeDeliveryTestDKIMKey(t), + HeadersToSign: []string{"Subject"}, + }}, + } + + manager, _, err := initQueueSystem(cfg, slog.Default()) + if manager != nil { + defer manager.Stop() + } + require.ErrorContains(t, err, "headers_to_sign must include From") +} + +func TestInitQueueSystem_SMTPModeAttachesDKIMSigner(t *testing.T) { + oldMetricsFactory := newQueueMetricsStore + defer func() { newQueueMetricsStore = oldMetricsFactory }() + newQueueMetricsStore = func(string) (queue.MetricsRecorder, error) { + return &deliveryMetricsStub{}, nil + } + + cfg := createTestConfig(t) + cfg.QueueProcessorEnabled = true + cfg.QueueProcessInterval = 1 + cfg.QueueWorkers = 1 + cfg.Delivery = &DeliveryConfig{Mode: "smtp"} + cfg.DKIM = &dkim.Config{ + Enabled: true, + Domains: []dkim.DomainConfig{{ + Domain: "example.com", + Selector: "sel1", + PrivateKeyPath: writeDeliveryTestDKIMKey(t), + }}, + } + + var logs bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logs, nil)) + manager, _, err := initQueueSystem(cfg, logger) + require.NoError(t, err) + defer manager.Stop() + + output := logs.String() + require.True(t, strings.Contains(output, "Creating SMTP delivery handler"), output) + require.True(t, strings.Contains(output, "DKIM outbound signing enabled"), output) + require.False(t, strings.Contains(output, "active delivery handler is not the remote SMTP handler"), output) +} + // TestDeliveryHandlerFactoryEnsuresMockUsed verifies that the injection seam // allows substituting a mock delivery handler. func TestDeliveryHandlerFactoryEnsuresMockUsed(t *testing.T) { diff --git a/internal/smtp/server.go b/internal/smtp/server.go index 8cb106e..22da27b 100644 --- a/internal/smtp/server.go +++ b/internal/smtp/server.go @@ -8,6 +8,7 @@ import ( "net" "os" "path/filepath" + "strings" "sync" "sync/atomic" "time" @@ -117,12 +118,18 @@ func initPlugins(config *Config, slogger *slog.Logger) (*plugin.Manager, *plugin type deliveryHandlerFactory func(host string, port int, maxPerDomain int, failedQueueRetentionHours int) queue.DeliveryHandler +type smtpDeliveryHandlerFactory func(failedQueueRetentionHours int) *queue.SMTPDeliveryHandler + type metricsStoreFactory func(addr string) (queue.MetricsRecorder, error) var newDeliveryHandler deliveryHandlerFactory = func(host string, port int, maxPerDomain int, failedQueueRetentionHours int) queue.DeliveryHandler { return queue.NewLMTPDeliveryHandler(host, port, maxPerDomain, failedQueueRetentionHours) } +var newSMTPDeliveryHandler smtpDeliveryHandlerFactory = func(failedQueueRetentionHours int) *queue.SMTPDeliveryHandler { + return queue.NewSMTPDeliveryHandler(failedQueueRetentionHours) +} + var newQueueMetricsStore metricsStoreFactory = func(addr string) (queue.MetricsRecorder, error) { return deliverymetrics.NewValkeyStore(addr) } @@ -154,6 +161,14 @@ func initAuthenticator(config *Config, slogger *slog.Logger) (Authenticator, err // initQueueSystem initializes the queue manager and optional queue processor. func initQueueSystem(config *Config, slogger *slog.Logger) (*queue.Manager, *queue.Processor, error) { + // Validate and load DKIM before starting queue resources. This applies even + // when queue processing is disabled and leaves no manager goroutine/backend + // to clean up if DKIM configuration is invalid. + dkimSigner, err := dkim.NewSigner(config.DKIM, slogger) + if err != nil { + return nil, nil, fmt.Errorf("failed to initialize DKIM signer: %w", err) + } + queueManager, err := queue.NewManagerFromBackend( config.QueueDir, config.QueueBackend, @@ -203,17 +218,27 @@ func initQueueSystem(config *Config, slogger *slog.Logger) (*queue.Manager, *que maxPerDomain = 10 } - slogger.Info("Creating LMTP delivery handler", "host", deliveryHost, "port", deliveryPort, "max_per_domain", maxPerDomain) - lmtpHandler := newDeliveryHandler(deliveryHost, deliveryPort, maxPerDomain, config.FailedQueueRetentionHours) + deliveryMode := "lmtp" + if config.Delivery != nil && strings.TrimSpace(config.Delivery.Mode) != "" { + deliveryMode = strings.ToLower(strings.TrimSpace(config.Delivery.Mode)) + } - // Build the DKIM signer (if enabled) and attach it to the remote SMTP - // delivery handler. Local LMTP delivery is intentionally not signed. - dkimSigner, err := dkim.NewSigner(config.DKIM, slogger) - if err != nil { - return nil, nil, fmt.Errorf("failed to initialize DKIM signer: %w", err) + var deliveryHandler queue.DeliveryHandler + switch deliveryMode { + case "lmtp": + slogger.Info("Creating LMTP delivery handler", "host", deliveryHost, "port", deliveryPort, "max_per_domain", maxPerDomain) + deliveryHandler = newDeliveryHandler(deliveryHost, deliveryPort, maxPerDomain, config.FailedQueueRetentionHours) + case "smtp": + slogger.Info("Creating SMTP delivery handler", "max_per_domain", maxPerDomain) + deliveryHandler = newSMTPDeliveryHandler(config.FailedQueueRetentionHours) + default: + return nil, nil, fmt.Errorf("unsupported delivery mode %q (want smtp or lmtp)", deliveryMode) } + + // Attach the already-validated DKIM signer only to remote SMTP delivery. + // Local LMTP delivery is intentionally not signed. if dkimSigner != nil { - if smtpHandler, ok := lmtpHandler.(*queue.SMTPDeliveryHandler); ok { + if smtpHandler, ok := deliveryHandler.(*queue.SMTPDeliveryHandler); ok { smtpHandler.SetDKIMSigner(dkimSigner) slogger.Info("DKIM outbound signing enabled") } else { @@ -235,7 +260,7 @@ func initQueueSystem(config *Config, slogger *slog.Logger) (*queue.Manager, *que "interval", processorConfig.Interval, "workers", processorConfig.MaxConcurrent) - queueProcessor = queue.NewProcessor(queueManager, processorConfig, lmtpHandler) + queueProcessor = queue.NewProcessor(queueManager, processorConfig, deliveryHandler) // Set up bounce engine for DSN generation on permanent failures bounceEngine := NewBounceEngine(queueManager, config.Hostname, slogger)