What version of Go are you using (go version)?

go version go1.11.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, x/crypto/ssh on master

What did you do?

We have a large codebase that is acting as a ProxyCommand. Our code acts as a bridge, being both a client and server. When running under Ansible, which spawns OpenSSH with ControlMaster enabled and heavily leverages “pipelining” of Ansible commands, we have encountered a race condition in the processing of a Channel.

In mux.go, there is a tight loop for processing messages:
https://github.com/golang/crypto/blob/ff983b9c42bc9fbf91556e191cc8efb585c16908/ssh/mux.go#L189-L191

Eventually, this calls ch.handlePacket:
https://github.com/golang/crypto/blob/ff983b9c42bc9fbf91556e191cc8efb585c16908/ssh/channel.go#L398

Three messages are received quickly on a session channel:

• exit-status
• eof
• close

Each message is handled differently:

• exit-status is made into a Request and sent to ch.incomingRequests. (goroutine A)
• eof sets .eof() on the buffer, which triggers any Readers of the buffer (stdout, stderr) to stop. (goroutine B)
• close is handled internally. (goroutine C)

When writing these “back” to our outbound channel, we must make calls in a specific order, so that the messages on the wire go out in the same order as we received them. In our ideal world, our proxy code, we should call these in order:

• SendRequest() for exit-status
• .CloseWrite() on the channel (generating eof messages)
• .Close() on the channel (generating close)

However, this is extremely difficult, and our code had race conditions because we’re processing the Channel IO and Requests in their own goroutines. In the “bad” path of the race condition, we would:

• Inbound channel Read() would return EOF, so we would call .CloseWrite() on the outbound channel, generating an EOF message to the Outbound Channel.
• We would then RECEIVE a Close from the Outbound Channel -- this would cause the internal handling to completely close the channel
• Inbound channel reading from the ->Requests Go-Channel would process the exit-status Request, and fail to write it to the outbound channel, because the outbound channel has already been hard closed.
• Ansible / OpenSSH would then break the ControlMaster muxing because it was expecting to receive an exit-status message, but never does, causing an error message like this:

debug3: mux_client_read_packet: read header failed: Broken pipe
debug2: Control master terminated unexpectedly

Using Go-channels and a Read() interface at the same time, requires processing across at least two goroutines. Additionally, because close messages are internally handled, there is a 3rd goroutine making races harder to control. Ordering of receiving messages is lost and hard to guarantee.

What did you expect to see?

I would prefer that all operations were written to a single Go-channel. (Data and Requests, including the Close message). This would allow in-order processing of a SSH Channel. This could maybe be written as an intermediate layer, and the current public API could be changed to run on top of it?

Relevant Logs

2019/01/11 11:39:35 decoding(1): data packet - 16393 bytes
2019/01/11 11:39:35 send(1): ssh.windowAdjustMsg{PeersID:0x0, AdditionalBytes:0x4000}
2019/01/11 11:39:35 decoding(1): data packet - 8201 bytes
2019/01/11 11:39:35 decoding(1): data packet - 1010 bytes
2019/01/11 11:39:35 send(1): ssh.windowAdjustMsg{PeersID:0x0, AdditionalBytes:0x23e9}
2019/01/11 11:39:35 decoding(1): 98 &ssh.channelRequestMsg{PeersID:0x1, Request:"exit-status", WantReply:false, RequestSpecificData:[]uint8{0x0, 0x0, 0x0, 0x0}} - 25 bytes
2019/01/11 11:39:35 decoding(1): 96 &ssh.channelEOFMsg{PeersID:0x1} - 5 bytes
2019/01/11 11:39:35 decoding(1): 97 &ssh.channelCloseMsg{PeersID:0x1} - 5 bytes
2019/01/11 11:39:35 send(1): ssh.channelCloseMsg{PeersID:0x0}
2019/01/11 11:39:35 send(2): ssh.channelEOFMsg{PeersID:0x2}
2019/01/11 11:39:35 decoding(2): 97 &ssh.channelCloseMsg{PeersID:0x2} - 5 bytes
2019/01/11 11:39:35 send(2): ssh.channelCloseMsg{PeersID:0x2}
2019/01/11 11:39:35 send(2): ssh.channelRequestMsg{PeersID:0x2, Request:"exit-status", WantReply:false, RequestSpecificData:[]uint8{0x0, 0x0, 0x0, 0x0}}
2019/01/11 11:39:35 send(2): ssh.channelCloseMsg{PeersID:0x2}
2019/01/11 11:39:35 send(1): ssh.channelCloseMsg{PeersID:0x0}