diff --git a/imapserver/fuzz_test.go b/imapserver/fuzz_test.go index e4cb36b..f0aa398 100644 --- a/imapserver/fuzz_test.go +++ b/imapserver/fuzz_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "errors" "fmt" + "io" "net" "os" "testing" @@ -106,7 +107,7 @@ func FuzzServer(f *testing.F) { return } err, ok := x.(error) - if !ok || !errors.Is(err, os.ErrDeadlineExceeded) { + if !ok || (!errors.Is(err, os.ErrDeadlineExceeded) && !errors.Is(err, io.EOF)) { panic(x) } }() diff --git a/imapserver/server.go b/imapserver/server.go index 16264e9..ce6ac07 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -130,9 +130,9 @@ func limitersInit() { } } -// Delay before reads and after 1-byte writes for probably spammers. Tests set this -// to zero. -var badClientDelay = time.Second +// Delay after bad/suspicious behaviour. Tests set these to zero. +var badClientDelay = time.Second // Before reads and after 1-byte writes for probably spammers. +var authFailDelay = time.Second // After authentication failure. // Capabilities (extensions) the server supports. Connections will add a few more, e.g. STARTTLS, LOGINDISABLED, AUTH=PLAIN. // ENABLE: ../rfc/5161 @@ -175,6 +175,7 @@ type conn struct { cmd string // Currently executing, for deciding to applyChanges and logging. cmdMetric string // Currently executing, for metrics. cmdStart time.Time + ncmds int // Number of commands processed. Used to abort connection when first incoming command is unknown/invalid. log *mlog.Log enabled map[capability]bool // All upper-case. @@ -666,7 +667,7 @@ func serve(listenerName string, cid int64, tlsConfig *tls.Config, nc net.Conn, x case <-mox.Shutdown.Done(): // ../rfc/9051:5381 c.writelinef("* BYE mox shutting down") - panic(errIO) + return default: } @@ -752,6 +753,13 @@ func (c *conn) command() { var serr serverError if errors.As(err, &sxerr) { result = "badsyntax" + if c.ncmds == 0 { + // Other side is likely speaking something else than IMAP, send error message and + // stop processing because there is a good chance whatever they sent has multiple + // lines. + c.writelinef("* BYE please try again speaking imap") + panic(errIO) + } c.log.Debugx("imap command syntax error", err, logFields...) c.log.Info("imap syntax error", mlog.Field("lastline", c.lastLine)) fatal := strings.HasSuffix(c.lastLine, "+}") @@ -806,6 +814,7 @@ func (c *conn) command() { xsyntaxErrorf("unknown command %q", cmd) } c.cmdMetric = c.cmd + c.ncmds++ // Check if command is allowed in this state. if _, ok1 := commandsStateAny[cmdlow]; ok1 { @@ -1425,8 +1434,8 @@ func (c *conn) cmdAuthenticate(tag, cmd string, p *parser) { // Examples: ../rfc/9051:1520 ../rfc/3501:1631 // For many failed auth attempts, slow down verification attempts. - if c.authFailed > 3 { - mox.Sleep(mox.Context, time.Duration(c.authFailed-3)*time.Second) + if c.authFailed > 3 && authFailDelay > 0 { + mox.Sleep(mox.Context, time.Duration(c.authFailed-3)*authFailDelay) } c.authFailed++ // Compensated on success. defer func() { @@ -1709,8 +1718,8 @@ func (c *conn) cmdLogin(tag, cmd string, p *parser) { } // For many failed auth attempts, slow down verification attempts. - if c.authFailed > 3 { - mox.Sleep(mox.Context, time.Duration(c.authFailed-3)*time.Second) + if c.authFailed > 3 && authFailDelay > 0 { + mox.Sleep(mox.Context, time.Duration(c.authFailed-3)*authFailDelay) } c.authFailed++ // Compensated on success. defer func() { diff --git a/imapserver/server_test.go b/imapserver/server_test.go index 03193f2..1dacdce 100644 --- a/imapserver/server_test.go +++ b/imapserver/server_test.go @@ -26,6 +26,7 @@ func init() { // Don't slow down tests. badClientDelay = 0 + authFailDelay = 0 } func tocrlf(s string) string { @@ -397,8 +398,6 @@ func TestLogin(t *testing.T) { func TestState(t *testing.T) { tc := start(t) - tc.transactf("bad", "boguscommand") - notAuthenticated := []string{"starttls", "authenticate", "login"} authenticatedOrSelected := []string{"enable", "select", "examine", "create", "delete", "rename", "subscribe", "unsubscribe", "list", "namespace", "status", "append", "idle", "lsub"} selected := []string{"close", "unselect", "expunge", "search", "fetch", "store", "copy", "move", "uid expunge"} @@ -421,6 +420,21 @@ func TestState(t *testing.T) { for _, cmd := range append(append([]string{}, notAuthenticated...), selected...) { tc.transactf("no", "%s", cmd) } + + tc.transactf("bad", "boguscommand") +} + +func TestNonIMAP(t *testing.T) { + tc := start(t) + defer tc.close() + + // imap greeting has already been read, we sidestep the imapclient. + _, err := fmt.Fprintf(tc.conn, "bogus\r\n") + tc.check(err, "write bogus command") + tc.readprefixline("* BYE ") + if _, err := tc.conn.Read(make([]byte, 1)); err == nil { + t.Fatalf("connection not closed after initial bad command") + } } func TestLiterals(t *testing.T) { diff --git a/smtpserver/server.go b/smtpserver/server.go index 000e890..b1acc37 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -94,11 +94,11 @@ func limitersInit() { } var ( - // Delay before reads and after 1-byte writes for probably spammers. Zero during tests. - badClientDelay = time.Second - - // Delay before accepting message from sender without reputation. Zero during tests. - reputationlessSenderDeliveryDelay = 15 * time.Second + // Delays for bad/suspicious behaviour. Zero during tests. + badClientDelay = time.Second // Before reads and after 1-byte writes for probably spammers. + authFailDelay = time.Second // Response to authentication failure. + reputationlessSenderDeliveryDelay = 15 * time.Second // Before accepting message from first-time sender. + unknownRecipientsDelay = 5 * time.Second // Response when all recipients are unknown. ) type codes struct { @@ -276,6 +276,7 @@ type conn struct { requireTLSForDelivery bool cmd string // Current command. cmdStart time.Time // Start of current command. + ncmds int // Number of commands processed. Used to abort connection when first incoming command is unknown/invalid. dnsBLs []dns.Domain // todo future: add a flag for "pedantic" mode, causing us to be strict. e.g. interpreting some SHOULD as MUST. ../rfc/5321:4076 @@ -706,10 +707,18 @@ func command(c *conn) { p := newParser(args, c.smtputf8, c) fn, ok := commands[cmdl] if !ok { + if c.ncmds == 0 { + // Other side is likely speaking something else than SMTP, send error message and + // stop processing because there is a good chance whatever they sent has multiple + // lines. + c.writecodeline(smtp.C500BadSyntax, smtp.SeProto5Syntax2, "please try again speaking smtp", nil) + panic(errIO) + } c.cmd = "(unknown)" // note: not "command not implemented", see ../rfc/5321:2934 ../rfc/5321:2539 xsmtpUserErrorf(smtp.C500BadSyntax, smtp.SeProto5BadCmdOrSeq1, "unknown command") } + c.ncmds++ fn(c, p) } @@ -867,9 +876,9 @@ func (c *conn) cmdAuth(p *parser) { // For many failed auth attempts, slow down verification attempts. // Dropping the connection could also work, but more so when we have a connection rate limiter. // ../rfc/4954:770 - if c.authFailed > 3 { + if c.authFailed > 3 && authFailDelay > 0 { // ../rfc/4954:770 - mox.Sleep(mox.Context, time.Duration(c.authFailed-3)*time.Second) + mox.Sleep(mox.Context, time.Duration(c.authFailed-3)*authFailDelay) } c.authFailed++ // Compensated on success. defer func() { @@ -1803,7 +1812,9 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW // Crude attempt to slow down someone trying to guess names. Would work better // with connection rate limiter. - mox.Sleep(ctx, 5*time.Second) + if unknownRecipientsDelay > 0 { + mox.Sleep(ctx, unknownRecipientsDelay) + } // todo future: if remote does not look like a properly configured mail system, respond with generic 451 error? to prevent any random internet system from discovering accounts. we could give proper response if spf for ehlo or mailfrom passes. xsmtpUserErrorf(smtp.C550MailboxUnavail, smtp.SeAddr1UnknownDestMailbox1, "no such user(s)") diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index cef65b9..c0b11ff 100644 --- a/smtpserver/server_test.go +++ b/smtpserver/server_test.go @@ -42,6 +42,8 @@ func init() { // Don't make tests slow. badClientDelay = 0 reputationlessSenderDeliveryDelay = 0 + authFailDelay = 0 + unknownRecipientsDelay = 0 } func tcheck(t *testing.T, err error, msg string) { @@ -877,3 +879,46 @@ func TestRatelimitDelivery(t *testing.T) { } }) } + +func TestNonSMTP(t *testing.T) { + ts := newTestServer(t, "../testdata/smtp/mox.conf", dns.MockResolver{}) + defer ts.close() + ts.cid += 2 + + serverConn, clientConn := net.Pipe() + defer serverConn.Close() + serverdone := make(chan struct{}) + defer func() { <-serverdone }() + + go func() { + tlsConfig := &tls.Config{ + Certificates: []tls.Certificate{fakeCert(ts.t)}, + } + serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, tlsConfig, serverConn, ts.resolver, ts.submission, false, 100<<20, false, false, ts.dnsbls) + close(serverdone) + }() + + defer clientConn.Close() + + buf := make([]byte, 128) + + // Read and ignore hello. + if _, err := clientConn.Read(buf); err != nil { + t.Fatalf("reading hello: %v", err) + } + + if _, err := fmt.Fprintf(clientConn, "bogus\r\n"); err != nil { + t.Fatalf("write command: %v", err) + } + n, err := clientConn.Read(buf) + if err != nil { + t.Fatalf("read response line: %v", err) + } + s := string(buf[:n]) + if !strings.HasPrefix(s, "500 5.5.2 ") { + t.Fatalf(`got %q, expected "500 5.5.2 ...`, s) + } + if _, err := clientConn.Read(buf); err == nil { + t.Fatalf("connection not closed after bogus command") + } +}