From fea808f8e3796c7d8eb0da884138b46ddd68bc73 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Thu, 3 Nov 2016 00:48:34 +0000 Subject: [PATCH] queue: Get the DSN domain from the message Picking the domain used in the DSN message "From" header is more complicated than it needs to be, causing confusing code paths and having different uses for the hostname, which should be purely aesthetic. This patch makes the queue pick the DSN "From" domain from the message itself, by looking for a local domain in either the sender or the original recipients. We should find at least one, otherwise it'd be relaying. This allows the code to be simplified, and we can narrow the scope of the hostname option even further. --- etc/chasquid/chasquid.conf | 10 +++------- internal/config/config.pb.go | 25 +++++++++++++++---------- internal/config/config.proto | 10 +++------- internal/queue/queue.go | 22 ++++++++++++++++------ internal/queue/queue_test.go | 10 +++++----- internal/smtpsrv/server.go | 2 +- 6 files changed, 43 insertions(+), 36 deletions(-) diff --git a/etc/chasquid/chasquid.conf b/etc/chasquid/chasquid.conf index 42e0bf0..a161f7c 100644 --- a/etc/chasquid/chasquid.conf +++ b/etc/chasquid/chasquid.conf @@ -1,11 +1,7 @@ -# Main/default hostname to use. -# This is used to say hello to clients, and by default as the domain -# we send delivery notifications errors from. -# It should be a domain we can send email from, and we should have a -# certificate for it. -# It usually helps if our IP address resolves to it. -# Default: machine hostname. +# Default hostname to use when saying hello. +# This is used to say hello to clients, for aesthetic purposes. +# Default: the system's hostname. #hostname: "mx.example.com" # Maximum email size, in megabytes. diff --git a/internal/config/config.pb.go b/internal/config/config.pb.go index e7a6c5e..92cf969 100644 --- a/internal/config/config.pb.go +++ b/internal/config/config.pb.go @@ -29,12 +29,9 @@ var _ = math.Inf const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package type Config struct { - // Main/default hostname to use. - // This is used to say hello to clients, and by default as the domain - // we send delivery notifications errors from. - // It should be a domain we can send email from. - // It usually helps if our IP address resolves to it. - // Default: machine hostname. + // Default hostname to use when saying hello. + // This is used to say hello to clients, for aesthetic purposes. + // Default: the system's hostname. Hostname string `protobuf:"bytes,1,opt,name=hostname" json:"hostname,omitempty"` // Maximum email size, in megabytes. // Default: 50. @@ -48,6 +45,7 @@ type Config struct { // systemd sockets must be named with "FileDescriptorName=submission". SubmissionAddress []string `protobuf:"bytes,4,rep,name=submission_address,json=submissionAddress" json:"submission_address,omitempty"` // Address for the monitoring http server. + // Do NOT expose this to the public internet. // Default: no monitoring http server. MonitoringAddress string `protobuf:"bytes,5,opt,name=monitoring_address,json=monitoringAddress" json:"monitoring_address,omitempty"` // Mail delivery agent (MDA, also known as LDA) to use. @@ -57,10 +55,17 @@ type Config struct { // Default: "procmail". MailDeliveryAgentBin string `protobuf:"bytes,6,opt,name=mail_delivery_agent_bin,json=mailDeliveryAgentBin" json:"mail_delivery_agent_bin,omitempty"` // Command line arguments for the mail delivery agent. One per argument. - // Some replacements will be done: - // - "%user%" -> local user (anything before the @) - // - "%domain%" -> domain (anything after the @) - // Default: "-d", "%user" (adequate for procmail) + // Some replacements will be done. + // On an email sent from marsnik@mars to venera@venus: + // - %from% -> from address (marsnik@mars) + // - %from_user% -> from user (marsnik) + // - %from_domain% -> from domain (mars) + // - %to% -> to address (venera@venus) + // - %to_user% -> to user (venera) + // - %to_domain% -> to domain (venus) + // + // Default: "-f", "%from%", "-d", "%to_user%" (adequate for procmail + // and maildrop). MailDeliveryAgentArgs []string `protobuf:"bytes,7,rep,name=mail_delivery_agent_args,json=mailDeliveryAgentArgs" json:"mail_delivery_agent_args,omitempty"` // Directory where we store our persistent data. // Default: "/var/lib/chasquid" diff --git a/internal/config/config.proto b/internal/config/config.proto index 3d49358..c4a487d 100644 --- a/internal/config/config.proto +++ b/internal/config/config.proto @@ -2,13 +2,9 @@ syntax = "proto3"; message Config { - // Main/default hostname to use. - // This is used to say hello to clients, and by default as the domain - // we send delivery notifications errors from. - // It should be a domain we can send email from, and we should have a - // certificate for it. - // It usually helps if our IP address resolves to it. - // Default: machine hostname. + // Default hostname to use when saying hello. + // This is used to say hello to clients, for aesthetic purposes. + // Default: the system's hostname. string hostname = 1; // Maximum email size, in megabytes. diff --git a/internal/queue/queue.go b/internal/queue/queue.go index ff84f2d..8b91384 100644 --- a/internal/queue/queue.go +++ b/internal/queue/queue.go @@ -105,14 +105,11 @@ type Queue struct { // Aliases resolver. aliases *aliases.Resolver - - // Domain we use to send delivery status notifications from. - dsnDomain string } // New creates a new Queue instance. func New(path string, localDomains *set.String, aliases *aliases.Resolver, - localC, remoteC courier.Courier, dsnDomain string) *Queue { + localC, remoteC courier.Courier) *Queue { os.MkdirAll(path, 0700) @@ -123,7 +120,6 @@ func New(path string, localDomains *set.String, aliases *aliases.Resolver, localDomains: localDomains, path: path, aliases: aliases, - dsnDomain: dsnDomain, } } @@ -434,7 +430,21 @@ func (item *Item) countRcpt(statuses ...Recipient_Status) int { func sendDSN(tr *trace.Trace, q *Queue, item *Item) { tr.Debugf("sending DSN") - msg, err := deliveryStatusNotification(q.dsnDomain, item) + // Pick a (local) domain to send the DSN from. We should always find one, + // as otherwise we're relaying. + domain := "unknown" + if item.From != "<>" && envelope.DomainIn(item.From, q.localDomains) { + domain = envelope.DomainOf(item.From) + } else { + for _, rcpt := range item.Rcpt { + if envelope.DomainIn(rcpt.OriginalAddress, q.localDomains) { + domain = envelope.DomainOf(rcpt.OriginalAddress) + break + } + } + } + + msg, err := deliveryStatusNotification(domain, item) if err != nil { tr.Errorf("failed to build DSN: %v", err) return diff --git a/internal/queue/queue_test.go b/internal/queue/queue_test.go index 5a37cd5..1b485d2 100644 --- a/internal/queue/queue_test.go +++ b/internal/queue/queue_test.go @@ -64,7 +64,7 @@ func TestBasic(t *testing.T) { localC := newTestCourier() remoteC := newTestCourier() q := New("/tmp/queue_test", set.NewString("loco"), aliases.NewResolver(), - localC, remoteC, "dsndomain") + localC, remoteC) localC.wg.Add(2) remoteC.wg.Add(1) @@ -117,7 +117,7 @@ func TestDSNOnTimeout(t *testing.T) { localC := newTestCourier() remoteC := newTestCourier() q := New("/tmp/queue_test", set.NewString("loco"), aliases.NewResolver(), - localC, remoteC, "dsndomain") + localC, remoteC) // Insert an expired item in the queue. item := &Item{ @@ -156,7 +156,7 @@ func TestAliases(t *testing.T) { localC := newTestCourier() remoteC := newTestCourier() q := New("/tmp/queue_test", set.NewString("loco"), aliases.NewResolver(), - localC, remoteC, "dsndomain") + localC, remoteC) q.aliases.AddDomain("loco") q.aliases.AddAliasForTesting("ab@loco", "pq@loco", aliases.EMAIL) @@ -207,7 +207,7 @@ var dumbCourier = DumbCourier{} func TestFullQueue(t *testing.T) { q := New("/tmp/queue_test", set.NewString(), aliases.NewResolver(), - dumbCourier, dumbCourier, "dsndomain") + dumbCourier, dumbCourier) // Force-insert maxQueueSize items in the queue. oneID := "" @@ -247,7 +247,7 @@ func TestFullQueue(t *testing.T) { func TestPipes(t *testing.T) { q := New("/tmp/queue_test", set.NewString("loco"), aliases.NewResolver(), - dumbCourier, dumbCourier, "dsndomain") + dumbCourier, dumbCourier) item := &Item{ Message: Message{ ID: <-newID, diff --git a/internal/smtpsrv/server.go b/internal/smtpsrv/server.go index b1378e0..2e46c94 100644 --- a/internal/smtpsrv/server.go +++ b/internal/smtpsrv/server.go @@ -123,7 +123,7 @@ func (s *Server) InitDomainInfo(dir string) *domaininfo.DB { } func (s *Server) InitQueue(path string, localC, remoteC courier.Courier) { - q := queue.New(path, s.localDomains, s.aliasesR, localC, remoteC, s.Hostname) + q := queue.New(path, s.localDomains, s.aliasesR, localC, remoteC) err := q.Load() if err != nil { log.Fatalf("Error loading queue: %v", err)