x11 forwarding (#9897)

.cloudbuild/scripts/cmd/unit-tests/main.go

@@ -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
 

build.assets/Makefile

@@ -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; \
@@ -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; \

lib/client/api.go

@@ -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
@@ -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) {
@@ -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
@@ -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()

lib/client/session.go

@@ -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"
 )
@@ -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
@@ -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)
 	}
@@ -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 {
@@ -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)
 	}
@@ -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)
@@ -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)
@@ -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, " "))

lib/client/x11_session.go

@@ -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)
+}

lib/config/configuration.go

@@ -956,6 +956,11 @@ func applySSHConfig(fc *FileConfig, cfg *service.Config) (err error) {
 
 	cfg.SSH.AllowTCPForwarding = fc.SSH.AllowTCPForwarding()
 
+	cfg.SSH.X11, err = fc.SSH.X11ServerConfig()
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
 	return nil
 }
 

lib/config/configuration_test.go

@@ -1712,23 +1712,25 @@ func TestAppsCLF(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		clf := CommandLineFlags{
-			Roles:   tt.inRoles,
-			AppName: tt.inAppName,
-			AppURI:  tt.inAppURI,
-		}
-		cfg := service.MakeDefaultConfig()
-		err := Configure(&clf, cfg)
-		if err != nil {
-			require.IsType(t, err, tt.outError, tt.desc)
-		} else {
-			require.NoError(t, err, tt.desc)
-		}
-		if tt.outError != nil {
-			continue
-		}
-		require.True(t, cfg.Apps.Enabled, tt.desc)
-		require.Len(t, cfg.Apps.Apps, 1, tt.desc)
+		t.Run(tt.desc, func(t *testing.T) {
+			clf := CommandLineFlags{
+				Roles:   tt.inRoles,
+				AppName: tt.inAppName,
+				AppURI:  tt.inAppURI,
+			}
+			cfg := service.MakeDefaultConfig()
+			err := Configure(&clf, cfg)
+			if err != nil {
+				require.IsType(t, err, tt.outError)
+			} else {
+				require.NoError(t, err)
+			}
+			if tt.outError != nil {
+				return
+			}
+			require.True(t, cfg.Apps.Enabled)
+			require.Len(t, cfg.Apps.Apps, 1)
+		})
 	}
 }
 

lib/config/fileconf.go

@@ -44,6 +44,7 @@ import (
 	restricted "github.com/gravitational/teleport/lib/restrictedsession"
 	"github.com/gravitational/teleport/lib/service"
 	"github.com/gravitational/teleport/lib/services"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/utils"
 
 	"github.com/gravitational/trace"
@@ -822,6 +823,9 @@ type SSH struct {
 	// Don't read this value directly: call the AllowTCPForwarding method
 	// instead.
 	MaybeAllowTCPForwarding *bool `yaml:"port_forwarding,omitempty"`
+
+	// X11 is used to configure X11 forwarding settings
+	X11 *X11 `yaml:"x11,omitempty"`
 }
 
 // AllowTCPForwarding checks whether the config file allows TCP forwarding or not.
@@ -832,6 +836,47 @@ func (ssh *SSH) AllowTCPForwarding() bool {
 	return *ssh.MaybeAllowTCPForwarding
 }
 
+// X11ServerConfig returns the X11 forwarding server configuration.
+func (ssh *SSH) X11ServerConfig() (*x11.ServerConfig, error) {
+	// Start with default configuration
+	cfg := &x11.ServerConfig{Enabled: false}
+	if ssh.X11 == nil {
+		return cfg, nil
+	}
+
+	var err error
+	cfg.Enabled, err = apiutils.ParseBool(ssh.X11.Enabled)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	if !cfg.Enabled {
+		return cfg, nil
+	}
+
+	cfg.MaxDisplay = x11.DefaultMaxDisplay
+	if ssh.X11.MaxDisplay != nil {
+		cfg.MaxDisplay = int(*ssh.X11.MaxDisplay)
+		// If set, max display must not be greater than the
+		// max display number supported by X Server
+		if cfg.MaxDisplay > x11.MaxDisplayNumber {
+			cfg.DisplayOffset = x11.MaxDisplayNumber
+		}
+	}
+
+	cfg.DisplayOffset = x11.DefaultDisplayOffset
+	if ssh.X11.DisplayOffset != nil {
+		cfg.DisplayOffset = int(*ssh.X11.DisplayOffset)
+		// If set, display offset must not be greater than the
+		// max display number
+		if cfg.DisplayOffset > cfg.MaxDisplay {
+			cfg.DisplayOffset = cfg.MaxDisplay
+		}
+	}
+
+	return cfg, nil
+}
+
 // CommandLabel is `command` section of `ssh_service` in the config file
 type CommandLabel struct {
 	Name    string        `yaml:"name"`
@@ -924,6 +969,18 @@ func (r *RestrictedSession) Parse() (*restricted.Config, error) {
 	}, nil
 }
 
+// X11 is a configuration for X11 forwarding
+type X11 struct {
+	// Enabled controls whether X11 forwarding requests can be granted by the server.
+	Enabled string `yaml:"enabled"`
+	// DisplayOffset tells the server what X11 display number to start from when
+	// searching for an open X11 unix socket for XServer proxies.
+	DisplayOffset *uint `yaml:"display_offset,omitempty"`
+	// DisplayOffset tells the server what X11 display number to stop at when
+	// searching for an open X11 unix socket for XServer proxies.
+	MaxDisplay *uint `yaml:"max_displays,omitempty"`
+}
+
 // Databases represents the database proxy service configuration.
 //
 // In the configuration file this section will be "db_service".

lib/config/fileconf_test.go

@@ -18,11 +18,13 @@ package config
 
 import (
 	"bytes"
+	"math"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v2"
@@ -388,6 +390,42 @@ func TestSSHSection(t *testing.T) {
 				cfg["ssh_service"].(cfgMap)["port_forwarding"] = "banana"
 			},
 			expectError: require.Error,
+		}, {
+			desc: "X11 enabled",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled": "yes",
+				}
+			},
+			expectError: require.NoError,
+		}, {
+			desc: "X11 disabled",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled": "no",
+				}
+			},
+			expectError: require.NoError,
+		}, {
+			desc:        "X11 display offset default",
+			mutate:      func(cfg cfgMap) {},
+			expectError: require.NoError,
+		}, {
+			desc: "X11 display offset 100",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"display_offset": 100,
+				}
+			},
+			expectError: require.NoError,
+		}, {
+			desc: "X11 display offset invalid value",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"display_offset": -100,
+				}
+			},
+			expectError: require.Error,
 		},
 	}
 
@@ -407,4 +445,160 @@ func TestSSHSection(t *testing.T) {
 			}
 		})
 	}
+
+}
+
+func TestX11Config(t *testing.T) {
+	for _, tc := range []struct {
+		desc            string
+		mutate          func(cfgMap)
+		expectError     require.ErrorAssertionFunc
+		expectX11Config *x11.ServerConfig
+	}{
+		{
+			desc:            "default",
+			mutate:          func(cfg cfgMap) {},
+			expectError:     require.NoError,
+			expectX11Config: &x11.ServerConfig{},
+		}, {
+			desc: "disabled",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled": "no",
+				}
+			},
+			expectError:     require.NoError,
+			expectX11Config: &x11.ServerConfig{},
+		}, {
+			desc: "x11 enabled",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled": "yes",
+				}
+			},
+			expectError: require.NoError,
+			expectX11Config: &x11.ServerConfig{
+				Enabled:       true,
+				DisplayOffset: x11.DefaultDisplayOffset,
+				MaxDisplay:    x11.DefaultMaxDisplay,
+			},
+		}, {
+			desc: "enabled value invalid",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":        "no",
+					"display_offset": "100",
+				}
+			},
+			expectError: require.Error,
+		}, {
+			desc: "display offset set",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":        "yes",
+					"display_offset": "100",
+				}
+			},
+			expectError: require.NoError,
+			expectX11Config: &x11.ServerConfig{
+				Enabled:       true,
+				DisplayOffset: 100,
+				MaxDisplay:    x11.DefaultMaxDisplay,
+			},
+		}, {
+			desc: "not enabled, display offset set",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":        "no",
+					"display_offset": "100",
+				}
+			},
+			expectError: require.NoError,
+			expectX11Config: &x11.ServerConfig{
+				Enabled: false,
+			},
+		}, {
+			desc: "display offset value invalid",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":        "yes",
+					"display_offset": "banana",
+				}
+			},
+			expectError: require.Error,
+		}, {
+			desc: "display offset value capped",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":        "yes",
+					"display_offset": math.MaxUint32,
+				}
+			},
+			expectError: require.Error,
+			expectX11Config: &x11.ServerConfig{
+				Enabled:       true,
+				DisplayOffset: x11.DefaultDisplayOffset,
+			},
+		}, {
+			desc: "max displays set",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":      "yes",
+					"max_displays": "100",
+				}
+			},
+			expectError: require.NoError,
+			expectX11Config: &x11.ServerConfig{
+				Enabled:       true,
+				DisplayOffset: x11.DefaultDisplayOffset,
+				MaxDisplay:    100,
+			},
+		}, {
+			desc: "not enabled, max displays set",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":      "no",
+					"max_displays": "100",
+				}
+			},
+			expectError: require.NoError,
+			expectX11Config: &x11.ServerConfig{
+				Enabled: false,
+			},
+		}, {
+			desc: "max displays value invalid",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":      "yes",
+					"max_displays": "banana",
+				}
+			},
+			expectError: require.Error,
+		}, {
+			desc: "max displays value capped",
+			mutate: func(cfg cfgMap) {
+				cfg["ssh_service"].(cfgMap)["x11"] = cfgMap{
+					"enabled":      "yes",
+					"max_displays": math.MaxUint32,
+				}
+			},
+			expectError: require.Error,
+			expectX11Config: &x11.ServerConfig{
+				Enabled:       true,
+				DisplayOffset: x11.DefaultDisplayOffset,
+				MaxDisplay:    x11.MaxDisplayNumber - x11.DefaultDisplayOffset,
+			},
+		},
+	} {
+		text := bytes.NewBuffer(editConfig(t, tc.mutate))
+		cfg, err := ReadConfig(text)
+		tc.expectError(t, err)
+		if err != nil {
+			return
+		}
+
+		serverCfg, err := cfg.SSH.X11ServerConfig()
+		require.NoError(t, err)
+		require.Equal(t, tc.expectX11Config, serverCfg)
+	}
 }

lib/service/cfg.go

@@ -54,6 +54,7 @@ import (
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/srv/app/common"
 	"github.com/gravitational/teleport/lib/sshca"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
 
@@ -568,6 +569,9 @@ type SSHConfig struct {
 	// the inactivity timeout expiring. The empty string indicates that no
 	// timeout message will be sent.
 	IdleTimeoutMessage string
+
+	// X11 holds x11 forwarding configuration for Teleport.
+	X11 *x11.ServerConfig
 }
 
 // KubeConfig specifies configuration for kubernetes service

lib/service/service.go

@@ -1957,6 +1957,7 @@ func (process *TeleportProcess) initSSH() error {
 			regular.SetOnHeartbeat(process.onHeartbeat(teleport.ComponentNode)),
 			regular.SetAllowTCPForwarding(cfg.SSH.AllowTCPForwarding),
 			regular.SetLockWatcher(lockWatcher),
+			regular.SetX11ForwardingConfig(cfg.SSH.X11),
 		)
 		if err != nil {
 			return trace.Wrap(err)

lib/srv/authhandlers.go

@@ -159,6 +159,14 @@ func (h *AuthHandlers) CheckAgentForward(ctx *ServerContext) error {
 	return nil
 }
 
+// CheckX11Forward checks if X11 forwarding is permitted for the user's RoleSet.
+func (h *AuthHandlers) CheckX11Forward(ctx *ServerContext) error {
+	if !ctx.Identity.RoleSet.PermitX11Forwarding() {
+		return trace.AccessDenied("x11 forwarding not permitted")
+	}
+	return nil
+}
+
 // CheckPortForward checks if port forwarding is allowed for the users RoleSet.
 func (h *AuthHandlers) CheckPortForward(addr string, ctx *ServerContext) error {
 	if ok := ctx.Identity.RoleSet.CanPortForward(); !ok {

lib/srv/ctx.go

@@ -22,6 +22,7 @@ import (
 	"io"
 	"net"
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -42,6 +43,7 @@ import (
 	rsession "github.com/gravitational/teleport/lib/session"
 	"github.com/gravitational/teleport/lib/srv/uacc"
 	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/lib/utils/parse"
 
@@ -319,6 +321,14 @@ type ServerContext struct {
 	// port to connect to in a "direct-tcpip" request. This value is only
 	// populated for port forwarding requests.
 	DstAddr string
+
+	// x11rdy{r,w} is used to signal from the child process to the
+	// parent process when X11 forwarding is set up.
+	x11rdyr *os.File
+	x11rdyw *os.File
+
+	// x11Config holds the xauth and XServer listener config for this session.
+	x11Config *X11Config
 }
 
 // NewServerContext creates a new *ServerContext which is used to pass and
@@ -427,6 +437,15 @@ func NewServerContext(ctx context.Context, parent *sshutils.ConnectionContext, s
 	child.AddCloser(child.contr)
 	child.AddCloser(child.contw)
 
+	// Create pipe used to get X11 forwarding ready signal from the child process.
+	child.x11rdyr, child.x11rdyw, err = os.Pipe()
+	if err != nil {
+		childErr := child.Close()
+		return nil, nil, trace.NewAggregate(err, childErr)
+	}
+	child.AddCloser(child.x11rdyr)
+	child.AddCloser(child.x11rdyw)
+
 	return ctx, child, nil
 }
 
@@ -584,6 +603,139 @@ func (c *ServerContext) getSession() *session {
 	return c.session
 }
 
+// OpenXServerListener opens a new XServer unix listener.
+func (c *ServerContext) OpenXServerListener(x11Req x11.ForwardRequestPayload, displayOffset, maxDisplays int) error {
+	l, display, err := x11.OpenNewXServerListener(displayOffset, maxDisplays, x11Req.ScreenNumber)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
+	err = c.setX11Config(&X11Config{
+		XServerUnixSocket: l.Addr().String(),
+		XAuthEntry: x11.XAuthEntry{
+			Display: display,
+			Proto:   x11Req.AuthProtocol,
+			Cookie:  x11Req.AuthCookie,
+		},
+	})
+	if err != nil {
+		l.Close()
+		return trace.Wrap(err)
+	}
+
+	c.AddCloser(l)
+
+	// Prepare X11 channel request payload
+	originHost, originPort, err := net.SplitHostPort(c.ServerConn.LocalAddr().String())
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	originPortI, err := strconv.Atoi(originPort)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	x11ChannelReqPayload := ssh.Marshal(x11.ChannelRequestPayload{
+		OriginatorAddress: originHost,
+		OriginatorPort:    uint32(originPortI),
+	})
+
+	go func() {
+		for {
+			xconn, err := l.Accept()
+			if err != nil {
+				// listener is closed
+				return
+			}
+
+			go func() {
+				defer xconn.Close()
+
+				// If the session has not signaled that X11 forwarding is
+				// fully set up yet, then ignore any incoming connections.
+				// The client's session hasn't been fully set up yet so this
+				// could potentially be a break-in attempt.
+				if ok, err := c.x11Ready(); err != nil {
+					log.WithError(err).Debug("Failed to get X11 ready status")
+					return
+				} else if !ok {
+					log.WithError(err).Debug("Rejecting X11 request, XServer Proxy is not ready")
+					return
+				}
+
+				xchan, sin, err := c.ServerConn.OpenChannel(sshutils.X11ChannelRequest, x11ChannelReqPayload)
+				if err != nil {
+					log.WithError(err).Debug("Failed to open a new X11 channel")
+					return
+				}
+				defer xchan.Close()
+
+				// Forward ssh requests on the X11 channels until X11 forwarding is complete
+				ctx, cancel := context.WithCancel(c.cancelContext)
+				defer cancel()
+
+				go func() {
+					err := sshutils.ForwardRequests(ctx, sin, c.RemoteSession)
+					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")
+				}
+			}()
+
+			if x11Req.SingleConnection {
+				l.Close()
+				return
+			}
+		}
+	}()
+
+	return nil
+}
+
+// getX11Config gets the x11 config for this server session.
+func (c *ServerContext) getX11Config() X11Config {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.x11Config != nil {
+		return *c.x11Config
+	}
+	return X11Config{}
+}
+
+// setX11Config sets X11 config for the session, or returns an error if already set.
+func (c *ServerContext) setX11Config(cfg *X11Config) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if c.x11Config != nil {
+		return trace.AlreadyExists("X11 forwarding is already set up for this session")
+	}
+
+	c.x11Config = cfg
+	return nil
+}
+
+// x11Ready returns whether the X11 unix listener is ready to accept connections.
+func (c *ServerContext) x11Ready() (bool, error) {
+	// Wait for child process to send signal (1 byte)
+	// or EOF if signal was already received.
+	_, err := io.ReadFull(c.x11rdyr, make([]byte, 1))
+	if err == io.EOF {
+		return true, nil
+	} else if err != nil {
+		return false, trace.Wrap(err)
+	}
+
+	// signal received, close writer so future calls read EOF.
+	if err := c.x11rdyw.Close(); err != nil {
+		return false, trace.Wrap(err)
+	}
+	return true, nil
+}
+
 // takeClosers returns all resources that should be closed and sets the properties to null
 // we do this to avoid calling Close() under lock to avoid potential deadlocks
 func (c *ServerContext) takeClosers() []io.Closer {
@@ -833,6 +985,7 @@ func (c *ServerContext) ExecCommand() (*ExecCommand, error) {
 		PAMConfig:             pamConfig,
 		IsTestStub:            c.IsTestStub,
 		UaccMetadata:          *uaccMetadata,
+		X11Config:             c.getX11Config(),
 	}, nil
 }
 

lib/srv/forward/sshserver.go

@@ -40,6 +40,7 @@ import (
 	"github.com/gravitational/teleport/lib/session"
 	"github.com/gravitational/teleport/lib/srv"
 	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/teleagent"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/trace"
@@ -947,129 +948,55 @@ func (s *Server) handleAgentForward(ch ssh.Channel, req *ssh.Request, ctx *srv.S
 // handleX11ChannelRequest accepts an X11 channel and forwards it back to the client.
 // Servers which support X11 forwarding request a separate channel for serving each
 // inbound connection on the X11 socket of the remote session.
-func (s *Server) handleX11ChannelRequest(ctx context.Context, xreq ssh.NewChannel) error {
+func (s *Server) handleX11ChannelRequest(ctx context.Context, nch ssh.NewChannel) {
 	// accept inbound X11 channel from server
-	sch, sin, err := xreq.Accept()
+	sch, sin, err := nch.Accept()
 	if err != nil {
-		return trace.Wrap(err)
+		s.log.Errorf("X11 channel fwd failed: %v", err)
+		return
 	}
 	defer sch.Close()
 
 	// setup outbound X11 channel to client
-	cch, cin, err := s.sconn.OpenChannel(sshutils.X11ChannelRequest, xreq.ExtraData())
+	cch, cin, err := s.sconn.OpenChannel(sshutils.X11ChannelRequest, nch.ExtraData())
 	if err != nil {
-		return trace.Wrap(err)
+		s.log.Errorf("X11 channel fwd failed: %v", err)
+		return
 	}
 	defer cch.Close()
 
-	// setup child context which will be canceled when io forwarding completes.
-	ctx, cancel := context.WithCancel(ctx)
-
-	// setup wait group for io forwarding goroutines
-	var wg sync.WaitGroup
-
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		// forward data from server to client
-		io.Copy(cch, sch)
-		// inform client that no more data is coming
-		cch.CloseWrite()
-	}()
-
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		// forward data from client to server
-		io.Copy(sch, cch)
-		// inform server that no more data is coming
-		sch.CloseWrite()
-	}()
-
-	// cancel the local context when wait group completes
-	go func() {
-		wg.Wait()
-		cancel()
-	}()
-
-	// multiplex requests until io forwarding goroutines
-	// have completed.
-	for {
-		select {
-		case sreq := <-sin:
-			if sreq == nil {
-				// channel closed, stop processing
-				sin = nil
-				continue
-			}
-			switch sreq.Type {
-			case sshutils.WindowChangeRequest:
-				if _, err := forwardRequest(cch, sreq); err != nil {
-					return trace.Wrap(err)
-				}
-			default:
-				s.log.Errorf("Unsupported x11 channel request type: %s", sreq.Type)
-				if sreq.WantReply {
-					sreq.Reply(false, nil)
-				}
-				continue
-			}
-		case creq := <-cin:
-			if creq == nil {
-				// channel closed, stop processing
-				cin = nil
-				continue
-			}
-			switch creq.Type {
-			case sshutils.WindowChangeRequest:
-				if _, err := forwardRequest(sch, creq); err != nil {
-					return trace.Wrap(err)
-				}
-			default:
-				s.log.Errorf("Unsupported x11 channel request type: %s", creq.Type)
-				if creq.WantReply {
-					creq.Reply(false, nil)
-				}
-				continue
-			}
-		case <-ctx.Done():
-			return nil
-		}
-	}
-}
-
-// serveX11Channels registers and runs a hander for X11 channel requests
-// generated by the server.  Must not be launched until X11 forwarding has
-// been requested and authorized.
-func (s *Server) serveX11Channels(ctx context.Context) error {
-	ncs := s.remoteClient.HandleChannelOpen(sshutils.X11ChannelRequest)
-	if ncs == nil {
-		return trace.AlreadyExists("x11 forwarding already active")
-	}
+	// Forward ssh requests on the X11 channels until X11 forwarding is complete
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
-	for {
-		select {
-		case req := <-ncs:
-			if req == nil {
-				return nil
-			}
-			go func() {
-				if err := s.handleX11ChannelRequest(ctx, req); err != nil {
-					s.log.Errorf("x11 channel fwd failed: %v", err)
-				}
-			}()
-		case <-ctx.Done():
-			return nil
+
+	go func() {
+		err := sshutils.ForwardRequests(ctx, cin, sch)
+		if err != nil {
+			s.log.WithError(err).Debug("Failed to forward ssh request from client during X11 forwarding")
 		}
+	}()
+
+	go func() {
+		err := sshutils.ForwardRequests(ctx, sin, cch)
+		if err != nil {
+			s.log.WithError(err).Debug("Failed to forward ssh request from server during X11 forwarding")
+		}
+	}()
+
+	if err := x11.Forward(ctx, cch, sch); err != nil {
+		s.log.WithError(err).Debug("Encountered error during x11 forwarding")
 	}
 }
 
 // handleX11Forward handles an X11 forwarding request from the client.
-func (s *Server) handleX11Forward(ctx context.Context, ch ssh.Channel, req *ssh.Request, scx *srv.ServerContext) error {
-	event := apievents.X11Forward{
+func (s *Server) handleX11Forward(ctx context.Context, ch ssh.Channel, req *ssh.Request, scx *srv.ServerContext) (err error) {
+	event := &apievents.X11Forward{
+		Status: apievents.Status{
+			Success: true,
+		},
 		Metadata: apievents.Metadata{
 			Type: events.X11ForwardEvent,
+			Code: events.X11ForwardCode,
 		},
 		UserMetadata: s.identityContext.GetUserMetadata(),
 		ConnectionMetadata: apievents.ConnectionMetadata{
@@ -1078,64 +1005,44 @@ func (s *Server) handleX11Forward(ctx context.Context, ch ssh.Channel, req *ssh.
 		},
 	}
 
-	// check if RBAC permits X11 forwarding
-	if !scx.Identity.RoleSet.PermitX11Forwarding() {
-		event.Metadata.Code = events.X11ForwardFailureCode
-		event.Status.Success = false
-		event.Status.Error = "x11 forwarding not permitted"
-		if err := s.EmitAuditEvent(s.closeContext, &event); err != nil {
-			s.log.WithError(err).Warn("Failed to emit X11 forward event.")
-		}
-		s.replyError(ch, req, trace.AccessDenied("x11 forwarding not permitted"))
-		// failed X11 requests are ok from a protocol perspective, so
-		// we don't actually return an error here.
-		return nil
-	}
-
-	// send X11 forwarding request to remote
-	ok, err := forwardRequest(scx.RemoteSession, req)
-	if err != nil || !ok {
-		// request failed or was denied
-		event.Metadata.Code = events.X11ForwardFailureCode
-		event.Status.Success = false
+	defer func() {
 		if err != nil {
+			event.Metadata.Code = events.X11ForwardFailureCode
+			event.Status.Success = false
 			event.Status.Error = err.Error()
 		}
-		if err := s.EmitAuditEvent(s.closeContext, &event); err != nil {
-			s.log.WithError(err).Warn("Failed to emit X11 forward event.")
+		if trace.IsAccessDenied(err) {
+			// denied X11 requests are ok from a protocol perspective so we
+			// don't return them, just reply over ssh and emit the audit log.
+			s.replyError(ch, req, err)
+			err = nil
 		}
-		return trace.Wrap(err)
-	}
-
-	go func() {
-		if err := s.serveX11Channels(ctx); err != nil {
-			s.log.Errorf("x11 channel srv failed: %v", err)
+		if err := s.EmitAuditEvent(ctx, event); err != nil {
+			s.log.WithError(err).Warn("Failed to emit x11-forward event.")
 		}
 	}()
 
-	event.Status.Success = true
-	event.Metadata.Code = events.X11ForwardCode
-	if err := s.EmitAuditEvent(s.closeContext, &event); err != nil {
-		s.log.WithError(err).Warn("Failed to emit X11 forward event.")
+	// Check if the user's RBAC role allows X11 forwarding.
+	if err := s.authHandlers.CheckX11Forward(scx); err != nil {
+		return trace.Wrap(err)
 	}
+
+	// send X11 forwarding request to remote
+	ok, err := sshutils.ForwardRequest(scx.RemoteSession, req)
+	if err != nil {
+		return trace.Wrap(err)
+	} else if !ok {
+		return trace.AccessDenied("X11 forwarding request denied by server")
+	}
+
+	err = x11.ServeChannelRequests(ctx, s.remoteClient, s.handleX11ChannelRequest)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
 	return nil
 }
 
-// sendRequest represents a resource capable of sending an ssh request such as
-// an ssh.Channel or ssh.Session.
-type sendRequest interface {
-	SendRequest(name string, wantReply bool, payload []byte) (bool, error)
-}
-
-// forwardRequest is a helper for forwarding a request across a session or channel.
-func forwardRequest(sender sendRequest, req *ssh.Request) (bool, error) {
-	reply, err := sender.SendRequest(req.Type, req.WantReply, req.Payload)
-	if err != nil || !req.WantReply {
-		return reply, trace.Wrap(err)
-	}
-	return reply, trace.Wrap(req.Reply(reply, nil))
-}
-
 func (s *Server) handleSubsystem(ch ssh.Channel, req *ssh.Request, ctx *srv.ServerContext) error {
 	subsystem, err := parseSubsystemRequest(req, ctx)
 	if err != nil {

lib/srv/reexec.go

@@ -18,6 +18,7 @@ package srv
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -37,6 +38,7 @@ import (
 	"github.com/gravitational/teleport/lib/shell"
 	"github.com/gravitational/teleport/lib/srv/uacc"
 	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/utils"
 
 	log "github.com/sirupsen/logrus"
@@ -89,6 +91,9 @@ type ExecCommand struct {
 
 	// UaccMetadata contains metadata needed for user accounting.
 	UaccMetadata UaccMetadata `json:"uacc_meta"`
+
+	// X11Config contains an xauth entry to be added to the command user's xauthority.
+	X11Config X11Config `json:"x11_config"`
 }
 
 // PAMConfig represents all the configuration data that needs to be passed to the child.
@@ -104,6 +109,14 @@ type PAMConfig struct {
 	Environment map[string]string `json:"environment"`
 }
 
+// X11Config contains information used by the child process to set up X11 forwarding.
+type X11Config struct {
+	// XAuthEntry contains xauth data used for X11 forwarding.
+	XAuthEntry x11.XAuthEntry `json:"xauth_entry,omitempty"`
+	// XServerUnixSocket is the name of an open XServer unix socket used for X11 forwarding.
+	XServerUnixSocket string `json:"xserver_unix_socket"`
+}
+
 // UaccMetadata contains information the child needs from the parent for user accounting.
 type UaccMetadata struct {
 	// The hostname of the node.
@@ -153,12 +166,12 @@ func RunCommand() (io.Writer, int, error) {
 	var pty *os.File
 	uaccEnabled := false
 
-	// If a terminal was requested, file descriptor 4 and 5 always point to the
+	// If a terminal was requested, file descriptors 6 and 7 always point to the
 	// PTY and TTY. Extract them and set the controlling TTY. Otherwise, connect
 	// std{in,out,err} directly.
 	if c.Terminal {
-		pty = os.NewFile(uintptr(5), "/proc/self/fd/5")
-		tty = os.NewFile(uintptr(6), "/proc/self/fd/6")
+		pty = os.NewFile(uintptr(6), "/proc/self/fd/6")
+		tty = os.NewFile(uintptr(7), "/proc/self/fd/7")
 		if pty == nil || tty == nil {
 			return errorWriter, teleport.RemoteCommandFailure, trace.BadParameter("pty and tty not found")
 		}
@@ -233,6 +246,66 @@ func RunCommand() (io.Writer, int, error) {
 		return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
 	}
 
+	if c.X11Config.XServerUnixSocket != "" {
+		// Set the open XServer unix socket's owner to the localuser
+		// to prevent a potential privilege escalation vulnerability.
+		uid, err := strconv.Atoi(localUser.Uid)
+		if err != nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+		gid, err := strconv.Atoi(localUser.Gid)
+		if err != nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+		if err := os.Chown(c.X11Config.XServerUnixSocket, uid, gid); err != nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+
+		// Update localUser's xauth database for X11 forwarding.
+		removeCmd := x11.NewXAuthCommand(context.Background(), "")
+		addCmd := x11.NewXAuthCommand(context.Background(), "")
+
+		// Copy the re-exec command's io and user environment fields.
+		cpyCmdFields := func(cmd *exec.Cmd, params *exec.Cmd) {
+			cmd.Stdout = params.Stdout
+			cmd.Stdin = params.Stdin
+			cmd.Stderr = params.Stderr
+			cmd.SysProcAttr = params.SysProcAttr
+			cmd.Env = params.Env
+			cmd.Dir = params.Dir
+		}
+		cpyCmdFields(removeCmd.Cmd, cmd)
+		cpyCmdFields(addCmd.Cmd, cmd)
+
+		if err := removeCmd.RemoveEntries(c.X11Config.XAuthEntry.Display); err != nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+		if err := addCmd.AddEntry(c.X11Config.XAuthEntry); err != nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+
+		// Set $DISPLAY so that XServer requests forwarded to the X11 unix listener.
+		cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", x11.DisplayEnv, c.X11Config.XAuthEntry.Display.String()))
+
+		// Open x11rdy fd to signal parent process once X11 forwarding is set up.
+		x11rdyfd := os.NewFile(uintptr(5), "/proc/self/fd/5")
+		if x11rdyfd == nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.BadParameter("continue pipe not found")
+		}
+
+		// Write a single byte to signal to the parent process that X11 forwarding is set up.
+		if _, err := x11rdyfd.Write([]byte{0}); err != nil {
+			if err2 := x11rdyfd.Close(); err2 != nil {
+				return errorWriter, teleport.RemoteCommandFailure, trace.NewAggregate(err, err2)
+			}
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+
+		if err := x11rdyfd.Close(); err != nil {
+			return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err)
+		}
+	}
+
 	// Start the command.
 	err = cmd.Start()
 	if err != nil {
@@ -568,6 +641,7 @@ func ConfigureCommand(ctx *ServerContext) (*exec.Cmd, error) {
 		ExtraFiles: []*os.File{
 			ctx.cmdr,
 			ctx.contr,
+			ctx.x11rdyw,
 		},
 	}
 

lib/srv/regular/sshserver.go

@@ -50,6 +50,7 @@ import (
 	rsession "github.com/gravitational/teleport/lib/session"
 	"github.com/gravitational/teleport/lib/srv"
 	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/teleagent"
 	"github.com/gravitational/teleport/lib/utils"
 
@@ -182,6 +183,9 @@ type Server struct {
 	// TCP port forwarding.
 	allowTCPForwarding bool
 
+	// x11 is the X11 forwarding configuration for the server
+	x11 *x11.ServerConfig
+
 	// lockWatcher is the server's lock watcher.
 	lockWatcher *services.LockWatcher
 }
@@ -551,6 +555,14 @@ func SetLockWatcher(lockWatcher *services.LockWatcher) ServerOption {
 	}
 }
 
+// SetX11ForwardingConfig sets the server's X11 forwarding configuration
+func SetX11ForwardingConfig(xc *x11.ServerConfig) ServerOption {
+	return func(s *Server) error {
+		s.x11 = xc
+		return nil
+	}
+}
+
 // New returns an unstarted server
 func New(addr utils.NetAddr,
 	hostname string,
@@ -1422,6 +1434,8 @@ func (s *Server) dispatch(ch ssh.Channel, req *ssh.Request, ctx *srv.ServerConte
 		// subsystems are SSH subsystems defined in http://tools.ietf.org/html/rfc4254 6.6
 		// they are in essence SSH session extensions, allowing to implement new SSH commands
 		return s.handleSubsystem(ch, req, ctx)
+	case sshutils.X11ForwardRequest:
+		return s.handleX11Forward(ch, req, ctx)
 	case sshutils.AgentForwardRequest:
 		// This happens when SSH client has agent forwarding enabled, in this case
 		// client sends a special request, in return SSH server opens new channel
@@ -1491,6 +1505,69 @@ func (s *Server) handleAgentForwardProxy(req *ssh.Request, ctx *srv.ServerContex
 	return nil
 }
 
+// handleX11Forward handles an X11 forwarding request from the client.
+func (s *Server) handleX11Forward(ch ssh.Channel, req *ssh.Request, ctx *srv.ServerContext) (err error) {
+	event := &apievents.X11Forward{
+		Metadata: apievents.Metadata{
+			Type: events.X11ForwardEvent,
+			Code: events.X11ForwardCode,
+		},
+		UserMetadata: apievents.UserMetadata{
+			Login:        ctx.Identity.Login,
+			User:         ctx.Identity.TeleportUser,
+			Impersonator: ctx.Identity.Impersonator,
+		},
+		ConnectionMetadata: apievents.ConnectionMetadata{
+			LocalAddr:  ctx.ServerConn.LocalAddr().String(),
+			RemoteAddr: ctx.ServerConn.RemoteAddr().String(),
+		},
+		Status: apievents.Status{
+			Success: true,
+		},
+	}
+
+	defer func() {
+		if err != nil {
+			event.Metadata.Code = events.X11ForwardFailureCode
+			event.Status.Success = false
+			event.Status.Error = err.Error()
+		}
+		if trace.IsAccessDenied(err) {
+			// denied X11 requests are ok from a protocol perspective so we
+			// don't return them, just reply over ssh and emit the audit log.
+			s.replyError(ch, req, err)
+			err = nil
+		}
+		if err := s.EmitAuditEvent(s.ctx, event); err != nil {
+			log.WithError(err).Warn("Failed to emit x11-forward event.")
+		}
+	}()
+
+	// check if X11 forwarding is disabled, or if xauth can't be handled.
+	if !s.x11.Enabled || x11.CheckXAuthPath() != nil {
+		return trace.AccessDenied("X11 forwarding is not enabled")
+	}
+
+	// Check if the user's RBAC role allows X11 forwarding.
+	if err := s.authHandlers.CheckX11Forward(ctx); err != nil {
+		return trace.Wrap(err)
+	}
+
+	var x11Req x11.ForwardRequestPayload
+	if err := ssh.Unmarshal(req.Payload, &x11Req); err != nil {
+		return trace.Wrap(err)
+	}
+
+	if err := ctx.OpenXServerListener(x11Req, s.x11.DisplayOffset, s.x11.MaxDisplay); err != nil {
+		if trace.IsLimitExceeded(err) {
+			return trace.AccessDenied("The server cannot support any more X11 forwarding sessions at this time")
+		}
+		return trace.Wrap(err)
+	}
+
+	return nil
+}
+
 func (s *Server) handleSubsystem(ch ssh.Channel, req *ssh.Request, ctx *srv.ServerContext) error {
 	sb, err := s.parseSubsystemRequest(req, ctx)
 	if err != nil {

lib/srv/regular/sshserver_test.go

@@ -51,6 +51,7 @@ import (
 	sess "github.com/gravitational/teleport/lib/session"
 	"github.com/gravitational/teleport/lib/srv"
 	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/utils"
 
 	"github.com/google/uuid"
@@ -177,6 +178,7 @@ func newCustomFixture(t *testing.T, mutateCfg func(*auth.TestServerConfig), sshO
 		SetRestrictedSessionManager(&restricted.NOP{}),
 		SetClock(clock),
 		SetLockWatcher(newLockWatcher(ctx, t, nodeClient)),
+		SetX11ForwardingConfig(&x11.ServerConfig{}),
 	}
 
 	serverOptions = append(serverOptions, sshOpts...)
@@ -684,6 +686,143 @@ func TestAgentForward(t *testing.T) {
 	require.FailNow(t, "expected socket to be closed, still could dial after 150 ms")
 }
 
+// TestX11Forward tests x11 forwarding via unix sockets
+func TestX11Forward(t *testing.T) {
+	if os.Getenv("TELEPORT_XAUTH_TEST") == "" {
+		t.Skip("Skipping test as xauth is not enabled")
+	}
+
+	t.Parallel()
+	f := newFixture(t)
+	f.ssh.srv.x11 = &x11.ServerConfig{
+		Enabled:       true,
+		DisplayOffset: x11.DefaultDisplayOffset,
+		MaxDisplay:    x11.DefaultMaxDisplay,
+	}
+
+	ctx := context.Background()
+	roleName := services.RoleNameForUser(f.user)
+	role, err := f.testSrv.Auth().GetRole(ctx, roleName)
+	require.NoError(t, err)
+	roleOptions := role.GetOptions()
+	roleOptions.PermitX11Forwarding = types.NewBool(true)
+	role.SetOptions(roleOptions)
+	err = f.testSrv.Auth().UpsertRole(ctx, role)
+	require.NoError(t, err)
+
+	se, err := f.ssh.clt.NewSession()
+	require.NoError(t, err)
+	defer se.Close()
+
+	// Client requests x11 forwarding for the server session.
+	clientXAuthEntry, err := x11.NewFakeXAuthEntry(x11.Display{})
+	require.NoError(t, err)
+	err = x11.RequestForwarding(se, clientXAuthEntry)
+	require.NoError(t, err)
+
+	// Create a fake client XServer listener which echos
+	// back whatever it receives.
+	fakeClientDisplay, err := net.Listen("tcp", ":0")
+	require.NoError(t, err)
+	go func() {
+		for {
+			conn, err := fakeClientDisplay.Accept()
+			require.NoError(t, err)
+			_, err = io.Copy(conn, conn)
+			require.NoError(t, err)
+			conn.Close()
+		}
+	}()
+
+	// Handle any x11 channel requests received from the server
+	// and start x11 forwarding to the client display.
+	err = x11.ServeChannelRequests(ctx, f.ssh.clt, func(ctx context.Context, nch ssh.NewChannel) {
+		sch, sin, err := nch.Accept()
+		require.NoError(t, err)
+		defer sch.Close()
+
+		clientConn, err := net.Dial("tcp", fakeClientDisplay.Addr().String())
+		require.NoError(t, err)
+		clientXConn, ok := clientConn.(*net.TCPConn)
+		require.True(t, ok)
+		defer clientConn.Close()
+
+		go func() {
+			err := sshutils.ForwardRequests(ctx, sin, se)
+			if err != nil {
+				log.WithError(err).Debug("Failed to forward ssh request from server during X11 forwarding")
+			}
+		}()
+
+		err = x11.Forward(ctx, clientXConn, sch)
+		require.NoError(t, err)
+	})
+	require.NoError(t, err)
+
+	// prepare to send virtual "keyboard input" into the shell:
+	keyboard, err := se.StdinPipe()
+	require.NoError(t, err)
+
+	// start interactive SSH session with x11 forwarding enabled (new shell):
+	err = se.Shell()
+	require.NoError(t, err)
+
+	// create a temp file to collect the shell output into:
+	tmpFile, err := os.CreateTemp(os.TempDir(), "teleport-x11-forward-test")
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		os.Remove(tmpFile.Name())
+	})
+
+	// type 'printenv DISPLAY > /path/to/tmp/file' into the session (dumping the value of DISPLAY into the temp file)
+	_, err = keyboard.Write([]byte(fmt.Sprintf("printenv %v >> %s\n\r", x11.DisplayEnv, tmpFile.Name())))
+	require.NoError(t, err)
+
+	// wait for the output
+	var display string
+	require.Eventually(t, func() bool {
+		output, err := os.ReadFile(tmpFile.Name())
+		if err == nil && len(output) != 0 {
+			display = strings.TrimSpace(string(output))
+			return true
+		}
+		return false
+	}, 5*time.Second, 10*time.Millisecond, "failed to read display")
+
+	// Make a new connection to the XServer proxy, the client
+	// XServer should echo back anything written on it.
+	serverDisplay, err := x11.ParseDisplay(display)
+	require.NoError(t, err)
+	conn, err := serverDisplay.Dial()
+	require.NoError(t, err)
+	defer conn.Close()
+
+	msg := "msg"
+	_, err = conn.Write([]byte(msg))
+	require.NoError(t, err)
+	err = conn.CloseWrite()
+	require.NoError(t, err)
+
+	echo, err := io.ReadAll(conn)
+	require.NoError(t, err)
+	require.Equal(t, msg, string(echo))
+	conn.Close()
+
+	// Make a new second connection to the XServer proxy
+	conn2, err := serverDisplay.Dial()
+	require.NoError(t, err)
+	defer conn2.Close()
+
+	_, err = conn2.Write([]byte(msg))
+	require.NoError(t, err)
+	err = conn2.CloseWrite()
+	require.NoError(t, err)
+
+	echo, err = io.ReadAll(conn2)
+	require.NoError(t, err)
+	require.Equal(t, msg, string(echo))
+}
+
 func TestAllowedUsers(t *testing.T) {
 	t.Parallel()
 	f := newFixture(t)
@@ -1484,7 +1623,7 @@ func startX11EchoServer(ctx context.Context, t *testing.T, authSrv *auth.Server)
 		select {
 		case req = <-creqs:
 		case <-time.After(time.Second * 3):
-			return trace.LimitExceeded("Timeout waiting for x11 forwarding request")
+			return trace.LimitExceeded("Timeout waiting for X11 forwarding request")
 		case <-ctx.Done():
 			return nil
 		}
@@ -1607,7 +1746,7 @@ func TestX11ProxySupport(t *testing.T) {
 	defer requireNoErrors(t, doneGathering)
 
 	// The error gathering routine needs this context to expire or it will wait
-	// forever on the x11 server to exit. Hence we defer a call to the x11cancel
+	// forever on the X11 server to exit. Hence we defer a call to the x11cancel
 	// here rather than directly below the context creation
 	defer x11Cancel()
 

lib/sshutils/forward.go

@@ -0,0 +1,67 @@
+// 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 sshutils
+
+import (
+	"context"
+
+	"github.com/gravitational/trace"
+	"golang.org/x/crypto/ssh"
+)
+
+// RequestForwarder represents a resource capable of sending
+// an ssh request such as an ssh.Channel or ssh.Session.
+type RequestForwarder interface {
+	SendRequest(name string, wantReply bool, payload []byte) (bool, error)
+}
+
+// ForwardRequest is a helper for forwarding a request across a session or channel.
+func ForwardRequest(sender RequestForwarder, req *ssh.Request) (bool, error) {
+	reply, err := sender.SendRequest(req.Type, req.WantReply, req.Payload)
+	if err != nil || !req.WantReply {
+		return reply, trace.Wrap(err)
+	}
+	return reply, trace.Wrap(req.Reply(reply, nil))
+}
+
+// ForwardRequests forwards all ssh requests received from the
+// given channel until the channel or context is closed.
+func ForwardRequests(ctx context.Context, sin <-chan *ssh.Request, sender RequestForwarder) error {
+	for {
+		select {
+		case sreq, ok := <-sin:
+			if !ok {
+				// channel closed, stop processing
+				return nil
+			}
+			switch sreq.Type {
+			case WindowChangeRequest:
+				if _, err := ForwardRequest(sender, sreq); err != nil {
+					return trace.Wrap(err)
+				}
+			default:
+				if sreq.WantReply {
+					sreq.Reply(false, nil)
+				}
+				continue
+			}
+		case <-ctx.Done():
+			if ctx.Err() != context.Canceled {
+				return trace.Wrap(ctx.Err())
+			}
+			return nil
+		}
+	}
+}

lib/sshutils/x11/auth.go

@@ -0,0 +1,260 @@
+// 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 x11
+
+import (
+	"context"
+	"crypto/rand"
+	"encoding/binary"
+	"encoding/hex"
+	"fmt"
+	"io"
+	"os/exec"
+	"strings"
+	"time"
+
+	"github.com/gravitational/trace"
+)
+
+const (
+	// mitMagicCookieProto is the default xauth protocol used for X11 forwarding.
+	mitMagicCookieProto = "MIT-MAGIC-COOKIE-1"
+	// mitMagicCookieSize is the number of bytes in an mit magic cookie.
+	mitMagicCookieSize = 16
+)
+
+// XAuthEntry is an entry in an XAuthority database which can be used to authenticate
+// and authorize requests from an XServer to the associated X display.
+type XAuthEntry struct {
+	// Display is an X display in the format - [hostname]:[display_number].[screen_number]
+	Display Display `json:"display"`
+	// Proto is an XAuthority protocol, generally "MIT-MAGIC-COOKIE-1"
+	Proto string `json:"proto"`
+	// Cookie is a hex encoded XAuthority cookie
+	Cookie string `json:"cookie"`
+}
+
+// NewFakeXAuthEntry creates a fake xauth entry with a randomly generated MIT-MAGIC-COOKIE-1.
+func NewFakeXAuthEntry(display Display) (*XAuthEntry, error) {
+	cookie, err := newCookie(mitMagicCookieSize)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return &XAuthEntry{
+		Display: display,
+		Proto:   mitMagicCookieProto,
+		Cookie:  cookie,
+	}, nil
+}
+
+// SpoofXAuthEntry creates a new xauth entry with a random cookie with the
+// same length as the original entry's cookie. This is used to create a
+// believable spoof of the client's xauth data to send to the server.
+func (e *XAuthEntry) SpoofXAuthEntry() (*XAuthEntry, error) {
+	spoofedCookie, err := newCookie(hex.DecodedLen(len(e.Cookie)))
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return &XAuthEntry{
+		Display: e.Display,
+		Proto:   e.Proto,
+		Cookie:  spoofedCookie,
+	}, nil
+}
+
+// newCookie makes a random hex-encoded cookie with the given byte length.
+func newCookie(byteLength int) (string, error) {
+	cookieBytes := make([]byte, byteLength)
+	if _, err := rand.Read(cookieBytes); err != nil {
+		return "", trace.Wrap(err)
+	}
+	return hex.EncodeToString(cookieBytes), nil
+}
+
+// XAuthCommand is a os/exec.Cmd wrapper for running xauth commands.
+type XAuthCommand struct {
+	*exec.Cmd
+}
+
+// NewXAuthCommand reate a new "xauth" command. xauthFile can be
+// optionally provided to run the xauth command against a specific xauth file.
+func NewXAuthCommand(ctx context.Context, xauthFile string) *XAuthCommand {
+	var args []string
+	if xauthFile != "" {
+		args = []string{"-f", xauthFile}
+	}
+	return &XAuthCommand{exec.CommandContext(ctx, "xauth", args...)}
+}
+
+// ReadEntry runs "xauth list" to read the first xauth entry for the given display.
+func (x *XAuthCommand) ReadEntry(display Display) (*XAuthEntry, error) {
+	x.Cmd.Args = append(x.Cmd.Args, "list", display.String())
+	out, err := x.output()
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	if len(out) == 0 {
+		return nil, trace.NotFound("no xauth entry found")
+	}
+
+	// Ignore entries beyond the first listed.
+	entry := strings.Split(string(out), "\n")[0]
+
+	splitEntry := strings.Split(entry, "  ")
+	if len(splitEntry) != 3 {
+		return nil, trace.Errorf("invalid xAuthEntry, expected entry to have three parts")
+	}
+	proto, cookie := splitEntry[1], splitEntry[2]
+
+	return &XAuthEntry{
+		Display: display,
+		Proto:   proto,
+		Cookie:  cookie,
+	}, nil
+}
+
+// RemoveEntries runs "xauth remove" to remove any xauth entries for the given display.
+func (x *XAuthCommand) RemoveEntries(display Display) error {
+	x.Cmd.Args = append(x.Cmd.Args, "remove", display.String())
+	return trace.Wrap(x.run())
+}
+
+// AddEntry runs "xauth add" to add the given xauth entry.
+func (x *XAuthCommand) AddEntry(entry XAuthEntry) error {
+	x.Cmd.Args = append(x.Cmd.Args, "add", entry.Display.String(), entry.Proto, entry.Cookie)
+	return trace.Wrap(x.run())
+}
+
+// GenerateUntrustedCookie runs "xauth generate untrusted" to create a new xauth entry with
+// an untrusted MIT-MAGIC-COOKIE-1. A timeout can optionally be set for the xauth entry, after
+// which the XServer will ignore this cookie.
+func (x *XAuthCommand) GenerateUntrustedCookie(display Display, timeout time.Duration) error {
+	x.Cmd.Args = append(x.Cmd.Args, "generate", display.String(), mitMagicCookieProto, "untrusted")
+	x.Cmd.Args = append(x.Cmd.Args, "timeout", fmt.Sprint(timeout/time.Second))
+	return trace.Wrap(x.run())
+}
+
+// run Run and wrap error with stderr.
+func (x *XAuthCommand) run() error {
+	err := x.Cmd.Run()
+	if err != nil {
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			return trace.Wrap(err, "stderr: %q", exitErr.Stderr)
+		}
+	}
+	return trace.Wrap(err)
+}
+
+// run Output and wrap error with stderr.
+func (x *XAuthCommand) output() ([]byte, error) {
+	out, err := x.Cmd.Output()
+	if err != nil {
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			return nil, trace.Wrap(err, "stderr: %q", exitErr.Stderr)
+		}
+	}
+	return out, trace.Wrap(err)
+}
+
+// CheckXAuthPath checks if xauth is runnable in the current environment.
+func CheckXAuthPath() error {
+	_, err := exec.LookPath("xauth")
+	return trace.Wrap(err)
+}
+
+// ReadAndRewriteXAuthPacket reads the initial xauth packet from an XServer request. The xauth packet has 2 parts:
+//  1. fixed size buffer (12 bytes) - holds byteOrder bit, and the sizes of the protocol string and auth data
+//  2. variable size xauth packet - holds xauth protocol and data used to connect to the remote XServer.
+//
+// Then it compares the received auth packet with the auth proto and fake cookie
+// sent to the server with the original "x11-req". If the data matches, the auth
+// packet is returned with the fake cookie replaced by the real cookie to provide
+// access to the client's X display.
+func ReadAndRewriteXAuthPacket(xreq io.Reader, spoofedXAuthEntry, realXAuthEntry *XAuthEntry) ([]byte, error) {
+	if spoofedXAuthEntry.Proto != realXAuthEntry.Proto || len(spoofedXAuthEntry.Cookie) != len(realXAuthEntry.Cookie) {
+		return nil, trace.BadParameter("spoofed and real xauth entries must use the same xauth protocol")
+	}
+
+	// xauth packet starts with a fixed sized buffer of 12 bytes
+	// which is used to size and decode the remaining bytes
+	initBuf := make([]byte, xauthPacketInitBufSize)
+	if _, err := io.ReadFull(xreq, initBuf); err != nil {
+		return nil, trace.Wrap(err, "X11 channel initial packet buffer missing or too short")
+	}
+
+	protoLen, dataLen, err := readXauthPacketInitBuf(initBuf)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// authPacket size is equal to protoLen (rounded up by 4) + dataLen.
+	// In openssh, the rounding is performed with: (protoLen + 3) & ~3
+	authPacketSize := protoLen + (4-protoLen%4)%4 + dataLen
+	authPacket := make([]byte, authPacketSize)
+	if _, err := io.ReadFull(xreq, authPacket); err != nil {
+		return nil, trace.Wrap(err, "X11 channel auth packet missing or too short")
+	}
+
+	proto := authPacket[:protoLen]
+	authData := authPacket[len(authPacket)-dataLen:]
+	if string(proto) != spoofedXAuthEntry.Proto || hex.EncodeToString(authData) != spoofedXAuthEntry.Cookie {
+		return nil, trace.AccessDenied("X11 channel has the wrong authentication data")
+	}
+
+	// Replace auth data with the real auth data
+	realAuthData, err := hex.DecodeString(realXAuthEntry.Cookie)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	copy(authData, realAuthData)
+
+	return append(initBuf, authPacket...), trace.Wrap(err)
+}
+
+const (
+	// xauthPacketInitBufSize is the size of the initial
+	// fixed portion of an xauth packet
+	xauthPacketInitBufSize = 12
+	// little endian byte order
+	littleEndian = 'l'
+	// big endian byte order
+	bigEndian = 'B'
+)
+
+// readXauthPacketInitBuf reads the initial fixed size portion of
+// an xauth packet to get the length of the auth proto and auth data
+// portions of the xauth packet.
+func readXauthPacketInitBuf(initBuf []byte) (protoLen int, dataLen int, err error) {
+	// The first byte in the packet determines the
+	// byte order of the initial buffer's bytes.
+	var e binary.ByteOrder
+	binary.BigEndian.GoString()
+	switch initBuf[0] {
+	case bigEndian:
+		e = binary.BigEndian
+	case littleEndian:
+		e = binary.LittleEndian
+	default:
+		return 0, 0, trace.BadParameter("X11 channel auth packet has invalid byte order: %v", initBuf[0])
+	}
+
+	// bytes 6-7 and 8-9 are used to determine the length of
+	// the auth proto and auth data fields respectively.
+	protoLen = int(e.Uint16(initBuf[6:8]))
+	dataLen = int(e.Uint16(initBuf[8:10]))
+	return protoLen, dataLen, nil
+}

lib/sshutils/x11/auth_test.go

@@ -0,0 +1,157 @@
+// 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 x11
+
+import (
+	"bytes"
+	"context"
+	"encoding/binary"
+	"encoding/hex"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/gravitational/trace"
+	"github.com/stretchr/testify/require"
+)
+
+func TestXAuthCommands(t *testing.T) {
+	if os.Getenv("TELEPORT_XAUTH_TEST") == "" {
+		t.Skip("Skipping test as xauth is not enabled")
+	}
+
+	ctx := context.Background()
+
+	tmpDir := t.TempDir()
+	xauthFile := filepath.Join(tmpDir, ".Xauthority")
+	display, err := ParseDisplay("unix:10")
+	require.NoError(t, err)
+
+	// New xauth file should have no entries
+	xauth := NewXAuthCommand(ctx, xauthFile)
+	xauthEntry, err := xauth.ReadEntry(display)
+	require.Error(t, err)
+	require.True(t, trace.IsNotFound(err))
+	require.Nil(t, xauthEntry)
+
+	// Add trusted xauth entry
+	trustedXauthEntry, err := NewFakeXAuthEntry(display)
+	require.NoError(t, err)
+	xauth = NewXAuthCommand(ctx, xauthFile)
+	err = xauth.AddEntry(*trustedXauthEntry)
+	require.NoError(t, err)
+
+	// Read back the xauth entry
+	xauth = NewXAuthCommand(ctx, xauthFile)
+	xauthEntry, err = xauth.ReadEntry(display)
+	require.NoError(t, err)
+	require.Equal(t, trustedXauthEntry, xauthEntry)
+
+	// Remove xauth entries
+	xauth = NewXAuthCommand(ctx, xauthFile)
+	err = xauth.RemoveEntries(xauthEntry.Display)
+	require.NoError(t, err)
+
+	xauth = NewXAuthCommand(ctx, xauthFile)
+	xauthEntry, err = xauth.ReadEntry(display)
+	require.Error(t, err)
+	require.True(t, trace.IsNotFound(err))
+	require.Nil(t, xauthEntry)
+
+	// TODO(Joerger): Currently this test will only run/pass locally if you have $DISPLAY
+	// set. We need to add Xorg to the buildbox and start an XServer in order to run
+	// this test in CI. Additionally, Xorg and x11-apps can be used to add more tests
+	// and rework current tests which depend on fake display listeners instead.
+	localDisplay, err := GetXDisplay()
+	if trace.IsBadParameter(err) {
+		t.Skip("skipping xauth generate test, DISPLAY isn't set")
+	}
+	xauth = NewXAuthCommand(ctx, xauthFile)
+	err = xauth.GenerateUntrustedCookie(localDisplay, 0)
+	require.NoError(t, err)
+
+	xauth = NewXAuthCommand(ctx, xauthFile)
+	xauthEntry, err = xauth.ReadEntry(localDisplay)
+	require.NoError(t, err)
+	require.NotNil(t, xauthEntry)
+}
+
+func TestReadAndRewriteXAuthPacket(t *testing.T) {
+	t.Parallel()
+
+	realXAuthEntry, err := NewFakeXAuthEntry(Display{})
+	require.NoError(t, err)
+	realXAuthPacket := mockXAuthPacket(t, realXAuthEntry)
+
+	spoofedXAuthEntry, err := realXAuthEntry.SpoofXAuthEntry()
+	require.NoError(t, err)
+	spoofedXAuthPacket := mockXAuthPacket(t, spoofedXAuthEntry)
+
+	otherXAuthEntry, err := NewFakeXAuthEntry(Display{})
+	require.NoError(t, err)
+	otherXAuthPacket := mockXAuthPacket(t, otherXAuthEntry)
+
+	t.Run("match and replace xauth packet", func(t *testing.T) {
+		in := bytes.NewBuffer(spoofedXAuthPacket)
+		out, err := ReadAndRewriteXAuthPacket(in, spoofedXAuthEntry, realXAuthEntry)
+		require.NoError(t, err)
+		require.Equal(t, realXAuthPacket, out)
+	})
+
+	t.Run("xauth packet doesn't match", func(t *testing.T) {
+		in := bytes.NewBuffer(otherXAuthPacket)
+		out, err := ReadAndRewriteXAuthPacket(in, spoofedXAuthEntry, realXAuthEntry)
+		require.True(t, trace.IsAccessDenied(err))
+		require.Empty(t, out)
+	})
+
+	t.Run("xauth packet missing xauth data", func(t *testing.T) {
+		in := bytes.NewBuffer(mockXAuthPacketInitial(len(mitMagicCookieProto), mitMagicCookieSize))
+		out, err := ReadAndRewriteXAuthPacket(in, spoofedXAuthEntry, realXAuthEntry)
+		require.Error(t, err)
+		require.Empty(t, out)
+	})
+
+	t.Run("xauth packet empty", func(t *testing.T) {
+		out, err := ReadAndRewriteXAuthPacket(bytes.NewBuffer([]byte{}), spoofedXAuthEntry, realXAuthEntry)
+		require.Error(t, err)
+		require.Empty(t, out)
+	})
+}
+
+// mockXAuthPacket creates an xauth packet for the given xauth entry.
+func mockXAuthPacket(t *testing.T, entry *XAuthEntry) []byte {
+	authData, err := hex.DecodeString(entry.Cookie)
+	require.NoError(t, err)
+
+	var xauthPacket []byte
+	initPacket := mockXAuthPacketInitial(len(entry.Proto), len(authData))
+	xauthPacket = append(xauthPacket, initPacket...)
+	xauthPacket = append(xauthPacket, []byte(entry.Proto)...)
+	xauthPacket = append(xauthPacket, 0, 0)
+	xauthPacket = append(xauthPacket, authData...)
+	return xauthPacket
+}
+
+// mockXAuthPacketInitial creates the fixed size initial
+// portion of an xauth packet, with little endian encoding.
+func mockXAuthPacketInitial(authProtoLen, authDataLen int) []byte {
+	initData := make([]byte, 12)
+	initData[0] = 'l'
+	e := binary.LittleEndian
+	e.PutUint16(initData[6:8], uint16(authProtoLen))
+	e.PutUint16(initData[8:10], uint16(authDataLen))
+	return initData
+}

lib/sshutils/x11/conn.go

@@ -0,0 +1,90 @@
+// 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 x11
+
+import (
+	"errors"
+	"io"
+	"math"
+	"net"
+	"os"
+	"syscall"
+
+	"github.com/gravitational/trace"
+)
+
+// XServerConn is a connection to an XServer through a direct or forwarded connection.
+// It can either be a tcp conn, unix conn, or an X11 channel.
+type XServerConn interface {
+	io.ReadWriteCloser
+	// must implement CloseWrite to prevent read loop from halting pre-maturely
+	CloseWrite() error
+}
+
+// XServerListener is a proxy XServer listener used to forward XServer requests.
+// to an actual XServer. The underlying listener may be a unix or tcp listener.
+type XServerListener interface {
+	// Accept waits for and returns the next connection to the listener.
+	Accept() (XServerConn, error)
+
+	// Close closes the listener.
+	// Any blocked Accept operations will be unblocked and return errors.
+	Close() error
+
+	// Addr returns the listener's network address.
+	Addr() net.Addr
+}
+
+type xserverUnixListener struct {
+	*net.UnixListener
+}
+
+func (l *xserverUnixListener) Accept() (XServerConn, error) {
+	return l.AcceptUnix()
+}
+
+type xserverTCPListener struct {
+	*net.TCPListener
+}
+
+func (l *xserverTCPListener) Accept() (XServerConn, error) {
+	return l.AcceptTCP()
+}
+
+// OpenNewXServerListener opens an XServerListener for the first available Display.
+// displayOffset will determine what display number to start from when searching for
+// an open display unix socket, and maxDisplays in optional limit for the number of
+// display sockets which can be opened at once.
+func OpenNewXServerListener(displayOffset int, maxDisplay int, screen uint32) (XServerListener, Display, error) {
+	if displayOffset > maxDisplay {
+		return nil, Display{}, trace.BadParameter("displayOffset (%d) cannot be larger than maxDisplay (%d)", displayOffset, maxDisplay)
+	} else if maxDisplay > MaxDisplayNumber {
+		return nil, Display{}, trace.BadParameter("maxDisplay (%d) cannot be larger than the max int32 (%d)", maxDisplay, math.MaxInt32)
+	}
+
+	// Create /tmp/.X11-unix if it doesn't exist (such as in CI)
+	if err := os.Mkdir(x11SockDir(), 1777); err != nil && !errors.Is(err, os.ErrExist) {
+		return nil, Display{}, trace.Wrap(err)
+	}
+
+	for displayNumber := displayOffset; displayNumber <= maxDisplay; displayNumber++ {
+		display := Display{DisplayNumber: displayNumber}
+		if l, err := display.Listen(); err == nil {
+			return l, display, nil
+		} else if !errors.Is(err, syscall.EADDRINUSE) {
+			return nil, Display{}, trace.Wrap(err)
+		}
+	}
+	return nil, Display{}, trace.LimitExceeded("No more X11 sockets are available")
+}

lib/sshutils/x11/display.go

@@ -0,0 +1,204 @@
+// 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 x11
+
+import (
+	"fmt"
+	"math"
+	"net"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"unicode"
+
+	"github.com/gravitational/trace"
+)
+
+const (
+	// DefaultDisplayOffset is the default display offset when
+	// searching for an open XServer unix socket.
+	DefaultDisplayOffset = 10
+	// DefaultMaxDisplay is the default maximum display number
+	// supported when searching for an open XServer unix socket.
+	DefaultMaxDisplay = 1000
+	// MaxDisplay is the theoretical max display value which
+	// X Clients and serverwill be able to parse into a unix socket.
+	MaxDisplayNumber = math.MaxInt32
+	// DisplayEnv is an environment variable used to determine what
+	// local display should be connected to during X11 forwarding.
+	DisplayEnv = "DISPLAY"
+
+	// x11BasePort is the base port used for XServer tcp addresses.
+	// e.g. DISPLAY=localhost:10 -> net.Dial("tcp", "localhost:6010")
+	// Used by some XServer clients, such as openSSH and MobaXTerm.
+	x11BasePort = 6000
+	// x11SocketDirName is the name of the directory where X11 unix sockets are kept.
+	x11SocketDirName = ".X11-unix"
+)
+
+// Display is an XServer display.
+type Display struct {
+	// HostName is the the display's hostname. For tcp display sockets, this will be
+	// an ip address. For unix display sockets, this will be empty or "unix".
+	HostName string `json:"hostname"`
+	// DisplayNumber is a number representing an x display.
+	DisplayNumber int `json:"display_number"`
+	// ScreenNumber is a specific screen number of an x display.
+	ScreenNumber int `json:"screen_number"`
+}
+
+// String returns the string representation of the display.
+func (d *Display) String() string {
+	return fmt.Sprintf("%s:%d.%d", d.HostName, d.DisplayNumber, d.ScreenNumber)
+}
+
+// Dial opens an XServer connection to the display
+func (d *Display) Dial() (XServerConn, error) {
+	var conn XServerConn
+
+	unixSock, unixErr := d.unixSocket()
+	if unixErr == nil {
+		if conn, unixErr = net.DialUnix("unix", nil, unixSock); unixErr == nil {
+			return conn, nil
+		}
+	}
+
+	tcpSock, tcpErr := d.tcpSocket()
+	if tcpErr == nil {
+		if conn, tcpErr = net.DialTCP("tcp", nil, tcpSock); tcpErr == nil {
+			return conn, nil
+		}
+	}
+
+	return nil, trace.NewAggregate(unixErr, tcpErr)
+}
+
+// Listen opens an XServer listener
+func (d *Display) Listen() (XServerListener, error) {
+	unixSock, unixErr := d.unixSocket()
+	if unixErr == nil {
+		var l *net.UnixListener
+		if l, unixErr = net.ListenUnix("unix", unixSock); unixErr == nil {
+			return &xserverUnixListener{l}, nil
+		}
+	}
+
+	tcpSock, tcpErr := d.tcpSocket()
+	if tcpErr == nil {
+		var l *net.TCPListener
+		if l, tcpErr = net.ListenTCP("tcp", tcpSock); tcpErr == nil {
+			return &xserverTCPListener{l}, nil
+		}
+	}
+
+	return nil, trace.NewAggregate(unixErr, tcpErr)
+}
+
+// xserverUnixSocket returns the display's associated unix socket.
+func (d *Display) unixSocket() (*net.UnixAddr, error) {
+	// For x11 unix domain sockets, the hostname must be "unix" or empty. In these cases
+	// we return the actual unix socket for the display "/tmp/.X11-unix/X<display_number>"
+	if d.HostName == "unix" || d.HostName == "" {
+		sockName := filepath.Join(x11SockDir(), fmt.Sprintf("X%d", d.DisplayNumber))
+		return net.ResolveUnixAddr("unix", sockName)
+	}
+	return nil, trace.BadParameter("display is not a unix socket")
+}
+
+// xserverTCPSocket returns the display's associated tcp socket.
+// e.g. "hostname:<6000+display_number>"
+func (d *Display) tcpSocket() (*net.TCPAddr, error) {
+	if d.HostName == "" {
+		return nil, trace.BadParameter("hostname can't be empty for an XServer tcp socket")
+	}
+
+	port := fmt.Sprint(d.DisplayNumber + x11BasePort)
+	rawAddr := net.JoinHostPort(d.HostName, port)
+	addr, err := net.ResolveTCPAddr("tcp", rawAddr)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return addr, nil
+}
+
+// GetXDisplay retrieves and validates the local XServer display
+// set in the environment variable $DISPLAY.
+func GetXDisplay() (Display, error) {
+	displayString := os.Getenv(DisplayEnv)
+	if displayString == "" {
+		return Display{}, trace.BadParameter("$DISPLAY not set")
+	}
+
+	display, err := ParseDisplay(displayString)
+	if err != nil {
+		return Display{}, trace.Wrap(err)
+	}
+	return display, nil
+}
+
+// ParseDisplay parses the given display value and returns the host,
+// display number, and screen number, or a parsing error. display must be
+//in one of the following formats - hostname:d[.s], unix:d[.s], :d[.s], ::d[.s].
+func ParseDisplay(displayString string) (Display, error) {
+	if displayString == "" {
+		return Display{}, trace.BadParameter("display cannot be an empty string")
+	}
+
+	// check the display for illegal characters in case of code injection attempt
+	allowedSpecialChars := ":/.-_" // chars used for hostname or display delimiters.
+	for _, c := range displayString {
+		if !(unicode.IsLetter(c) || unicode.IsNumber(c) || strings.ContainsRune(allowedSpecialChars, c)) {
+			return Display{}, trace.BadParameter("display contains invalid character %q", c)
+		}
+	}
+
+	// Parse hostname.
+	colonIdx := strings.LastIndex(displayString, ":")
+	if colonIdx == -1 || len(displayString) == colonIdx+1 {
+		return Display{}, trace.BadParameter("display value is missing display number")
+	}
+
+	var display Display
+	if displayString[0] == ':' {
+		display.HostName = ""
+	} else {
+		display.HostName = displayString[:colonIdx]
+	}
+
+	// Parse display number and screen number
+	splitDot := strings.Split(displayString[colonIdx+1:], ".")
+	displayNumber, err := strconv.ParseUint(splitDot[0], 10, 0)
+	if err != nil {
+		return Display{}, trace.Wrap(err)
+	}
+
+	display.DisplayNumber = int(displayNumber)
+	if len(splitDot) < 2 {
+		return display, nil
+	}
+
+	screenNumber, err := strconv.ParseUint(splitDot[1], 10, 0)
+	if err != nil {
+		return Display{}, trace.Wrap(err)
+	}
+
+	display.ScreenNumber = int(screenNumber)
+	return display, nil
+}
+
+func x11SockDir() string {
+	return filepath.Join(os.TempDir(), x11SocketDirName)
+}

lib/sshutils/x11/display_test.go

@@ -0,0 +1,172 @@
+// 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 x11
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseDisplay(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		desc          string
+		displayString string
+		expectDisplay Display
+		assertErr     require.ErrorAssertionFunc
+		validSocket   string
+	}{
+		{
+			desc:          "unix socket",
+			displayString: ":10",
+			expectDisplay: Display{DisplayNumber: 10},
+			assertErr:     require.NoError,
+			validSocket:   "unix",
+		}, {
+			desc:          "unix socket",
+			displayString: "::10",
+			expectDisplay: Display{DisplayNumber: 10},
+			assertErr:     require.NoError,
+			validSocket:   "unix",
+		}, {
+			desc:          "unix socket",
+			displayString: "unix:10",
+			expectDisplay: Display{HostName: "unix", DisplayNumber: 10},
+			assertErr:     require.NoError,
+			validSocket:   "unix",
+		}, {
+			desc:          "unix socket with screen number",
+			displayString: "unix:10.1",
+			expectDisplay: Display{HostName: "unix", DisplayNumber: 10, ScreenNumber: 1},
+			assertErr:     require.NoError,
+			validSocket:   "unix",
+		}, {
+			desc:          "localhost",
+			displayString: "localhost:10",
+			expectDisplay: Display{HostName: "localhost", DisplayNumber: 10},
+			assertErr:     require.NoError,
+			validSocket:   "tcp",
+		}, {
+			desc:          "some hostname",
+			displayString: "example.com:10",
+			expectDisplay: Display{HostName: "example.com", DisplayNumber: 10},
+			assertErr:     require.NoError,
+			validSocket:   "tcp",
+		}, {
+			desc:          "some ip address",
+			displayString: "1.2.3.4:10",
+			expectDisplay: Display{HostName: "1.2.3.4", DisplayNumber: 10},
+			assertErr:     require.NoError,
+			validSocket:   "tcp",
+		}, {
+			desc:          "empty",
+			displayString: "",
+			expectDisplay: Display{},
+			assertErr:     require.Error,
+		}, {
+			desc:          "no display number",
+			displayString: ":",
+			expectDisplay: Display{},
+			assertErr:     require.Error,
+		}, {
+			desc:          "negative display number",
+			displayString: ":-10",
+			expectDisplay: Display{},
+			assertErr:     require.Error,
+		}, {
+			desc:          "negative screen number",
+			displayString: ":10.-1",
+			expectDisplay: Display{},
+			assertErr:     require.Error,
+		}, {
+			desc:          "invalid characters",
+			displayString: "$(exec ls)",
+			expectDisplay: Display{},
+			assertErr:     require.Error,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			display, err := ParseDisplay(tc.displayString)
+			tc.assertErr(t, err)
+			require.Equal(t, tc.expectDisplay, display)
+
+			switch tc.validSocket {
+			case "unix":
+				_, err := display.unixSocket()
+				require.NoError(t, err)
+			case "tcp":
+				_, err := display.tcpSocket()
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestDisplaySocket(t *testing.T) {
+	testCases := []struct {
+		desc           string
+		display        Display
+		expectUnixAddr string
+		expectTCPAddr  string
+	}{
+		{
+			desc:           "unix socket no hostname",
+			display:        Display{DisplayNumber: 10},
+			expectUnixAddr: "/tmp/.X11-unix/X10",
+		}, {
+			desc:           "unix socket with hostname",
+			display:        Display{HostName: "unix", DisplayNumber: 10},
+			expectUnixAddr: "/tmp/.X11-unix/X10",
+		}, {
+			desc:          "localhost",
+			display:       Display{HostName: "localhost", DisplayNumber: 10},
+			expectTCPAddr: "127.0.0.1:6010",
+		}, {
+			desc:          "some ip address",
+			display:       Display{HostName: "1.2.3.4", DisplayNumber: 10},
+			expectTCPAddr: "1.2.3.4:6010",
+		}, {
+			desc:    "invalid ip address",
+			display: Display{HostName: "1.2.3.4.5", DisplayNumber: 10},
+		}, {
+			desc:    "invalid unix socket",
+			display: Display{HostName: "/tmp/socket", DisplayNumber: 10},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			unixSock, err := tc.display.unixSocket()
+			if tc.expectUnixAddr == "" {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.expectUnixAddr, unixSock.String())
+			}
+
+			tcpSock, err := tc.display.tcpSocket()
+			if tc.expectTCPAddr == "" {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.expectTCPAddr, tcpSock.String())
+			}
+		})
+	}
+}

lib/sshutils/x11/forward.go

@@ -0,0 +1,147 @@
+// 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 X11 contains contains the ssh client/server helper functions
+// for performing X11 forwarding.
+package x11
+
+import (
+	"context"
+	"io"
+	"sync"
+
+	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/trace"
+	"golang.org/x/crypto/ssh"
+)
+
+// forwardIO forwards io between two XServer connections until
+// one of the connections is closed. If the ctx is closed early,
+// the function will return, but forwarding will continue until
+// the XServer connnections are closed.
+func Forward(ctx context.Context, client, server XServerConn) error {
+	errs := make(chan error)
+	var wg sync.WaitGroup
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		_, err := io.Copy(client, server)
+		errs <- trace.Wrap(err)
+		// Send other goroutine an EOF
+		err = client.CloseWrite()
+		errs <- trace.Wrap(err)
+	}()
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		_, err := io.Copy(server, client)
+		errs <- trace.Wrap(err)
+		// Send other goroutine an EOF
+		err = server.CloseWrite()
+		errs <- trace.Wrap(err)
+	}()
+
+	go func() {
+		wg.Wait()
+		close(errs)
+	}()
+
+	return trace.NewAggregateFromChannel(errs, ctx)
+}
+
+// ForwardRequestPayload according to http://www.ietf.org/rfc/rfc4254.txt
+type ForwardRequestPayload struct {
+	// SingleConnection determines whether any connections will be forwarded
+	// after the first connection, or after the session is closed. In OpenSSH
+	// and Teleport SSH clients, SingleConnection is always set to false.
+	SingleConnection bool
+	// AuthProtocol is the name of the X11 authentication protocol being used.
+	AuthProtocol string
+	// AuthCookie is a hexadecimal encoded X11 authentication cookie. This should
+	// be a fake, random cookie, which will be checked and replaced by the real
+	// cookie once the connection request is received.
+	AuthCookie string
+	// ScreenNumber determines which screen will be.
+	ScreenNumber uint32
+}
+
+// RequestForwarding sends an "x11-req" to the server to set up X11 forwarding for the given session.
+// authProto and authCookie are required to set up authentication with the Server. screenNumber is used
+// by the server to determine which screen should be connected to for X11 forwarding. singleConnection is
+// an optional argument to request X11 forwarding for a single connection.
+func RequestForwarding(sess *ssh.Session, xauthEntry *XAuthEntry) error {
+	payload := ForwardRequestPayload{
+		AuthProtocol: xauthEntry.Proto,
+		AuthCookie:   xauthEntry.Cookie,
+		ScreenNumber: uint32(xauthEntry.Display.ScreenNumber),
+	}
+
+	ok, err := sess.SendRequest(sshutils.X11ForwardRequest, true, ssh.Marshal(payload))
+	if err != nil {
+		return trace.Wrap(err)
+	} else if !ok {
+		return trace.Errorf("X11 forward request failed")
+	}
+
+	return nil
+}
+
+// ChannelRequestPayload according to http://www.ietf.org/rfc/rfc4254.txt
+type ChannelRequestPayload struct {
+	// OriginatorAddress is the address of the server requesting an X11 channel
+	OriginatorAddress string
+	// OriginatorPort is the port of the server requesting an X11 channel
+	OriginatorPort uint32
+}
+
+type x11ChannelHandler func(ctx context.Context, nch ssh.NewChannel)
+
+// ServeChannelRequests opens an X11 channel handler and starts a
+// goroutine to serve any channels received with the handler provided.
+func ServeChannelRequests(ctx context.Context, clt *ssh.Client, handler x11ChannelHandler) error {
+	channels := clt.HandleChannelOpen(sshutils.X11ChannelRequest)
+	if channels == nil {
+		return trace.AlreadyExists("X11 forwarding channel already open")
+	}
+
+	go func() {
+		ctx, cancel := context.WithCancel(ctx)
+		defer cancel()
+		for {
+			select {
+			case nch := <-channels:
+				if nch == nil {
+					return
+				}
+				go handler(ctx, nch)
+			case <-ctx.Done():
+				return
+			}
+		}
+	}()
+	return nil
+}
+
+// ServerConfig is a server configuration for X11 forwarding
+type ServerConfig struct {
+	// Enabled controls whether X11 forwarding requests can be granted by the server.
+	Enabled bool `yaml:"enabled"`
+	// DisplayOffset tells the server what X11 display number to start from when
+	// searching for an open X11 unix socket for XServer proxies.
+	DisplayOffset int `yaml:"display_offset,omitempty"`
+	// MaxDisplay tells the server what X11 display number to stop at when
+	// searching for an open X11 unix socket for XServer proxies.
+	MaxDisplay int `yaml:"max_displays,omitempty"`
+}

lib/sshutils/x11/forward_test.go

@@ -0,0 +1,110 @@
+// 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 x11
+
+import (
+	"context"
+	"io"
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestForward(t *testing.T) {
+	ctx := context.Background()
+
+	// Create a fake client display. In practice, the display
+	// set in $DISPLAY is used to connect to the client display.
+	fakeClientDisplay, err := net.Listen("tcp", ":0")
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, fakeClientDisplay.Close())
+	})
+
+	// Handle connections to the client XServer
+	echoMsg := "msg"
+	go func() {
+		for {
+			localConn, err := fakeClientDisplay.Accept()
+			if err != nil {
+				// listener is closed, test is done.
+				return
+			}
+
+			go func() {
+				defer localConn.Close()
+
+				// read request and expect what was written to server
+				bytes, err := io.ReadAll(localConn)
+				require.NoError(t, err)
+				require.Equal(t, echoMsg, string(bytes))
+			}()
+		}
+	}()
+
+	// Create a fake XServer proxy just like the one in sshserver.
+	sl, serverDisplay, err := OpenNewXServerListener(DefaultDisplayOffset, DefaultMaxDisplay, 0)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, sl.Close())
+	})
+
+	// Handle connection to XServer proxy
+	go func() {
+		for {
+			serverConn, err := sl.Accept()
+			if err != nil {
+				// listener is closed, test is done.
+				return
+			}
+
+			go func() {
+				defer serverConn.Close()
+
+				clientConn, err := net.Dial("tcp", fakeClientDisplay.Addr().String())
+				if err != nil {
+					// fakeClientDisplay is closed, test is done.
+					return
+				}
+
+				clientXConn, ok := clientConn.(*net.TCPConn)
+				require.True(t, ok)
+				defer clientConn.Close()
+
+				err = Forward(ctx, clientXConn, serverConn)
+				require.NoError(t, err)
+			}()
+		}
+	}()
+
+	// Create a fake XServer request to the XServer proxy
+	xreq, err := serverDisplay.Dial()
+	require.NoError(t, err)
+	_, err = xreq.Write([]byte(echoMsg))
+	require.NoError(t, err)
+
+	// Create a second request simultaneously
+	xreq2, err := serverDisplay.Dial()
+	require.NoError(t, err)
+	_, err = xreq2.Write([]byte(echoMsg))
+	require.NoError(t, err)
+	xreq2.Close()
+
+	// Close XServer requests, forwarding should stop as soon as
+	// the open connection has been read and forwarded fully.
+	xreq.Close()
+	xreq2.Close()
+}

tool/tsh/options.go

@@ -18,7 +18,9 @@ package main
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
+	"time"
 
 	"github.com/gravitational/teleport/lib/client"
 	"github.com/gravitational/teleport/lib/utils"
@@ -26,122 +28,94 @@ import (
 	"github.com/gravitational/trace"
 )
 
-const (
-	forwardAgentTextYes   = "yes"
-	forwardAgentTextNo    = "no"
-	forwardAgentTextLocal = "local"
-)
-
-// AllOptions is a listing of all known OpenSSH options.
-var AllOptions = map[string]map[string]bool{
-	"AddKeysToAgent":                  map[string]bool{"yes": true},
-	"AddressFamily":                   map[string]bool{},
-	"BatchMode":                       map[string]bool{},
-	"BindAddress":                     map[string]bool{},
-	"CanonicalDomains":                map[string]bool{},
-	"CanonicalizeFallbackLocal":       map[string]bool{},
-	"CanonicalizeHostname":            map[string]bool{},
-	"CanonicalizeMaxDots":             map[string]bool{},
-	"CanonicalizePermittedCNAMEs":     map[string]bool{},
-	"CertificateFile":                 map[string]bool{},
-	"ChallengeResponseAuthentication": map[string]bool{},
-	"CheckHostIP":                     map[string]bool{},
-	"Cipher":                          map[string]bool{},
-	"Ciphers":                         map[string]bool{},
-	"ClearAllForwardings":             map[string]bool{},
-	"Compression":                     map[string]bool{},
-	"CompressionLevel":                map[string]bool{},
-	"ConnectionAttempts":              map[string]bool{},
-	"ConnectTimeout":                  map[string]bool{},
-	"ControlMaster":                   map[string]bool{},
-	"ControlPath":                     map[string]bool{},
-	"ControlPersist":                  map[string]bool{},
-	"DynamicForward":                  map[string]bool{},
-	"EscapeChar":                      map[string]bool{},
-	"ExitOnForwardFailure":            map[string]bool{},
-	"FingerprintHash":                 map[string]bool{},
-	"ForwardAgent": map[string]bool{
-		forwardAgentTextYes:   true,
-		forwardAgentTextNo:    true,
-		forwardAgentTextLocal: true,
-	},
-	"ForwardX11":                       map[string]bool{},
-	"ForwardX11Timeout":                map[string]bool{},
-	"ForwardX11Trusted":                map[string]bool{},
-	"GatewayPorts":                     map[string]bool{},
-	"GlobalKnownHostsFile":             map[string]bool{},
-	"GSSAPIAuthentication":             map[string]bool{},
-	"GSSAPIDelegateCredentials":        map[string]bool{},
-	"HashKnownHosts":                   map[string]bool{},
-	"Host":                             map[string]bool{},
-	"HostbasedAuthentication":          map[string]bool{},
-	"HostbasedKeyTypes":                map[string]bool{},
-	"HostKeyAlgorithms":                map[string]bool{},
-	"HostKeyAlias":                     map[string]bool{},
-	"HostName":                         map[string]bool{},
-	"IdentityFile":                     map[string]bool{},
-	"IdentitiesOnly":                   map[string]bool{},
-	"IPQoS":                            map[string]bool{},
-	"KbdInteractiveAuthentication":     map[string]bool{},
-	"KbdInteractiveDevices":            map[string]bool{},
-	"KexAlgorithms":                    map[string]bool{},
-	"LocalCommand":                     map[string]bool{},
-	"LocalForward":                     map[string]bool{},
-	"LogLevel":                         map[string]bool{},
-	"MACs":                             map[string]bool{},
-	"Match":                            map[string]bool{},
-	"NoHostAuthenticationForLocalhost": map[string]bool{},
-	"NumberOfPasswordPrompts":          map[string]bool{},
-	"PasswordAuthentication":           map[string]bool{},
-	"PermitLocalCommand":               map[string]bool{},
-	"PKCS11Provider":                   map[string]bool{},
-	"Port":                             map[string]bool{},
-	"PreferredAuthentications":         map[string]bool{},
-	"Protocol":                         map[string]bool{},
-	"ProxyCommand":                     map[string]bool{},
-	"ProxyUseFdpass":                   map[string]bool{},
-	"PubkeyAcceptedKeyTypes":           map[string]bool{},
-	"PubkeyAuthentication":             map[string]bool{},
-	"RekeyLimit":                       map[string]bool{},
-	"RemoteForward":                    map[string]bool{},
-	"RequestTTY":                       map[string]bool{"yes": true, "no": true},
-	"RhostsRSAAuthentication":          map[string]bool{},
-	"RSAAuthentication":                map[string]bool{},
-	"SendEnv":                          map[string]bool{},
-	"ServerAliveInterval":              map[string]bool{},
-	"ServerAliveCountMax":              map[string]bool{},
-	"StreamLocalBindMask":              map[string]bool{},
-	"StreamLocalBindUnlink":            map[string]bool{},
-	"StrictHostKeyChecking":            map[string]bool{"yes": true, "no": true},
-	"TCPKeepAlive":                     map[string]bool{},
-	"Tunnel":                           map[string]bool{},
-	"TunnelDevice":                     map[string]bool{},
-	"UpdateHostKeys":                   map[string]bool{},
-	"UsePrivilegedPort":                map[string]bool{},
-	"User":                             map[string]bool{},
-	"UserKnownHostsFile":               map[string]bool{},
-	"VerifyHostKeyDNS":                 map[string]bool{},
-	"VisualHostKey":                    map[string]bool{},
-	"XAuthLocation":                    map[string]bool{},
-}
-
-func asAgentForwardingMode(s string) client.AgentForwardingMode {
-	switch strings.ToLower(s) {
-	case forwardAgentTextNo:
-		return client.ForwardAgentNo
-
-	case forwardAgentTextYes:
-		return client.ForwardAgentYes
-
-	case forwardAgentTextLocal:
-		return client.ForwardAgentLocal
-
-	default:
-		log.Errorf(
-			"Invalid agent forwarding mode %q. Defaulting to %q.",
-			s, forwardAgentTextNo)
-		return client.ForwardAgentNo
-	}
+// supportedOptions is a listing of all known OpenSSH options
+// and a parser/setter if the option is supported by tsh.
+var supportedOptions = map[string]setOption{
+	"AddKeysToAgent":                   setAddKeysToAgentOption,
+	"AddressFamily":                    nil,
+	"BatchMode":                        nil,
+	"BindAddress":                      nil,
+	"CanonicalDomains":                 nil,
+	"CanonicalizeFallbackLocal":        nil,
+	"CanonicalizeHostname":             nil,
+	"CanonicalizeMaxDots":              nil,
+	"CanonicalizePermittedCNAMEs":      nil,
+	"CertificateFile":                  nil,
+	"ChallengeResponseAuthentication":  nil,
+	"CheckHostIP":                      nil,
+	"Cipher":                           nil,
+	"Ciphers":                          nil,
+	"ClearAllForwardings":              nil,
+	"Compression":                      nil,
+	"CompressionLevel":                 nil,
+	"ConnectionAttempts":               nil,
+	"ConnectTimeout":                   nil,
+	"ControlMaster":                    nil,
+	"ControlPath":                      nil,
+	"ControlPersist":                   nil,
+	"DynamicForward":                   nil,
+	"EscapeChar":                       nil,
+	"ExitOnForwardFailure":             nil,
+	"FingerprintHash":                  nil,
+	"ForwardAgent":                     setAgentForwardingModeOption,
+	"ForwardX11":                       setForwardX11Option,
+	"ForwardX11Timeout":                setForwardX11TimeoutOption,
+	"ForwardX11Trusted":                setForwardX11TrustedOption,
+	"GatewayPorts":                     nil,
+	"GlobalKnownHostsFile":             nil,
+	"GSSAPIAuthentication":             nil,
+	"GSSAPIDelegateCredentials":        nil,
+	"HashKnownHosts":                   nil,
+	"Host":                             nil,
+	"HostbasedAuthentication":          nil,
+	"HostbasedKeyTypes":                nil,
+	"HostKeyAlgorithms":                nil,
+	"HostKeyAlias":                     nil,
+	"HostName":                         nil,
+	"IdentityFile":                     nil,
+	"IdentitiesOnly":                   nil,
+	"IPQoS":                            nil,
+	"KbdInteractiveAuthentication":     nil,
+	"KbdInteractiveDevices":            nil,
+	"KexAlgorithms":                    nil,
+	"LocalCommand":                     nil,
+	"LocalForward":                     nil,
+	"LogLevel":                         nil,
+	"MACs":                             nil,
+	"Match":                            nil,
+	"NoHostAuthenticationForLocalhost": nil,
+	"NumberOfPasswordPrompts":          nil,
+	"PasswordAuthentication":           nil,
+	"PermitLocalCommand":               nil,
+	"PKCS11Provider":                   nil,
+	"Port":                             nil,
+	"PreferredAuthentications":         nil,
+	"Protocol":                         nil,
+	"ProxyCommand":                     nil,
+	"ProxyUseFdpass":                   nil,
+	"PubkeyAcceptedKeyTypes":           nil,
+	"PubkeyAuthentication":             nil,
+	"RekeyLimit":                       nil,
+	"RemoteForward":                    nil,
+	"RequestTTY":                       setRequestTTYOption,
+	"RhostsRSAAuthentication":          nil,
+	"RSAAuthentication":                nil,
+	"SendEnv":                          nil,
+	"ServerAliveInterval":              nil,
+	"ServerAliveCountMax":              nil,
+	"StreamLocalBindMask":              nil,
+	"StreamLocalBindUnlink":            nil,
+	"StrictHostKeyChecking":            setStrictHostKeyCheckingOption,
+	"TCPKeepAlive":                     nil,
+	"Tunnel":                           nil,
+	"TunnelDevice":                     nil,
+	"UpdateHostKeys":                   nil,
+	"UsePrivilegedPort":                nil,
+	"User":                             nil,
+	"UserKnownHostsFile":               nil,
+	"VerifyHostKeyDNS":                 nil,
+	"VisualHostKey":                    nil,
+	"XAuthLocation":                    nil,
 }
 
 // Options holds parsed values of OpenSSH options.
@@ -163,15 +137,128 @@ type Options struct {
 	// keys to the ~/.tsh/known_hosts file. Supported option values are "yes"
 	// and "no".
 	StrictHostKeyChecking bool
+
+	// ForwardX11 specifies whether X11 forwarding should be enabled for
+	// ssh sessions started by the client. Supported option values are "yes".
+	//
+	// When this option is to true, ForwardX11Trusted will default to true.
+	ForwardX11 bool
+
+	// ForwardX11Trusted determines what trust mode should be used for X11Forwarding.
+	// Supported option values are "yes" and "no"
+	//
+	// When set to yes, X11 forwarding will always be in trusted mode if requested.
+	// When set to no, X11 forwarding will default to untrusted mode, unless used with
+	// the -Y flag
+	ForwardX11Trusted *bool
+
+	// ForwardX11Timeout specifies a timeout in seconds after which X11 forwarding
+	// attempts will be rejected when in untrusted forwarding mode.
+	ForwardX11Timeout time.Duration
 }
 
-func parseOptions(opts []string) (Options, error) {
-	// By default, Teleport prefers strict host key checking and adding keys
-	// to system SSH agent.
-	options := Options{
+type setOption func(*Options, string) error
+
+func setAddKeysToAgentOption(o *Options, val string) error {
+	parsedValue, err := parseBoolTrueOption(val)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	o.AddKeysToAgent = parsedValue
+	return nil
+}
+
+func setAgentForwardingModeOption(o *Options, val string) error {
+	switch strings.ToLower(val) {
+	case "no":
+		o.ForwardAgent = client.ForwardAgentNo
+	case "yes":
+		o.ForwardAgent = client.ForwardAgentYes
+	case "local":
+		o.ForwardAgent = client.ForwardAgentLocal
+	default:
+		return trace.BadParameter("invalid agent forwarding mode: %q, supported are: yes, no, local", val)
+	}
+	return nil
+}
+
+func setForwardX11Option(o *Options, val string) error {
+	parsedValue, err := parseBoolTrueOption(val)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	o.ForwardX11 = parsedValue
+	if o.ForwardX11Trusted == nil {
+		trusted := true
+		o.ForwardX11Trusted = &trusted
+	}
+	return nil
+}
+
+func setForwardX11TimeoutOption(o *Options, val string) error {
+	// ForwardX11Timeout should be a non-negative integer
+	// representing a duration in seconds.
+	timeoutSeconds, err := strconv.ParseUint(val, 10, 0)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
+	o.ForwardX11Timeout = time.Duration(timeoutSeconds) * time.Second
+	return nil
+}
+
+func setForwardX11TrustedOption(o *Options, val string) error {
+	parsedValue, err := parseBoolOption(val)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	o.ForwardX11Trusted = &parsedValue
+	return nil
+}
+
+func setRequestTTYOption(o *Options, val string) error {
+	parsedValue, err := parseBoolOption(val)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	o.RequestTTY = parsedValue
+	return nil
+}
+
+func setStrictHostKeyCheckingOption(o *Options, val string) error {
+	parsedValue, err := parseBoolOption(val)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	o.StrictHostKeyChecking = parsedValue
+	return nil
+}
+
+func parseBoolOption(val string) (bool, error) {
+	if val != "yes" && val != "no" {
+		return false, trace.BadParameter("invalid bool option value: %s", val)
+	}
+	return utils.AsBool(val), nil
+}
+
+func parseBoolTrueOption(val string) (bool, error) {
+	if val != "yes" {
+		return false, trace.BadParameter("invalid true-only bool option value: %s", val)
+	}
+	return utils.AsBool(val), nil
+}
+
+func defaultOptions() Options {
+	return Options{
+		// By default, Teleport prefers strict host key checking and adding keys
+		// to system SSH agent.
 		StrictHostKeyChecking: true,
 		AddKeysToAgent:        true,
 	}
+}
+
+func parseOptions(opts []string) (Options, error) {
+	options := defaultOptions()
 
 	for _, o := range opts {
 		key, value, err := splitOption(o)
@@ -179,30 +266,18 @@ func parseOptions(opts []string) (Options, error) {
 			return Options{}, trace.Wrap(err)
 		}
 
-		supportedValues, ok := AllOptions[key]
+		setOption, ok := supportedOptions[key]
 		if !ok {
 			return Options{}, trace.BadParameter("unsupported option key: %v", key)
 		}
 
-		if len(supportedValues) == 0 {
+		if setOption == nil {
 			fmt.Printf("WARNING: Option '%v' is not supported.\n", key)
 			continue
 		}
 
-		_, ok = supportedValues[value]
-		if !ok {
-			return Options{}, trace.BadParameter("unsupported option value: %v", value)
-		}
-
-		switch key {
-		case "AddKeysToAgent":
-			options.AddKeysToAgent = utils.AsBool(value)
-		case "ForwardAgent":
-			options.ForwardAgent = asAgentForwardingMode(value)
-		case "RequestTTY":
-			options.RequestTTY = utils.AsBool(value)
-		case "StrictHostKeyChecking":
-			options.StrictHostKeyChecking = utils.AsBool(value)
+		if err := setOption(&options, value); err != nil {
+			return Options{}, trace.BadParameter("unsupported option value %q: %s", value, err)
 		}
 	}
 
@@ -221,10 +296,8 @@ func splitOption(option string) (string, string, error) {
 
 // fieldsFunc splits key-value pairs off ' ' and '='.
 func fieldsFunc(c rune) bool {
-	switch {
-	case c == ' ':
-		return true
-	case c == '=':
+	switch c {
+	case ' ', '=':
 		return true
 	default:
 		return false

tool/tsh/options_test.go

@@ -0,0 +1,194 @@
+/*
+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 main
+
+import (
+	"testing"
+	"time"
+
+	"github.com/gravitational/teleport/lib/client"
+	"github.com/stretchr/testify/require"
+)
+
+func TestOptions(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		desc          string
+		inOptions     []string
+		assertError   require.ErrorAssertionFunc
+		assertOptions func(t *testing.T, opts Options)
+	}{
+		// Default options
+		{
+			desc:        "default options",
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, defaultOptions(), opts)
+			},
+		},
+		// AddKeysToAgent Tests and Generic option-parsing tests
+		{
+			desc:        "Space Delimited",
+			inOptions:   []string{"AddKeysToAgent yes"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, true, opts.AddKeysToAgent)
+			},
+		},
+		{
+			desc:        "Equals Sign Delimited",
+			inOptions:   []string{"AddKeysToAgent=yes"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, true, opts.AddKeysToAgent)
+			},
+		},
+		{
+			desc:        "Unsupported key",
+			inOptions:   []string{"foo foo"},
+			assertError: require.Error,
+		},
+		{
+			desc:        "Unsupported option gets skipped",
+			inOptions:   []string{"AddressFamily val"},
+			assertError: require.NoError,
+		},
+		{
+			desc:        "Incomplete option",
+			inOptions:   []string{"AddKeysToAgent"},
+			assertError: require.Error,
+		},
+		{
+			desc:        "AddKeysToAgent Invalid Value",
+			inOptions:   []string{"AddKeysToAgent foo"},
+			assertError: require.Error,
+		},
+		// ForwardAgent Tests
+		{
+			desc:        "Forward Agent Yes",
+			inOptions:   []string{"ForwardAgent yes"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, client.ForwardAgentYes, opts.ForwardAgent)
+			},
+		},
+		{
+			desc:        "Forward Agent No",
+			inOptions:   []string{"ForwardAgent no"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, client.ForwardAgentNo, opts.ForwardAgent)
+			},
+		},
+		{
+			desc:        "Forward Agent Local",
+			inOptions:   []string{"ForwardAgent local"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, client.ForwardAgentLocal, opts.ForwardAgent)
+			},
+		},
+		{
+			desc:        "Forward Agent InvalidValue",
+			inOptions:   []string{"ForwardAgent potato"},
+			assertError: require.Error,
+		},
+		// ForwardX11 tests
+		{
+			desc:        "Forward X11",
+			inOptions:   []string{"ForwardX11 yes"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, true, opts.ForwardX11)
+				require.Equal(t, true, *opts.ForwardX11Trusted)
+			},
+		},
+		{
+			desc:        "Forward X11 InvalidValue",
+			inOptions:   []string{"ForwardX11 potato"},
+			assertError: require.Error,
+		},
+		// ForwardX11Trusted tests
+		{
+			desc:        "Forward X11 Trusted yes",
+			inOptions:   []string{"ForwardX11Trusted yes"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, true, *opts.ForwardX11Trusted)
+			},
+		},
+		{
+			desc:        "Forward X11 Trusted no",
+			inOptions:   []string{"ForwardX11Trusted no"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, false, *opts.ForwardX11Trusted)
+			},
+		},
+		{
+			desc:        "Forward X11 Trusted with Forward X11",
+			inOptions:   []string{"ForwardX11 yes", "ForwardX11Trusted no"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, true, opts.ForwardX11)
+				require.Equal(t, false, *opts.ForwardX11Trusted)
+			},
+		},
+		{
+			desc:        "Forward X11 Trusted InvalidValue",
+			inOptions:   []string{"ForwardX11Trusted potato"},
+			assertError: require.Error,
+		},
+		// ForwardX11Trusted tests
+		{
+			desc:        "Forward X11 Timeout",
+			inOptions:   []string{"ForwardX11Timeout 10"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, time.Second*10, opts.ForwardX11Timeout)
+			},
+		},
+		{
+			desc:        "Forward X11 Timeout 0",
+			inOptions:   []string{"ForwardX11Timeout 0"},
+			assertError: require.NoError,
+			assertOptions: func(t *testing.T, opts Options) {
+				require.Equal(t, time.Duration(0), opts.ForwardX11Timeout)
+			},
+		},
+		{
+			desc:        "Forward X11 Timeout negative",
+			inOptions:   []string{"ForwardX11Timeout -1"},
+			assertError: require.Error,
+		},
+		{
+			desc:        "Forward X11 Timeout devimal",
+			inOptions:   []string{"ForwardX11Timeout 1.5"},
+			assertError: require.Error,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			options, err := parseOptions(tt.inOptions)
+			tt.assertError(t, err)
+			if tt.assertOptions != nil {
+				tt.assertOptions(t, options)
+			}
+		})
+	}
+}

tool/tsh/tsh.go

@@ -56,6 +56,7 @@ import (
 	"github.com/gravitational/teleport/lib/session"
 	"github.com/gravitational/teleport/lib/sshutils"
 	"github.com/gravitational/teleport/lib/sshutils/scp"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
 
@@ -211,6 +212,15 @@ type CLIConf struct {
 	// will block instead. Useful when port forwarding. Equivalent of -N for OpenSSH.
 	NoRemoteExec bool
 
+	// X11ForwardingUntrusted will set up untrusted X11 forwarding for the session ('ssh -X')
+	X11ForwardingUntrusted bool
+
+	// X11Forwarding will set up trusted X11 forwarding for the session ('ssh -Y')
+	X11ForwardingTrusted bool
+
+	// X11ForwardingTimeout can optionally set to set a timeout for untrusted X11 forwarding.
+	X11ForwardingTimeout time.Duration
+
 	// Debug sends debug logs to stdout.
 	Debug bool
 
@@ -384,6 +394,9 @@ func Run(args []string, opts ...cliOption) error {
 	ssh.Flag("cluster", clusterHelp).StringVar(&cf.SiteName)
 	ssh.Flag("option", "OpenSSH options in the format used in the configuration file").Short('o').AllowDuplicate().StringsVar(&cf.Options)
 	ssh.Flag("no-remote-exec", "Don't execute remote command, useful for port forwarding").Short('N').BoolVar(&cf.NoRemoteExec)
+	ssh.Flag("x11-untrusted", "Requests untrusted (secure) X11 forwarding for this session").Short('X').BoolVar(&cf.X11ForwardingUntrusted)
+	ssh.Flag("x11-trusted", "Requests trusted (insecure) X11 forwarding for this session. This can make your local displays vulnerable to attacks, use with caution").Short('Y').BoolVar(&cf.X11ForwardingTrusted)
+	ssh.Flag("x11-untrusted-timeout", "Sets a timeout for untrusted X11 forwarding, after which the client will reject any forwarding requests from the server").Default("10m").DurationVar((&cf.X11ForwardingTimeout))
 
 	// AWS.
 	aws := app.Command("aws", "Access AWS API.")
@@ -1978,6 +1991,10 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro
 		c.ForwardAgent = client.ForwardAgentYes
 	}
 
+	if err := setX11Config(c, cf, options, os.Getenv); err != nil {
+		log.WithError(err).Info("X11 forwarding is not properly configured, continuing without it.")
+	}
+
 	// If the caller does not want to check host keys, pass in a insecure host
 	// key checker.
 	if !options.StrictHostKeyChecking {
@@ -2039,6 +2056,30 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro
 	return tc, nil
 }
 
+// setX11Config sets X11 config using CLI and SSH option flags.
+func setX11Config(c *client.Config, cf *CLIConf, o Options, fn envGetter) error {
+	// X11 forwarding can be enabled with -X, -Y, or -oForwardX11=yes
+	c.EnableX11Forwarding = cf.X11ForwardingUntrusted || cf.X11ForwardingTrusted || o.ForwardX11
+
+	if c.EnableX11Forwarding && fn(x11.DisplayEnv) == "" {
+		c.EnableX11Forwarding = false
+		return trace.BadParameter("$DISPLAY must be set for X11 forwarding")
+	}
+
+	c.X11ForwardingTrusted = cf.X11ForwardingTrusted
+	if o.ForwardX11Trusted != nil && *o.ForwardX11Trusted {
+		c.X11ForwardingTrusted = true
+	}
+
+	// Set X11 forwarding timeout, prioritizing the SSH option if set.
+	c.X11ForwardingTimeout = o.ForwardX11Timeout
+	if c.X11ForwardingTimeout == 0 {
+		c.X11ForwardingTimeout = cf.X11ForwardingTimeout
+	}
+
+	return nil
+}
+
 // defaultWebProxyPorts is the order of default proxy ports to try, in order that
 // they will be tried.
 var defaultWebProxyPorts = []int{

tool/tsh/tsh_test.go

@@ -46,6 +46,7 @@ import (
 	"github.com/gravitational/teleport/lib/service"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/srv"
+	"github.com/gravitational/teleport/lib/sshutils/x11"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
 )
@@ -659,111 +660,6 @@ func TestIdentityRead(t *testing.T) {
 	require.Greater(t, len(conf.RootCAs.Subjects()), 0)
 }
 
-func TestOptions(t *testing.T) {
-	t.Parallel()
-	tests := []struct {
-		desc        string
-		inOptions   []string
-		assertError require.ErrorAssertionFunc
-		outOptions  Options
-	}{
-		// Generic option-parsing tests
-		{
-			desc:        "Space Delimited",
-			inOptions:   []string{"AddKeysToAgent yes"},
-			assertError: require.NoError,
-			outOptions: Options{
-				AddKeysToAgent:        true,
-				ForwardAgent:          client.ForwardAgentNo,
-				RequestTTY:            false,
-				StrictHostKeyChecking: true,
-			},
-		},
-		{
-			desc:        "Equals Sign Delimited",
-			inOptions:   []string{"AddKeysToAgent=yes"},
-			assertError: require.NoError,
-			outOptions: Options{
-				AddKeysToAgent:        true,
-				ForwardAgent:          client.ForwardAgentNo,
-				RequestTTY:            false,
-				StrictHostKeyChecking: true,
-			},
-		},
-		{
-			desc:        "Invalid key",
-			inOptions:   []string{"foo foo"},
-			assertError: require.Error,
-			outOptions:  Options{},
-		},
-		{
-			desc:        "Incomplete option",
-			inOptions:   []string{"AddKeysToAgent"},
-			assertError: require.Error,
-			outOptions:  Options{},
-		},
-		// AddKeysToAgent Tests
-		{
-			desc:        "AddKeysToAgent Invalid Value",
-			inOptions:   []string{"AddKeysToAgent foo"},
-			assertError: require.Error,
-			outOptions:  Options{},
-		},
-		// ForwardAgent Tests
-		{
-			desc:        "Forward Agent Yes",
-			inOptions:   []string{"ForwardAgent yes"},
-			assertError: require.NoError,
-			outOptions: Options{
-				AddKeysToAgent:        true,
-				ForwardAgent:          client.ForwardAgentYes,
-				RequestTTY:            false,
-				StrictHostKeyChecking: true,
-			},
-		},
-		{
-			desc:        "Forward Agent No",
-			inOptions:   []string{"ForwardAgent no"},
-			assertError: require.NoError,
-			outOptions: Options{
-				AddKeysToAgent:        true,
-				ForwardAgent:          client.ForwardAgentNo,
-				RequestTTY:            false,
-				StrictHostKeyChecking: true,
-			},
-		},
-		{
-			desc:        "Forward Agent Local",
-			inOptions:   []string{"ForwardAgent local"},
-			assertError: require.NoError,
-			outOptions: Options{
-				AddKeysToAgent:        true,
-				ForwardAgent:          client.ForwardAgentLocal,
-				RequestTTY:            false,
-				StrictHostKeyChecking: true,
-			},
-		},
-		{
-			desc:        "Forward Agent InvalidValue",
-			inOptions:   []string{"ForwardAgent potato"},
-			assertError: require.Error,
-			outOptions:  Options{},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.desc, func(t *testing.T) {
-			options, err := parseOptions(tt.inOptions)
-			tt.assertError(t, err)
-
-			require.Equal(t, tt.outOptions.AddKeysToAgent, options.AddKeysToAgent)
-			require.Equal(t, tt.outOptions.ForwardAgent, options.ForwardAgent)
-			require.Equal(t, tt.outOptions.RequestTTY, options.RequestTTY)
-			require.Equal(t, tt.outOptions.StrictHostKeyChecking, options.StrictHostKeyChecking)
-		})
-	}
-}
-
 func TestFormatConnectCommand(t *testing.T) {
 	tests := []struct {
 		clusterFlag string
@@ -1143,6 +1039,205 @@ func TestMakeTableWithTruncatedColumn(t *testing.T) {
 	}
 }
 
+func TestSetX11Config(t *testing.T) {
+	t.Parallel()
+
+	envMapGetter := func(envMap map[string]string) envGetter {
+		return func(s string) string {
+			return envMap[s]
+		}
+	}
+
+	for _, tc := range []struct {
+		desc         string
+		cf           CLIConf
+		opts         []string
+		envMap       map[string]string
+		assertError  require.ErrorAssertionFunc
+		expectConfig client.Config
+	}{
+		// Test Teleport flag usage
+		{
+			desc: "-X",
+			cf: CLIConf{
+				X11ForwardingUntrusted: true,
+			},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: false,
+			},
+		}, {
+			desc: "-Y",
+			cf: CLIConf{
+				X11ForwardingTrusted: true,
+			},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc: "--x11-untrustedTimeout=1m",
+			cf: CLIConf{
+				X11ForwardingTimeout: time.Minute,
+			},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				X11ForwardingTimeout: time.Minute,
+			},
+		}, {
+			desc: "$DISPLAY not set",
+			cf: CLIConf{
+				X11ForwardingUntrusted: true,
+			},
+			assertError: require.Error,
+			expectConfig: client.Config{
+				EnableX11Forwarding: false,
+			},
+		},
+		// Test OpenSSH flag usage
+		{
+			desc:        "-oForwardX11=yes",
+			opts:        []string{"ForwardX11=yes"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc:        "-oForwardX11Trusted=yes",
+			opts:        []string{"ForwardX11Trusted=yes"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc:        "-oForwardX11Trusted=yes",
+			opts:        []string{"ForwardX11Trusted=no"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				X11ForwardingTrusted: false,
+			},
+		}, {
+			desc:        "-oForwardX11=yes with -oForwardX11Trusted=yes",
+			opts:        []string{"ForwardX11=yes", "ForwardX11Trusted=yes"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc:        "-oForwardX11=yes with -oForwardX11Trusted=no",
+			opts:        []string{"ForwardX11=yes", "ForwardX11Trusted=no"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: false,
+			},
+		}, {
+			desc:        "-oForwardX11Timeout=60",
+			opts:        []string{"ForwardX11Timeout=60"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				X11ForwardingTimeout: time.Minute,
+			},
+		},
+		// Test Combined usage - options generally take priority
+		{
+			desc: "-X with -oForwardX11=yes",
+			cf: CLIConf{
+				X11ForwardingUntrusted: true,
+			},
+			opts:        []string{"ForwardX11=yes"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc: "-X with -oForwardX11Trusted=yes",
+			cf: CLIConf{
+				X11ForwardingUntrusted: true,
+			},
+			opts:        []string{"ForwardX11Trusted=yes"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc: "-X with -oForwardX11Trusted=no",
+			cf: CLIConf{
+				X11ForwardingUntrusted: true,
+			},
+			opts:        []string{"ForwardX11Trusted=no"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: false,
+			},
+		}, {
+			desc: "-Y with -oForwardX11Trusted=yes",
+			cf: CLIConf{
+				X11ForwardingTrusted: true,
+			},
+			opts:        []string{"ForwardX11Trusted=yes"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc: "-Y with -oForwardX11Trusted=no",
+			cf: CLIConf{
+				X11ForwardingTrusted: true,
+			},
+			opts:        []string{"ForwardX11Trusted=no"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				EnableX11Forwarding:  true,
+				X11ForwardingTrusted: true,
+			},
+		}, {
+			desc: "--x11-untrustedTimeout=1m with -oForwardX11Timeout=120",
+			cf: CLIConf{
+				X11ForwardingTimeout: time.Minute,
+			},
+			opts:        []string{"ForwardX11Timeout=120"},
+			envMap:      map[string]string{x11.DisplayEnv: ":0"},
+			assertError: require.NoError,
+			expectConfig: client.Config{
+				X11ForwardingTimeout: time.Minute * 2,
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			opts, err := parseOptions(tc.opts)
+			require.NoError(t, err)
+
+			clt := client.Config{}
+			err = setX11Config(&clt, &tc.cf, opts, envMapGetter(tc.envMap))
+			tc.assertError(t, err)
+			require.Equal(t, tc.expectConfig, clt)
+		})
+	}
+}
+
 type testServersOpts struct {
 	bootstrap      []types.Resource
 	authConfigFunc func(cfg *service.AuthConfig)