From 2ee8009354ce3cf41f67f3d968422d0cca9b35d4 Mon Sep 17 00:00:00 2001 From: Samir Aguiar Date: Sat, 6 Jul 2024 22:23:58 -0300 Subject: [PATCH 1/3] 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 5466f3bda68a..b892a1c3c60c 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -3605,6 +3605,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 { sshFunc := func() error { diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index c2f96b070a3c..f2bf4c2c04fd 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -2298,6 +2298,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 From dfef0805d934a89d4b0b86f205a527adde053ba2 Mon Sep 17 00:00:00 2001 From: Samir Aguiar Date: Sat, 6 Jul 2024 22:23:58 -0300 Subject: [PATCH 2/3] 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_test.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index f2bf4c2c04fd..ae2eb829b0a4 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -2345,6 +2345,17 @@ func TestSSHCommands(t *testing.T) { } }, 10*time.Second, 100*time.Millisecond) + 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) + tests := []struct { name string cmd []string @@ -2400,16 +2411,6 @@ func TestSSHCommands(t *testing.T) { 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{}} From d283c92c767d1dad849a776b4026042be55b23e5 Mon Sep 17 00:00:00 2001 From: Tim Ross Date: Wed, 11 Sep 2024 15:03:41 -0400 Subject: [PATCH 3/3] fix: update tests --- tool/tsh/common/tsh_test.go | 86 ++++++++++++++++++++++++++----------- 1 file changed, 60 insertions(+), 26 deletions(-) diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index ae2eb829b0a4..37e06c0ef228 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -2358,50 +2358,82 @@ func TestSSHCommands(t *testing.T) { tests := []struct { name string - cmd []string + args []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"}, + name: "ssh simple command", + expected: "this is a test message", + args: []string{ + fmt.Sprintf("%s@%s", user.Username, sshHostname), + "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"}, + name: "ssh command with double dash", + expected: "this is a test message", + args: []string{ + fmt.Sprintf("%s@%s", user.Username, sshHostname), + "--", + "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"}, + name: "ssh command with double dash in the middle", + expected: "-- this is a test message", + args: []string{ + fmt.Sprintf("%s@%s", user.Username, sshHostname), + "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"}, + name: "ssh command literal", + expected: "this is a test message", + args: []string{ + fmt.Sprintf("%s@%s", user.Username, sshHostname), + "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"}, + name: "ssh command literal with double dash err", + expected: "", + args: []string{ + fmt.Sprintf("%s@%s", user.Username, sshHostname), + "-- 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"}, + name: "ssh command literal with double dash in the middle", + expected: "-- this is a test message", + args: []string{ + fmt.Sprintf("%s@%s", user.Username, sshHostname), + "echo", "-- this is a test message", + }, + shouldErr: false, + }, + { + // Test tsh ssh -- hostname command + name: "delimiter before host and command", + expected: "this is a test message", + args: []string{ + "--", sshHostname, "echo", "this is a test message", + }, shouldErr: false, }, } @@ -2414,13 +2446,15 @@ func TestSSHCommands(t *testing.T) { 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...) + args := append( + []string{ + "ssh", + "--insecure", + "--proxy", rootProxyAddr.String(), + }, + test.args..., + ) + err := Run(ctx, args, setHomePath(tmpHomePath), func(conf *CLIConf) error { conf.overrideStdin = &bytes.Buffer{}