Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client and server support for SMTP DSN extension #135

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ const (
BodyBinaryMIME BodyType = "BINARYMIME"
)

type DSNReturn string

const (
// Attach the full copy of the message to any DSN that indicates
// a failure. Non-failure DSNs always contain the header only.
ReturnFull DSNReturn = "FULL"

// Attach only header of the message to any DSN that indicates a
// failure.
ReturnHeaders DSNReturn = "HDRS"
)

// MailOptions contains custom arguments that were
// passed as an argument to the MAIL command.
type MailOptions struct {
Expand Down Expand Up @@ -56,6 +68,45 @@ type MailOptions struct {
//
// Defined in RFC 4954.
Auth *string

// Whether the full message or header only should be returned in
// failure DSNs.
//
// Defined in RFC 3461. Ignored if the server does not support DSN
// extension.
Return DSNReturn

// Envelope ID identifier. Returned in any DSN for the message.
//
// Not in xtext encoding. go-smtp restricts value to printable US-ASCII
// as required by specification.
//
// Defined in RFC 3461. Ignored if the server does not support DSN
// extension.
EnvelopeID string
}

type DSNNotify string

const (
NotifyNever DSNNotify = "NEVER"
NotifySuccess DSNNotify = "SUCCESS"
NotifyDelayed DSNNotify = "DELAYED"
NotifyFailure DSNNotify = "FAILURE"
)

type RcptOptions struct {
// When DSN should be generated for this recipient.
// As described in RFC 3461.
Notify []DSNNotify

// Original message recipient as described in RFC 3461.
//
// Value of OriginalRecipient is preserved as is. No xtext
// encoding/decoding or sanitization is done irregardless of
// OriginalRecipientType.
OriginalRecipient string
OriginalRecipientType string
}

// Session is used by servers to respond to an SMTP client.
Expand All @@ -71,7 +122,7 @@ type Session interface {
// Set return path for currently processed message.
Mail(from string, opts MailOptions) error
// Add recipient for currently processed message.
Rcpt(to string) error
Rcpt(to string, opts RcptOptions) error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If #145 is accepted, opts should be switched to a pointer as well.

// Set currently processed message contents and send it.
Data(r io.Reader) error
}
Expand Down
4 changes: 2 additions & 2 deletions backendutil/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ func (s *transformSession) Mail(from string, opts smtp.MailOptions) error {
return s.Session.Mail(from, opts)
}

func (s *transformSession) Rcpt(to string) error {
func (s *transformSession) Rcpt(to string, opts smtp.RcptOptions) error {
if s.be.TransformRcpt != nil {
var err error
to, err = s.be.TransformRcpt(to)
if err != nil {
return err
}
}
return s.Session.Rcpt(to)
return s.Session.Rcpt(to, opts)
}

func (s *transformSession) Data(r io.Reader) error {
Expand Down
2 changes: 1 addition & 1 deletion backendutil/transform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (s *session) Mail(from string, opts smtp.MailOptions) error {
return nil
}

func (s *session) Rcpt(to string) error {
func (s *session) Rcpt(to string, opts smtp.RcptOptions) error {
s.msg.To = append(s.msg.To, to)
return nil
}
Expand Down
85 changes: 66 additions & 19 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,25 +372,36 @@ func (c *Client) Mail(from string, opts *MailOptions) error {
if _, ok := c.ext["SIZE"]; ok && opts != nil && opts.Size != 0 {
cmdStr += " SIZE=" + strconv.Itoa(opts.Size)
}
if opts != nil && opts.RequireTLS {
if _, ok := c.ext["REQUIRETLS"]; ok {
cmdStr += " REQUIRETLS"
} else {
return errors.New("smtp: server does not support REQUIRETLS")
if opts != nil {
if opts.RequireTLS {
if _, ok := c.ext["REQUIRETLS"]; ok {
cmdStr += " REQUIRETLS"
} else {
return errors.New("smtp: server does not support REQUIRETLS")
}
}
}
if opts != nil && opts.UTF8 {
if _, ok := c.ext["SMTPUTF8"]; ok {
cmdStr += " SMTPUTF8"
} else {
return errors.New("smtp: server does not support SMTPUTF8")
if opts.UTF8 {
if _, ok := c.ext["SMTPUTF8"]; ok {
cmdStr += " SMTPUTF8"
} else {
return errors.New("smtp: server does not support SMTPUTF8")
}
}
}
if opts != nil && opts.Auth != nil {
if _, ok := c.ext["AUTH"]; ok {
cmdStr += " AUTH=" + encodeXtext(*opts.Auth)
if opts.Auth != nil {
if _, ok := c.ext["AUTH"]; ok {
cmdStr += " AUTH=" + encodeXtext(*opts.Auth)
}
// We can safely discard parameter if server does not support AUTH.
}
if _, dsn := c.ext["DSN"]; dsn && opts.Return != "" {
cmdStr += " RET=" + string(opts.Return)
}
if _, dsn := c.ext["DSN"]; dsn && opts.EnvelopeID != "" {
if !checkPrintableASCII(opts.EnvelopeID) {
return errors.New("smtp: characters outside of printable ASCII are not allowed in EnvelopeID")
}
cmdStr += " ENVID=" + encodeXtext(opts.EnvelopeID)
}
// We can safely discard parameter if server does not support AUTH.
}
_, _, err := c.cmd(250, cmdStr, from)
return err
Expand All @@ -400,12 +411,48 @@ func (c *Client) Mail(from string, opts *MailOptions) error {
// A call to Rcpt must be preceded by a call to Mail and may be followed by
// a Data call or another Rcpt call.
//
// If opts is not nil, RCPT arguments provided in the structure will be added
// to the command. Handling of unsupported options depends on the extension.
//
// If server returns an error, it will be of type *SMTPError.
func (c *Client) Rcpt(to string) error {
func (c *Client) Rcpt(to string, opts *RcptOptions) error {
if err := validateLine(to); err != nil {
return err
}
if _, _, err := c.cmd(25, "RCPT TO:<%s>", to); err != nil {
cmdStr := "RCPT TO:<%s>"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to just append content to cmdStr? The new content could have user-supplied %v sequences, which fmt will choke on.

We should probably set cmdStr = "RCPT TO:<%s>%s" and have a separate extStr string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It applies to MAIL FROM too which uses similar code.

if opts != nil {
if len(opts.Notify) != 0 {
if _, ok := c.ext["DSN"]; ok {
notifyFlags := make([]string, len(opts.Notify))
for i := range opts.Notify {
if opts.Notify[i] == NotifyNever && len(opts.Notify) != 1 {
return errors.New("smtp: NotifyNever cannot be combined with other options")
}
notifyFlags[i] = string(opts.Notify[i])
}
cmdStr += " NOTIFY=" + strings.Join(notifyFlags, ",")
} else {
return errors.New("smtp: server does not support DSN")
}
}
if opts.OriginalRecipientType != "" {
// This is not strictly speaking a requirement of RFC that requires a registered
// address type but we verify it nonetheless to prevent injections.
if !checkPrintableASCII(opts.OriginalRecipientType) {
return errors.New("smtp: characters outside of printable ASCII are not allowed in OriginalRecipientType")
}
if !checkPrintableASCII(opts.OriginalRecipient) {
return errors.New("smtp: characters outside of printable ASCII are not allowed in OriginalRecipient")
}
if _, ok := c.ext["DSN"]; ok {
cmdStr += " ORCPT=" + opts.OriginalRecipientType + ";" + encodeXtext(opts.OriginalRecipient)
}
// ORCPT is unlikely to be critical for message handling so we can
// silently disregard it if server actually does not support DSN
// extension.
}
}
if _, _, err := c.cmd(25, cmdStr, to); err != nil {
return err
}
c.rcpts = append(c.rcpts, to)
Expand Down Expand Up @@ -546,7 +593,7 @@ func SendMail(addr string, a sasl.Client, from string, to []string, r io.Reader)
return err
}
for _, addr := range to {
if err = c.Rcpt(addr); err != nil {
if err = c.Rcpt(addr, nil); err != nil {
return err
}
}
Expand Down
10 changes: 5 additions & 5 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestBasic(t *testing.T) {
t.Fatalf("AUTH failed: %s", err)
}

if err := c.Rcpt("[email protected]>\r\nDATA\r\nInjected message body\r\n.\r\nQUIT\r\n"); err == nil {
if err := c.Rcpt("[email protected]>\r\nDATA\r\nInjected message body\r\n.\r\nQUIT\r\n", nil); err == nil {
t.Fatalf("RCPT should have failed due to a message injection attempt")
}
if err := c.Mail("[email protected]>\r\nDATA\r\nAnother injected message body\r\n.\r\nQUIT\r\n", nil); err == nil {
Expand All @@ -130,7 +130,7 @@ func TestBasic(t *testing.T) {
if err := c.Mail("[email protected]", nil); err != nil {
t.Fatalf("MAIL failed: %s", err)
}
if err := c.Rcpt("[email protected]"); err != nil {
if err := c.Rcpt("[email protected]", nil); err != nil {
t.Fatalf("RCPT failed: %s", err)
}
msg := `From: [email protected]
Expand Down Expand Up @@ -922,7 +922,7 @@ func TestLMTP(t *testing.T) {
if err := c.Mail("[email protected]", nil); err != nil {
t.Fatalf("MAIL failed: %s", err)
}
if err := c.Rcpt("[email protected]"); err != nil {
if err := c.Rcpt("[email protected]", nil); err != nil {
t.Fatalf("RCPT failed: %s", err)
}
msg := `From: [email protected]
Expand Down Expand Up @@ -1007,10 +1007,10 @@ func TestLMTPData(t *testing.T) {
if err := c.Mail("[email protected]", nil); err != nil {
t.Fatalf("MAIL failed: %s", err)
}
if err := c.Rcpt("[email protected]"); err != nil {
if err := c.Rcpt("[email protected]", nil); err != nil {
t.Fatalf("RCPT failed: %s", err)
}
if err := c.Rcpt("[email protected]"); err != nil {
if err := c.Rcpt("[email protected]", nil); err != nil {
t.Fatalf("RCPT failed: %s", err)
}
msg := `From: [email protected]
Expand Down
2 changes: 1 addition & 1 deletion cmd/smtp-debug-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (s *session) Mail(from string, opts smtp.MailOptions) error {
return nil
}

func (s *session) Rcpt(to string) error {
func (s *session) Rcpt(to string, opts smtp.RcptOptions) error {
return nil
}

Expand Down
90 changes: 84 additions & 6 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ func (c *Conn) handleGreet(enhanced bool, arg string) {
if c.server.EnableBINARYMIME {
caps = append(caps, "BINARYMIME")
}
if c.server.EnableDSN {
caps = append(caps, "DSN")
}
if c.server.MaxMessageBytes > 0 {
caps = append(caps, fmt.Sprintf("SIZE %v", c.server.MaxMessageBytes))
}
Expand Down Expand Up @@ -396,6 +399,24 @@ func (c *Conn) handleMail(arg string) {
}
decodedMbox := value[1 : len(value)-1]
opts.Auth = &decodedMbox
case "RET":
value := DSNReturn(strings.ToUpper(value))
if value != ReturnFull && value != ReturnHeaders {
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Unsupported RET value")
return
}
opts.Return = value
case "ENVID":
value, err := decodeXtext(value)
if err != nil {
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Malformed xtext in ENVID")
return
}
if !checkPrintableASCII(value) {
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Only printable ASCII allowed in ENVID")
return
}
opts.EnvelopeID = value
default:
c.WriteResponse(500, EnhancedCode{5, 5, 4}, "Unknown MAIL FROM argument")
return
Expand Down Expand Up @@ -467,6 +488,15 @@ func encodeXtext(raw string) string {
return out.String()
}

func checkPrintableASCII(s string) bool {
for _, c := range s {
if c < 32 && c > 127 {
return false
}
}
return true
}

// MAIL state -> waiting for RCPTs followed by DATA
func (c *Conn) handleRcpt(arg string) {
if !c.fromReceived {
Expand All @@ -478,20 +508,68 @@ func (c *Conn) handleRcpt(arg string) {
return
}

if (len(arg) < 4) || (strings.ToUpper(arg[0:3]) != "TO:") {
c.WriteResponse(501, EnhancedCode{5, 5, 2}, "Was expecting RCPT arg syntax of TO:<address>")
if len(arg) < 4 || strings.ToUpper(arg[0:3]) != "TO:" {
c.WriteResponse(501, EnhancedCode{5, 5, 1}, "Was expecting TO arg syntax of TO:<address>")
return
}

// TODO: This trim is probably too forgiving
recipient := strings.Trim(arg[3:], "<> ")
toArgs := strings.Split(strings.Trim(arg[3:], " "), " ")
if c.server.Strict {
if !strings.HasPrefix(toArgs[0], "<") || !strings.HasSuffix(toArgs[0], ">") {
c.WriteResponse(501, EnhancedCode{5, 5, 1}, "Was expecting TO arg syntax of TO:<address>")
return
}
}
recipient := toArgs[0]
if recipient == "" {
c.WriteResponse(501, EnhancedCode{5, 5, 2}, "Was expecting MAIL arg syntax of FROM:<address>")
return
}
recipient = strings.Trim(recipient, "<>")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the TODO about this trim being not quite what we want?

Copy link
Contributor

@kayrus kayrus Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't like the existing TODO, and proposed to use mail.ParseAddress. How is it implemented in postfix/exim mail servers? Shall we introduce a similar function?


if c.server.MaxRecipients > 0 && len(c.recipients) >= c.server.MaxRecipients {
c.WriteResponse(552, EnhancedCode{5, 5, 3}, fmt.Sprintf("Maximum limit of %v recipients reached", c.server.MaxRecipients))
return
}

if err := c.Session().Rcpt(recipient); err != nil {
opts := RcptOptions{}

if len(toArgs) > 1 {
args, err := parseArgs(toArgs[1:])
if err != nil {
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Unable to parse TO ESMTP parameters")
return
}

for key, value := range args {
switch key {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to error out on unrecognized parameter?

case "ORCPT":
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a // This space is intentionally left blank comment on the next line? IMHO it's too easy to misread this as a fall-through.

case "NOTIFY":
notifyFlags := strings.Split(strings.ToUpper(value), ",")
seenFlags := make(map[string]struct{})
for _, f := range notifyFlags {
if _, ok := seenFlags[f]; ok {
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "NOTIFY parameters cannot be specified multiple times")
return
}
switch DSNNotify(f) {
case NotifyNever:
if len(notifyFlags) != 1 {
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "NOTIFY=NEVER cannot be combined with other options")
return
}
case NotifyDelayed, NotifySuccess, NotifyFailure:
default:
c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Unknown NOTIFY parameter")
return
}
seenFlags[f] = struct{}{}
opts.Notify = append(opts.Notify, DSNNotify(f))
}
}
}
}

if err := c.Session().Rcpt(recipient, RcptOptions{}); err != nil {
if smtpErr, ok := err.(*SMTPError); ok {
c.WriteResponse(smtpErr.Code, smtpErr.EnhancedCode, smtpErr.Message)
return
Expand Down
Loading