From e68b8b971ceab23a7bb8475e61d5c6137d786282 Mon Sep 17 00:00:00 2001 From: Samir Aguiar Date: Sat, 6 Jul 2024 22:23:58 -0300 Subject: [PATCH] Support double dash delimiter in tsh ssh This PR extends the tsh ssh command by adding support for the double dash (--) delimiter before remote commands (e.g. `tsh ssh -- echo test`), aligning its behavior with the standard ssh binary. This improves compatibility with tools that rely on the standard ssh binary behavior, such as sshuttle. Fixes #18453, #16589. Signed-off-by: Tim Ross --- tool/tsh/common/tsh.go | 5 ++ tool/tsh/common/tsh_test.go | 142 ++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index 1a8f73b7db6a..d90cb79939cc 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -3492,6 +3492,11 @@ func onSSH(cf *CLIConf) error { tc.AllowHeadless = true + // Support calling `tsh ssh -- ` (with a double dash before the command) + if len(cf.RemoteCommand) > 0 && strings.TrimSpace(cf.RemoteCommand[0]) == "--" { + cf.RemoteCommand = cf.RemoteCommand[1:] + } + tc.Stdin = os.Stdin err = retryWithAccessRequest(cf, tc, func() error { err = client.RetryWithRelogin(cf.Context, tc, func() error { diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index 36b767805c39..754285611a73 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -2040,6 +2040,148 @@ func TestAccessRequestOnLeaf(t *testing.T) { require.NoError(t, err) } +// TestSSHCommand tests that a user can access a single SSH node and run commands. +func TestSSHCommands(t *testing.T) { + modules.SetTestModules(t, &modules.TestModules{TestBuildType: modules.BuildEnterprise}) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + accessRoleName := "access" + sshHostname := "test-ssh-server" + + accessUser, err := types.NewUser(accessRoleName) + require.NoError(t, err) + accessUser.SetRoles([]string{accessRoleName}) + + user, err := user.Current() + require.NoError(t, err) + accessUser.SetLogins([]string{user.Username}) + + traits := map[string][]string{ + constants.TraitLogins: {user.Username}, + } + accessUser.SetTraits(traits) + + connector := mockConnector(t) + rootServerOpts := []testserver.TestServerOptFunc{ + testserver.WithBootstrap(connector, accessUser), + testserver.WithHostname(sshHostname), + testserver.WithClusterName(t, "root"), + testserver.WithSSHLabel(accessRoleName, "true"), + testserver.WithSSHPublicAddrs("127.0.0.1:0"), + testserver.WithConfig(func(cfg *servicecfg.Config) { + cfg.SSH.Enabled = true + cfg.SSH.PublicAddrs = []utils.NetAddr{cfg.SSH.Addr} + cfg.SSH.DisableCreateHostUser = true + }), + } + rootServer := testserver.MakeTestServer(t, rootServerOpts...) + + rootProxyAddr, err := rootServer.ProxyWebAddr() + require.NoError(t, err) + + require.EventuallyWithT(t, func(t *assert.CollectT) { + rootNodes, err := rootServer.GetAuthServer().GetNodes(ctx, apidefaults.Namespace) + if !assert.NoError(t, err) || !assert.Len(t, rootNodes, 1) { + return + } + }, 10*time.Second, 100*time.Millisecond) + + tests := []struct { + name string + cmd []string + expected string + shouldErr bool + }{ + { + // Test that a simple echo works. + name: "ssh simple command", + expected: "this is a test message", + cmd: []string{"echo", "this is a test message"}, + shouldErr: false, + }, + { + // Test that commands can be prefixed with a double dash. + name: "ssh command with double dash", + expected: "this is a test message", + cmd: []string{"--", "echo", "this is a test message"}, + shouldErr: false, + }, + { + // Test that a double dash is not removed from the middle of a command. + name: "ssh command with double dash in the middle", + expected: "-- this is a test message", + cmd: []string{"echo", "--", "this is a test message"}, + shouldErr: false, + }, + { + // Test that quoted commands work (e.g. `tsh ssh 'echo test'`) + name: "ssh command literal", + expected: "this is a test message", + cmd: []string{"echo this is a test message"}, + shouldErr: false, + }, + { + // Test that a double dash is passed as-is in a quoted command (which should fail). + name: "ssh command literal with double dash err", + expected: "", + cmd: []string{"-- echo this is a test message"}, + shouldErr: true, + }, + { + // Test that a double dash is not removed from the middle of a quoted command. + name: "ssh command literal with double dash in the middle", + expected: "-- this is a test message", + cmd: []string{"echo -- this is a test message"}, + shouldErr: false, + }, + } + + for _, test := range tests { + test := test + ctx := context.Background() + t.Run(test.name, func(t *testing.T) { + t.Parallel() + tmpHomePath := t.TempDir() + rootAuth := rootServer.GetAuthServer() + + err = Run(ctx, []string{ + "login", + "--insecure", + "--proxy", rootProxyAddr.String(), + "--user", user.Username, + }, setHomePath(tmpHomePath), setMockSSOLogin(rootAuth, accessUser, connector.GetName())) + require.NoError(t, err) + + stdout := &output{buf: bytes.Buffer{}} + stderr := &output{buf: bytes.Buffer{}} + args := []string{ + "ssh", + "--insecure", + "--proxy", rootProxyAddr.String(), + fmt.Sprintf("%s@%s", user.Username, sshHostname), + } + args = append(args, test.cmd...) + err := Run(ctx, args, setHomePath(tmpHomePath), + func(conf *CLIConf) error { + conf.overrideStdin = &bytes.Buffer{} + conf.OverrideStdout = stdout + conf.overrideStderr = stderr + return nil + }, + ) + + if test.shouldErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, test.expected, strings.TrimSpace(stdout.String())) + require.Empty(t, stderr.String()) + } + }) + } +} + // tryCreateTrustedCluster performs several attempts to create a trusted cluster, // retries on connection problems and access denied errors to let caches // propagate and services to start