diff --git a/endpoint/smtp/smtp.go b/endpoint/smtp/smtp.go index 6a7d0b0..5ae0811 100644 --- a/endpoint/smtp/smtp.go +++ b/endpoint/smtp/smtp.go @@ -159,7 +159,7 @@ func (s *Session) Data(r io.Reader) error { } if s.endp.submission { - if err := SubmissionPrepare(s.msgMeta, header, s.endp.serv.Domain); err != nil { + if err := s.submissionPrepare(header); err != nil { s.log.Error("DATA error", err) return s.endp.wrapErr(s.msgMeta.ID, err) } diff --git a/endpoint/smtp/submission.go b/endpoint/smtp/submission.go index 9584a73..c6066fc 100644 --- a/endpoint/smtp/submission.go +++ b/endpoint/smtp/submission.go @@ -7,38 +7,45 @@ import ( "time" "github.com/emersion/go-message/textproto" - "github.com/emersion/go-smtp" - "github.com/foxcpp/maddy/log" - "github.com/foxcpp/maddy/module" + "github.com/foxcpp/maddy/exterrors" "github.com/google/uuid" ) -func SubmissionPrepare(msgMeta *module.MsgMetadata, header textproto.Header, ourHostname string) error { - msgMeta.DontTraceSender = true +func (s *Session) submissionPrepare(header textproto.Header) error { + s.msgMeta.DontTraceSender = true if header.Get("Message-ID") == "" { msgId, err := uuid.NewRandom() if err != nil { return errors.New("Message-ID generation failed") } - // TODO: Move this function to smtpSession and use its logger. - log.Debugf("adding missing Message-ID header to message from %s (%s)", msgMeta.SrcHostname, msgMeta.SrcAddr) - header.Set("Message-ID", "<"+msgId.String()+"@"+ourHostname+">") + s.log.Msg("adding missing Message-ID") + header.Set("Message-ID", "<"+msgId.String()+"@"+s.endp.serv.Domain+">") } if header.Get("From") == "" { - return &smtp.SMTPError{ - Code: 554, - Message: "Message does not contains a From header field", + return &exterrors.SMTPError{ + Code: 554, + EnhancedCode: exterrors.EnhancedCode{5, 6, 0}, + Message: "Message does not contains a From header field", + Misc: map[string]interface{}{ + "modifier": "submission_prepare", + }, } } for _, hdr := range [...]string{"Sender"} { if value := header.Get(hdr); value != "" { if _, err := mail.ParseAddress(value); err != nil { - return &smtp.SMTPError{ - Code: 554, - Message: fmt.Sprintf("Invalid address in %s: %v", hdr, err), + return &exterrors.SMTPError{ + Code: 554, + EnhancedCode: exterrors.EnhancedCode{5, 6, 0}, + Message: fmt.Sprintf("Invalid address in %s", hdr), + Misc: map[string]interface{}{ + "modifier": "submission_prepare", + "addr": value, + }, + Err: err, } } } @@ -46,9 +53,15 @@ func SubmissionPrepare(msgMeta *module.MsgMetadata, header textproto.Header, our for _, hdr := range [...]string{"To", "Cc", "Bcc", "Reply-To"} { if value := header.Get(hdr); value != "" { if _, err := mail.ParseAddressList(value); err != nil { - return &smtp.SMTPError{ - Code: 554, - Message: fmt.Sprintf("Invalid address in %s: %v", hdr, err), + return &exterrors.SMTPError{ + Code: 554, + EnhancedCode: exterrors.EnhancedCode{5, 6, 0}, + Message: fmt.Sprintf("Invalid address in %s", hdr), + Misc: map[string]interface{}{ + "modifier": "submission_prepare", + "addr": value, + }, + Err: err, } } } @@ -56,31 +69,47 @@ func SubmissionPrepare(msgMeta *module.MsgMetadata, header textproto.Header, our addrs, err := mail.ParseAddressList(header.Get("From")) if err != nil { - return &smtp.SMTPError{ - Code: 554, - Message: fmt.Sprintf("Invalid address in From: %v", err), + return &exterrors.SMTPError{ + Code: 554, + EnhancedCode: exterrors.EnhancedCode{5, 6, 0}, + Message: fmt.Sprintf("Invalid address in From"), + Misc: map[string]interface{}{ + "modifier": "submission_prepare", + "addr": header.Get("From"), + }, + Err: err, } } // https://tools.ietf.org/html/rfc5322#section-3.6.2 // If From contains multiple addresses, Sender field must be present. if len(addrs) > 1 && header.Get("Sender") == "" { - return &smtp.SMTPError{ - Code: 554, - Message: "Missing Sender header field", + return &exterrors.SMTPError{ + Code: 554, + EnhancedCode: exterrors.EnhancedCode{5, 6, 0}, + Message: "Missing Sender header field", + Misc: map[string]interface{}{ + "modifier": "submission_prepare", + "from": header.Get("From"), + }, } } if dateHdr := header.Get("Date"); dateHdr != "" { _, err := time.Parse("Mon, 2 Jan 2006 15:04:05 -0700", dateHdr) if err != nil { - return &smtp.SMTPError{ + return &exterrors.SMTPError{ Code: 554, Message: "Malformed Data header", + Misc: map[string]interface{}{ + "modifier": "submission_prepare", + "date": dateHdr, + }, + Err: err, } } } else { - log.Debugf("adding missing Date header to message from %s (%s)", msgMeta.SrcHostname, msgMeta.SrcAddr) + s.log.Msg("adding missing Date header") header.Set("Date", time.Now().Format("Mon, 2 Jan 2006 15:04:05 -0700")) } diff --git a/log/log.go b/log/log.go index ff37fe9..91ebc44 100644 --- a/log/log.go +++ b/log/log.go @@ -97,7 +97,11 @@ func (l Logger) Error(msg string, err error, fields ...interface{}) { } sort.Strings(errKeys) - allFields = append(allFields, "reason", err.Error()) + // If there is already a 'reason' field - use it, it probably + // provides a better explaination than error text itself. + if errFields["reason"] == nil { + allFields = append(allFields, "reason", err.Error()) + } for _, key := range errKeys { allFields = append(allFields, key, errFields[key]) } diff --git a/msgpipeline/config.go b/msgpipeline/config.go index 48f2447..aba789a 100644 --- a/msgpipeline/config.go +++ b/msgpipeline/config.go @@ -5,10 +5,10 @@ import ( "strconv" "strings" - "github.com/emersion/go-smtp" "github.com/foxcpp/maddy/address" "github.com/foxcpp/maddy/config" modconfig "github.com/foxcpp/maddy/config/module" + "github.com/foxcpp/maddy/exterrors" "github.com/foxcpp/maddy/modify" "github.com/foxcpp/maddy/module" ) @@ -241,10 +241,10 @@ func parseMsgPipelineRcptCfg(globals map[string]interface{}, nodes []config.Node return &rcpt, nil } -func parseRejectDirective(node config.Node) (*smtp.SMTPError, error) { +func parseRejectDirective(node config.Node) (*exterrors.SMTPError, error) { code := 554 - enchCode := smtp.EnhancedCode{5, 7, 0} - msg := "Message rejected due to local policy" + enchCode := exterrors.EnhancedCode{5, 7, 0} + msg := "Message rejected due to a local policy" var err error switch len(node.Args) { case 3: @@ -274,20 +274,23 @@ func parseRejectDirective(node config.Node) (*smtp.SMTPError, error) { default: return nil, config.NodeErr(&node, "invalid count of arguments") } - return &smtp.SMTPError{ - Message: msg, + return &exterrors.SMTPError{ Code: code, EnhancedCode: enchCode, + Message: msg, + Misc: map[string]interface{}{ + "reason": "reject directive used", + }, }, nil } -func parseEnhancedCode(s string) (smtp.EnhancedCode, error) { +func parseEnhancedCode(s string) (exterrors.EnhancedCode, error) { parts := strings.Split(s, ".") if len(parts) != 3 { - return smtp.EnhancedCode{}, fmt.Errorf("wrong amount of enhanced code parts") + return exterrors.EnhancedCode{}, fmt.Errorf("wrong amount of enhanced code parts") } - code := smtp.EnhancedCode{} + code := exterrors.EnhancedCode{} for i, part := range parts { num, err := strconv.Atoi(part) if err != nil { diff --git a/msgpipeline/config_test.go b/msgpipeline/config_test.go index 91d1af9..a5198e2 100644 --- a/msgpipeline/config_test.go +++ b/msgpipeline/config_test.go @@ -5,15 +5,18 @@ import ( "strings" "testing" - "github.com/emersion/go-smtp" "github.com/foxcpp/maddy/config/parser" + "github.com/foxcpp/maddy/exterrors" ) func policyError(code int) error { - return &smtp.SMTPError{ - Message: "Message rejected due to local policy", + return &exterrors.SMTPError{ + Message: "Message rejected due to a local policy", Code: code, - EnhancedCode: smtp.EnhancedCode{5, 7, 0}, + EnhancedCode: exterrors.EnhancedCode{5, 7, 0}, + Misc: map[string]interface{}{ + "reason": "reject directive used", + }, } } diff --git a/msgpipeline/msgpipeline.go b/msgpipeline/msgpipeline.go index 81e9219..c023c28 100644 --- a/msgpipeline/msgpipeline.go +++ b/msgpipeline/msgpipeline.go @@ -8,6 +8,7 @@ import ( "github.com/foxcpp/maddy/address" "github.com/foxcpp/maddy/buffer" "github.com/foxcpp/maddy/config" + "github.com/foxcpp/maddy/exterrors" "github.com/foxcpp/maddy/log" "github.com/foxcpp/maddy/modify" "github.com/foxcpp/maddy/module" @@ -159,10 +160,14 @@ func (dd *msgpipelineDelivery) srcBlockForAddr(mailFrom string) (sourceBlock, er // is not a valid RFC 282 address and only a special // value for SMTP. if err != nil && mailFrom != "" { - return sourceBlock{}, &smtp.SMTPError{ + return sourceBlock{}, &exterrors.SMTPError{ Code: 501, - EnhancedCode: smtp.EnhancedCode{5, 1, 3}, - Message: "Invalid sender address: " + err.Error(), + EnhancedCode: exterrors.EnhancedCode{5, 1, 3}, + Message: "Invalid sender address", + Err: err, + Misc: map[string]interface{}{ + "reason": "Can't extract local-part and host-part", + }, } } @@ -418,10 +423,14 @@ func (dd *msgpipelineDelivery) rcptBlockForAddr(rcptTo string) (*rcptBlock, erro // Then try domain-only. _, domain, err := address.Split(rcptTo) if err != nil { - return nil, &smtp.SMTPError{ + return nil, &exterrors.SMTPError{ Code: 501, - EnhancedCode: smtp.EnhancedCode{5, 1, 3}, - Message: "Invalid recipient address: " + err.Error(), + EnhancedCode: exterrors.EnhancedCode{5, 1, 3}, + Message: "Invalid recipient address", + Err: err, + Misc: map[string]interface{}{ + "reason": "Can't extract local-part and host-part", + }, } } @@ -452,9 +461,8 @@ func (dd *msgpipelineDelivery) getRcptModifiers(rcptBlock *rcptBlock, rcptTo str newSender, err := rcptModifiersState.RewriteSender(dd.sourceAddr) if err == nil && newSender != dd.sourceAddr { - dd.log.Printf("Per-recipient modifier changed sender address. This is not supported and will "+ - "be ignored. RCPT TO = %s, original MAIL FROM = %s, modified MAIL FROM = %s", - rcptTo, dd.sourceAddr, newSender) + dd.log.Msg("Per-recipient modifier changed sender address. This is not supported and will "+ + "be ignored.", "rcpt", rcptTo, "originalFrom", dd.sourceAddr, "modifiedFrom", newSender) } dd.rcptModifiersState[rcptBlock] = rcptModifiersState diff --git a/storage/sql/sql.go b/storage/sql/sql.go index d144ce1..6263ba8 100644 --- a/storage/sql/sql.go +++ b/storage/sql/sql.go @@ -18,13 +18,13 @@ import ( specialuse "github.com/emersion/go-imap-specialuse" "github.com/emersion/go-imap/backend" "github.com/emersion/go-message/textproto" - "github.com/emersion/go-smtp" imapsql "github.com/foxcpp/go-imap-sql" "github.com/foxcpp/maddy/address" "github.com/foxcpp/maddy/auth" "github.com/foxcpp/maddy/buffer" "github.com/foxcpp/maddy/config" "github.com/foxcpp/maddy/dns" + "github.com/foxcpp/maddy/exterrors" "github.com/foxcpp/maddy/log" "github.com/foxcpp/maddy/module" "github.com/foxcpp/maddy/target" @@ -65,10 +65,12 @@ func (d *delivery) AddRcpt(rcptTo string) error { var err error accountName, _, err = address.Split(rcptTo) if err != nil { - return &smtp.SMTPError{ + return &exterrors.SMTPError{ Code: 501, - EnhancedCode: smtp.EnhancedCode{5, 1, 3}, - Message: "Invalid recipient address: " + err.Error(), + EnhancedCode: exterrors.EnhancedCode{5, 1, 3}, + Message: "Invalid recipient address", + TargetName: "sql", + Err: err, } } } @@ -86,10 +88,12 @@ func (d *delivery) AddRcpt(rcptTo string) error { if err := d.d.AddRcpt(strings.ToLower(accountName), userHeader); err != nil { if err == imapsql.ErrUserDoesntExists || err == backend.ErrNoSuchMailbox { - return &smtp.SMTPError{ + return &exterrors.SMTPError{ Code: 550, - EnhancedCode: smtp.EnhancedCode{5, 1, 1}, + EnhancedCode: exterrors.EnhancedCode{5, 1, 1}, Message: "User doesn't exist", + TargetName: "sql", + Err: err, } } return err diff --git a/target/smtp_downstream/sasl.go b/target/smtp_downstream/sasl.go index de958c2..59998db 100644 --- a/target/smtp_downstream/sasl.go +++ b/target/smtp_downstream/sasl.go @@ -2,8 +2,8 @@ package smtp_downstream import ( "github.com/emersion/go-sasl" - "github.com/emersion/go-smtp" "github.com/foxcpp/maddy/config" + "github.com/foxcpp/maddy/exterrors" ) type saslClientFactory func(downstreamUser, downstreamPass string) (sasl.Client, error) @@ -29,10 +29,14 @@ func (u *Downstream) saslAuthDirective(m *config.Map, node *config.Node) (interf return func(downstreamUser, downstreamPass string) (sasl.Client, error) { if downstreamUser == "" || downstreamPass == "" { u.log.Printf("client is not authenticated, can't forward credentials") - return nil, &smtp.SMTPError{ + return nil, &exterrors.SMTPError{ Code: 530, - EnhancedCode: smtp.EnhancedCode{5, 7, 0}, + EnhancedCode: exterrors.EnhancedCode{5, 7, 0}, Message: "Authentication is required", + TargetName: "smtp_downstream", + Misc: map[string]interface{}{ + "reason": "Credentials forwarding is requested but the client is not authenticated", + }, } } // TODO: See if it is useful to support custom identity argument.