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.

Sign up for GitHub

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

Jump to bottom

Fix command line length bug #221

Merged
merged 2 commits into from
Jul 11, 2021
Merged

Fix command line length bug #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f5f653
handler: Don't fail on 8 character command lines
jhillyerd Jul 11, 2021
e90c87e
handler: Test that STARTTLS is parsed correctly.
jhillyerd Jul 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 18 additions & 19 deletions pkg/server/smtp/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (s *Server) startSession(id int, conn net.Conn) {
}
break
}
// not an EOF
// Not an EOF
ssn.logger.Warn().Msgf("Connection error: %v", err)
if netErr, ok := err.(net.Error); ok {
if netErr.Timeout() {
Expand Down Expand Up @@ -281,7 +281,7 @@ func (s *Session) greetHandler(cmd string, arg string) {
return
}
s.remoteDomain = domain
// features before SIZE per RFC
// Features before SIZE per RFC
s.send("250-" + readyBanner)
s.send("250-8BITMIME")
s.send("250-AUTH PLAIN LOGIN")
Expand Down Expand Up @@ -331,20 +331,21 @@ func (s *Session) passwordHandler(line string) {
func (s *Session) readyHandler(cmd string, arg string) {
if cmd == "STARTTLS" {
if !s.Server.config.TLSEnabled {
// invalid command since unconfigured
// Invalid command since TLS unconfigured.
s.logger.Debug().Msgf("454 TLS unavailable on the server")
s.send("454 TLS unavailable on the server")
return
}
if s.tlsState != nil {
// tls state previously valid
// TLS state previously valid.
s.logger.Debug().Msg("454 A TLS session already agreed upon.")
s.send("454 A TLS session already agreed upon.")
return
}
s.logger.Debug().Msg("Initiating TLS context.")

// Start TLS connection handshake.
s.send("220 STARTTLS")
// start tls connection handshake
tlsConn := tls.Server(s.conn, s.Server.tlsConfig)
s.conn = tlsConn
s.text = textproto.NewConn(s.conn)
Expand Down Expand Up @@ -591,30 +592,28 @@ func (s *Session) readLine() (line string, err error) {

func (s *Session) parseCmd(line string) (cmd string, arg string, ok bool) {
line = strings.TrimRight(line, "\r\n")
l := len(line)

// Find length of command or entire line.
hasArg := true
l := strings.IndexByte(line, ' ')
if l == -1 {
hasArg = false
l = len(line)
}

switch {
case l == 0:
return "", "", true
case l < 4:
s.logger.Warn().Msgf("Command too short: %q", line)
return "", "", false
case l == 4 || l == 8:
return strings.ToUpper(line), "", true
case l == 5:
// Too long to be only command, too short to have args
s.logger.Warn().Msgf("Mangled command: %q", line)
return "", "", false
}

// If we made it here, command is long enough to have args
if line[4] != ' ' {
// There wasn't a space after the command?
s.logger.Warn().Msgf("Mangled command: %q", line)
return "", "", false
if hasArg {
return strings.ToUpper(line[0:l]), strings.Trim(line[l+1:], " "), true
}

// I'm not sure if we should trim the args or not, but we will for now
return strings.ToUpper(line[0:4]), strings.Trim(line[5:], " "), true
return strings.ToUpper(line), "", true
}

// parseArgs takes the arguments proceeding a command and files them
Expand Down
15 changes: 15 additions & 0 deletions pkg/server/smtp/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func TestGreetState(t *testing.T) {
if err := playSession(t, server, []scriptStep{{"helo 127.0.0.1", 250}}); err != nil {
t.Error(err)
}
if err := playSession(t, server, []scriptStep{{"HELO ABC", 250}}); err != nil {
t.Error(err)
}

// Valid EHLOs
if err := playSession(t, server, []scriptStep{{"EHLO mydomain", 250}}); err != nil {
Expand All @@ -70,6 +73,9 @@ func TestGreetState(t *testing.T) {
if err := playSession(t, server, []scriptStep{{"ehlo 127.0.0.1", 250}}); err != nil {
t.Error(err)
}
if err := playSession(t, server, []scriptStep{{"EHLO a", 250}}); err != nil {
t.Error(err)
}

if t.Failed() {
// Wait for handler to finish logging
Expand Down Expand Up @@ -199,6 +205,15 @@ func TestReadyState(t *testing.T) {
t.Error(err)
}

// Test Start TLS parsing.
script = []scriptStep{
{"HELO localhost", 250},
{"STARTTLS", 454}, // TLS unconfigured.
}
if err := playSession(t, server, script); err != nil {
t.Error(err)
}

if t.Failed() {
// Wait for handler to finish logging
time.Sleep(2 * time.Second)
Expand Down