Prevent zombie sessions being left behind for web sessions (#32141)

The ssh session was not being closed for web sessions which resulted
in zombie sessions being left around until the ssh service was
restarted. TestTerminal was updated to assert that the session
tracker eventually transitions to the terminated state when the
client terminates the web socket.

Fixes #32120

lib/srv/regular/sshserver.go

@@ -937,6 +937,12 @@ func (s *Server) Addr() string {
 	return s.srv.Addr()
 }
 
+// ActiveConnections returns the number of connections that are
+// being served.
+func (s *Server) ActiveConnections() int32 {
+	return s.srv.ActiveConnections()
+}
+
 // ID returns server ID
 func (s *Server) ID() string {
 	return s.uuid

lib/sshutils/server.go

@@ -441,6 +441,12 @@ func (s *Server) trackUserConnections(delta int32) int32 {
 	return atomic.AddInt32(&s.userConns, delta)
 }
 
+// ActiveConnections returns the number of connections that are
+// being served.
+func (s *Server) ActiveConnections() int32 {
+	return atomic.LoadInt32(&s.userConns)
+}
+
 // HandleConnection is called every time an SSH server accepts a new
 // connection from a client.
 //

lib/web/apiserver_test.go

@@ -1837,13 +1837,27 @@ func TestTerminal(t *testing.T) {
 			t.Parallel()
 			s := newWebSuite(t)
 
+			// Set the recording config
 			require.NoError(t, s.server.Auth().SetSessionRecordingConfig(context.Background(), &tt.recordingConfig))
 
+			// Create a new session
 			ws, _, err := s.makeTerminal(t, s.authPack(t, "foo"))
 			require.NoError(t, err)
-			t.Cleanup(func() { require.NoError(t, ws.Close()) })
+			t.Cleanup(func() { require.True(t, utils.IsOKNetworkError(ws.Close())) })
 
+			// Send a command and validate the output
 			validateTerminalStream(t, ws)
+
+			// Validate that the session is active on the node
+			require.Equal(t, int32(1), s.node.ActiveConnections())
+
+			// Close the web socket to emulate a user closing the browser window
+			require.NoError(t, ws.Close())
+
+			// Validate that the node terminates the session
+			require.EventuallyWithT(t, func(t *assert.CollectT) {
+				assert.Zero(t, s.node.ActiveConnections())
+			}, 30*time.Second, 250*time.Millisecond)
 		})
 	}
 }

lib/web/terminal.go

@@ -357,10 +357,14 @@ func (t *TerminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 func (t *TerminalHandler) Close() error {
 	var err error
 	t.closeOnce.Do(func() {
-		// If the terminal handler was closed (most likely due to the *SessionContext
-		// closing) then the stream should be closed as well.
-		if t.stream != nil {
-			err = t.stream.Close()
+		if t.stream == nil {
+			return
+		}
+
+		if t.stream.sshSession != nil {
+			err = trace.NewAggregate(t.stream.sshSession.Close(), t.stream.Close())
+		} else {
+			err = trace.Wrap(t.stream.Close())
 		}
 	})
 	return trace.Wrap(err)
@@ -1352,10 +1356,8 @@ func (t *WSStream) SendCloseMessage(event sessionEndEvent) error {
 
 func (t *WSStream) close() {
 	t.once.Do(func() {
-		defer func() {
-			close(t.rawC)
-			close(t.challengeC)
-		}()
+		close(t.rawC)
+		close(t.challengeC)
 	})
 }