From 22bf8f62ceed32e2e60433545716471e2677640c Mon Sep 17 00:00:00 2001 From: Michal Horejsek Date: Wed, 14 Apr 2021 13:09:28 +0200 Subject: [PATCH] GODT-1143 Turn off SMTP server while no connection --- internal/app/bridge/bridge.go | 1 + internal/imap/server.go | 15 +++--- internal/smtp/server.go | 99 ++++++++++++++++++++++++++++++++--- internal/smtp/server_test.go | 65 +++++++++++++++++++++++ test/context/smtp.go | 2 +- 5 files changed, 168 insertions(+), 14 deletions(-) create mode 100644 internal/smtp/server_test.go diff --git a/internal/app/bridge/bridge.go b/internal/app/bridge/bridge.go index aa27bfbe..fb39b4cc 100644 --- a/internal/app/bridge/bridge.go +++ b/internal/app/bridge/bridge.go @@ -95,6 +95,7 @@ func run(b *base.Base, c *cli.Context) error { // nolint[funlen] smtpPort := b.Settings.GetInt(settings.SMTPPortKey) useSSL := b.Settings.GetBool(settings.SMTPSSLKey) smtp.NewSMTPServer( + b.CrashHandler, c.Bool(flagLogSMTP), smtpPort, useSSL, tlsConfig, smtpBackend, b.Listener).ListenAndServe() }() diff --git a/internal/imap/server.go b/internal/imap/server.go index 4fa2e84c..4a07c0df 100644 --- a/internal/imap/server.go +++ b/internal/imap/server.go @@ -132,24 +132,25 @@ func (s *imapServer) listenAndServe(retries int) { } s.isRunning.Store(true) - log.Info("IMAP server listening at ", s.server.Addr) - l, err := net.Listen("tcp", s.server.Addr) + l := log.WithField("address", s.server.Addr) + l.Info("IMAP server is starting") + listener, err := net.Listen("tcp", s.server.Addr) if err != nil { s.isRunning.Store(false) if retries > 0 { - log.WithError(err).WithField("retries", retries).Warn("IMAP listener failed") + l.WithError(err).WithField("retries", retries).Warn("IMAP listener failed") time.Sleep(15 * time.Second) s.listenAndServe(retries - 1) return } - log.WithError(err).Error("IMAP listener failed") + l.WithError(err).Error("IMAP listener failed") s.eventListener.Emit(events.ErrorEvent, "IMAP failed: "+err.Error()) return } err = s.server.Serve(&connListener{ - Listener: l, + Listener: listener, server: s, userAgent: s.userAgent, }) @@ -158,13 +159,13 @@ func (s *imapServer) listenAndServe(retries int) { // but it should in case it was not closed by `s.Close()`. if err != nil && s.isRunning.Load().(bool) { s.isRunning.Store(false) - log.WithError(err).Error("IMAP server failed") + l.WithError(err).Error("IMAP server failed") s.eventListener.Emit(events.ErrorEvent, "IMAP failed: "+err.Error()) return } defer s.server.Close() //nolint[errcheck] - log.Info("IMAP server stopped") + l.Info("IMAP server stopped") } // Stops the server. diff --git a/internal/smtp/server.go b/internal/smtp/server.go index 3d6ceac6..245506f4 100644 --- a/internal/smtp/server.go +++ b/internal/smtp/server.go @@ -20,23 +20,30 @@ package smtp import ( "crypto/tls" "fmt" + "net" + "sync/atomic" + "time" "github.com/ProtonMail/proton-bridge/internal/bridge" "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/ports" "github.com/emersion/go-sasl" goSMTP "github.com/emersion/go-smtp" "github.com/sirupsen/logrus" ) type smtpServer struct { + panicHandler panicHandler server *goSMTP.Server eventListener listener.Listener useSSL bool + port int + isRunning atomic.Value } // NewSMTPServer returns an SMTP server configured with the given options. -func NewSMTPServer(debug bool, port int, useSSL bool, tls *tls.Config, smtpBackend goSMTP.Backend, eventListener listener.Listener) *smtpServer { //nolint[golint] +func NewSMTPServer(panicHandler panicHandler, debug bool, port int, useSSL bool, tls *tls.Config, smtpBackend goSMTP.Backend, eventListener listener.Listener) *smtpServer { //nolint[golint] s := goSMTP.NewServer(smtpBackend) s.Addr = fmt.Sprintf("%v:%v", bridge.Host, port) s.TLSConfig = tls @@ -69,28 +76,64 @@ func NewSMTPServer(debug bool, port int, useSSL bool, tls *tls.Config, smtpBacke }) }) - return &smtpServer{ + server := &smtpServer{ + panicHandler: panicHandler, server: s, eventListener: eventListener, useSSL: useSSL, + port: port, } + server.isRunning.Store(false) + return server } // Starts the server. func (s *smtpServer) ListenAndServe() { go s.monitorDisconnectedUsers() - l := log.WithField("useSSL", s.useSSL).WithField("address", s.server.Addr) + go s.monitorInternetConnection() + // When starting the Bridge, we don't want to retry to notify user + // quickly about the issue. Very probably retry will not help anyway. + s.listenAndServe(0) +} + +func (s *smtpServer) listenAndServe(retries int) { + if s.isRunning.Load().(bool) { + return + } + s.isRunning.Store(true) + + l := log.WithField("useSSL", s.useSSL).WithField("address", s.server.Addr) l.Info("SMTP server is starting") + var listener net.Listener var err error if s.useSSL { - err = s.server.ListenAndServeTLS() + listener, err = tls.Listen("tcp", s.server.Addr, s.server.TLSConfig) } else { - err = s.server.ListenAndServe() + listener, err = net.Listen("tcp", s.server.Addr) } if err != nil { + s.isRunning.Store(false) + if retries > 0 { + l.WithError(err).WithField("retries", retries).Warn("SMTP listener failed") + time.Sleep(15 * time.Second) + s.listenAndServe(retries - 1) + return + } + + l.WithError(err).Error("SMTP listener failed") + s.eventListener.Emit(events.ErrorEvent, "SMTP failed: "+err.Error()) + return + } + + err = s.server.Serve(listener) + // Serve returns error every time, even after closing the server. + // User shouldn't be notified about error if server shouldn't be running, + // but it should in case it was not closed by `s.Close()`. + if err != nil && s.isRunning.Load().(bool) { + s.isRunning.Store(false) + l.WithError(err).Error("SMTP server failed") s.eventListener.Emit(events.ErrorEvent, "SMTP failed: "+err.Error()) - l.Error("SMTP failed: ", err) return } defer s.server.Close() //nolint[errcheck] @@ -100,11 +143,55 @@ func (s *smtpServer) ListenAndServe() { // Stops the server. func (s *smtpServer) Close() { + if !s.isRunning.Load().(bool) { + return + } + s.isRunning.Store(false) + if err := s.server.Close(); err != nil { log.WithError(err).Error("Failed to close the connection") } } +func (s *smtpServer) monitorInternetConnection() { + on := make(chan string) + s.eventListener.Add(events.InternetOnEvent, on) + off := make(chan string) + s.eventListener.Add(events.InternetOffEvent, off) + + for { + var expectedIsPortFree bool + select { + case <-on: + go func() { + defer s.panicHandler.HandlePanic() + // We had issues on Mac that from time to time something + // blocked our port for a bit after we closed IMAP server + // due to connection issues. + // Restart always helped, so we do retry to not bother user. + s.listenAndServe(10) + }() + expectedIsPortFree = false + case <-off: + s.Close() + expectedIsPortFree = true + } + + start := time.Now() + for { + if ports.IsPortFree(s.port) == expectedIsPortFree { + break + } + // Safety stop if something went wrong. + if time.Since(start) > 15*time.Second { + log.WithField("expectedIsPortFree", expectedIsPortFree).Warn("Server start/stop check timeouted") + break + } + time.Sleep(100 * time.Millisecond) + } + } +} + func (s *smtpServer) monitorDisconnectedUsers() { ch := make(chan string) s.eventListener.Add(events.CloseConnectionEvent, ch) diff --git a/internal/smtp/server_test.go b/internal/smtp/server_test.go new file mode 100644 index 00000000..d3f421c9 --- /dev/null +++ b/internal/smtp/server_test.go @@ -0,0 +1,65 @@ +// Copyright (c) 2021 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ProtonMail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with ProtonMail Bridge. If not, see . + +package smtp + +import ( + "fmt" + "testing" + "time" + + "github.com/ProtonMail/proton-bridge/internal/bridge" + "github.com/ProtonMail/proton-bridge/internal/events" + "github.com/ProtonMail/proton-bridge/pkg/listener" + "github.com/ProtonMail/proton-bridge/pkg/ports" + goSMTP "github.com/emersion/go-smtp" + + "github.com/stretchr/testify/require" +) + +type testPanicHandler struct{} + +func (ph *testPanicHandler) HandlePanic() {} + +func TestSMTPServerTurnOffAndOnAgain(t *testing.T) { + panicHandler := &testPanicHandler{} + + eventListener := listener.New() + + port := ports.FindFreePortFrom(12345) + server := goSMTP.NewServer(nil) + server.Addr = fmt.Sprintf("%v:%v", bridge.Host, port) + + s := &smtpServer{ + panicHandler: panicHandler, + server: server, + eventListener: eventListener, + } + s.isRunning.Store(false) + + go s.ListenAndServe() + time.Sleep(5 * time.Second) + require.False(t, ports.IsPortFree(port)) + + eventListener.Emit(events.InternetOffEvent, "") + time.Sleep(10 * time.Second) + require.True(t, ports.IsPortFree(port)) + + eventListener.Emit(events.InternetOnEvent, "") + time.Sleep(10 * time.Second) + require.False(t, ports.IsPortFree(port)) +} diff --git a/test/context/smtp.go b/test/context/smtp.go index 86cae97e..bc88a253 100644 --- a/test/context/smtp.go +++ b/test/context/smtp.go @@ -60,7 +60,7 @@ func (ctx *TestContext) withSMTPServer() { useSSL := ctx.settings.GetBool(settings.SMTPSSLKey) backend := smtp.NewSMTPBackend(ph, ctx.listener, ctx.settings, ctx.bridge) - server := smtp.NewSMTPServer(true, port, useSSL, tls, backend, ctx.listener) + server := smtp.NewSMTPServer(ph, true, port, useSSL, tls, backend, ctx.listener) go server.ListenAndServe() require.NoError(ctx.t, waitForPort(port, 5*time.Second))