Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

(v7) Fix interactive sessions always exiting with code 0 #8252

Merged
merged 1 commit into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 149 additions & 6 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ func TestIntegrations(t *testing.T) {
t.Run("RotateSuccess", suite.bind(testRotateSuccess))
t.Run("RotateTrustedClusters", suite.bind(testRotateTrustedClusters))
t.Run("SessionStartContainsAccessRequest", suite.bind(testSessionStartContainsAccessRequest))
t.Run("SessionStreaming", suite.bind(testSessionStreaming))
t.Run("SSHExitCode", suite.bind(testSSHExitCode))
t.Run("Shutdown", suite.bind(testShutdown))
t.Run("TrustedClusters", suite.bind(testTrustedClusters))
t.Run("TrustedClustersWithLabels", suite.bind(testTrustedClustersWithLabels))
Expand All @@ -211,7 +213,6 @@ func TestIntegrations(t *testing.T) {
t.Run("TwoClustersTunnel", suite.bind(testTwoClustersTunnel))
t.Run("UUIDBasedProxy", suite.bind(testUUIDBasedProxy))
t.Run("WindowChange", suite.bind(testWindowChange))
t.Run("SessionStreaming", suite.bind(testSessionStreaming))
}

// testAuditOn creates a live session, records a bunch of data through it
Expand Down Expand Up @@ -1218,10 +1219,11 @@ func runDisconnectTest(t *testing.T, suite *integrationTestSuite, tc disconnectT
return
default:
}

if tc.assertExpected != nil {
tc.assertExpected(t, err)
} else if err != nil && !trace.IsEOF(err) {
require.FailNowf(t, "Missing EOF", "expected EOF or nil, got %v instead", err)
} else if err != nil && !trace.IsEOF(err) && !isSSHError(err) {
require.FailNowf(t, "Missing EOF", "expected EOF, ExitError, or nil, got %v instead", err)
}
}

Expand All @@ -1245,6 +1247,15 @@ func runDisconnectTest(t *testing.T, suite *integrationTestSuite, tc disconnectT
}
}

func isSSHError(err error) bool {
switch trace.Unwrap(err).(type) {
case *ssh.ExitError, *ssh.ExitMissingError:
return true
default:
return false
}
}

func timeNow() string {
return time.Now().Format(time.StampMilli)
}
Expand Down Expand Up @@ -3365,7 +3376,9 @@ func testPAM(t *testing.T, suite *integrationTestSuite) {

termSession.Type("\aecho hi\n\r\aexit\n\r\a")
err = cl.SSH(context.TODO(), []string{}, false)
require.NoError(t, err)
if !isSSHError(err) {
require.NoError(t, err)
}

cancel()
}()
Expand Down Expand Up @@ -4163,7 +4176,9 @@ func testWindowChange(t *testing.T, suite *integrationTestSuite) {
cl.Stdin = personA

err = cl.SSH(context.TODO(), []string{}, false)
require.NoError(t, err)
if !isSSHError(err) {
require.NoError(t, err)
}
}

// joinSession will join the existing session on a server.
Expand Down Expand Up @@ -4197,10 +4212,12 @@ func testWindowChange(t *testing.T, suite *integrationTestSuite) {

for i := 0; i < 10; i++ {
err = cl.Join(context.TODO(), apidefaults.Namespace, session.ID(sessionID), personB)
if err == nil {
if err == nil || isSSHError(err) {
err = nil
break
}
}

require.NoError(t, err)
}

Expand Down Expand Up @@ -4758,6 +4775,132 @@ func testBPFExec(t *testing.T, suite *integrationTestSuite) {
}
}

func testSSHExitCode(t *testing.T, suite *integrationTestSuite) {
lsPath, err := exec.LookPath("ls")
require.NoError(t, err)

var tests = []struct {
desc string
command []string
input string
interactive bool
errorAssertion require.ErrorAssertionFunc
statusCode int
}{
// A successful noninteractive session should have a zero status code
{
desc: "Run Command and Exit Successfully",
command: []string{lsPath},
interactive: false,
errorAssertion: require.NoError,
},
// A failed noninteractive session should have a non-zero status code
{
desc: "Run Command and Fail With Code 2",
command: []string{"exit 2"},
interactive: false,
errorAssertion: require.Error,
statusCode: 2,
},
// A failed interactive session should have a non-zero status code
{
desc: "Run Command Interactively and Fail With Code 2",
command: []string{"exit 2"},
interactive: true,
errorAssertion: require.Error,
statusCode: 2,
},
// A failed interactive session should have a non-zero status code
{
desc: "Interactively Fail With Code 3",
input: "exit 3\n\r",
interactive: true,
errorAssertion: require.Error,
statusCode: 3,
},
// A failed interactive session should have a non-zero status code
{
desc: "Interactively Fail With Code 3",
input: fmt.Sprintf("%v\n\rexit 3\n\r", lsPath),
interactive: true,
errorAssertion: require.Error,
statusCode: 3,
},
// A successful interactive session should have a zero status code
{
desc: "Interactively Run Command and Exit Successfully",
input: fmt.Sprintf("%v\n\rexit\n\r", lsPath),
interactive: true,
errorAssertion: require.NoError,
},
// A successful interactive session should have a zero status code
{
desc: "Interactively Exit",
input: "exit\n\r",
interactive: true,
errorAssertion: require.NoError,
},
}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
// Create and start a Teleport cluster.
makeConfig := func() (*testing.T, []string, []*InstanceSecrets, *service.Config) {
// Create default config.
tconf := suite.defaultServiceConfig()

// Configure Auth.
tconf.Auth.Preference.SetSecondFactor("off")
tconf.Auth.Enabled = true
tconf.Auth.NoAudit = true

// Configure Proxy.
tconf.Proxy.Enabled = true
tconf.Proxy.DisableWebService = false
tconf.Proxy.DisableWebInterface = true

// Configure Node.
tconf.SSH.Enabled = true
return t, nil, nil, tconf
}
main := suite.newTeleportWithConfig(makeConfig())
t.Cleanup(func() { main.StopAll() })

// context to signal when the client is done with the terminal.
doneContext, doneCancel := context.WithTimeout(context.Background(), time.Second*10)
defer doneCancel()

cli, err := main.NewClient(t, ClientConfig{
Login: suite.me.Username,
Cluster: Site,
Host: Host,
Port: main.GetPortSSHInt(),
Interactive: tt.interactive,
})
require.NoError(t, err)

if tt.interactive {
// Create a new terminal and connect it to std{in,out} of client.
term := NewTerminal(250)
cli.Stdout = term
cli.Stdin = term
term.Type(tt.input)
}

// run the ssh command
err = cli.SSH(doneContext, tt.command, false)
tt.errorAssertion(t, err)

// check that the exit code of the session matches the expected one
if err != nil {
var exitError *ssh.ExitError
require.ErrorAs(t, trace.Unwrap(err), &exitError)
require.Equal(t, tt.statusCode, exitError.ExitStatus())
}
})
}
}

// testBPFSessionDifferentiation verifies that the bpf package can
// differentiate events from two different sessions. This test in turn also
// verifies the cgroup package.
Expand Down
7 changes: 7 additions & 0 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1926,6 +1926,13 @@ func (tc *TeleportClient) runShell(nodeClient *NodeClient, sessToJoin *session.S
return trace.Wrap(err)
}
if err = nodeSession.runShell(tc.OnShellCreated); err != nil {
switch e := trace.Unwrap(err).(type) {
case *ssh.ExitError:
tc.ExitStatus = e.ExitStatus()
case *ssh.ExitMissingError:
tc.ExitStatus = 1
}

return trace.Wrap(err)
}
if nodeSession.ExitMsg == "" {
Expand Down
3 changes: 2 additions & 1 deletion lib/client/session.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !windows
// +build !windows

/*
Expand Down Expand Up @@ -265,7 +266,7 @@ func (ns *NodeSession) interactiveSession(callback interactiveCallback) error {
}
// wait for the session to end
<-ns.closer.C
return nil
return sess.Wait()
}

// allocateTerminal creates (allocates) a server-side terminal for this session.
Expand Down