diff --git a/.cloudbuild/scripts/cmd/unit-tests/main.go b/.cloudbuild/scripts/cmd/unit-tests/main.go index 76d8cb09686..5e3d8b40255 100644 --- a/.cloudbuild/scripts/cmd/unit-tests/main.go +++ b/.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 diff --git a/build.assets/Makefile b/build.assets/Makefile index 383fdf446b7..cc9e8b7b6c1 100644 --- a/build.assets/Makefile +++ b/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; \ diff --git a/lib/client/api.go b/lib/client/api.go index 5fe0e7dd9c6..dadf89ae0e3 100644 --- a/lib/client/api.go +++ b/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() diff --git a/lib/client/session.go b/lib/client/session.go index a80a9b1e041..65f3ddf557f 100644 --- a/lib/client/session.go +++ b/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, " ")) diff --git a/lib/client/x11_session.go b/lib/client/x11_session.go new file mode 100644 index 00000000000..9987d5fd61e --- /dev/null +++ b/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) +} diff --git a/lib/config/configuration.go b/lib/config/configuration.go index 592049047b1..6983f014100 100644 --- a/lib/config/configuration.go +++ b/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 } diff --git a/lib/config/configuration_test.go b/lib/config/configuration_test.go index 662e246f2e6..ab50dc121b9 100644 --- a/lib/config/configuration_test.go +++ b/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) + }) } } diff --git a/lib/config/fileconf.go b/lib/config/fileconf.go index 40696d2d049..eb4285f5ce5 100644 --- a/lib/config/fileconf.go +++ b/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". diff --git a/lib/config/fileconf_test.go b/lib/config/fileconf_test.go index b678e77fa00..db3f68a10cc 100644 --- a/lib/config/fileconf_test.go +++ b/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) + } } diff --git a/lib/service/cfg.go b/lib/service/cfg.go index b1870b5a152..6aa0159ae65 100644 --- a/lib/service/cfg.go +++ b/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 diff --git a/lib/service/service.go b/lib/service/service.go index a0e94efcf04..004847f232f 100644 --- a/lib/service/service.go +++ b/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) diff --git a/lib/srv/authhandlers.go b/lib/srv/authhandlers.go index 3f6940361f9..4ecce9c9403 100644 --- a/lib/srv/authhandlers.go +++ b/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 { diff --git a/lib/srv/ctx.go b/lib/srv/ctx.go index d70caf04411..4239bce24d0 100644 --- a/lib/srv/ctx.go +++ b/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 } diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index def9e0990b9..2abc52aec42 100644 --- a/lib/srv/forward/sshserver.go +++ b/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 { diff --git a/lib/srv/reexec.go b/lib/srv/reexec.go index b21fc86f61b..f69467c6e12 100644 --- a/lib/srv/reexec.go +++ b/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, }, } diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 4247c60e6b4..21f65bfebfd 100644 --- a/lib/srv/regular/sshserver.go +++ b/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 { diff --git a/lib/srv/regular/sshserver_test.go b/lib/srv/regular/sshserver_test.go index ad3d2b08a88..e1feea6ff81 100644 --- a/lib/srv/regular/sshserver_test.go +++ b/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() diff --git a/lib/sshutils/forward.go b/lib/sshutils/forward.go new file mode 100644 index 00000000000..e22ed973721 --- /dev/null +++ b/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 + } + } +} diff --git a/lib/sshutils/x11/auth.go b/lib/sshutils/x11/auth.go new file mode 100644 index 00000000000..7be6ef0647d --- /dev/null +++ b/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 +} diff --git a/lib/sshutils/x11/auth_test.go b/lib/sshutils/x11/auth_test.go new file mode 100644 index 00000000000..8fda6b468e4 --- /dev/null +++ b/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 +} diff --git a/lib/sshutils/x11/conn.go b/lib/sshutils/x11/conn.go new file mode 100644 index 00000000000..2a9190a14b8 --- /dev/null +++ b/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") +} diff --git a/lib/sshutils/x11/display.go b/lib/sshutils/x11/display.go new file mode 100644 index 00000000000..46cfe1723e3 --- /dev/null +++ b/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" + 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) +} diff --git a/lib/sshutils/x11/display_test.go b/lib/sshutils/x11/display_test.go new file mode 100644 index 00000000000..3b744829455 --- /dev/null +++ b/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()) + } + }) + } +} diff --git a/lib/sshutils/x11/forward.go b/lib/sshutils/x11/forward.go new file mode 100644 index 00000000000..7f0b3d833d7 --- /dev/null +++ b/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"` +} diff --git a/lib/sshutils/x11/forward_test.go b/lib/sshutils/x11/forward_test.go new file mode 100644 index 00000000000..ef477fe6ef4 --- /dev/null +++ b/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() +} diff --git a/tool/tsh/options.go b/tool/tsh/options.go index b772a88e33b..3b13b0c4ca3 100644 --- a/tool/tsh/options.go +++ b/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 diff --git a/tool/tsh/options_test.go b/tool/tsh/options_test.go new file mode 100644 index 00000000000..aac0fec776b --- /dev/null +++ b/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) + } + }) + } +} diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index d0204519204..e5899eac3c1 100644 --- a/tool/tsh/tsh.go +++ b/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{ diff --git a/tool/tsh/tsh_test.go b/tool/tsh/tsh_test.go index 71c52b3e3c3..83e247c97a7 100644 --- a/tool/tsh/tsh_test.go +++ b/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)