diff --git a/services/protocols/cmd/lambdamail-protocols/main.go b/services/protocols/cmd/lambdamail-protocols/main.go index 52d759f..056fe25 100644 --- a/services/protocols/cmd/lambdamail-protocols/main.go +++ b/services/protocols/cmd/lambdamail-protocols/main.go @@ -135,6 +135,8 @@ func run(cfg config) { // wired the scripts were stored and never read, so a vacation responder // that was switched on never replied and no filter ever filed anything. inboundUC.SetRules(usecase.NewApplyRulesUseCase(postgres.NewSieveRepository(pool))) + // Senders the mailbox trusts are not filed as spam. + inboundUC.SetTrustedSenders(postgres.NewTrustedSenders(pool)) // ------------------------------------------------------------ outbound var signer port.DkimSigner diff --git a/services/protocols/internal/application/port/trusted_senders.go b/services/protocols/internal/application/port/trusted_senders.go new file mode 100644 index 0000000..c8e306f --- /dev/null +++ b/services/protocols/internal/application/port/trusted_senders.go @@ -0,0 +1,18 @@ +package port + +import ( + "context" + + "github.com/google/uuid" +) + +// TrustedSenders is the list of senders a mailbox has said it trusts. +// +// Mail from one of them is not filed as spam, which is what makes "this is not +// spam" mean something: before, moving a message out of Junk taught nothing and +// the next reply from the same person went straight back. +type TrustedSenders interface { + // IsTrusted reports whether the mailbox trusts this address, which is + // already lowercased. + IsTrusted(ctx context.Context, mailboxID uuid.UUID, address string) (bool, error) +} diff --git a/services/protocols/internal/application/usecase/message_headers.go b/services/protocols/internal/application/usecase/message_headers.go index 788e685..65c685e 100644 --- a/services/protocols/internal/application/usecase/message_headers.go +++ b/services/protocols/internal/application/usecase/message_headers.go @@ -18,6 +18,8 @@ const snippetLength = 200 type MessageHeaders struct { Subject string FromDisplayName string + // FromAddress is the address in From, lowercased, or empty when it does not parse. + FromAddress string MessageID string Snippet string HasAttachments bool @@ -58,6 +60,7 @@ func ExtractMessageHeaders(payload []byte) MessageHeaders { if from := msg.Header.Get("From"); from != "" { if addr, err := mail.ParseAddress(from); err == nil { + out.FromAddress = strings.ToLower(addr.Address) out.FromDisplayName = truncateUTF8(decodeHeader(addr.Name), 255) } else { // A From that does not parse still often carries a usable name diff --git a/services/protocols/internal/application/usecase/process_inbound_email.go b/services/protocols/internal/application/usecase/process_inbound_email.go index 325c695..a341112 100644 --- a/services/protocols/internal/application/usecase/process_inbound_email.go +++ b/services/protocols/internal/application/usecase/process_inbound_email.go @@ -67,6 +67,7 @@ type ProcessInboundEmailUseCase struct { trackerManager *MailboxTrackerManager reportIngestor *IngestDeliveredReportsUseCase rules *ApplyRulesUseCase + trusted port.TrustedSenders submission *ProcessOutboundEmailUseCase auth port.AuthRepository replyHost string @@ -294,6 +295,11 @@ func (uc *ProcessInboundEmailUseCase) Handle(ctx context.Context, input ProcessI // Each recipient's own rules decide their own copy: two mailboxes on // this server receiving the same message may file it differently. folder := targetFolder + // Before the rules, so a filter still has the last word: the spam + // verdict is the filter's guess and the rules are the mailbox's own. + if folder == "Junk" && uc.rescuedFromJunk(ctx, recipient.ID, headers.FromAddress, input.Sender, authResult) { + folder = "INBOX" + } var decision RuleDecision if uc.rules != nil { decision = uc.rules.For(ctx, recipient.ID, input.RecipientAddresses[i], input.Sender, payload) diff --git a/services/protocols/internal/application/usecase/trusted_delivery.go b/services/protocols/internal/application/usecase/trusted_delivery.go new file mode 100644 index 0000000..38d8c7b --- /dev/null +++ b/services/protocols/internal/application/usecase/trusted_delivery.go @@ -0,0 +1,65 @@ +package usecase + +import ( + "context" + "log" + "strings" + + "github.com/google/uuid" + + "lambdamail/protocols/internal/application/port" +) + +// SetTrustedSenders lets a mailbox's trusted senders skip the spam folder. +// Optional: without it a spam verdict files the message in Junk for everyone. +func (uc *ProcessInboundEmailUseCase) SetTrustedSenders(trusted port.TrustedSenders) { + uc.trusted = trusted +} + +// rescuedFromJunk reports whether a message the spam filter condemned should +// reach the inbox because this mailbox trusts who it is from. +// +// The From header is what a person trusts, and it is also the easiest field to +// forge, so it only counts when the message proves it came from that address: +// DMARC passed, or SPF passed for the very domain written in From. Without the +// proof, trusting a contact would hand a spammer a way in by writing that +// contact's address on their mail. +// +// Fails closed: a lookup that errors leaves the message in Junk, which is where +// it was going anyway. +func (uc *ProcessInboundEmailUseCase) rescuedFromJunk( + ctx context.Context, + mailboxID uuid.UUID, + fromAddress, envelopeSender string, + auth port.InboundAuthResult, +) bool { + if uc.trusted == nil || fromAddress == "" { + return false + } + if !provesSender(fromAddress, envelopeSender, auth) { + return false + } + trusted, err := uc.trusted.IsTrusted(ctx, mailboxID, fromAddress) + if err != nil { + log.Printf("trusted senders: %v", err) + return false + } + return trusted +} + +func provesSender(fromAddress, envelopeSender string, auth port.InboundAuthResult) bool { + if auth.DMARC == port.AuthResultPass { + return true + } + return auth.SPF == port.AuthResultPass && + domainOf(fromAddress) != "" && + domainOf(fromAddress) == domainOf(normaliseAddress(envelopeSender)) +} + +func domainOf(address string) string { + at := strings.LastIndex(address, "@") + if at < 0 || at == len(address)-1 { + return "" + } + return address[at+1:] +} diff --git a/services/protocols/internal/application/usecase/trusted_delivery_test.go b/services/protocols/internal/application/usecase/trusted_delivery_test.go new file mode 100644 index 0000000..86ca777 --- /dev/null +++ b/services/protocols/internal/application/usecase/trusted_delivery_test.go @@ -0,0 +1,100 @@ +package usecase + +import ( + "bytes" + "context" + "errors" + "testing" + + "github.com/google/uuid" + + "lambdamail/protocols/internal/application/port" + "lambdamail/protocols/internal/domain/valueobject" +) + +type fakeTrusted struct { + trusted map[string]bool + err error +} + +func (f *fakeTrusted) IsTrusted(_ context.Context, _ uuid.UUID, address string) (bool, error) { + return f.trusted[address], f.err +} + +func deliverAsSpam(t *testing.T, trusted port.TrustedSenders, auth port.InboundAuthResult, envelope string) string { + t.Helper() + repo := &recordingMessageRepository{} + uc := NewProcessInboundEmailUseCase(nil, &capturingBlobStorage{}, repo) + uc.SetScanner(&stubScanner{result: &valueobject.ScanResult{Verdict: valueobject.ScanVerdictSpamJunk}}) + uc.SetAuthenticator(&stubAuthenticator{result: auth}) + if trusted != nil { + uc.SetTrustedSenders(trusted) + } + err := uc.Handle(context.Background(), ProcessInboundEmailInput{ + Sender: envelope, + Recipients: []port.MailboxRecord{{ID: uuid.New()}}, + RecipientAddresses: []string{"me@example.test"}, + Body: bytes.NewBufferString("From: Jenny \r\nSubject: re: hi\r\n\r\nbody"), + }) + if err != nil { + t.Fatalf("Handle: %v", err) + } + return repo.persisted[0].TargetFolderName +} + +var passDMARC = port.InboundAuthResult{SPF: "pass", DKIM: "pass", DMARC: "pass"} + +func TestTrustedSenderIsNotFiledAsSpam(t *testing.T) { + trusted := &fakeTrusted{trusted: map[string]bool{"jenny@friend.test": true}} + if got := deliverAsSpam(t, trusted, passDMARC, "jenny@friend.test"); got != "INBOX" { + t.Fatalf("a trusted, authenticated sender was filed in %q", got) + } +} + +func TestUntrustedSenderStillGoesToJunk(t *testing.T) { + trusted := &fakeTrusted{trusted: map[string]bool{"someone@else.test": true}} + if got := deliverAsSpam(t, trusted, passDMARC, "jenny@friend.test"); got != "Junk" { + t.Fatalf("an untrusted sender was filed in %q", got) + } +} + +// Writing a contact's address in From is all a spammer needs to do, so trust +// only counts when the message proves it came from there. +func TestForgedTrustedSenderStillGoesToJunk(t *testing.T) { + trusted := &fakeTrusted{trusted: map[string]bool{"jenny@friend.test": true}} + forged := port.InboundAuthResult{SPF: "fail", DKIM: "none", DMARC: "fail"} + if got := deliverAsSpam(t, trusted, forged, "bounce@spammer.test"); got != "Junk" { + t.Fatalf("a forged From reached the inbox: %q", got) + } +} + +// SPF that passed for the spammer's own domain proves nothing about From. +func TestSPFForAnotherDomainDoesNotProveFrom(t *testing.T) { + trusted := &fakeTrusted{trusted: map[string]bool{"jenny@friend.test": true}} + other := port.InboundAuthResult{SPF: "pass", DKIM: "none", DMARC: "none"} + if got := deliverAsSpam(t, trusted, other, "bounce@spammer.test"); got != "Junk" { + t.Fatalf("SPF for a different domain let a forged From in: %q", got) + } +} + +// A domain with no DMARC record can still be proven by SPF for its own domain. +func TestSPFForTheSameDomainProvesFrom(t *testing.T) { + trusted := &fakeTrusted{trusted: map[string]bool{"jenny@friend.test": true}} + spfOnly := port.InboundAuthResult{SPF: "pass", DKIM: "none", DMARC: "none"} + if got := deliverAsSpam(t, trusted, spfOnly, "mailer@friend.test"); got != "INBOX" { + t.Fatalf("SPF for the sender's own domain was not accepted: %q", got) + } +} + +func TestLookupFailureLeavesTheMessageInJunk(t *testing.T) { + trusted := &fakeTrusted{trusted: map[string]bool{"jenny@friend.test": true}, err: errors.New("db down")} + if got := deliverAsSpam(t, trusted, passDMARC, "jenny@friend.test"); got != "Junk" { + t.Fatalf("a failed lookup let mail through: %q", got) + } +} + +func TestWithoutTrustedSendersSpamGoesToJunk(t *testing.T) { + if got := deliverAsSpam(t, nil, passDMARC, "jenny@friend.test"); got != "Junk" { + t.Fatalf("filed in %q", got) + } +} diff --git a/services/protocols/internal/infrastructure/postgres/trusted_senders.go b/services/protocols/internal/infrastructure/postgres/trusted_senders.go new file mode 100644 index 0000000..112620f --- /dev/null +++ b/services/protocols/internal/infrastructure/postgres/trusted_senders.go @@ -0,0 +1,32 @@ +package postgres + +import ( + "context" + "fmt" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" +) + +// TrustedSenders reads the senders a mailbox trusts. The webmail writes them +// through the auth service; delivery only ever asks. +type TrustedSenders struct { + pool *pgxpool.Pool +} + +func NewTrustedSenders(pool *pgxpool.Pool) *TrustedSenders { + return &TrustedSenders{pool: pool} +} + +func (t *TrustedSenders) IsTrusted(ctx context.Context, mailboxID uuid.UUID, address string) (bool, error) { + var trusted bool + err := t.pool.QueryRow(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM trusted_senders WHERE mailbox_id = $1 AND address = $2 + ) + `, mailboxID, address).Scan(&trusted) + if err != nil { + return false, fmt.Errorf("look up a trusted sender: %w", err) + } + return trusted, nil +}