diff options
| author | James Tucker <james@tailscale.com> | 2026-01-05 12:54:41 -0800 |
|---|---|---|
| committer | James Tucker <james@tailscale.com> | 2026-01-05 13:36:03 -0800 |
| commit | e9a28ff0dbe9e32e82bed94261f7f1e5c1df6967 (patch) | |
| tree | eb097844551a8dc584330d75998a88e0263d8253 /tempfork | |
| parent | d451cd54a70152a95ad708592a981cb5e37395a8 (diff) | |
| download | tailscale-raggi/ssh-shutdown.tar.xz tailscale-raggi/ssh-shutdown.zip | |
ssh/tailssh: fix exit-status ordering and improve signal/exit code handlingraggi/ssh-shutdown
Fixes a race where CloseWrite() could be called before Exit(), causing
SSH clients (especially on macOS) to miss the exit status. Simplified
run() to use sync.WaitGroup and guarantee Exit() is sent before EOF per
RFC 4254 section 6.10.
Also:
- Send SIGHUP instead of SIGKILL on session termination
- Use exit code 127 for command not found
- Use exit code 255 for SSH permission/protocol errors
- Use exit code 254 for recording failures
- Complete TCP handlers only after I/O completes
Fixes tailscale/tailscale#18256
Signed-off-by: James Tucker <james@tailscale.com>
Diffstat (limited to 'tempfork')
| -rw-r--r-- | tempfork/gliderlabs/ssh/session.go | 9 | ||||
| -rw-r--r-- | tempfork/gliderlabs/ssh/tcpip.go | 29 |
2 files changed, 33 insertions, 5 deletions
diff --git a/tempfork/gliderlabs/ssh/session.go b/tempfork/gliderlabs/ssh/session.go index a7a9a3eeb..ef068355e 100644 --- a/tempfork/gliderlabs/ssh/session.go +++ b/tempfork/gliderlabs/ssh/session.go @@ -188,7 +188,12 @@ func (sess *session) Exit(code int) error { if err != nil { return err } - return sess.Close() + // Don't close the channel here. Per RFC 4254 section 6.10, the exit-status + // message should be sent before the channel is closed. By not closing immediately, + // we allow the session handler to complete any remaining I/O operations (like + // flushing output and sending EOF via CloseWrite) before the channel is closed + // by the request handler's cleanup code. + return nil } func (sess *session) User() string { @@ -273,6 +278,7 @@ func (sess *session) handleRequests(reqs <-chan *gossh.Request) { go func() { sess.handler(sess) sess.Exit(0) + sess.Close() }() case "subsystem": if sess.handled { @@ -307,6 +313,7 @@ func (sess *session) handleRequests(reqs <-chan *gossh.Request) { go func() { handler(sess) sess.Exit(0) + sess.Close() }() case "env": if sess.handled { diff --git a/tempfork/gliderlabs/ssh/tcpip.go b/tempfork/gliderlabs/ssh/tcpip.go index 335fda657..307cc53cb 100644 --- a/tempfork/gliderlabs/ssh/tcpip.go +++ b/tempfork/gliderlabs/ssh/tcpip.go @@ -53,16 +53,37 @@ func DirectTCPIPHandler(srv *Server, conn *gossh.ServerConn, newChan gossh.NewCh } go gossh.DiscardRequests(reqs) + defer ch.Close() + defer dconn.Close() + + done := make(chan struct{}, 2) go func() { - defer ch.Close() - defer dconn.Close() + defer ch.CloseWrite() + defer closeRead(dconn) io.Copy(ch, dconn) + done <- struct{}{} }() go func() { - defer ch.Close() - defer dconn.Close() + defer closeWrite(dconn) io.Copy(dconn, ch) + done <- struct{}{} }() + <-done + <-done +} + +func closeWrite(c net.Conn) error { + if cw, ok := c.(interface{ CloseWrite() error }); ok { + return cw.CloseWrite() + } + return c.Close() +} + +func closeRead(c net.Conn) error { + if cr, ok := c.(interface{ CloseRead() error }); ok { + return cr.CloseRead() + } + return c.Close() } type remoteForwardRequest struct { |
