From aa1cfd19b4b25cedb732e1eaf4d8995b675690b3 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Thu, 21 Nov 2024 13:15:45 +0100 Subject: [PATCH] feat(BRIDGE-268): add option to disable AUTHENTICATE command. (#417) --- builder.go | 76 ++++++++++---------- imap/command/authenticate_test.go | 17 ++++- imap/command/list_test.go | 4 +- imap/command/parser.go | 113 ++++++++++++++++++++---------- imap/command/parser_test.go | 4 +- internal/session/command.go | 9 ++- internal/session/session.go | 41 ++++++----- option.go | 10 +++ server.go | 5 +- tests/authenticate_test.go | 7 ++ tests/capability_test.go | 15 ++++ tests/server_test.go | 41 +++++++---- 12 files changed, 230 insertions(+), 112 deletions(-) diff --git a/builder.go b/builder.go index 2cf9cf90..23a418ef 100644 --- a/builder.go +++ b/builder.go @@ -23,24 +23,25 @@ import ( ) type serverBuilder struct { - dataDir string - databaseDir string - delim string - loginJailTime time.Duration - tlsConfig *tls.Config - idleBulkTime time.Duration - inLogger io.Writer - outLogger io.Writer - versionInfo version.Info - cmdExecProfBuilder profiling.CmdProfilerBuilder - storeBuilder store.Builder - reporter reporter.Reporter - disableParallelism bool - imapLimits limits.IMAP - uidValidityGenerator imap.UIDValidityGenerator - panicHandler async.PanicHandler - dbCI db.ClientInterface - observabilitySender observability.Sender + dataDir string + databaseDir string + delim string + loginJailTime time.Duration + tlsConfig *tls.Config + idleBulkTime time.Duration + inLogger io.Writer + outLogger io.Writer + versionInfo version.Info + cmdExecProfBuilder profiling.CmdProfilerBuilder + storeBuilder store.Builder + reporter reporter.Reporter + disableParallelism bool + imapLimits limits.IMAP + disableIMAPAuthenticate bool + uidValidityGenerator imap.UIDValidityGenerator + panicHandler async.PanicHandler + dbCI db.ClientInterface + observabilitySender observability.Sender } func newBuilder() (*serverBuilder, error) { @@ -106,25 +107,26 @@ func (builder *serverBuilder) build() (*Server, error) { } s := &Server{ - dataDir: builder.dataDir, - databaseDir: builder.databaseDir, - backend: backend, - sessions: make(map[int]*session.Session), - serveErrCh: async.NewQueuedChannel[error](1, 1, builder.panicHandler, "server-err-ch"), - serveDoneCh: make(chan struct{}), - serveWG: async.MakeWaitGroup(builder.panicHandler), - inLogger: builder.inLogger, - outLogger: builder.outLogger, - tlsConfig: builder.tlsConfig, - idleBulkTime: builder.idleBulkTime, - storeBuilder: builder.storeBuilder, - cmdExecProfBuilder: builder.cmdExecProfBuilder, - versionInfo: builder.versionInfo, - reporter: builder.reporter, - disableParallelism: builder.disableParallelism, - uidValidityGenerator: builder.uidValidityGenerator, - panicHandler: builder.panicHandler, - observabilitySender: builder.observabilitySender, + dataDir: builder.dataDir, + databaseDir: builder.databaseDir, + backend: backend, + sessions: make(map[int]*session.Session), + serveErrCh: async.NewQueuedChannel[error](1, 1, builder.panicHandler, "server-err-ch"), + serveDoneCh: make(chan struct{}), + serveWG: async.MakeWaitGroup(builder.panicHandler), + inLogger: builder.inLogger, + outLogger: builder.outLogger, + tlsConfig: builder.tlsConfig, + idleBulkTime: builder.idleBulkTime, + storeBuilder: builder.storeBuilder, + cmdExecProfBuilder: builder.cmdExecProfBuilder, + versionInfo: builder.versionInfo, + reporter: builder.reporter, + disableParallelism: builder.disableParallelism, + disableIMAPAuthenticate: builder.disableIMAPAuthenticate, + uidValidityGenerator: builder.uidValidityGenerator, + panicHandler: builder.panicHandler, + observabilitySender: builder.observabilitySender, } return s, nil diff --git a/imap/command/authenticate_test.go b/imap/command/authenticate_test.go index 9cae3ae2..1c8eb10b 100644 --- a/imap/command/authenticate_test.go +++ b/imap/command/authenticate_test.go @@ -29,7 +29,7 @@ func TestParser_Authenticate(t *testing.T) { authString := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("\x00%s\x00%s", data.UserID, data.Password))) input := toIMAPLine(tag+` AUTHENTICATE PLAIN`, authString) s := rfcparser.NewScanner(bytes.NewReader(input)) - p := NewParserWithLiteralContinuationCb(s, continuationChecker(&continued)) + p := NewParser(s, WithLiteralContinuationCallback(continuationChecker(&continued))) cmd, err := p.Parse() message := fmt.Sprintf(" test failed for input %#v", data) @@ -46,7 +46,7 @@ func TestParser_AuthenticationWithIdentity(t *testing.T) { authString := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("identity\x00user\x00pass"))) s := rfcparser.NewScanner(bytes.NewReader(toIMAPLine(`A0001 authenticate plain`, authString))) - p := NewParserWithLiteralContinuationCb(s, continuationChecker(&continued)) + p := NewParser(s, WithLiteralContinuationCallback(continuationChecker(&continued))) cmd, err := p.Parse() require.NoError(t, err, "error test failed") @@ -123,7 +123,7 @@ func TestParser_AuthenticateFailures(t *testing.T) { var continued bool s := rfcparser.NewScanner(bytes.NewReader(toIMAPLine(test.input...))) - p := NewParserWithLiteralContinuationCb(s, continuationChecker(&continued)) + p := NewParser(s, WithLiteralContinuationCallback(continuationChecker(&continued))) _, err := p.Parse() failureDescription := fmt.Sprintf(" test failed for input %#v", test) @@ -134,3 +134,14 @@ func TestParser_AuthenticateFailures(t *testing.T) { require.Equal(t, test.continuationExpected, continued, "continuation"+failureDescription) } } + +func TestParser_AuthenticateDisabled(t *testing.T) { + s := rfcparser.NewScanner(bytes.NewReader(toIMAPLine(`A0001 authenticate plain`))) + p := NewParser(s, WithDisableIMAPAuthenticate()) + _, err := p.Parse() + + var parserError *rfcparser.Error + + require.ErrorAs(t, err, &parserError) + require.Equal(t, "unknown command 'authenticate'", parserError.Message) +} diff --git a/imap/command/list_test.go b/imap/command/list_test.go index d6f8a90e..3dc6cee4 100644 --- a/imap/command/list_test.go +++ b/imap/command/list_test.go @@ -63,10 +63,10 @@ func TestParser_ListCommandLiteral(t *testing.T) { input := toIMAPLine(`tag LIST {5}`, `"bar" %`) s := rfcparser.NewScanner(bytes.NewReader(input)) continuationCalled := false - p := NewParserWithLiteralContinuationCb(s, func(string) error { + p := NewParser(s, WithLiteralContinuationCallback(func(string) error { continuationCalled = true return nil - }) + })) expected := Command{Tag: "tag", Payload: &List{ Mailbox: `"bar"`, ListMailbox: "%", diff --git a/imap/command/parser.go b/imap/command/parser.go index 0ff1e2e7..1222e207 100644 --- a/imap/command/parser.go +++ b/imap/command/parser.go @@ -11,6 +11,39 @@ type Builder interface { FromParser(p *rfcparser.Parser) (Payload, error) } +type parserBuilder struct { + continuationCallback func(string) error + disableIMAPAuthenticate bool +} + +type Option interface { + config(*parserBuilder) +} + +type withLiteralContinuationCallback struct { + callback func(string) error +} + +func (opt withLiteralContinuationCallback) config(builder *parserBuilder) { + builder.continuationCallback = opt.callback +} + +func WithLiteralContinuationCallback(callback func(string) error) Option { + return &withLiteralContinuationCallback{ + callback: callback, + } +} + +type withDisableIMAPAuthenticate struct{} + +func (opt withDisableIMAPAuthenticate) config(builder *parserBuilder) { + builder.disableIMAPAuthenticate = true +} + +func WithDisableIMAPAuthenticate() Option { + return &withDisableIMAPAuthenticate{} +} + // Parser parses IMAP Commands. type Parser struct { parser *rfcparser.Parser @@ -20,45 +53,51 @@ type Parser struct { lastCmd string } -func NewParser(s *rfcparser.Scanner) *Parser { - return NewParserWithLiteralContinuationCb(s, nil) -} +func NewParser(s *rfcparser.Scanner, options ...Option) *Parser { + var builder parserBuilder + for _, option := range options { + option.config(&builder) + } + + commands := map[string]Builder{ + "list": &ListCommandParser{}, + "append": &AppendCommandParser{}, + "search": &SearchCommandParser{}, + "fetch": &FetchCommandParser{}, + "capability": &CapabilityCommandParser{}, + "idle": &IdleCommandParser{}, + "noop": &NoopCommandParser{}, + "logout": &LogoutCommandParser{}, + "check": &CheckCommandParser{}, + "close": &CloseCommandParser{}, + "expunge": &ExpungeCommandParser{}, + "unselect": &UnselectCommandParser{}, + "starttls": &StartTLSCommandParser{}, + "status": &StatusCommandParser{}, + "select": &SelectCommandParser{}, + "examine": &ExamineCommandParser{}, + "create": &CreateCommandParser{}, + "delete": &DeleteCommandParser{}, + "subscribe": &SubscribeCommandParser{}, + "unsubscribe": &UnsubscribeCommandParser{}, + "rename": &RenameCommandParser{}, + "lsub": &LSubCommandParser{}, + "login": &LoginCommandParser{}, + "store": &StoreCommandParser{}, + "copy": &CopyCommandParser{}, + "move": &MoveCommandParser{}, + "uid": NewUIDCommandParser(), + "id": &IDCommandParser{}, + } + + if !builder.disableIMAPAuthenticate { + commands["authenticate"] = &AuthenticateCommandParser{} + } -func NewParserWithLiteralContinuationCb(s *rfcparser.Scanner, cb func(string) error) *Parser { return &Parser{ - scanner: s, - parser: rfcparser.NewParserWithLiteralContinuationCb(s, cb), - commands: map[string]Builder{ - "list": &ListCommandParser{}, - "append": &AppendCommandParser{}, - "search": &SearchCommandParser{}, - "fetch": &FetchCommandParser{}, - "capability": &CapabilityCommandParser{}, - "idle": &IdleCommandParser{}, - "noop": &NoopCommandParser{}, - "logout": &LogoutCommandParser{}, - "check": &CheckCommandParser{}, - "close": &CloseCommandParser{}, - "expunge": &ExpungeCommandParser{}, - "unselect": &UnselectCommandParser{}, - "starttls": &StartTLSCommandParser{}, - "status": &StatusCommandParser{}, - "select": &SelectCommandParser{}, - "examine": &ExamineCommandParser{}, - "create": &CreateCommandParser{}, - "delete": &DeleteCommandParser{}, - "subscribe": &SubscribeCommandParser{}, - "unsubscribe": &UnsubscribeCommandParser{}, - "rename": &RenameCommandParser{}, - "lsub": &LSubCommandParser{}, - "login": &LoginCommandParser{}, - "store": &StoreCommandParser{}, - "copy": &CopyCommandParser{}, - "move": &MoveCommandParser{}, - "uid": NewUIDCommandParser(), - "id": &IDCommandParser{}, - "authenticate": &AuthenticateCommandParser{}, - }, + scanner: s, + parser: rfcparser.NewParserWithLiteralContinuationCb(s, builder.continuationCallback), + commands: commands, } } diff --git a/imap/command/parser_test.go b/imap/command/parser_test.go index b960f19d..0e44394e 100644 --- a/imap/command/parser_test.go +++ b/imap/command/parser_test.go @@ -105,10 +105,10 @@ func TestParser_LiteralWithContinuationSubmission(t *testing.T) { }() s := rfcparser.NewScanner(reader) - p := NewParserWithLiteralContinuationCb(s, func(string) error { + p := NewParser(s, WithLiteralContinuationCallback(func(string) error { close(continueCh) return nil - }) + })) expected := Command{Tag: "A003", Payload: &Append{ Mailbox: "saved-messages", diff --git a/internal/session/command.go b/internal/session/command.go index cb762b8d..919a140a 100644 --- a/internal/session/command.go +++ b/internal/session/command.go @@ -33,7 +33,14 @@ func (s *Session) startCommandReader(ctx context.Context) <-chan commandResult { {0x16, 0x00, 0x00}, // 0.0 } - parser := command.NewParserWithLiteralContinuationCb(s.scanner, func(message string) error { return response.Continuation().Send(s, message) }) + options := []command.Option{ + command.WithLiteralContinuationCallback(func(message string) error { return response.Continuation().Send(s, message) }), + } + if s.disableIMAPAuthenticate { + options = append(options, command.WithDisableIMAPAuthenticate()) + } + + parser := command.NewParser(s.scanner, options...) for { s.inputCollector.Reset() diff --git a/internal/session/session.go b/internal/session/session.go index 48ead4be..04df36ff 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -21,7 +21,6 @@ import ( "github.com/ProtonMail/gluon/internal/backend" "github.com/ProtonMail/gluon/internal/response" "github.com/ProtonMail/gluon/internal/state" - "github.com/ProtonMail/gluon/limits" "github.com/ProtonMail/gluon/profiling" "github.com/ProtonMail/gluon/reporter" "github.com/ProtonMail/gluon/rfcparser" @@ -84,13 +83,16 @@ type Session struct { // handleWG is used to wait for all commands to finish before closing the session. handleWG async.WaitGroup - /// errorCount error counter + /// errorCount error counter. errorCount int - imapLimits limits.IMAP + // disableIMAPAuthenticate disables the IMAP AUTHENTICATE command (client can then only authenticate using LOGIN). + disableIMAPAuthenticate bool + // panicHandler The panic handler. panicHandler async.PanicHandler + // log The log for the session. log *logrus.Entry } @@ -102,25 +104,32 @@ func New( profiler profiling.CmdProfilerBuilder, eventCh chan<- events.Event, idleBulkTime time.Duration, + disableIMAPAuthenticate bool, panicHandler async.PanicHandler, ) *Session { inputCollector := command.NewInputCollector(bufio.NewReader(conn)) scanner := rfcparser.NewScannerWithReader(inputCollector) + caps := []imap.Capability{imap.IMAP4rev1, imap.IDLE, imap.UNSELECT, imap.UIDPLUS, imap.MOVE, imap.ID} + if !disableIMAPAuthenticate { + caps = append(caps, imap.AUTHPLAIN) + } + return &Session{ - conn: conn, - inputCollector: inputCollector, - scanner: scanner, - backend: backend, - caps: []imap.Capability{imap.IMAP4rev1, imap.IDLE, imap.UNSELECT, imap.UIDPLUS, imap.MOVE, imap.ID, imap.AUTHPLAIN}, - sessionID: sessionID, - eventCh: eventCh, - idleBulkTime: idleBulkTime, - version: version, - cmdProfilerBuilder: profiler, - handleWG: async.MakeWaitGroup(panicHandler), - panicHandler: panicHandler, - log: logrus.WithField("pkg", "gluon/session").WithField("session", sessionID), + conn: conn, + inputCollector: inputCollector, + scanner: scanner, + backend: backend, + caps: caps, + sessionID: sessionID, + eventCh: eventCh, + idleBulkTime: idleBulkTime, + version: version, + cmdProfilerBuilder: profiler, + handleWG: async.MakeWaitGroup(panicHandler), + disableIMAPAuthenticate: disableIMAPAuthenticate, + panicHandler: panicHandler, + log: logrus.WithField("pkg", "gluon/session").WithField("session", sessionID), } } diff --git a/option.go b/option.go index 525a0242..dc748293 100644 --- a/option.go +++ b/option.go @@ -221,6 +221,16 @@ func WithIMAPLimits(limits limits2.IMAP) Option { } } +type withDisableIMAPAuthenticate struct{} + +func (withDisableIMAPAuthenticate) config(builder *serverBuilder) { + builder.disableIMAPAuthenticate = true +} + +func WithDisableIMAPAuthenticate() Option { + return &withDisableIMAPAuthenticate{} +} + type withUIDValidityGenerator struct { generator imap.UIDValidityGenerator } diff --git a/server.go b/server.go index 8abf3782..3466bf0f 100644 --- a/server.go +++ b/server.go @@ -86,6 +86,9 @@ type Server struct { // disableParallelism indicates whether the server is allowed to parallelize certain IMAP commands. disableParallelism bool + // disableIMAPAuthenticate disables the IMAP AUTHENTICATE command (client can then only authenticate using LOGIN). + disableIMAPAuthenticate bool + uidValidityGenerator imap.UIDValidityGenerator panicHandler async.PanicHandler @@ -293,7 +296,7 @@ func (s *Server) addSession(ctx context.Context, conn net.Conn) (*session.Sessio nextID := s.getNextID() - s.sessions[nextID] = session.New(conn, s.backend, nextID, s.versionInfo, s.cmdExecProfBuilder, s.newEventCh(ctx), s.idleBulkTime, s.panicHandler) + s.sessions[nextID] = session.New(conn, s.backend, nextID, s.versionInfo, s.cmdExecProfBuilder, s.newEventCh(ctx), s.idleBulkTime, s.disableIMAPAuthenticate, s.panicHandler) if s.tlsConfig != nil { s.sessions[nextID].SetTLSConfig(s.tlsConfig) diff --git a/tests/authenticate_test.go b/tests/authenticate_test.go index b1ab2428..4ad2cba1 100644 --- a/tests/authenticate_test.go +++ b/tests/authenticate_test.go @@ -25,6 +25,13 @@ func TestAuthenticateSuccess(t *testing.T) { }) } +func TestAuthenticateDisabled(t *testing.T) { + runOneToOneTest(t, defaultServerOptions(t, withDisableIMAPAuthenticate()), func(c *testConnection, _ *testSession) { + c.C("A001 AUTHENTICATE PLAIN") + c.BAD("A001") + }) +} + func TestAuthenticateFailure(t *testing.T) { authString := base64AuthString("user", "badPass") diff --git a/tests/capability_test.go b/tests/capability_test.go index 3d7ade3b..4afb52a3 100644 --- a/tests/capability_test.go +++ b/tests/capability_test.go @@ -18,3 +18,18 @@ func TestCapability(t *testing.T) { c.S("A003 OK CAPABILITY") }) } + +func TestCapabilityAuthenticateDisabled(t *testing.T) { + runOneToOneTest(t, defaultServerOptions(t, withDisableIMAPAuthenticate()), func(c *testConnection, _ *testSession) { + c.C("A001 Capability") + c.S(`* CAPABILITY ID IDLE IMAP4rev1 STARTTLS`) + c.S("A001 OK CAPABILITY") + + c.C(`A002 login "user" "pass"`) + c.S(`A002 OK [CAPABILITY ID IDLE IMAP4rev1 MOVE STARTTLS UIDPLUS UNSELECT] Logged in`) + + c.C("A003 Capability") + c.S(`* CAPABILITY ID IDLE IMAP4rev1 MOVE STARTTLS UIDPLUS UNSELECT`) + c.S("A003 OK CAPABILITY") + }) +} diff --git a/tests/server_test.go b/tests/server_test.go index b5b78458..2d8d3751 100644 --- a/tests/server_test.go +++ b/tests/server_test.go @@ -67,19 +67,20 @@ func (*dummyConnectorBuilder) New(usernames []string, password []byte, period ti } type serverOptions struct { - credentials []credentials - delimiter string - loginJailTime time.Duration - dataDir string - databaseDir string - idleBulkTime time.Duration - storeBuilder store.Builder - connectorBuilder connectorBuilder - disableParallelism bool - imapLimits limits.IMAP - reporter reporter.Reporter - uidValidityGenerator imap.UIDValidityGenerator - database db.ClientInterface + credentials []credentials + delimiter string + loginJailTime time.Duration + dataDir string + databaseDir string + idleBulkTime time.Duration + storeBuilder store.Builder + connectorBuilder connectorBuilder + disableParallelism bool + imapLimits limits.IMAP + disableIMAPAuthenticate bool + reporter reporter.Reporter + uidValidityGenerator imap.UIDValidityGenerator + database db.ClientInterface } func (s *serverOptions) defaultUsername() string { @@ -190,6 +191,12 @@ func (w withDatabaseOption) apply(options *serverOptions) { options.database = w.database } +type disableIMAPAuthenticateOption struct{} + +func (disableIMAPAuthenticateOption) apply(options *serverOptions) { + options.disableIMAPAuthenticate = true +} + func (u uidValidityGeneratorOption) apply(options *serverOptions) { options.uidValidityGenerator = u.generator } @@ -242,6 +249,10 @@ func withDatabase(ci db.ClientInterface) serverOption { return &withDatabaseOption{database: ci} } +func withDisableIMAPAuthenticate() serverOption { + return &disableIMAPAuthenticateOption{} +} + func defaultServerOptions(tb testing.TB, modifiers ...serverOption) *serverOptions { options := &serverOptions{ credentials: []credentials{{ @@ -320,6 +331,10 @@ func runServer(tb testing.TB, options *serverOptions, tests func(session *testSe gluonOptions = append(gluonOptions, gluon.WithUIDValidityGenerator(options.uidValidityGenerator)) } + if options.disableIMAPAuthenticate { + gluonOptions = append(gluonOptions, gluon.WithDisableIMAPAuthenticate()) + } + // Create a new gluon server. server, err := gluon.New(gluonOptions...) require.NoError(tb, err)