From b16764a65d7f978f084f114fd400eb46815519ba Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Thu, 13 Jun 2024 12:38:07 -0700 Subject: [PATCH] fix(pop3): Prevent STLS cmd triggered crashes (#516) * fix(pop3): Prevent STLS cmd triggered crashes Signed-off-by: James Hillyerd * err lint fix Signed-off-by: James Hillyerd --------- Signed-off-by: James Hillyerd --- pkg/server/pop3/handler.go | 4 +- pkg/server/pop3/handler_test.go | 132 ++++++++++++++++++++++++++++++-- 2 files changed, 128 insertions(+), 8 deletions(-) diff --git a/pkg/server/pop3/handler.go b/pkg/server/pop3/handler.go index ce7df98..7181a4a 100644 --- a/pkg/server/pop3/handler.go +++ b/pkg/server/pop3/handler.go @@ -218,13 +218,13 @@ func (s *Session) authorizationHandler(cmd string, args []string) { // 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) + return } 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) + return } s.logger.Debug().Msg("Initiating TLS context.") diff --git a/pkg/server/pop3/handler_test.go b/pkg/server/pop3/handler_test.go index 480dbb3..ac3c3d5 100644 --- a/pkg/server/pop3/handler_test.go +++ b/pkg/server/pop3/handler_test.go @@ -29,8 +29,7 @@ func TestNoTLS(t *testing.T) { pipe := setupPOPSession(t, server) c := textproto.NewConn(pipe) defer func() { - _ = c.PrintfLine("QUIT") - _, _ = c.ReadLine() + _ = c.Close() server.Drain() }() @@ -63,14 +62,44 @@ func TestNoTLS(t *testing.T) { } } +func TestSTLSWithTLSDisabled(t *testing.T) { + ds := test.NewStore() + server := setupPOPServer(t, ds, false, false) + pipe := setupPOPSession(t, server) + _ = pipe.SetDeadline(time.Now().Add(10 * time.Second)) + c := textproto.NewConn(pipe) + defer func() { + _ = c.Close() + 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("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, "-ERR") { + t.Errorf("STLS should have errored: %s", reply) + } +} + 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() + _ = c.Close() server.Drain() }() @@ -149,6 +178,98 @@ func TestStartTLS(t *testing.T) { } } +func TestDupStartTLS(t *testing.T) { + ds := test.NewStore() + server := setupPOPServer(t, ds, true, false) + pipe := setupPOPSession(t, server) + _ = pipe.SetDeadline(time.Now().Add(10 * time.Second)) + c := textproto.NewConn(pipe) + defer func() { + _ = c.Close() + 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 { + 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.") + } + + t.Log("Sending first STLS command, expected to succeed") + 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) + + t.Log("Sending second STLS command, expected to fail") + 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, "-ERR") { + t.Fatalf("STLS failed: %s", reply) + } + + // Send STAT to verify handler has not crashed. + if err := c.PrintfLine("STAT"); err != nil { + t.Fatalf("Failed to send STAT; %v.", err) + } + reply, err = c.ReadLine() + if err != nil { + t.Fatalf("Reading STAT reply line failed %v", err) + } + if !strings.HasPrefix(reply, "-ERR") { + t.Fatalf("STAT failed: %s", reply) + } +} + func TestForceTLS(t *testing.T) { ds := test.NewStore() server := setupPOPServer(t, ds, true, true) @@ -165,8 +286,7 @@ func TestForceTLS(t *testing.T) { } c := textproto.NewConn(tlsConn) defer func() { - _ = c.PrintfLine("QUIT") - _, _ = c.ReadLine() + _ = c.Close() server.Drain() }()