diff --git a/services/protocols/internal/application/usecase/outbound_dsn_test.go b/services/protocols/internal/application/usecase/outbound_dsn_test.go index 61a8417..f5fd024 100644 --- a/services/protocols/internal/application/usecase/outbound_dsn_test.go +++ b/services/protocols/internal/application/usecase/outbound_dsn_test.go @@ -139,7 +139,9 @@ func TestOutboundWorker_DsnToFanOutSenderDeliversToEveryTarget(t *testing.T) { repo := &fakeOutboundRepo{} repo.Enqueue(context.Background(), newTestJob()) - mailboxes := &fakeAliasFanOutRepository{targets: []port.MailboxRecord{ + // The sender is the local, aliased address; the recipient is remote, which + // is what makes this a delivery that goes out and then bounces back. + mailboxes := &fakeAliasFanOutRepository{localAddress: "user@domain.test", targets: []port.MailboxRecord{ {ID: uuid.New(), QuotaBytes: 1000}, {ID: uuid.New(), QuotaBytes: 1000}, }} diff --git a/services/protocols/internal/application/usecase/outbound_local_delivery_test.go b/services/protocols/internal/application/usecase/outbound_local_delivery_test.go new file mode 100644 index 0000000..399c8cf --- /dev/null +++ b/services/protocols/internal/application/usecase/outbound_local_delivery_test.go @@ -0,0 +1,114 @@ +package usecase + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + + "lambdamail/protocols/internal/application/port" + "lambdamail/protocols/internal/domain/entity" +) + +// countingMXResolver fails the test if delivery ever tries to leave the host. +type countingMXResolver struct{ lookups int } + +func (c *countingMXResolver) LookupMX(_ context.Context, _ string) ([]string, error) { + c.lookups++ + // An address that cannot be dialled, so a test that does reach here fails + // on the assertion rather than on a timeout. + return []string{"127.0.0.1:9"}, nil +} + +func newLocalDeliveryJob(to string) *entity.OutboundJob { + mbID := uuid.New() + return &entity.OutboundJob{ + ID: uuid.New(), + MailboxID: &mbID, + BlobID: uuid.New(), + EnvelopeFrom: "me@example.test", + EnvelopeTo: to, + DestinationDomain: "example.test", + Status: entity.OutboundJobStatusQueued, + NextAttemptAt: time.Now(), + ExpiresAt: time.Now().Add(time.Hour), + } +} + +// Mail between two accounts on this server must not depend on outbound port +// 25. Every message used to be resolved through MX and delivered over SMTP, +// including one addressed to a mailbox on this very host - so on a provider +// that blocks port 25, users could not write to each other on their own +// server. +func TestLocalRecipientIsDeliveredWithoutLeavingTheHost(t *testing.T) { + recipient := "colleague@example.test" + mailboxes := &fakeMailboxRepository{byAddress: map[string]port.MailboxRecord{ + recipient: {ID: uuid.New(), MaxMessageBytes: 1_000_000}, + }} + messages := &recordingMessageRepository{} + inbound := NewProcessInboundEmailUseCase( + mailboxes, &stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "s", SizeBytes: 4}}, messages, + ) + + mx := &countingMXResolver{} + repo := &fakeOutboundRepo{} + job := newLocalDeliveryJob(recipient) + repo.jobs = append(repo.jobs, job) + + worker := NewOutboundWorkerUseCase( + repo, mx, + &fakeBlobReader{payload: []byte("From: me@example.test\r\nTo: " + recipient + "\r\n\r\nhello\r\n")}, + inbound, mailboxes, "mail.example.test", + ) + + if _, err := worker.ProcessBatch(context.Background(), "w1", 10); err != nil { + t.Fatalf("ProcessBatch: %v", err) + } + + if mx.lookups != 0 { + t.Errorf("a local recipient triggered %d MX lookups; delivery left the host", mx.lookups) + } + if len(messages.persisted) != 1 { + t.Fatalf("want the message stored once for the local recipient, got %d", len(messages.persisted)) + } + if job.Status != entity.OutboundJobStatusDelivered { + t.Errorf("job status = %q, want delivered", job.Status) + } + if job.TlsPolicyUsed != entity.TLSModeLocal { + t.Errorf("tls policy = %q, want %q - nothing crossed a network", + job.TlsPolicyUsed, entity.TLSModeLocal) + } +} + +// The shortcut must be narrow: an address this server does not host still goes +// out over SMTP, or local delivery would swallow the internet's mail. +func TestRemoteRecipientStillGoesOutOverSmtp(t *testing.T) { + mailboxes := &fakeMailboxRepository{byAddress: map[string]port.MailboxRecord{}} + messages := &recordingMessageRepository{} + inbound := NewProcessInboundEmailUseCase( + mailboxes, &stubBlobStorage{ref: port.BlobRef{ID: uuid.New()}}, messages, + ) + + mx := &countingMXResolver{} + repo := &fakeOutboundRepo{} + job := newLocalDeliveryJob("stranger@remote.test") + job.DestinationDomain = "remote.test" + repo.jobs = append(repo.jobs, job) + + worker := NewOutboundWorkerUseCase( + repo, mx, &fakeBlobReader{payload: []byte("From: me\r\n\r\nhi\r\n")}, + inbound, mailboxes, "mail.example.test", + ) + + if _, err := worker.ProcessBatch(context.Background(), "w1", 10); err != nil { + t.Fatalf("ProcessBatch: %v", err) + } + + if mx.lookups == 0 { + t.Error("a remote recipient was not resolved through MX") + } + if len(messages.persisted) != 0 { + t.Errorf("a remote message was filed into a local mailbox: %+v", messages.persisted) + } +} diff --git a/services/protocols/internal/application/usecase/outbound_worker.go b/services/protocols/internal/application/usecase/outbound_worker.go index 235fb11..81bc8ef 100644 --- a/services/protocols/internal/application/usecase/outbound_worker.go +++ b/services/protocols/internal/application/usecase/outbound_worker.go @@ -98,6 +98,29 @@ func (w *OutboundWorkerUseCase) processSingleJob(ctx context.Context, job *entit return } + // Mail addressed to a mailbox on this server is delivered here, without + // going out to the internet and back. + // + // Nothing did this before: every message was resolved through MX and + // delivered over SMTP, including one user writing to another on the same + // host. That makes internal mail depend on outbound port 25 - so on a + // provider that blocks it, which is most of them, colleagues could not + // write to each other on their own server. It also fails ahead of the + // relay check on purpose: sending local mail out to a smarthost only to + // have it delivered back is a slower path with more ways to break. + if delivered, err := w.deliverLocally(ctx, job, payload); delivered { + if err != nil { + w.handleFailure(ctx, job, fmt.Sprintf("local delivery: %v", err), true) + return + } + job.Status = entity.OutboundJobStatusDelivered + job.LastError = "" + job.LastSmtpCode = "" + job.TlsPolicyUsed = entity.TLSModeLocal + _ = w.outboundRepo.UpdateJob(ctx, job) + return + } + // With a smarthost every message goes to the relay regardless of // destination, and the relay owns transport security from there on. if w.relay.Configured() { @@ -379,3 +402,44 @@ func (w *OutboundWorkerUseCase) deliverToMX(ctx context.Context, mxHost string, return client.Quit() } + +// deliverLocally hands a message to a mailbox on this server. +// +// Returns (handled, err): handled is false when the recipient is not local, so +// the caller falls through to relay or MX delivery. A local recipient whose +// delivery then fails returns (true, err) and is retried like any other job - +// the queue row stays the durable record either way. +// +// The message is still content-scanned. It arrives DKIM-signed by this server, +// so it authenticates cleanly, and a virus mailed between two accounts on the +// same host is no less a virus. +func (w *OutboundWorkerUseCase) deliverLocally( + ctx context.Context, job *entity.OutboundJob, payload []byte, +) (bool, error) { + if w.inboundUC == nil || w.mailboxes == nil { + return false, nil + } + + targets, err := w.mailboxes.ResolveDeliveryTargets(ctx, job.EnvelopeTo) + if err != nil { + // Not knowing whether the address is local is not the same as knowing + // it is not: falling through to SMTP would deliver to the internet a + // message that may belong here. Retrying is the safe answer. + return true, err + } + if len(targets) == 0 { + return false, nil + } + + addresses := make([]string, len(targets)) + for i := range targets { + addresses[i] = job.EnvelopeTo + } + + return true, w.inboundUC.Handle(ctx, ProcessInboundEmailInput{ + Sender: job.EnvelopeFrom, + Recipients: targets, + RecipientAddresses: addresses, + Body: bytes.NewReader(payload), + }) +} diff --git a/services/protocols/internal/application/usecase/process_inbound_email_test.go b/services/protocols/internal/application/usecase/process_inbound_email_test.go index 1285a0f..196e5e3 100644 --- a/services/protocols/internal/application/usecase/process_inbound_email_test.go +++ b/services/protocols/internal/application/usecase/process_inbound_email_test.go @@ -83,15 +83,25 @@ func TestUseCase_ResolveRecipient_ReturnsErrMailboxQuotaExceededWhenAnyFanOutTar } // fakeAliasFanOutRepository simulates an alias resolving to multiple mailboxes. +// +// localAddress is the one address it hosts. It used to answer for any address +// at all, which made every recipient look local - harmless while nothing +// consulted it on the delivery path, and wrong the moment local delivery +// started asking. The real repository only ever resolves addresses on an +// active local domain. type fakeAliasFanOutRepository struct { - targets []port.MailboxRecord + localAddress string + targets []port.MailboxRecord } func (f *fakeAliasFanOutRepository) FindActiveByAddress(_ context.Context, _ string) (*port.MailboxRecord, error) { return nil, nil } -func (f *fakeAliasFanOutRepository) ResolveDeliveryTargets(_ context.Context, _ string) ([]port.MailboxRecord, error) { +func (f *fakeAliasFanOutRepository) ResolveDeliveryTargets(_ context.Context, address string) ([]port.MailboxRecord, error) { + if f.localAddress != "" && address != f.localAddress { + return []port.MailboxRecord{}, nil + } return f.targets, nil } diff --git a/services/protocols/internal/domain/entity/tls_policy.go b/services/protocols/internal/domain/entity/tls_policy.go index 82a4702..0a5e0b5 100644 --- a/services/protocols/internal/domain/entity/tls_policy.go +++ b/services/protocols/internal/domain/entity/tls_policy.go @@ -19,6 +19,11 @@ const ( // TLSModeRelay records that the message left through a smarthost, which // owns transport security from that point on (PLAN.md section 10.4). TLSModeRelay = "relay" + // TLSModeLocal records a message that never crossed a network: the + // recipient is a mailbox on this same server, so there was no transport + // to secure. Distinct from opportunistic, which means a network hop was + // made without a policy to validate against. + TLSModeLocal = "local" ) // TLSPolicy is the effective transport security policy for one outbound