From 3071dd014f3a93f05133fbcd0d8e6ed32c85f379 Mon Sep 17 00:00:00 2001 From: Daniel Cormier Date: Mon, 11 Jun 2018 16:31:04 -0400 Subject: [PATCH 1/3] Don't remove HELO/EHLO domain on RSET Per RFC 5321, sections 4.1.1.5 and 4.1.4, issuing an RSET command should not result in the HELO/EHLO domain being removed from the session. From section 4.1.1.5: > It is effectively equivalent to a NOOP (i.e., it has no > effect) if issued immediately after EHLO, before EHLO is > issued in the session, after an end of data indicator has > been sent and acknowledged, or immediately before a QUIT. And: > Since EHLO implies some additional processing and response by the > server, RSET will normally be more efficient than reissuing that > command, even though the formal semantics are the same. --- protocol.go | 2 ++ protocol_test.go | 1 + 2 files changed, 3 insertions(+) diff --git a/protocol.go b/protocol.go index a09f82d..11793b5 100644 --- a/protocol.go +++ b/protocol.go @@ -229,8 +229,10 @@ func (proto *Protocol) Command(command *Command) (reply *Reply) { return ReplyBye() case "RSET" == command.verb: proto.logf("Got RSET command, switching to MAIL state") + helo := proto.Message.Helo proto.State = MAIL proto.Message = &data.SMTPMessage{} + proto.Message.Helo = helo return ReplyOk() case "NOOP" == command.verb: proto.logf("Got NOOP verb, staying in %s state", StateMap[proto.State]) diff --git a/protocol_test.go b/protocol_test.go index 45f0125..20f1c75 100644 --- a/protocol_test.go +++ b/protocol_test.go @@ -485,6 +485,7 @@ func TestRSET(t *testing.T) { So(reply.Status, ShouldEqual, 250) So(reply.Lines(), ShouldResemble, []string{"250 Ok\r\n"}) So(proto.State, ShouldEqual, MAIL) + So(proto.Message.Helo, ShouldEqual, "localhost") So(proto.Message.From, ShouldEqual, "") So(len(proto.Message.To), ShouldEqual, 0) }) From 074b367bc37d2e16807b4da3918a3b8ef5b9ac92 Mon Sep 17 00:00:00 2001 From: Daniel Cormier Date: Tue, 12 Jun 2018 08:34:38 -0400 Subject: [PATCH 2/3] Reset values on subsequent HELO/EHLO commands Based on RFC 5321, section 4.1.4, any stored values should be reset if a HELO/EHLO command is sent subsequent to the original: > An EHLO command MAY be issued by a client later in the session. If > it is issued after the session begins and the EHLO command is > acceptable to the SMTP server, the SMTP server MUST clear all buffers > and reset the state exactly as if a RSET command had been issued. In > other words, the sequence of RSET followed immediately by EHLO is > redundant, but not harmful other than in the performance cost of > executing unnecessary commands. --- protocol.go | 2 ++ protocol_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/protocol.go b/protocol.go index 11793b5..6cd38e6 100644 --- a/protocol.go +++ b/protocol.go @@ -412,6 +412,7 @@ func (proto *Protocol) Command(command *Command) (reply *Reply) { // HELO creates a reply to a HELO command func (proto *Protocol) HELO(args string) (reply *Reply) { proto.logf("Got HELO command, switching to MAIL state") + proto.resetState() proto.State = MAIL proto.Message.Helo = args return ReplyOk("Hello " + args) @@ -420,6 +421,7 @@ func (proto *Protocol) HELO(args string) (reply *Reply) { // EHLO creates a reply to a EHLO command func (proto *Protocol) EHLO(args string) (reply *Reply) { proto.logf("Got EHLO command, switching to MAIL state") + proto.resetState() proto.State = MAIL proto.Message.Helo = args replyArgs := []string{"Hello " + args, "PIPELINING"} diff --git a/protocol_test.go b/protocol_test.go index 20f1c75..b1dcfa3 100644 --- a/protocol_test.go +++ b/protocol_test.go @@ -296,7 +296,7 @@ func TestEHLO(t *testing.T) { So(proto.State, ShouldEqual, MAIL) So(proto.Message.Helo, ShouldEqual, "localhost") }) - Convey("HELO should work in MAIL state", t, func() { + Convey("EHLO should work in MAIL state", t, func() { proto := NewProtocol() proto.Start() proto.Command(ParseCommand("HELO localhost")) @@ -309,19 +309,43 @@ func TestEHLO(t *testing.T) { So(proto.State, ShouldEqual, MAIL) So(proto.Message.Helo, ShouldEqual, "localhost") }) - Convey("HELO should work in RCPT state", t, func() { + Convey("EHLO should work in RCPT state", t, func() { proto := NewProtocol() proto.Start() - proto.Command(ParseCommand("HELO localhost")) + proto.Command(ParseCommand("EHLO localhost")) proto.Command(ParseCommand("MAIL From:")) So(proto.State, ShouldEqual, RCPT) So(proto.Message.Helo, ShouldEqual, "localhost") - reply := proto.Command(ParseCommand("EHLO localhost")) + So(proto.Message.From, ShouldEqual, "test") + So(proto.Message.To, ShouldBeZeroValue) + reply := proto.Command(ParseCommand("EHLO localhost2")) So(reply, ShouldNotBeNil) So(reply.Status, ShouldEqual, 250) - So(reply.Lines(), ShouldResemble, []string{"250-Hello localhost\r\n", "250 PIPELINING\r\n"}) + So(reply.Lines(), ShouldResemble, []string{"250-Hello localhost2\r\n", "250 PIPELINING\r\n"}) So(proto.State, ShouldEqual, MAIL) + So(proto.Message.Helo, ShouldEqual, "localhost2") + So(proto.Message.From, ShouldBeZeroValue) + So(proto.Message.To, ShouldBeZeroValue) + }) + Convey("EHLO should work in RCPT state with recipients", t, func() { + proto := NewProtocol() + proto.Start() + proto.Command(ParseCommand("EHLO localhost")) + proto.Command(ParseCommand("MAIL From:")) + proto.Command(ParseCommand("RCPT To:")) + proto.Command(ParseCommand("RCPT To:")) + So(proto.State, ShouldEqual, RCPT) So(proto.Message.Helo, ShouldEqual, "localhost") + So(proto.Message.From, ShouldEqual, "test") + So(proto.Message.To, ShouldResemble, []string{"rcpt1", "rcpt2"}) + reply := proto.Command(ParseCommand("EHLO localhost2")) + So(reply, ShouldNotBeNil) + So(reply.Status, ShouldEqual, 250) + So(reply.Lines(), ShouldResemble, []string{"250-Hello localhost2\r\n", "250 PIPELINING\r\n"}) + So(proto.State, ShouldEqual, MAIL) + So(proto.Message.Helo, ShouldEqual, "localhost2") + So(proto.Message.From, ShouldBeZeroValue) + So(proto.Message.To, ShouldBeZeroValue) }) } @@ -370,12 +394,36 @@ func TestHELO(t *testing.T) { proto.Command(ParseCommand("MAIL From:")) So(proto.State, ShouldEqual, RCPT) So(proto.Message.Helo, ShouldEqual, "localhost") - reply := proto.Command(ParseCommand("HELO localhost")) + So(proto.Message.From, ShouldEqual, "test") + So(proto.Message.To, ShouldBeZeroValue) + reply := proto.Command(ParseCommand("HELO localhost2")) So(reply, ShouldNotBeNil) So(reply.Status, ShouldEqual, 250) - So(reply.Lines(), ShouldResemble, []string{"250 Hello localhost\r\n"}) + So(reply.Lines(), ShouldResemble, []string{"250 Hello localhost2\r\n"}) So(proto.State, ShouldEqual, MAIL) + So(proto.Message.Helo, ShouldEqual, "localhost2") + So(proto.Message.From, ShouldBeZeroValue) + So(proto.Message.To, ShouldBeZeroValue) + }) + Convey("HELO should work in RCPT state with recipients", t, func() { + proto := NewProtocol() + proto.Start() + proto.Command(ParseCommand("HELO localhost")) + proto.Command(ParseCommand("MAIL From:")) + proto.Command(ParseCommand("RCPT To:")) + proto.Command(ParseCommand("RCPT To:")) + So(proto.State, ShouldEqual, RCPT) So(proto.Message.Helo, ShouldEqual, "localhost") + So(proto.Message.From, ShouldEqual, "test") + So(proto.Message.To, ShouldResemble, []string{"rcpt1", "rcpt2"}) + reply := proto.Command(ParseCommand("HELO localhost2")) + So(reply, ShouldNotBeNil) + So(reply.Status, ShouldEqual, 250) + So(reply.Lines(), ShouldResemble, []string{"250 Hello localhost2\r\n"}) + So(proto.State, ShouldEqual, MAIL) + So(proto.Message.Helo, ShouldEqual, "localhost2") + So(proto.Message.From, ShouldBeZeroValue) + So(proto.Message.To, ShouldBeZeroValue) }) } From 6660dbc3f2bba40e4e3f23d3cebc57ff1aa5ee48 Mon Sep 17 00:00:00 2001 From: Daniel Cormier Date: Tue, 12 Jun 2018 12:10:39 -0400 Subject: [PATCH 3/3] Fixed one last EHLO test to use EHLO rather than HELO --- protocol_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol_test.go b/protocol_test.go index b1dcfa3..c6ecf61 100644 --- a/protocol_test.go +++ b/protocol_test.go @@ -299,7 +299,7 @@ func TestEHLO(t *testing.T) { Convey("EHLO should work in MAIL state", t, func() { proto := NewProtocol() proto.Start() - proto.Command(ParseCommand("HELO localhost")) + proto.Command(ParseCommand("EHLO localhost")) So(proto.State, ShouldEqual, MAIL) So(proto.Message.Helo, ShouldEqual, "localhost") reply := proto.Command(ParseCommand("EHLO localhost"))