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

x11 forwarding #9897

Merged
merged 31 commits into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6f2dbdc
Add X11 forwarding flag to tsh ssh; Implement basic X11 forwarding cl…
Joerger Nov 9, 2021
47cb206
Implement x11 server.
Joerger Nov 11, 2021
29d64e1
Implement xauth with trusted and untrusted mode; move x11 code to ssh…
Joerger Dec 9, 2021
5b0f5c5
Update x11 forwarding flags and add openssh options.
Joerger Dec 10, 2021
31c3b72
Clean up implementation and add proper comments; reuse new x11 helpers
Joerger Dec 16, 2021
e371a6e
Add x11 server options; handle x11 request failures gracefully; Fix f…
Joerger Dec 16, 2021
9ab5194
Fix x11 forwarding for XQuartz and MobaXTerm.
Joerger Dec 21, 2021
a62ff3d
Update comment for ServerConfig.UseLocalhost.
Joerger Jan 10, 2022
eb766c7
Refactor xauth command logic to make the command run more customizable;
Joerger Jan 11, 2022
c765c11
Update xserver unix listner owner in re-exec block.
Joerger Jan 12, 2022
df9d5e0
Add x11rdy pipe to signal parent process when x11 is set up.
Joerger Jan 19, 2022
26a4e68
Update non-graceful restart TODO comment.
Joerger Jan 20, 2022
fc88b02
Clean up.
Joerger Jan 20, 2022
4dc4f33
Resolve PR comments and apply suggestions;
Joerger Jan 24, 2022
93ff2cb
Fix ForwardX11Timeout functionality;
Joerger Jan 29, 2022
e01d0b3
Add x11-untrusted-timeout flag.
Joerger Jan 29, 2022
1c12e95
Update UX to match updated RFD.
Joerger Jan 31, 2022
07453b7
Fix re-exec chown syscall; Fix sshserver test.
Joerger Jan 31, 2022
dba63c8
Handle X Server listener limit exceeded error.
Joerger Jan 31, 2022
564ae1b
Remove todo.
Joerger Jan 31, 2022
a1a7678
Cleanup.
Joerger Feb 1, 2022
6146215
Fix linter and fileconf tests.
Joerger Feb 2, 2022
8e97f18
Check for EADDRINUSE when opening XServer unix socket.
Joerger Feb 2, 2022
32a6eb3
Address comments.
Joerger Feb 3, 2022
ed31f03
Fix CI error.
Joerger Feb 3, 2022
c028a5f
Make suggested changes.
Joerger Feb 3, 2022
2a53fa2
Continue session when forwarded x11 request denied; Print log to user…
Joerger Feb 4, 2022
2fcafb8
Add debug log.
Joerger Feb 4, 2022
23cf2e0
Add xauth tests; Disable xauth tests unless requested.
Joerger Feb 4, 2022
39be324
Add todo for x11 test improvments.
Joerger Feb 4, 2022
3fcd4a8
Merge branch 'master' into joerger/x11-forwarding
Joerger Feb 4, 2022
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
1 change: 1 addition & 0 deletions .cloudbuild/scripts/cmd/unit-tests/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func runUnitTests(workspace string) error {
cmd := exec.Command("make", "test")
cmd.Dir = workspace
cmd.Env = append(os.Environ(), "TELEPORT_ETCD_TEST=yes")
cmd.Env = append(os.Environ(), "TELEPORT_XAUTH_TEST=yes")
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

Expand Down
2 changes: 2 additions & 0 deletions build.assets/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ clean:
test: buildbox
docker run \
--env TELEPORT_ETCD_TEST="yes" \
--env TELEPORT_XAUTH_TEST="yes" \
$(DOCKERFLAGS) $(NOROOT) -t $(BUILDBOX) \
/bin/bash -c \
"examples/etcd/start-etcd.sh & sleep 1; \
Expand All @@ -248,6 +249,7 @@ test: buildbox
test-root: buildbox
docker run \
--env TELEPORT_ETCD_TEST="yes" \
--env TELEPORT_XAUTH_TEST="yes" \
$(DOCKERFLAGS) -t $(BUILDBOX) \
/bin/bash -c \
"examples/etcd/start-etcd.sh & sleep 1; \
Expand Down
20 changes: 15 additions & 5 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,16 @@ type Config struct {
// ForwardAgent is used by the client to request agent forwarding from the server.
ForwardAgent AgentForwardingMode

// EnableX11Forwarding specifies whether X11 forwarding should be enabled.
EnableX11Forwarding bool

// X11ForwardingTimeout can be set to set a X11 forwarding timeout in seconds,
// after which any X11 forwarding requests in that session will be rejected.
X11ForwardingTimeout time.Duration

// X11ForwardingTrusted specifies the X11 forwarding security mode.
X11ForwardingTrusted bool

// AuthMethods are used to login into the cluster. If specified, the client will
// use them in addition to certs stored in its local agent (from disk)
AuthMethods []ssh.AuthMethod
Expand Down Expand Up @@ -1449,7 +1459,7 @@ func (tc *TeleportClient) SSH(ctx context.Context, command []string, runLocally
if len(nodeAddrs) > 1 {
fmt.Printf("\x1b[1mWARNING\x1b[0m: Multiple nodes match the label selector, picking first: %v\n", nodeAddrs[0])
}
return tc.runShell(nodeClient, nil)
return tc.runShell(ctx, nodeClient, nil)
}

func (tc *TeleportClient) startPortForwarding(ctx context.Context, nodeClient *NodeClient) {
Expand Down Expand Up @@ -1559,7 +1569,7 @@ func (tc *TeleportClient) Join(ctx context.Context, namespace string, sessionID
tc.startPortForwarding(ctx, nc)

// running shell with a given session means "join" it:
return tc.runShell(nc, session)
return tc.runShell(ctx, nc, session)
}

// Play replays the recorded session
Expand Down Expand Up @@ -2028,13 +2038,13 @@ func (tc *TeleportClient) runCommand(ctx context.Context, nodeClient *NodeClient
}

// runShell starts an interactive SSH session/shell.
// sessionID : when empty, creates a new shell. otherwise it tries to join the existing session.
func (tc *TeleportClient) runShell(nodeClient *NodeClient, sessToJoin *session.Session) error {
// sessToJoin : when empty, creates a new shell. otherwise it tries to join the existing session.
func (tc *TeleportClient) runShell(ctx context.Context, nodeClient *NodeClient, sessToJoin *session.Session) error {
nodeSession, err := newSession(nodeClient, sessToJoin, tc.Env, tc.Stdin, tc.Stdout, tc.Stderr, tc.useLegacyID(nodeClient), tc.EnableEscapeSequences)
if err != nil {
return trace.Wrap(err)
}
if err = nodeSession.runShell(tc.OnShellCreated); err != nil {
if err = nodeSession.runShell(ctx, tc.OnShellCreated); err != nil {
switch e := trace.Unwrap(err).(type) {
case *ssh.ExitError:
tc.ExitStatus = e.ExitStatus()
Expand Down
40 changes: 30 additions & 10 deletions lib/client/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/sshutils/x11"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/trace"
)
Expand Down Expand Up @@ -75,6 +76,17 @@ type NodeSession struct {
enableEscapeSequences bool

terminal *terminal.Terminal

// clientXAuthEntry contains xauth data which provides
// access to the client's local XServer.
clientXAuthEntry *x11.XAuthEntry
// spoofedXAuthEntry is a copy of the client's xauth data with a
// spoofed cookie. This cookie will be used to authenticate server
// requests without exposing the client's cookie.
spoofedXAuthEntry *x11.XAuthEntry
// x11RefuseTime is an optional time at which X11 channel
// requests using the xauth cookie will be rejected.
x11RefuseTime time.Time
}

// newSession creates a new Teleport session with the given remote node
Expand Down Expand Up @@ -164,8 +176,8 @@ func (ns *NodeSession) NodeClient() *NodeClient {
return ns.nodeClient
}

func (ns *NodeSession) regularSession(callback func(s *ssh.Session) error) error {
session, err := ns.createServerSession()
func (ns *NodeSession) regularSession(ctx context.Context, callback func(s *ssh.Session) error) error {
session, err := ns.createServerSession(ctx)
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -177,11 +189,19 @@ func (ns *NodeSession) regularSession(callback func(s *ssh.Session) error) error

type interactiveCallback func(serverSession *ssh.Session, shell io.ReadWriteCloser) error

func (ns *NodeSession) createServerSession() (*ssh.Session, error) {
func (ns *NodeSession) createServerSession(ctx context.Context) (*ssh.Session, error) {
sess, err := ns.nodeClient.Client.NewSession()
if err != nil {
return nil, trace.Wrap(err)
}

// If X11 forwading is requested and the server accepts,
// X11 channel requests from the server will be accepted.
// Otherwise, all X11 channel requests must be rejected.
if err := ns.handleX11Forwarding(ctx, sess); err != nil {
return nil, trace.Wrap(err)
}

// pass language info into the remote session.
evarsToPass := []string{"LANG", "LANGUAGE"}
for _, evar := range evarsToPass {
Expand Down Expand Up @@ -238,14 +258,14 @@ func selectKeyAgent(tc *TeleportClient) agent.Agent {

// interactiveSession creates an interactive session on the remote node, executes
// the given callback on it, and waits for the session to end
func (ns *NodeSession) interactiveSession(callback interactiveCallback) error {
func (ns *NodeSession) interactiveSession(ctx context.Context, callback interactiveCallback) error {
// determine what kind of a terminal we need
termType := os.Getenv("TERM")
if termType == "" {
termType = teleport.SafeTerminalType
}
// create the server-side session:
sess, err := ns.createServerSession()
sess, err := ns.createServerSession(ctx)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -442,8 +462,8 @@ func (ns *NodeSession) updateTerminalSize(s *ssh.Session) {
}

// runShell executes user's shell on the remote node under an interactive session
func (ns *NodeSession) runShell(callback ShellCreatedCallback) error {
return ns.interactiveSession(func(s *ssh.Session, shell io.ReadWriteCloser) error {
func (ns *NodeSession) runShell(ctx context.Context, callback ShellCreatedCallback) error {
return ns.interactiveSession(ctx, func(s *ssh.Session, shell io.ReadWriteCloser) error {
// start the shell on the server:
if err := s.Shell(); err != nil {
return trace.Wrap(err)
Expand Down Expand Up @@ -475,7 +495,7 @@ func (ns *NodeSession) runCommand(ctx context.Context, cmd []string, callback Sh
// keyboard based signals will be propogated to the TTY on the server which is
// where all signal handling will occur.
if interactive {
return ns.interactiveSession(func(s *ssh.Session, term io.ReadWriteCloser) error {
return ns.interactiveSession(ctx, func(s *ssh.Session, term io.ReadWriteCloser) error {
err := s.Start(strings.Join(cmd, " "))
if err != nil {
return trace.Wrap(err)
Expand All @@ -502,10 +522,10 @@ func (ns *NodeSession) runCommand(ctx context.Context, cmd []string, callback Sh
// Unfortunately at the moment the Go SSH library Teleport uses does not
// support sending SSH_MSG_DISCONNECT. Instead we close the SSH channel and
// SSH client, and try and exit as gracefully as possible.
return ns.regularSession(func(s *ssh.Session) error {
return ns.regularSession(ctx, func(s *ssh.Session) error {
var err error

runContext, cancel := context.WithCancel(context.Background())
runContext, cancel := context.WithCancel(ctx)
go func() {
defer cancel()
err = s.Run(strings.Join(cmd, " "))
Expand Down
206 changes: 206 additions & 0 deletions lib/client/x11_session.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
/*
Copyright 2022 Gravitational, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package client

import (
"context"
"os"
"time"

"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/sshutils/x11"
"github.com/gravitational/trace"
"golang.org/x/crypto/ssh"
)

// handleX11Forwarding handles X11 channel requests for the given server session.
// If X11 forwarding is not requested by the client, or it is rejected by the server,
// then X11 channel requests will be rejected.
func (ns *NodeSession) handleX11Forwarding(ctx context.Context, sess *ssh.Session) error {
if !ns.nodeClient.TC.EnableX11Forwarding {
return ns.rejectX11Channels(ctx)
}

display, err := x11.GetXDisplay()
if err != nil {
log.WithError(err).Info("X11 forwarding requested but $DISPLAY is invalid")
return ns.rejectX11Channels(ctx)
}

if err := ns.setXAuthData(ctx, display); err != nil {
return trace.Wrap(err)
}

// The client's xauth cookie should never be exposed to the server, so we
// create a spoof of the cookie to send to the server for authentication.
// During X11 forwarding, the spoofed cookie will be replaced
// with the client's cookie to connect to the client's XServer.
ns.spoofedXAuthEntry, err = ns.clientXAuthEntry.SpoofXAuthEntry()
if err != nil {
return trace.Wrap(err)
}

if err := x11.RequestForwarding(sess, ns.spoofedXAuthEntry); err != nil {
// Notify the user that x11 forwarding request failed regardless of debug level
log.Print("X11 forwarding request failed")
log.WithError(err).Debug("X11 forwarding request error")
// If the X11 forwarding request fails, we must reject all X11 channel requests.
return ns.rejectX11Channels(ctx)
}

// Start listening for new X11 channel requests from the server
// and start X11 forwarding on those channels
err = ns.serveX11Channels(ctx, sess)
return trace.Wrap(err)
}

// setXAuthData generates new xauth data for the client's local XServer.
// This will be used during X11 forwarding to forward and authorize
// XServer requests from the remote server to the client's XServer.
func (ns *NodeSession) setXAuthData(ctx context.Context, display x11.Display) error {
if ns.nodeClient.TC.X11ForwardingTrusted {
// For trusted X11 forwarding, we can create a random cookie without xauth
// as it is only used to validate the server-client connection. Locally,
// the client's XServer will ignore the cookie and use whatever authentication
// mechanisms it would use as if the client made the request locally.
log.Info("Creating a fake xauth cookie for trusted X11 forwarding.")
log.Warn("Trusted X11 forwarding provides unmitigated access to your local XServer, use with caution")

var err error
ns.clientXAuthEntry, err = x11.NewFakeXAuthEntry(display)
return trace.Wrap(err)
}

if err := x11.CheckXAuthPath(); err != nil {
log.Info("trusted X11 forwarding requested but xauth is not installed")
return trace.Wrap(err)
}

// Generate the xauth entry in a temporary file so it only exists within the context of this request.
// The XServer will recognize the xauth data regardless of it's existence within the file system.
xauthFile, err := os.CreateTemp("", "tsh-xauthfile-*")
if err != nil {
return trace.Wrap(err)
}
defer func() {
if err := os.Remove(xauthFile.Name()); err != nil {
log.WithError(err).Debug("Failed to remove temporary xauth file")
}
}()

// When an untrusted cookie expires, X requests with that cookie are not rejected, rather
// the X Server ignores the unrecognized cookie and fail over to whatever authentication
// mechanisms are in place. This is the same behavior used with the fake cookie used
// above in trusted forwarding. Therefore it is essential that we deny any X requests made
// after the cookie has expired, and so we set this timeout before generating the cookie.
if ns.nodeClient.TC.X11ForwardingTimeout != 0 {
ns.x11RefuseTime = time.Now().Add(ns.nodeClient.TC.X11ForwardingTimeout)
}

log.Info("creating an untrusted xauth cookie for untrusted X11 forwarding")
cmd := x11.NewXAuthCommand(ctx, xauthFile.Name())
if err := cmd.GenerateUntrustedCookie(display, ns.nodeClient.TC.X11ForwardingTimeout); err != nil {
return trace.Wrap(err)
}

ns.clientXAuthEntry, err = x11.NewXAuthCommand(ctx, xauthFile.Name()).ReadEntry(display)
if err != nil {
return trace.Wrap(err)
}

return nil
}

// serveX11Channels serves incoming X11 channels by starting X11 forwarding with the session.
func (ns *NodeSession) serveX11Channels(ctx context.Context, sess *ssh.Session) error {
err := x11.ServeChannelRequests(ctx, ns.nodeClient.Client, func(ctx context.Context, nch ssh.NewChannel) {
if !ns.x11RefuseTime.IsZero() && time.Now().After(ns.x11RefuseTime) {
nch.Reject(ssh.Prohibited, "rejected X11 channel request after ForwardX11Timeout")
log.Warn("rejected X11 forwarding attempt after the ForwardX11Timeout")
return
}

var req x11.ChannelRequestPayload
if err := ssh.Unmarshal(nch.ExtraData(), &req); err != nil {
nch.Reject(ssh.Prohibited, "invalid payload")
log.WithError(err).Debug("rejected X11 channel request with invalid payload")
return
}

log.Debugf("received X11 channel request from %s:%d", req.OriginatorAddress, req.OriginatorPort)
xchan, sin, err := nch.Accept()
if err != nil {
log.WithError(err).Debug("failed to accept X11 channel request")
return
}
defer xchan.Close()

// Scan the XServer request from the X11 channel for an xauth packet. If the xauth packet
// is present and contains the spoofed cookie, then the cookie will be replaced with the
// client's xauth cookie. Otherwise, the request will be denied.
authPacket, err := x11.ReadAndRewriteXAuthPacket(xchan, ns.spoofedXAuthEntry, ns.clientXAuthEntry)
if trace.IsAccessDenied(err) {
log.Error("X11 connection rejected due to wrong authentication")
return
} else if err != nil {
log.WithError(err).Debug("Failed to read xauth packet from X11 channel request")
return
}

// Dial a connection to the client's XServer.
xconn, err := ns.clientXAuthEntry.Display.Dial()
if err != nil {
log.WithError(err).Debug("Failed to connect to client's display")
return
}
defer xconn.Close()

// Send the processed X11 auth packet to the client's XServer connection.
if _, err := xconn.Write(authPacket); err != nil {
log.WithError(err).Debug("Failed to write xauth packet")
return
}

// Forward ssh requests on the X11 channels until X11 forwarding is complete
ctx, cancel := context.WithCancel(ctx)
defer cancel()

go func() {
err := sshutils.ForwardRequests(ctx, sin, sess)
if err != nil {
log.WithError(err).Debug("Failed to forward ssh request from server during X11 forwarding")
}
}()

if err := x11.Forward(ctx, xconn, xchan); err != nil {
log.WithError(err).Debug("Encountered error during X11 forwarding")
}
})
return trace.Wrap(err)
}

// rejectX11Channels rejects any incomign X11 channels for this node session.
func (ns *NodeSession) rejectX11Channels(ctx context.Context) error {
err := x11.ServeChannelRequests(ctx, ns.nodeClient.Client, func(_ context.Context, nch ssh.NewChannel) {
// According to RFC 4254, client "implementations MUST reject any X11 channel
// open requests if they have not requested X11 forwarding. Following openssh's
// example, we treat such a request as a break in attempt and warn the user.
log.Warn("server tried X11 forwarding without client requesting it, this is likely a break-in attempt by a malicious user")
nch.Reject(ssh.Prohibited, "")
})
return trace.Wrap(err)
}
Loading