From 9f0fef3180ca1a239ceb2cdb1a73af739808e97f Mon Sep 17 00:00:00 2001 From: Benson Margulies Date: Tue, 5 Sep 2023 14:28:26 -0700 Subject: [PATCH] Implement STLS for pop3 (#384) --- .gitignore | 7 + pkg/config/config.go | 12 +- pkg/policy/address.go | 2 +- pkg/rest/testutils_test.go | 5 +- pkg/server/lifecycle.go | 5 +- pkg/server/pop3/handler.go | 60 ++++++- pkg/server/pop3/handler_test.go | 305 ++++++++++++++++++++++++++++++++ pkg/server/pop3/listener.go | 43 +++-- pkg/server/smtp/handler_test.go | 14 +- pkg/webui/root_controller.go | 2 +- 10 files changed, 426 insertions(+), 29 deletions(-) create mode 100644 pkg/server/pop3/handler_test.go diff --git a/.gitignore b/.gitignore index 8842df9..4472964 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,9 @@ *.a *.so +# Emacs messiness. +*~ + # Folders _obj _test @@ -58,3 +61,7 @@ repl-temp-* # Test lua files /inbucket.lua + +# IntelliJ +.idea +inbucket.iml diff --git a/pkg/config/config.go b/pkg/config/config.go index b27a428..d7c934f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -94,10 +94,14 @@ type SMTP struct { // POP3 contains the POP3 server configuration. type POP3 struct { - Addr string `required:"true" default:"0.0.0.0:1100" desc:"POP3 server IP4 host:port"` - Domain string `required:"true" default:"inbucket" desc:"HELLO domain"` - Timeout time.Duration `required:"true" default:"600s" desc:"Idle network timeout"` - Debug bool `ignored:"true"` + Addr string `required:"true" default:"0.0.0.0:1100" desc:"POP3 server IP4 host:port"` + Domain string `required:"true" default:"inbucket" desc:"HELLO domain"` + Timeout time.Duration `required:"true" default:"600s" desc:"Idle network timeout"` + Debug bool `ignored:"true"` + TLSEnabled bool `default:"false" desc:"Enable TLS"` + TLSPrivKey string `default:"cert.key" desc:"X509 Private Key file for TLS Support"` + TLSCert string `default:"cert.crt" desc:"X509 Public Certificate file for TLS Support"` + ForceTLS bool `default:"false" desc:"If true, TLS is always on. If false, enable STLS"` } // Web contains the HTTP server configuration. diff --git a/pkg/policy/address.go b/pkg/policy/address.go index 824c49e..5988258 100644 --- a/pkg/policy/address.go +++ b/pkg/policy/address.go @@ -117,7 +117,7 @@ func (a *Addressing) ShouldStoreDomain(domain string) bool { // ShouldAcceptOriginDomain indicates if Inbucket accept mail from the specified domain. func (a *Addressing) ShouldAcceptOriginDomain(domain string) bool { domain = strings.ToLower(domain) - return !stringutil.SliceContains(a.Config.SMTP.RejectOriginDomains, domain) + return !stringutil.SliceContains(a.Config.SMTP.RejectOriginDomains, domain) } // ParseEmailAddress unescapes an email address, and splits the local part from the domain part. diff --git a/pkg/rest/testutils_test.go b/pkg/rest/testutils_test.go index a64f411..a08eb87 100644 --- a/pkg/rest/testutils_test.go +++ b/pkg/rest/testutils_test.go @@ -110,12 +110,11 @@ func decodedStringEquals(t *testing.T, json interface{}, path string, want strin // Named path elements require the parent element to be a map[string]interface{}, numbers in square // brackets require the parent element to be a []interface{}. // -// getDecodedPath(o, "users", "[1]", "name") +// getDecodedPath(o, "users", "[1]", "name") // // is equivalent to the JavaScript: // -// o.users[1].name -// +// o.users[1].name func getDecodedPath(o interface{}, path ...string) (interface{}, string) { if len(path) == 0 { return o, "" diff --git a/pkg/server/lifecycle.go b/pkg/server/lifecycle.go index a6b6faa..b754460 100644 --- a/pkg/server/lifecycle.go +++ b/pkg/server/lifecycle.go @@ -61,7 +61,10 @@ func FullAssembly(conf *config.Root) (*Services, error) { rest.SetupRoutes(web.Router.PathPrefix(prefix("/api/")).Subrouter()) webServer := web.NewServer(conf, mmanager, msgHub) - pop3Server := pop3.NewServer(conf.POP3, store) + pop3Server, err := pop3.NewServer(conf.POP3, store) + if err != nil { + return nil, err + } smtpServer := smtp.NewServer(conf.SMTP, mmanager, addrPolicy, extHost) return &Services{ diff --git a/pkg/server/pop3/handler.go b/pkg/server/pop3/handler.go index d07dd96..f2a41d4 100644 --- a/pkg/server/pop3/handler.go +++ b/pkg/server/pop3/handler.go @@ -2,6 +2,8 @@ package pop3 import ( "bufio" + "context" + "crypto/tls" "fmt" "io" "net" @@ -53,6 +55,7 @@ var commands = map[string]bool{ "PASS": true, "APOP": true, "CAPA": true, + "STLS": true, } // Session defines an active POP3 session @@ -102,11 +105,32 @@ func (s *Session) String() string { func (s *Server) startSession(id int, conn net.Conn) { logger := log.With().Str("module", "pop3").Str("remote", conn.RemoteAddr().String()). Int("session", id).Logger() + logger.Debug().Msgf("ForceTLS: %t", s.config.ForceTLS) + connToClose := conn + if s.config.ForceTLS { + logger.Debug().Msg("Setting up TLS for ForceTLS") + tlsConn := tls.Server(conn, s.tlsConfig) + toCtx, toCtxCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer toCtxCancel() + if err := tlsConn.HandshakeContext(toCtx); err != nil { + logger.Error().Msgf("TLS handshake failed: %v.", err) + conn.Close() + s.wg.Done() + return + } + s.tlsState = new(tls.ConnectionState) + *s.tlsState = tlsConn.ConnectionState() + conn = tlsConn + } + logger.Info().Msg("Starting POP3 session") defer func() { - if err := conn.Close(); err != nil { + logger.Debug().Msg("closing at end of session") + // Closing the tlsConn hangs. + if err := connToClose.Close(); err != nil { logger.Warn().Err(err).Msg("Closing connection") } + logger.Debug().Msg("End of session") s.wg.Done() }() @@ -117,6 +141,7 @@ func (s *Server) startSession(id int, conn net.Conn) { // This is our command reading loop for ssn.state != QUIT && ssn.sendError == nil { line, err := ssn.readLine() + ssn.logger.Debug().Msgf("read %s", line) if err == nil { if cmd, arg, ok := ssn.parseCmd(line); ok { // Check against valid SMTP commands @@ -139,6 +164,9 @@ func (s *Server) startSession(id int, conn net.Conn) { ssn.send("USER") ssn.send("UIDL") ssn.send("IMPLEMENTATION Inbucket") + if s.tlsConfig != nil && s.tlsState == nil && !s.config.ForceTLS { + ssn.send("STLS") + } ssn.send(".") continue } @@ -193,7 +221,37 @@ func (s *Session) authorizationHandler(cmd string, args []string) { switch cmd { case "QUIT": s.send("+OK Goodnight and good luck") + s.logger.Debug().Msg("Quitting.") s.enterState(QUIT) + + case "STLS": + if !s.Server.config.TLSEnabled || s.Server.config.ForceTLS { + // Invalid command since TLS unconfigured. + s.logger.Debug().Msgf("-ERR TLS unavailable on the server") + s.send("-ERR TLS unavailable on the server") + s.ooSeq(cmd) + } + if s.tlsState != nil { + // TLS state previously valid. + s.logger.Debug().Msg("-ERR A TLS session already agreed upon.") + s.send("-ERR A TLS session already agreed upon.") + s.ooSeq(cmd) + } + s.logger.Debug().Msg("Initiating TLS context.") + + // Start TLS connection handshake. + s.send("+OK Begin TLS Negotiation") + tlsConn := tls.Server(s.conn, s.Server.tlsConfig) + if err := tlsConn.Handshake(); err != nil { + s.logger.Error().Msgf("-ERR TLS handshake failed %v", err) + s.ooSeq(cmd) + } + s.conn = tlsConn + s.reader = bufio.NewReader(tlsConn) + s.tlsState = new(tls.ConnectionState) + *s.tlsState = tlsConn.ConnectionState() + s.logger.Debug().Msgf("TLS set %v", *s.tlsState) + case "USER": if len(args) > 0 { s.user = args[0] diff --git a/pkg/server/pop3/handler_test.go b/pkg/server/pop3/handler_test.go new file mode 100644 index 0000000..db5573d --- /dev/null +++ b/pkg/server/pop3/handler_test.go @@ -0,0 +1,305 @@ +package pop3 + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "net/textproto" + "os" + "path" + "strings" + "testing" + "time" + + "github.com/inbucket/inbucket/pkg/config" + "github.com/inbucket/inbucket/pkg/storage" + "github.com/inbucket/inbucket/pkg/test" +) + +func TestNoTLS(t *testing.T) { + ds := test.NewStore() + server := setupPOPServer(t, ds, false, false) + pipe := setupPOPSession(t, server) + c := textproto.NewConn(pipe) + defer func() { + _ = c.PrintfLine("QUIT") + _, _ = c.ReadLine() + server.Drain() + }() + + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading initial line failed %v", err) + } + if !strings.HasPrefix(reply, "+OK") { + t.Fatalf("Initial line is not +OK") + } + if err := c.PrintfLine("CAPA"); err != nil { + t.Fatalf("Failed to send CAPA; %v.", err) + } + replies := []string{} + for true { + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading CAPA line failed %v", err) + } + if reply == "." { + break + } + replies = append(replies, reply) + } + + for _, r := range replies { + if r == "STLS" { + t.Errorf("TLS not enabled but received STLS.") + } + } +} + +func TestStartTLS(t *testing.T) { + ds := test.NewStore() + server := setupPOPServer(t, ds, true, false) + pipe := setupPOPSession(t, server) + c := textproto.NewConn(pipe) + defer func() { + _ = c.PrintfLine("QUIT") + _, _ = c.ReadLine() + server.Drain() + }() + + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading initial line failed %v", err) + } + if !strings.HasPrefix(reply, "+OK") { + t.Fatalf("Initial line is not +OK") + } + if err := c.PrintfLine("CAPA"); err != nil { + t.Fatalf("Failed to send CAPA; %v.", err) + } + replies := []string{} + for true { + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading CAPA line failed %v", err) + } + if reply == "." { + break + } + replies = append(replies, reply) + } + + sawTLS := false + for _, r := range replies { + if r == "STLS" { + sawTLS = true + } + } + + if !sawTLS { + t.Errorf("TLS enabled but no STLS capability.") + } + + if err := c.PrintfLine("STLS"); err != nil { + t.Fatalf("Failed to send STLS; %v.", err) + } + reply, err = c.ReadLine() + if err != nil { + t.Fatalf("Reading STLS reply line failed %v", err) + } + if !strings.HasPrefix(reply, "+OK") { + t.Fatalf("STLS failed: %s", reply) + } + + tlsConfig := &tls.Config{ + InsecureSkipVerify: true, + } + tlsConn := tls.Client(pipe, tlsConfig) + ctx, toCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer toCancel() + if err := tlsConn.HandshakeContext(ctx); err != nil { + t.Fatalf("TLS handshake failed; %v", err) + } + c = textproto.NewConn(tlsConn) + if err := c.PrintfLine("CAPA"); err != nil { + t.Fatalf("Failed to send CAPA; %v.", err) + } + reply, err = c.ReadLine() + if err != nil { + t.Fatalf("Reading CAPA reply line failed %v", err) + } + if !strings.HasPrefix(reply, "+OK") { + t.Fatalf("CAPA failed: %s", reply) + } + for true { + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading CAPA line failed %v", err) + } + if reply == "." { + break + } + } +} + +func TestForceTLS(t *testing.T) { + ds := test.NewStore() + server := setupPOPServer(t, ds, true, true) + pipe := setupPOPSession(t, server) + + tlsConfig := &tls.Config{ + InsecureSkipVerify: true, + } + tlsConn := tls.Client(pipe, tlsConfig) + ctx, toCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer toCancel() + if err := tlsConn.HandshakeContext(ctx); err != nil { + t.Fatalf("TLS handshake failed; %v", err) + } + c := textproto.NewConn(tlsConn) + defer func() { + _ = c.PrintfLine("QUIT") + _, _ = c.ReadLine() + server.Drain() + }() + + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading initial line failed %v", err) + } + if !strings.HasPrefix(reply, "+OK") { + t.Fatalf("Initial line is not +OK") + } + + if err := c.PrintfLine("CAPA"); err != nil { + t.Fatalf("Failed to send CAPA; %v.", err) + } + reply, err = c.ReadLine() + if err != nil { + t.Fatalf("Reading CAPA reply line failed %v", err) + } + if !strings.HasPrefix(reply, "+OK") { + t.Fatalf("CAPA failed: %s", reply) + } + for true { + reply, err := c.ReadLine() + if err != nil { + t.Fatalf("Reading CAPA line failed %v", err) + } + if reply == "STLS" { + t.Errorf("STLS in CAPA in forceTLS mode.") + } + if reply == "." { + break + } + } +} + +// net.Pipe does not implement deadlines +type mockConn struct { + net.Conn +} + +func (m *mockConn) SetDeadline(t time.Time) error { return nil } +func (m *mockConn) SetReadDeadline(t time.Time) error { return nil } +func (m *mockConn) SetWriteDeadline(t time.Time) error { return nil } + +func setupPOPServer(t *testing.T, ds storage.Store, tls bool, forceTLS bool) *Server { + t.Helper() + cfg := config.POP3{ + Addr: "127.0.0.1:2500", + Domain: "inbucket.local", + Timeout: 5, + Debug: true, + ForceTLS: forceTLS, + } + if tls { + cert, privKey, err := generateCertificate(t) + if err != nil { + t.Fatalf("Failed to generate x.509 certificate; %v", err) + } + // we have to write these things into files. + + cfg.TLSEnabled = true + td := t.TempDir() + certPath := path.Join(td, "cert.pem") + keyPath := path.Join(td, "key.pem") + if err := os.WriteFile(certPath, certToPem(cert), 0700); err != nil { + t.Fatalf("Failed to write cert PEM file; %v", err) + } + if err := os.WriteFile(keyPath, privKeyToPem(privKey), 0700); err != nil { + t.Fatalf("Failed to write privKey PEM file; %v", err) + } + + cfg.TLSCert = certPath + cfg.TLSPrivKey = keyPath + } + + s, err := NewServer(cfg, ds) + if err != nil { + t.Fatalf("Failed to create server: %v.", err) + } + return s +} + +var sessionNum int + +func setupPOPSession(t *testing.T, server *Server) net.Conn { + t.Helper() + serverConn, clientConn := net.Pipe() + + // Start the session. + server.wg.Add(1) + sessionNum++ + go server.startSession(sessionNum, &mockConn{serverConn}) + + return clientConn +} + +func privKeyToPem(privkey *rsa.PrivateKey) []byte { + privkeyBytes := x509.MarshalPKCS1PrivateKey(privkey) + return pem.EncodeToMemory( + &pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: privkeyBytes, + }, + ) +} + +func certToPem(cert []byte) []byte { + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert}) +} + +func generateCertificate(t *testing.T) ([]byte, *rsa.PrivateKey, error) { + t.Helper() + priv, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatalf("Failed to generate key; %v", err) + } + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + CommonName: "localhost.local", + }, + DNSNames: []string{"localhost", "127.0.0.1", "inbucket.local"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageDataEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageEmailProtection}, + } + + cert, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv) + if err != nil { + return nil, nil, fmt.Errorf("certificate generation failed; %v", err) + } + return cert, priv, nil +} diff --git a/pkg/server/pop3/listener.go b/pkg/server/pop3/listener.go index 8216593..32d72ea 100644 --- a/pkg/server/pop3/listener.go +++ b/pkg/server/pop3/listener.go @@ -2,6 +2,8 @@ package pop3 import ( "context" + "crypto/tls" + "fmt" "net" "sync" "time" @@ -13,21 +15,39 @@ import ( // Server defines an instance of the POP3 server. type Server struct { - config config.POP3 // POP3 configuration. - store storage.Store // Mail store. - listener net.Listener // TCP listener. - wg *sync.WaitGroup // Waitgroup tracking sessions. - notify chan error // Notify on fatal error. + config config.POP3 // POP3 configuration. + store storage.Store // Mail store. + listener net.Listener // TCP listener. + wg *sync.WaitGroup // Waitgroup tracking sessions. + notify chan error // Notify on fatal error. + tlsConfig *tls.Config // TLS encryption configuration. + tlsState *tls.ConnectionState } // NewServer creates a new, unstarted, POP3 server. -func NewServer(pop3Config config.POP3, store storage.Store) *Server { - return &Server{ - config: pop3Config, - store: store, - wg: new(sync.WaitGroup), - notify: make(chan error, 1), +func NewServer(pop3Config config.POP3, store storage.Store) (*Server, error) { + slog := log.With().Str("module", "pop3").Str("phase", "tls").Logger() + tlsConfig := &tls.Config{} + if pop3Config.TLSEnabled { + var err error + tlsConfig.Certificates = make([]tls.Certificate, 1) + tlsConfig.Certificates[0], err = tls.LoadX509KeyPair(pop3Config.TLSCert, pop3Config.TLSPrivKey) + if err != nil { + slog.Error().Msgf("Failed loading X509 KeyPair: %v", err) + return nil, fmt.Errorf("Failed to configure TLS; %v", err) + // Do not silently turn off Security. + } + slog.Debug().Msg("TLS config available") + } else { + tlsConfig = nil } + return &Server{ + config: pop3Config, + store: store, + wg: new(sync.WaitGroup), + notify: make(chan error, 1), + tlsConfig: tlsConfig, + }, nil } // Start the server and listen for connections @@ -110,6 +130,7 @@ func (s *Server) serve(ctx context.Context) { // Drain causes the caller to block until all active POP3 sessions have finished func (s *Server) Drain() { // Wait for sessions to close + log.Debug().Str("module", "pop3").Str("phase", "shutdown").Msg("waiting for connections to complete.") s.wg.Wait() log.Debug().Str("module", "pop3").Str("phase", "shutdown").Msg("POP3 connections have drained") } diff --git a/pkg/server/smtp/handler_test.go b/pkg/server/smtp/handler_test.go index 7754d33..126ae83 100644 --- a/pkg/server/smtp/handler_test.go +++ b/pkg/server/smtp/handler_test.go @@ -576,14 +576,14 @@ func setupSMTPServer(ds storage.Store, extHost *extension.Host) *Server { cfg := &config.Root{ MailboxNaming: config.FullNaming, SMTP: config.SMTP{ - Addr: "127.0.0.1:2500", - Domain: "inbucket.local", - MaxRecipients: 5, - MaxMessageBytes: 5000, - DefaultAccept: true, - RejectDomains: []string{"deny.com"}, + Addr: "127.0.0.1:2500", + Domain: "inbucket.local", + MaxRecipients: 5, + MaxMessageBytes: 5000, + DefaultAccept: true, + RejectDomains: []string{"deny.com"}, RejectOriginDomains: []string{"invalidomain.com"}, - Timeout: 5, + Timeout: 5, }, } diff --git a/pkg/webui/root_controller.go b/pkg/webui/root_controller.go index 05d3431..ca71e0e 100644 --- a/pkg/webui/root_controller.go +++ b/pkg/webui/root_controller.go @@ -2,8 +2,8 @@ package webui import ( "fmt" - "os" "net/http" + "os" "github.com/inbucket/inbucket/pkg/config" "github.com/inbucket/inbucket/pkg/server/web"