diff --git a/lib/srv/reexec.go b/lib/srv/reexec.go index 9da373b139159..35fa3c73ba82c 100644 --- a/lib/srv/reexec.go +++ b/lib/srv/reexec.go @@ -359,7 +359,10 @@ func RunCommand() (errw io.Writer, code int, err error) { // xauthority files is put into the correct place ($HOME/.Xauthority) // with the right permissions. removeCmd := x11.NewXAuthCommand(context.Background(), "") - removeCmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} + removeCmd.SysProcAttr = &syscall.SysProcAttr{ + Setsid: true, + Credential: cmd.SysProcAttr.Credential, + } removeCmd.Env = cmd.Env removeCmd.Dir = cmd.Dir if err := removeCmd.RemoveEntries(c.X11Config.XAuthEntry.Display); err != nil { @@ -367,7 +370,10 @@ func RunCommand() (errw io.Writer, code int, err error) { } addCmd := x11.NewXAuthCommand(context.Background(), "") - addCmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} + addCmd.SysProcAttr = &syscall.SysProcAttr{ + Setsid: true, + Credential: cmd.SysProcAttr.Credential, + } addCmd.Env = cmd.Env addCmd.Dir = cmd.Dir if err := addCmd.AddEntry(c.X11Config.XAuthEntry); err != nil { diff --git a/lib/srv/regular/sshserver_test.go b/lib/srv/regular/sshserver_test.go index 8d716720c7a78..1cfebbf64e514 100644 --- a/lib/srv/regular/sshserver_test.go +++ b/lib/srv/regular/sshserver_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "io/fs" "net" "net/http" "net/http/httptest" @@ -30,6 +31,7 @@ import ( "strconv" "strings" "sync" + "syscall" "testing" "time" @@ -131,6 +133,35 @@ func newFixtureWithoutDiskBasedLogging(t *testing.T) *sshTestFixture { return f } +func (f *sshTestFixture) newSSHClient(ctx context.Context, t *testing.T, user *user.User) *tracessh.Client { + // set up SSH client using the user private key for signing + up, err := newUpack(f.testSrv, user.Username, []string{user.Username}, wildcardAllow) + require.NoError(t, err) + + // set up an agent server and a client that uses agent for forwarding + keyring := agent.NewKeyring() + addedKey := agent.AddedKey{ + PrivateKey: up.pkey, + Certificate: up.pcert, + } + require.NoError(t, keyring.Add(addedKey)) + + cltConfig := &ssh.ClientConfig{ + User: user.Username, + Auth: []ssh.AuthMethod{ssh.PublicKeys(up.certSigner)}, + HostKeyCallback: ssh.FixedHostKey(f.signer.PublicKey()), + } + + client, err := tracessh.Dial(ctx, "tcp", f.ssh.srvAddress, cltConfig) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, client.Close()) + }) + + return client +} + func newCustomFixture(t *testing.T, mutateCfg func(*auth.TestServerConfig), sshOpts ...ServerOption) *sshTestFixture { ctx := context.Background() @@ -852,12 +883,12 @@ func TestX11Forward(t *testing.T) { err = f.testSrv.Auth().UpsertRole(ctx, role) require.NoError(t, err) - // Open two x11 sessions, the server should handle multiple - // concurrent X11 sessions. + // Open two x11 sessions from two separate clients to + // ensure concurrent X11 forwarding sessions are supported. serverDisplay := x11EchoSession(ctx, t, f.ssh.clt) - - client2, err := tracessh.Dial(ctx, "tcp", f.ssh.srv.Addr(), f.ssh.cltConfig) + user, err := user.Current() require.NoError(t, err) + client2 := f.newSSHClient(ctx, t, user) serverDisplay2 := x11EchoSession(ctx, t, client2) // Create multiple XServer requests, the server should @@ -886,6 +917,57 @@ func TestX11Forward(t *testing.T) { wg.Wait() } +// TestRootX11ForwardPermissions tests that X11 forwarding sessions are set up +// with the connecting user's file permissions (where needed), rather than root. +func TestRootX11ForwardPermissions(t *testing.T) { + requireRoot(t) + if os.Getenv("TELEPORT_XAUTH_TEST") == "" { + t.Skip("Skipping test as xauth is not enabled") + } + + t.Parallel() + f := newFixtureWithoutDiskBasedLogging(t) + f.ssh.srv.x11 = &x11.ServerConfig{ + Enabled: true, + DisplayOffset: x11.DefaultDisplayOffset, + MaxDisplay: x11.DefaultMaxDisplays, + } + + 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) + + // Create a new X11 session as a non-root nonroot in the system. + nonroot, err := user.LookupId("1000") + require.NoError(t, err) + client := f.newSSHClient(ctx, t, nonroot) + serverDisplay := x11EchoSession(ctx, t, client) + + // Check that the xauth entry is readable for the connecting user. + xauthCmd := x11.NewXAuthCommand(ctx, "") + uid, err := strconv.ParseUint(nonroot.Uid, 10, 32) + require.NoError(t, err) + gid, err := strconv.ParseUint(nonroot.Gid, 10, 32) + require.NoError(t, err) + xauthCmd.SysProcAttr = &syscall.SysProcAttr{ + Setsid: true, + Credential: &syscall.Credential{ + Uid: uint32(uid), + Gid: uint32(gid), + }, + } + xauthCmd.Dir = nonroot.HomeDir + xauthCmd.Env = []string{fmt.Sprintf("HOME=%v", nonroot.HomeDir)} + _, err = xauthCmd.ReadEntry(serverDisplay) + require.NoError(t, err) +} + // x11EchoSession creates a new ssh session and handles x11 forwarding for the session, // echoing XServer requests received back to the client. Returns the Display opened on the // session, which is set in $DISPLAY. @@ -953,6 +1035,10 @@ func x11EchoSession(ctx context.Context, t *testing.T, clt *tracessh.Client) x11 // create a temp file to collect the shell output into: tmpFile, err := os.CreateTemp(os.TempDir(), "teleport-x11-forward-test") require.NoError(t, err) + + // Allow non-root user to write to the temp file + err = tmpFile.Chmod(fs.FileMode(0777)) + require.NoError(t, err) t.Cleanup(func() { os.Remove(tmpFile.Name()) }) @@ -2371,6 +2457,13 @@ func newCertAuthorityWatcher(ctx context.Context, t *testing.T, client types.Eve return caWatcher } +func requireRoot(t *testing.T) { + t.Helper() + if os.Geteuid() != 0 { + t.Skip("This test will be skipped because tests are not being run as root.") + } +} + // maxPipeSize is one larger than the maximum pipe size for most operating // systems which appears to be 65536 bytes. //