fix(pop3): Prevent STLS cmd triggered crashes (#516)

* fix(pop3): Prevent STLS cmd triggered crashes

Signed-off-by: James Hillyerd <james@hillyerd.com>

* err lint fix

Signed-off-by: James Hillyerd <james@hillyerd.com>

---------

Signed-off-by: James Hillyerd <james@hillyerd.com>

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.")
 

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.Close()
-		_, _ = c.ReadLine()
 		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.Close()
-		_, _ = c.ReadLine()
 		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.Close()
-		_, _ = c.ReadLine()
 		server.Drain()
 	}()