From 30fdcf35622ae231d53b7a9126d446fc6f8aec06 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Tue, 14 May 2024 07:26:21 -0400 Subject: [PATCH] Prevent web ui SSH connections from leaking (#41501) The session closing in TerminalHandler.Close did not always close established ssh sessions if the client closed the web socket at any point between the new browser tab being opened and the session being established. In these cases the SSH connection was leaked for the duration of the Proxy process. To ensure sessions are always terminated a check to see if the client closed the web socket was added prior to writing the session metadata and after the shell is established for the ssh session. --- lib/web/terminal.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index 3d88a2c90a766..c8990115d6b3c 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -489,6 +489,14 @@ func (t *TerminalHandler) makeClient(ctx context.Context, stream *TerminalStream // to allow future window changes. tc.OnShellCreated = func(s *tracessh.Session, c *tracessh.Client, _ io.ReadWriteCloser) (bool, error) { t.stream.sessionCreated(s) + + // The web session was closed by the client while the ssh connection was being established. + // Attempt to close the SSH session instead of proceeding with the window change request. + if t.closedByClient.Load() { + t.log.Debug("websocket was closed by client, terminating established ssh connection to host") + return false, trace.Wrap(s.Close()) + } + if err := s.WindowChange(ctx, t.term.H, t.term.W); err != nil { t.log.Error(err) } @@ -808,9 +816,17 @@ func (t *TerminalHandler) streamTerminal(ctx context.Context, tc *client.Telepor t.stream.writeError(err.Error()) return } - defer nc.Close() + // If the session was terminated by client while the connection to the host + // was being established, then return early before creating the shell. Any terminations + // by the client from here on out should either get caught in the OnShellCreated callback + // set on the [tc] or in [TerminalHandler.Close]. + if t.closedByClient.Load() { + t.log.Debug("websocket was closed by client, aborting establishing ssh connection to host") + return + } + var beforeStart func(io.Writer) if t.participantMode == types.SessionModeratorMode { beforeStart = func(out io.Writer) { @@ -850,7 +866,7 @@ func (t *TerminalHandler) streamTerminal(ctx context.Context, tc *client.Telepor } if err := t.stream.Close(); err != nil { - t.log.WithError(err).Error("Unable to send close event to web client.") + t.log.WithError(err).Error("Unable to close client web socket.") return }