From 0a0f33b303d9664dd6b4d930fdaf7538f64843a8 Mon Sep 17 00:00:00 2001 From: Andrew LeFevre Date: Wed, 22 Apr 2026 14:49:35 -0400 Subject: [PATCH] unset XAUTHORITY env var for the networking command (#65713) * unset XAUTHORITY env var for the networking command When Teleport passes almost all the environment variables to the child processes that are born of a reexec. This is an issue when XAUTHORITY is set as it will be passed to the networking process, causing `xauth` to use a different file than the one that the SSH session process will use. This is because the SSH session process only inherits a small subset of the environment variables from the parent process, and XAUTHORITY is not one of them. This causes X11 forwarding to fail. * add Remove method to SafeEnv --- integration/integration_test.go | 6 +++++- lib/srv/exec.go | 14 ++++++++++---- session/envutils/environment.go | 9 +++++++++ session/networking/x11/auth.go | 4 ++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index 7475a0fb090..4849acb41ec 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -1018,7 +1018,6 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { term.Type("exit\n\r") waitSessionTermination(t, errCh, require.NoError) }) - }) } } @@ -4636,6 +4635,11 @@ func testX11Forwarding(t *testing.T, suite *integrationTestSuite) { t.Skip("Skipping TestX11Forwarding, no external SSH binary found.") } + // Set XAUTHORITY to a non-existent path to ensure that XAUTHORITY + // is unset for the networking command, if it isn't xauth will attempt + // to read /does/not/exist and fail, causing X11 forwarding to fail. + t.Setenv(x11.XAuthFileEnvVar, "/does/not/exist") + // Create a fake client XServer listener. clientXServer, clientDisplay, err := x11.OpenNewXServerListener(x11.DefaultDisplayOffset, x11.DefaultMaxDisplays, 0) require.NoError(t, err) diff --git a/lib/srv/exec.go b/lib/srv/exec.go index 2614d622d6b..ed7e45bf928 100644 --- a/lib/srv/exec.go +++ b/lib/srv/exec.go @@ -44,6 +44,7 @@ import ( "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/sshutils/reexec" "github.com/gravitational/teleport/session/envutils" + "github.com/gravitational/teleport/session/networking/x11" sessionreexec "github.com/gravitational/teleport/session/reexec" "github.com/gravitational/teleport/session/reexec/reexecconstants" ) @@ -701,11 +702,20 @@ func ConfigureCommand(ctx *ServerContext, extraFiles ...*os.File) (*exec.Cmd, er return nil, trace.Wrap(err) } + // Build env for `teleport exec`. + env := &envutils.SafeEnv{} + env.AddExecEnvironment() + // The channel/request type determines the subcommand to execute. var subCommand string switch ctx.ExecType { case reexecconstants.NetworkingSubCommand: subCommand = reexecconstants.NetworkingSubCommand + + // Unset XAUTHORITY for the networking command as the SSH session + // process given to the user will not have it set which can cause + // issues with the X11 forwarding. + env.Remove(x11.XAuthFileEnvVar) default: subCommand = reexecconstants.ExecSubCommand } @@ -714,10 +724,6 @@ func ConfigureCommand(ctx *ServerContext, extraFiles ...*os.File) (*exec.Cmd, er // is appended if Teleport is running in debug mode. args := []string{executable, subCommand} - // build env for `teleport exec` - env := &envutils.SafeEnv{} - env.AddExecEnvironment() - // Build the "teleport exec" command. cmd := &exec.Cmd{ Path: executable, diff --git a/session/envutils/environment.go b/session/envutils/environment.go index c4945467de6..01ce00257ed 100644 --- a/session/envutils/environment.go +++ b/session/envutils/environment.go @@ -25,6 +25,7 @@ import ( "io" "log/slog" "os" + "slices" "strings" "github.com/gravitational/trace" @@ -174,3 +175,11 @@ func (e *SafeEnv) isUnsafeKey(preventDuplicates bool, key string) bool { func (e *SafeEnv) AddExecEnvironment() { e.addFull(true, os.Environ()) } + +// Remove deletes the key and value from the environment if it exists. +func (e *SafeEnv) Remove(key string) { + keyPrefix := key + "=" + *e = slices.DeleteFunc(*e, func(s string) bool { + return strings.HasPrefix(s, keyPrefix) + }) +} diff --git a/session/networking/x11/auth.go b/session/networking/x11/auth.go index 974d1558365..6618ff3dc97 100644 --- a/session/networking/x11/auth.go +++ b/session/networking/x11/auth.go @@ -33,6 +33,10 @@ import ( ) const ( + // XAuthFileEnvVar is the environment variable used to specify the path to the + // X11 authorization file. + XAuthFileEnvVar = "XAUTHORITY" + // 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.