From 752519b0c8be28bffd3e2bbf9827a04a3dd8348a Mon Sep 17 00:00:00 2001 From: Nathan Ollerenshaw Date: Fri, 29 Jun 2018 14:13:01 -0700 Subject: [PATCH 1/4] Modified address parsing to use the net/mail ParseAddress function. --- address.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/address.go b/address.go index 68ae8e6..2a792ac 100644 --- a/address.go +++ b/address.go @@ -2,18 +2,29 @@ package smtpd import ( "fmt" + "net/mail" "strings" ) func parseAddress(src string) (string, error) { - if len(src) == 0 || src[0] != '<' || src[len(src)-1] != '>' { - return "", fmt.Errorf("Ill-formatted e-mail address: %s", src) + // Mailbox specifications as per RFC5321 must have a local part and a + // domain part separated by '@' + if strings.Count(src, "@") != 1 { + return "", fmt.Errorf("malformed e-mail address: %s", src) } - if strings.Count(src, "@") > 1 { - return "", fmt.Errorf("Ill-formatted e-mail address: %s", src) + // While a RFC5321 mailbox specification is not the same as an RFC5322 + // email address specification, it is better to accept that format and + // parse it down to the actual address, as there are a lot of badly + // behaving MTAs and MUAs that do it wrongly. It therefore makes sense + // to rely on Go's built-in address parser. This does have the benefit + // of allowing "email@example.com" as input as thats commonly used, + // though not RFC compliant. + addr, err := mail.ParseAddress(src) + if err != nil { + return "", fmt.Errorf("malformed e-mail address: %s", src) } - return src[1 : len(src)-1], nil + return addr.Address, nil } From c009354c02d8c51b2a0975b9624bc4dbb31fde29 Mon Sep 17 00:00:00 2001 From: Nathan Ollerenshaw Date: Fri, 29 Jun 2018 14:32:02 -0700 Subject: [PATCH 2/4] Removed check for single '@' in address. --- address.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/address.go b/address.go index 2a792ac..5cf93d8 100644 --- a/address.go +++ b/address.go @@ -3,17 +3,9 @@ package smtpd import ( "fmt" "net/mail" - "strings" ) func parseAddress(src string) (string, error) { - - // Mailbox specifications as per RFC5321 must have a local part and a - // domain part separated by '@' - if strings.Count(src, "@") != 1 { - return "", fmt.Errorf("malformed e-mail address: %s", src) - } - // While a RFC5321 mailbox specification is not the same as an RFC5322 // email address specification, it is better to accept that format and // parse it down to the actual address, as there are a lot of badly From eb300d92ccfe8de3727a9653f0b38d26718d1528 Mon Sep 17 00:00:00 2001 From: Nathan Ollerenshaw Date: Fri, 29 Jun 2018 14:41:59 -0700 Subject: [PATCH 3/4] Allow the '<>' null sender. --- protocol.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/protocol.go b/protocol.go index c4fdca8..bf6c427 100644 --- a/protocol.go +++ b/protocol.go @@ -212,11 +212,17 @@ func (session *session) handleMAIL(cmd command) { return } - addr, err := parseAddress(cmd.params[1]) + var err error + addr := "" // null sender - if err != nil { - session.reply(502, "Ill-formatted e-mail address") - return + // We must accept a null sender as per rfc5321 section-6.1. + if cmd.params[1] != "<>" { + addr, err = parseAddress(cmd.params[1]) + + if err != nil { + session.reply(502, "Malformed e-mail address") + return + } } if session.server.SenderChecker != nil { @@ -256,7 +262,7 @@ func (session *session) handleRCPT(cmd command) { addr, err := parseAddress(cmd.params[1]) if err != nil { - session.reply(502, "Ill-formatted e-mail address") + session.reply(502, "Malformed e-mail address") return } From 6441050e005a0886188eb5422c46eb028d527c31 Mon Sep 17 00:00:00 2001 From: Christian Joergensen Date: Wed, 26 Sep 2018 21:10:58 +0200 Subject: [PATCH 4/4] Added extra and cleaned old test cases for pull request #3. --- smtpd_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/smtpd_test.go b/smtpd_test.go index 3b7a5b7..8d7ae70 100644 --- a/smtpd_test.go +++ b/smtpd_test.go @@ -1003,19 +1003,19 @@ func TestHELO(t *testing.T) { } if err := cmd(c.Text, 502, "MAIL FROM:"); err != nil { - t.Fatalf("MAIL didn't fail: %v", err) + t.Fatalf("MAIL before HELO didn't fail: %v", err) } if err := cmd(c.Text, 250, "HELO localhost"); err != nil { t.Fatalf("HELO failed: %v", err) } - if err := cmd(c.Text, 502, "MAIL FROM:christian@technobabble.dk"); err != nil { - t.Fatalf("MAIL didn't fail: %v", err) + if err := cmd(c.Text, 250, "MAIL FROM:"); err != nil { + t.Fatalf("MAIL after HELO failed: %v", err) } if err := cmd(c.Text, 250, "HELO localhost"); err != nil { - t.Fatalf("HELO failed: %v", err) + t.Fatalf("double HELO failed: %v", err) } if err := c.Quit(); err != nil { @@ -1079,6 +1079,56 @@ func TestLOGINAuth(t *testing.T) { } +func TestNullSender(t *testing.T) { + + addr, closer := runserver(t, &smtpd.Server{}) + + defer closer() + + c, err := smtp.Dial(addr) + if err != nil { + t.Fatalf("Dial failed: %v", err) + } + + if err := cmd(c.Text, 250, "HELO localhost"); err != nil { + t.Fatalf("HELO failed: %v", err) + } + + if err := cmd(c.Text, 250, "MAIL FROM:<>"); err != nil { + t.Fatalf("MAIL with null sender failed: %v", err) + } + + if err := c.Quit(); err != nil { + t.Fatalf("Quit failed: %v", err) + } + +} + +func TestNoBracketsSender(t *testing.T) { + + addr, closer := runserver(t, &smtpd.Server{}) + + defer closer() + + c, err := smtp.Dial(addr) + if err != nil { + t.Fatalf("Dial failed: %v", err) + } + + if err := cmd(c.Text, 250, "HELO localhost"); err != nil { + t.Fatalf("HELO failed: %v", err) + } + + if err := cmd(c.Text, 250, "MAIL FROM:christian@technobabble.dk"); err != nil { + t.Fatalf("MAIL without brackets failed: %v", err) + } + + if err := c.Quit(); err != nil { + t.Fatalf("Quit failed: %v", err) + } + +} + func TestErrors(t *testing.T) { cert, err := tls.X509KeyPair(localhostCert, localhostKey) @@ -1111,10 +1161,6 @@ func TestErrors(t *testing.T) { t.Fatalf("AUTH didn't fail: %v", err) } - if err := cmd(c.Text, 502, "MAIL FROM:christian@technobabble.dk"); err != nil { - t.Fatalf("MAIL didn't fail: %v", err) - } - if err := c.Mail("sender@example.org"); err != nil { t.Fatalf("MAIL failed: %v", err) }