Description
Go version
go version go1.21.4 linux/amd64
Output of go env in your module/workspace:
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/jeffwill/.cache/go-build'
GOENV='/home/jeffwill/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/jeffwill/gows/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/jeffwill/gows'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/jeffwill/Downloads/go-1.21.4'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/jeffwill/Downloads/go-1.21.4/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3235840173=/tmp/go-build -gno-record-gcc-switches'What did you do?
I called Listen on an SSH Client, and then started an HTTP server on the Listener. When an HTTP Get was received on the HTTP server, I tried to call Hijack on the ResponseWriter.
For context, the purpose of the Hijack was to upgrade the connection to a Websocket; Hijack was called indirectly by github.com/gorilla/websocket.
See the attached program which reproduces the issue on Linux.
hijack_unresponsive_repro.tar.gz
What did you see happen?
Hijack hung forever.
What did you expect to see?
I expected Hijack to return.
The reason seems to be that Hijack tries to abort any pending reads on the connection before returning it to the caller of Hijack. In this scenario there is indeed a pending read, because the http server is trying to read data using http.*connReader).backgroundRead. Hijack tries to abort the pending read by setting the read deadline of the connection to a time that has already passed using SetReadDeadline, and then waits for the read to be aborted by waiting on a condition variable which gets signalled when the read unblocks.
However, the SSH channel struct which implements the connection that HTTP is using does not support SetReadDeadline. Thus, the read is never terminated and so the Hijack method blocks forever.
See the following gorouting stacktraces for what I believe is the cause:
goroutine 94 [sync.Cond.Wait, 32 minutes]:
sync.runtime_notifyListWait(0xc001ece490, 0x0)
/home/jeffwill/Downloads/go-1.21.4/src/runtime/sema.go:527 +0x159
sync.(*Cond).Wait(0xc0015ac060?)
/home/jeffwill/Downloads/go-1.21.4/src/sync/cond.go:70 +0x85
net/http.(*connReader).abortPendingRead(0xc0015ac060)
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:731 +0xa6
net/http.(*conn).hijackLocked(0xc002f94870)
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:320 +0x33
net/http.(*response).Hijack(0xc00205c540)
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:2086 +0xd0
github.com/gorilla/websocket.(*Upgrader).Upgrade(0xc006f4f930, {0x12f6b98, 0xc00205c540}, 0xc0000f3800, 0x10c?)
/home/jeffwill/gows/pkg/mod/github.com/gorilla/[email protected]/server.go:180 +0x3d7
main.ApiHandler.serveWebsocket({}, 0xc000afe6c0, {0x12f6b98, 0xc00205c540}, 0xc0000f3800)
/home/jeffwill/src/anvil-suite/anvil/src/anvil/api.go:880 +0x145
main.ApiHandler.ServeHTTP({}, {0x12f6b98, 0xc00205c540}, 0xc0000f3800)
/home/jeffwill/src/anvil-suite/anvil/src/anvil/api.go:130 +0x65a
net/http.serverHandler.ServeHTTP({0xc0015ac060?}, {0x12f6b98?, 0xc00205c540?}, 0x6?)
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:2938 +0x8e
net/http.(*conn).serve(0xc002f94870, {0x12f81b0, 0xc00158c870})
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:2009 +0x5f4
created by net/http.(*Server).Serve in goroutine 98
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:3086 +0x5cb
goroutine 106 [sync.Cond.Wait, 32 minutes]:
sync.runtime_notifyListWait(0xc001ece390, 0x1)
/home/jeffwill/Downloads/go-1.21.4/src/runtime/sema.go:527 +0x159
sync.(*Cond).Wait(0x60?)
/home/jeffwill/Downloads/go-1.21.4/src/sync/cond.go:70 +0x85
golang.org/x/crypto/ssh.(*buffer).Read(0xc001274440, {0xc0015ac071, 0x1, 0x1})
/home/jeffwill/gows/pkg/mod/golang.org/x/[email protected]/ssh/buffer.go:94 +0x1fb
golang.org/x/crypto/ssh.(*channel).ReadExtended(0xc000eab380, {0xc0015ac071?, 0xc00158cdb0?, 0x0?}, 0x0?)
/home/jeffwill/gows/pkg/mod/golang.org/x/[email protected]/ssh/channel.go:351 +0x93
golang.org/x/crypto/ssh.(*channel).Read(0xc00251ff78?, {0xc0015ac071?, 0xc00158cdb0?, 0xc002f94750?})
/home/jeffwill/gows/pkg/mod/golang.org/x/[email protected]/ssh/channel.go:528 +0x25
net/http.(*connReader).backgroundRead(0xc0015ac060)
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:683 +0x37
created by net/http.(*connReader).startBackgroundRead in goroutine 94
/home/jeffwill/Downloads/go-1.21.4/src/net/http/server.go:679 +0xba
One way to fix the issue is to implement SetReadDeadline on the channel. I've attached a patch that implements it and solves this particular starvation issue, but requires more thorough testing.
An alternative solution might be to provide the user a mechanism in the HTTP package to prevent background reads. I assume the background read is just an optimization to help improve response times, in which case having a method to disable it seems reasonable.
One other thought is that some other protocol for aborting the read could be supported between the HTTP package and the underlying connection to abort any pending reads, rather than relying on SetReadDeadline. Perhaps the connection could be checked to see if it implements an Aborter interface with an Abort method, and if present that method could be called before SetReadDeadline. That could reduce the need to implement full support for SetReadDeadline in ssh.channel and instead just implement a simpler functionality.