From f9eae88aba8906af5aacb6200519998805781ef6 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Fri, 10 Mar 2023 11:32:34 +0100 Subject: [PATCH] for imap/smtp syntax errors, only echo the remaining buffer if the connection is authenticated --- imapserver/error.go | 14 +++++++++----- imapserver/parse.go | 19 +++++++++++++++---- imapserver/server.go | 13 ++++++++++--- smtpserver/error.go | 11 +++++++---- smtpserver/parse.go | 14 +++++++++++++- smtpserver/server.go | 2 +- 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/imapserver/error.go b/imapserver/error.go index 3f8e399..43b3068 100644 --- a/imapserver/error.go +++ b/imapserver/error.go @@ -1,6 +1,7 @@ package imapserver import ( + "errors" "fmt" ) @@ -36,13 +37,14 @@ func xserverErrorf(format string, args ...any) { } type syntaxError struct { - line string // Optional line to write before BAD result. For untagged response. CRLF will be added. - code string // Optional result code (between []) to write in BAD result. - err error // BAD response message. + line string // Optional line to write before BAD result. For untagged response. CRLF will be added. + code string // Optional result code (between []) to write in BAD result. + errmsg string // BAD response message. + err error // Typically with same info as errmsg, but sometimes more. } func (e syntaxError) Error() string { - s := "bad syntax: " + e.err.Error() + s := "bad syntax: " + e.errmsg if e.code != "" { s += " [" + e.code + "]" } @@ -51,5 +53,7 @@ func (e syntaxError) Error() string { func (e syntaxError) Unwrap() error { return e.err } func xsyntaxErrorf(format string, args ...any) { - panic(syntaxError{"", "", fmt.Errorf(format, args...)}) + errmsg := fmt.Sprintf(format, args...) + err := errors.New(errmsg) + panic(syntaxError{"", "", errmsg, err}) } diff --git a/imapserver/parse.go b/imapserver/parse.go index 115d41b..d6236f6 100644 --- a/imapserver/parse.go +++ b/imapserver/parse.go @@ -1,6 +1,7 @@ package imapserver import ( + "errors" "fmt" "net/textproto" "strconv" @@ -74,11 +75,20 @@ func newParser(s string, conn *conn) *parser { } func (p *parser) xerrorf(format string, args ...any) { - var context string + var err error + errmsg := fmt.Sprintf(format, args...) + remaining := fmt.Sprintf("remaining %q", p.orig[p.o:]) if len(p.contexts) > 0 { - context = strings.Join(p.contexts, ",") + remaining += ", context " + strings.Join(p.contexts, ",") } - panic(syntaxError{"", "", fmt.Errorf("%s (%sremaining data %q)", fmt.Sprintf(format, args...), context, p.orig[p.o:])}) + remaining = " (" + remaining + ")" + if p.conn.account != nil { + errmsg += remaining + err = errors.New(errmsg) + } else { + err = errors.New(errmsg + remaining) + } + panic(syntaxError{"", "", errmsg, err}) } func (p *parser) context(s string) func() { @@ -724,7 +734,8 @@ func (p *parser) xliteralSize(maxSize int64, lit8 bool) (size int64, sync bool) if maxSize > 0 && size > maxSize { // ../rfc/7888:249 line := fmt.Sprintf("* BYE [ALERT] Max literal size %d is larger than allowed %d in this context", size, maxSize) - panic(syntaxError{line, "TOOBIG", fmt.Errorf("literal too big")}) + err := errors.New("literal too big") + panic(syntaxError{line, "TOOBIG", err.Error(), err}) } sync = !p.take("+") diff --git a/imapserver/server.go b/imapserver/server.go index ce6ac07..bdb0856 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -760,14 +760,21 @@ func (c *conn) command() { c.writelinef("* BYE please try again speaking imap") panic(errIO) } - c.log.Debugx("imap command syntax error", err, logFields...) + c.log.Debugx("imap command syntax error", sxerr.err, logFields...) c.log.Info("imap syntax error", mlog.Field("lastline", c.lastLine)) fatal := strings.HasSuffix(c.lastLine, "+}") if fatal { err := c.conn.SetWriteDeadline(time.Now().Add(5 * time.Second)) c.log.Check(err, "setting write deadline") } - c.bwriteresultf("%s BAD %s unrecognized syntax/command: %v", tag, cmd, err) + if sxerr.line != "" { + c.bwritelinef("%s", sxerr.line) + } + code := "" + if sxerr.code != "" { + code = "[" + sxerr.code + "] " + } + c.bwriteresultf("%s BAD %s%s unrecognized syntax/command: %v", tag, code, cmd, sxerr.errmsg) if fatal { c.xflush() panic(fmt.Errorf("aborting connection after syntax error for command with non-sync literal: %w", errProtocol)) @@ -1618,7 +1625,7 @@ func (c *conn) cmdAuthenticate(tag, cmd string, p *parser) { c0 := xreadInitial() ss, err := scram.NewServer(h, c0) if err != nil { - xsyntaxErrorf("starting scram: %w", err) + xsyntaxErrorf("starting scram: %s", err) } c.log.Debug("scram auth", mlog.Field("authentication", ss.Authentication)) acc, _, err := store.OpenEmail(ss.Authentication) diff --git a/smtpserver/error.go b/smtpserver/error.go index 1bc43d7..8984190 100644 --- a/smtpserver/error.go +++ b/smtpserver/error.go @@ -8,23 +8,26 @@ import ( func xcheckf(err error, format string, args ...any) { if err != nil { - panic(smtpError{smtp.C451LocalErr, smtp.SeSys3Other0, fmt.Errorf("%s: %w", fmt.Sprintf(format, args...), err), true, false}) + err := fmt.Errorf("%s: %w", fmt.Sprintf(format, args...), err) + panic(smtpError{smtp.C451LocalErr, smtp.SeSys3Other0, err.Error(), err, true, false}) } } type smtpError struct { code int secode string - err error + errmsg string // Sent in response. + err error // If set, used in logging. Typically has same information as errmsg. printStack bool userError bool // If this is an error on the user side, which causes logging at a lower level. } -func (e smtpError) Error() string { return e.err.Error() } +func (e smtpError) Error() string { return e.errmsg } func (e smtpError) Unwrap() error { return e.err } func xsmtpErrorf(code int, secode string, userError bool, format string, args ...any) { - panic(smtpError{code, secode, fmt.Errorf(format, args...), false, userError}) + err := fmt.Errorf(format, args...) + panic(smtpError{code, secode, err.Error(), err, false, userError}) } func xsmtpServerErrorf(codes codes, format string, args ...any) { diff --git a/smtpserver/parse.go b/smtpserver/parse.go index 3c7bb99..7f614cf 100644 --- a/smtpserver/parse.go +++ b/smtpserver/parse.go @@ -1,6 +1,7 @@ package smtpserver import ( + "errors" "fmt" "net" "strconv" @@ -40,8 +41,19 @@ func newParser(s string, smtputf8 bool, conn *conn) *parser { } func (p *parser) xerrorf(format string, args ...any) { + // For submission, send the remaining unparsed line. Otherwise, only log it. + var err error + errmsg := "bad syntax: " + fmt.Sprintf(format, args...) + remaining := fmt.Sprintf(" (remaining %q)", p.orig[p.o:]) + if p.conn.account != nil { + errmsg += remaining + err = errors.New(errmsg) + } else { + err = errors.New(errmsg + remaining) + } + // ../rfc/5321:2377 - xsmtpUserErrorf(smtp.C501BadParamSyntax, smtp.SeProto5Syntax2, "%s (remaining: %q)", fmt.Sprintf(format, args...), p.orig[p.o:]) + panic(smtpError{smtp.C501BadParamSyntax, smtp.SeProto5Syntax2, errmsg, err, false, true}) } func (p *parser) xutf8localparterrorf() { diff --git a/smtpserver/server.go b/smtpserver/server.go index b1acc37..c69be90 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -669,7 +669,7 @@ func command(c *conn) { var serr smtpError if errors.As(err, &serr) { - c.writecodeline(serr.code, serr.secode, fmt.Sprintf("%s (%s)", serr.err, mox.ReceivedID(c.cid)), serr.err) + c.writecodeline(serr.code, serr.secode, fmt.Sprintf("%s (%s)", serr.errmsg, mox.ReceivedID(c.cid)), serr.err) if serr.printStack { debug.PrintStack() }