Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] Prevent web ui SSH connections from leaking #41518

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10007,3 +10007,109 @@ type loginGetterFunc func(resource services.AccessCheckable) ([]string, error)
func (f loginGetterFunc) GetAllowedLoginsForResource(resource services.AccessCheckable) ([]string, error) {
return f(resource)
}

func TestWebSocketClosedBeforeSSHSessionCreated(t *testing.T) {
t.Parallel()
s := newWebSuiteWithConfig(t, webSuiteConfig{disableDiskBasedRecording: true})

ctx, cancel := context.WithCancel(s.ctx)
t.Cleanup(cancel)

pack := s.authPack(t, "foo")

req := TerminalRequest{
Server: s.node.ID(),
Login: pack.login,
Term: session.TerminalParams{
W: 100,
H: 100,
},
}

data, err := json.Marshal(req)
require.NoError(t, err)

u := url.URL{
Host: s.webServer.Listener.Addr().String(),
Scheme: client.WSS,
Path: "/v1/webapi/sites/-current-/connect/ws",
}

q := u.Query()
q.Set("params", string(data))
u.RawQuery = q.Encode()

header := http.Header{}
header.Add("Origin", "http://localhost")
for _, cookie := range pack.cookies {
header.Add("Cookie", cookie.String())
}

dialer := websocket.Dialer{
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
}

ws, resp, err := dialer.Dial(u.String(), header)
if err != nil {
var sb strings.Builder
sb.WriteString("websocket dial")
if resp != nil {
fmt.Fprintf(&sb, "; status code %v;", resp.StatusCode)
fmt.Fprintf(&sb, "headers: %v; body: ", resp.Header)
io.Copy(&sb, resp.Body)
}
require.NoError(t, err, sb.String())
}
require.NoError(t, resp.Body.Close())

require.NoError(t, makeAuthReqOverWS(ws, pack.session.Token))

wsClosedChan := make(chan struct{})

// Create a stream that closes the web socket when the server writes the session metadata
// to the client. At this point, the SSH connection to the target should be in flight but
// not yet established.
stream := NewTerminalStream(ctx, TerminalStreamConfig{
WS: ws,
Logger: utils.NewLogger(),
Handlers: map[string]WSHandlerFunc{
defaults.WebsocketSessionMetadata: func(ctx context.Context, envelope Envelope) {
if envelope.Type != defaults.WebsocketSessionMetadata {
return
}

var sessResp siteSessionGenerateResponse
if err := json.Unmarshal([]byte(envelope.Payload), &sessResp); err != nil {
return
}

assert.NoError(t, ws.WriteControl(websocket.CloseMessage, nil, time.Now().Add(time.Second)))
close(wsClosedChan)
},
},
})
t.Cleanup(func() { require.NoError(t, stream.Close()) })

// Set a read deadline to unblock ReadAll below in the event of a bug
// preventing the ws from closing above.
require.NoError(t, stream.SetReadDeadline(time.Now().Add(30*time.Second)))

// Wait for the web socket to be closed above.
select {
case <-wsClosedChan:
case <-time.After(10 * time.Second):
t.Fatal("timed out waiting for session metadata")
}

// Validate that the SSH connection is terminated in response to the WS closing.
require.Eventually(t, func() bool {
return s.node.ActiveConnections() == 0
}, 10*time.Second, 100*time.Millisecond)

// Validate that reading nothing was permitted.
out, err := io.ReadAll(stream)
require.NoError(t, err)
require.Empty(t, out)
}
24 changes: 22 additions & 2 deletions lib/web/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,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)
}
Expand Down Expand Up @@ -783,9 +791,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
}

if err := t.writeSessionData(ctx); err != nil {
t.log.WithError(err).Warn("Unable to stream terminal - failure sending session data")
}
Expand Down Expand Up @@ -829,7 +845,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
}

Expand Down Expand Up @@ -1107,6 +1123,10 @@ func isOKWebsocketCloseError(err error) bool {
)
}

func (t *WSStream) SetReadDeadline(deadline time.Time) error {
return t.ws.SetReadDeadline(deadline)
}

func (t *WSStream) processMessages(ctx context.Context) {
defer func() {
t.close()
Expand Down
Loading