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

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

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,

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

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.