Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

crypto/tls: TLS handshake swallows errors #14539

Closed
tamird opened this issue Feb 27, 2016 · 2 comments
Closed

crypto/tls: TLS handshake swallows errors #14539

tamird opened this issue Feb 27, 2016 · 2 comments

Comments

@tamird
Copy link
Contributor

@tamird tamird commented Feb 27, 2016

  1. What version of Go are you using (go version)?
    go version go1.6 darwin/amd64
  2. What operating system and processor architecture are you using (go env)?
    GOARCH="amd64"
    GOHOSTARCH="amd64"
    GOHOSTOS="darwin"
    GOOS="darwin"
  3. What did you do?
    Runnable example here: https://github.com/tamird/go-conn-repro
  4. What did you expect to see?
    I expect that requests will either succeed or fail.
  5. What did you see instead?
    The test times out because a connection has hung in TLS handshake (both sides are trying to read). Stack trace:
=== RUN   TestFoo
panic: test timed out after 5s

goroutine 453 [running]:
panic(0x2e2860, 0xc820017920)
    /Users/tamird/src/go1.6/src/runtime/panic.go:464 +0x3e6
testing.startAlarm.func1()
    /Users/tamird/src/go1.6/src/testing/testing.go:725 +0x14b
created by time.goFunc
    /Users/tamird/src/go1.6/src/time/sleep.go:129 +0x3a

goroutine 1 [chan receive]:
testing.RunTests(0x492080, 0x5c78d0, 0x1, 0x1, 0x373001)
    /Users/tamird/src/go1.6/src/testing/testing.go:583 +0x8d2
testing.(*M).Run(0xc820191f08, 0x492388)
    /Users/tamird/src/go1.6/src/testing/testing.go:515 +0x81
main.main()
    _/Users/tamird/Desktop/go-conn-repro/_test/_testmain.go:54 +0x117

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
    /Users/tamird/src/go1.6/src/runtime/asm_amd64.s:1998 +0x1

goroutine 6 [select]:
net/http.(*Transport).getConn(0xc8200fc0c0, 0xc8201d0e00, 0x0, 0xc82029aae0, 0x5, 0xc82029aae8, 0xf, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/transport.go:707 +0x4ef
net/http.(*Transport).RoundTrip(0xc8200fc0c0, 0xc8201d0e00, 0xc8200fc0c0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/transport.go:307 +0x7e9
net/http.send(0xc8201d0e00, 0x12047f8, 0xc8200fc0c0, 0x0, 0x0, 0x0, 0xc82029ab00, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/client.go:260 +0x6b7
net/http.(*Client).send(0xc82040bf20, 0xc8201d0e00, 0x0, 0x0, 0x0, 0xd8, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/client.go:155 +0x185
net/http.(*Client).doFollowingRedirects(0xc82040bf20, 0xc8201d0e00, 0x492408, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/client.go:475 +0x8a4
net/http.(*Client).Get(0xc82040bf20, 0xc82029aae0, 0x17, 0xc82014cde0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/client.go:422 +0xaa
_/Users/tamird/Desktop/go-conn-repro.TestFoo(0xc820080090)
    /Users/tamird/Desktop/go-conn-repro/main_test.go:92 +0x611
testing.tRunner(0xc820080090, 0x5c78d0)
    /Users/tamird/src/go1.6/src/testing/testing.go:473 +0x98
created by testing.RunTests
    /Users/tamird/src/go1.6/src/testing/testing.go:582 +0x892

goroutine 7 [IO wait]:
net.runtime_pollWait(0x12059a8, 0x72, 0x1200050)
    /Users/tamird/src/go1.6/src/runtime/netpoll.go:160 +0x60
net.(*pollDesc).Wait(0xc820131fe0, 0x72, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/fd_poll_runtime.go:73 +0x3a
net.(*pollDesc).WaitRead(0xc820131fe0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/fd_poll_runtime.go:78 +0x36
net.(*netFD).accept(0xc820131f80, 0x0, 0x1240000, 0xc82029aba0)
    /Users/tamird/src/go1.6/src/net/fd_unix.go:426 +0x27c
net.(*TCPListener).AcceptTCP(0xc82002a068, 0xc8203c6020, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/tcpsock_posix.go:254 +0x4d
net.(*TCPListener).Accept(0xc82002a068, 0x0, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/tcpsock_posix.go:264 +0x3d
crypto/tls.(*listener).Accept(0xc82013f880, 0x0, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/tls.go:52 +0x60
net/http.(*Server).Serve(0xc82012ee80, 0x1205a90, 0xc82013f880, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/server.go:2117 +0x129
created by _/Users/tamird/Desktop/go-conn-repro.TestFoo
    /Users/tamird/Desktop/go-conn-repro/main_test.go:80 +0x465

goroutine 452 [IO wait]:
net.runtime_pollWait(0x1205768, 0x72, 0xc82022a000)
    /Users/tamird/src/go1.6/src/runtime/netpoll.go:160 +0x60
net.(*pollDesc).Wait(0xc8201f0c30, 0x72, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/fd_poll_runtime.go:73 +0x3a
net.(*pollDesc).WaitRead(0xc8201f0c30, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/fd_poll_runtime.go:78 +0x36
net.(*netFD).Read(0xc8201f0bd0, 0xc82022a000, 0x400, 0x400, 0x0, 0x1200050, 0xc820016180)
    /Users/tamird/src/go1.6/src/net/fd_unix.go:250 +0x23a
net.(*conn).Read(0xc82035a020, 0xc82022a000, 0x400, 0x400, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/net.go:172 +0xe4
crypto/tls.(*block).readFromUntil(0xc820359ec0, 0x1240078, 0xc82035a020, 0x5, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:460 +0xcc
crypto/tls.(*Conn).readRecord(0xc82035e900, 0x1208116, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:562 +0x2d1
crypto/tls.(*Conn).readHandshake(0xc82035e900, 0x0, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:800 +0xd3
crypto/tls.(*Conn).clientHandshake(0xc82035e900, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/handshake_client.go:149 +0xfcb
crypto/tls.(*Conn).Handshake(0xc82035e900, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:1033 +0xec
net/http.(*Transport).dialConn.func3(0xc82035e900, 0x0, 0xc82030c840)
    /Users/tamird/src/go1.6/src/net/http/transport.go:824 +0x21
created by net/http.(*Transport).dialConn
    /Users/tamird/src/go1.6/src/net/http/transport.go:829 +0x986

goroutine 413 [chan receive]:
net/http.(*Transport).dialConn(0xc8200fc0c0, 0x0, 0xc82029aae0, 0x5, 0xc82029aae8, 0xf, 0xc820300360, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/http/transport.go:830 +0x9c2
net/http.(*Transport).getConn.func4(0xc8200fc0c0, 0x0, 0xc82029aae0, 0x5, 0xc82029aae8, 0xf, 0xc8202a6480)
    /Users/tamird/src/go1.6/src/net/http/transport.go:702 +0x66
created by net/http.(*Transport).getConn
    /Users/tamird/src/go1.6/src/net/http/transport.go:704 +0x262

goroutine 414 [IO wait]:
net.runtime_pollWait(0x1205828, 0x72, 0xc820202400)
    /Users/tamird/src/go1.6/src/runtime/netpoll.go:160 +0x60
net.(*pollDesc).Wait(0xc8201f0ca0, 0x72, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/fd_poll_runtime.go:73 +0x3a
net.(*pollDesc).WaitRead(0xc8201f0ca0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/fd_poll_runtime.go:78 +0x36
net.(*netFD).Read(0xc8201f0c40, 0xc820202400, 0x400, 0x400, 0x0, 0x1200050, 0xc820016180)
    /Users/tamird/src/go1.6/src/net/fd_unix.go:250 +0x23a
net.(*conn).Read(0xc8203c6020, 0xc820202400, 0x400, 0x400, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/net/net.go:172 +0xe4
crypto/tls.(*block).readFromUntil(0xc820178480, 0x1240078, 0xc8203c6020, 0x5, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:460 +0xcc
crypto/tls.(*Conn).readRecord(0xc820330f00, 0xe7c16, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:562 +0x2d1
crypto/tls.(*Conn).readHandshake(0xc820330f00, 0x0, 0x0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:800 +0xd3
crypto/tls.(*serverHandshakeState).readClientHello(0xc820285ab8, 0xc820195aa0, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/handshake_server.go:107 +0x5b
crypto/tls.(*Conn).serverHandshake(0xc820330f00, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/handshake_server.go:48 +0xac
crypto/tls.(*Conn).Handshake(0xc820330f00, 0x0, 0x0)
    /Users/tamird/src/go1.6/src/crypto/tls/conn.go:1035 +0x169
net/http.(*conn).serve(0xc8202a4480)
    /Users/tamird/src/go1.6/src/net/http/server.go:1405 +0x382
created by net/http.(*Server).Serve
    /Users/tamird/src/go1.6/src/net/http/server.go:2137 +0x44e


ERROR: exit status 2

This appears to be caused by errors not being propagated from the TLS handshake code.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Feb 27, 2016

CL https://golang.org/cl/19990 mentions this issue.

gopherbot pushed a commit that referenced this issue Mar 2, 2016
This promotes a connection hang during TLS handshake to a proper error.
This doesn't fully address #14539 because the error reported in that
case is a write-on-socket-not-connected error, which implies that an
earlier error during connection setup is not being checked, but it is
an improvement over the current behaviour.

Updates #14539.

Change-Id: I0571a752d32d5303db48149ab448226868b19495
Reviewed-on: https://go-review.googlesource.com/19990
Reviewed-by: Adam Langley <agl@golang.org>
@tamird

This comment has been minimized.

Copy link
Contributor Author

@tamird tamird commented Mar 2, 2016

Fixed by https://golang.org/cl/19990. The rest of the issue is tracked in #14548.

@tamird tamird closed this Mar 2, 2016
tamird added a commit to tamird/cockroach that referenced this issue Aug 2, 2016
I'd go to rc4, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
tamird added a commit to tamird/cockroach that referenced this issue Aug 2, 2016
I'd go to rc4, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
tamird added a commit to tamird/cockroach that referenced this issue Aug 2, 2016
I'd go to rc4, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
tamird added a commit to tamird/cockroach that referenced this issue Aug 3, 2016
I'd go to rc4, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
tamird added a commit to tamird/cockroach that referenced this issue Aug 3, 2016
I'd go to rc4, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
tamird added a commit to tamird/cockroach that referenced this issue Aug 3, 2016
I'd go to rc5, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
tamird added a commit to tamird/cockroach that referenced this issue Aug 3, 2016
I'd go to rc5, but I don't feel like waiting for docker. See
docker-library/official-images#2013.

Also unignore grpc.Errorf in `go vet`, now that it's fixed upstream.
See golang/go#12294.

Also remove TLS handshakes from leaktest's whitelist, since these leaks
should be gone in 1.7. See golang/go#14539.

Closes cockroachdb#8053.
Closes cockroachdb#8136.
Closes cockroachdb#8215.
Closes cockroachdb#8217.
Closes cockroachdb#8218.
Closes cockroachdb#8219.
Closes cockroachdb#8237.
Closes cockroachdb#8250.
Closes cockroachdb#8255.
@golang golang locked and limited conversation to collaborators Mar 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.