Skip to content

Commit

Permalink
feat(GODT-1891): kick client that send too many invalid IMAP command
Browse files Browse the repository at this point in the history
  • Loading branch information
rlejeune74 authored and LBeernaertProton committed Oct 5, 2022
1 parent 7d14433 commit 44257cf
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 0 deletions.
2 changes: 2 additions & 0 deletions internal/session/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ var (
ErrNotAuthenticated = errors.New("session is not authenticated")
ErrAlreadyAuthenticated = errors.New("session is already authenticated")

ErrTooManyInvalid = errors.New("too many invalid IMAP commands")

ErrNotImplemented = errors.New("not implemented")
)
16 changes: 16 additions & 0 deletions internal/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ import (
"github.com/ProtonMail/gluon/internal/response"
"github.com/ProtonMail/gluon/internal/state"
"github.com/ProtonMail/gluon/profiling"
"github.com/ProtonMail/gluon/reporter"
"github.com/ProtonMail/gluon/version"
"github.com/ProtonMail/gluon/wait"
"github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
)

const maxSessionError = 20

type Session struct {
// conn is the underlying TCP connection to the client. It is wrapped by a buffered liner.
conn net.Conn
Expand Down Expand Up @@ -73,6 +76,9 @@ type Session struct {

// handleWG is used to wait for all commands to finish before closing the session.
handleWG wait.Group

/// errorCount error counter
errorCount int
}

func New(
Expand Down Expand Up @@ -165,17 +171,27 @@ func (s *Session) serve(ctx context.Context) error {

if res.err != nil {
logrus.WithError(res.err).Debugf("Error during command parsing")
s.errorCount++

if errors.Is(res.err, io.EOF) {
logrus.Debugf("Connection to client lost")
return nil
} else if s.errorCount >= maxSessionError {
_ = response.Bad(tag).WithError(ErrTooManyInvalid).Send(s)
reporter.MessageWithContext(ctx,
ErrTooManyInvalid.Error(),
reporter.Context{"error": ErrTooManyInvalid, "ID": s.imapID.String()},
)
return ErrTooManyInvalid
} else if err := response.Bad(tag).WithError(res.err).Send(s); err != nil {
return err
}

continue
}

s.errorCount = 0

case <-s.state.Done():
return nil

Expand Down
26 changes: 26 additions & 0 deletions tests/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,29 @@ func parseDateFromDelimiter(messageDelimiter string) (t time.Time, err error) {

return time.Parse("Mon Jan _2 15:04:05 2006", strings.TrimSpace(strings.Join(split[2:], " ")))
}

func TestTooManyInvalidCommand(t *testing.T) {
runOneToOneTestWithAuth(t, defaultServerOptions(t), func(c *testConnection, s *testSession) {
for i := 1; i <= 19; i++ {
c.Cf("%d FOO", i).BAD(fmt.Sprintf("%d", i))
}
c.Cf("100 FOO").BAD("100")
// expect server to trigger the error as well
require.Errorf(t, <-s.server.GetErrorCh(), "100 BAD too many invalid IMAP commands")
})
}

func TestResetTooManyInvalidCommand(t *testing.T) {
runOneToOneTestWithAuth(t, defaultServerOptions(t), func(c *testConnection, s *testSession) {
for i := 1; i <= 19; i++ {
c.Cf("%d FOO", i).BAD(fmt.Sprintf("%d", i))
}
c.C("100 NOOP").OK("100")
for i := 1; i <= 19; i++ {
c.Cf("%d FOO", i).BAD(fmt.Sprintf("%d", i))
}
c.Cf("100 FOO").BAD("100")
// expect server to trigger the error as well
require.Errorf(t, <-s.server.GetErrorCh(), "100 BAD too many invalid IMAP commands")
})
}

0 comments on commit 44257cf

Please sign in to comment.