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
x/crypto/ssh: msgNewKeys interpreted too soon #18850
Comments
Ah, thanks for opening this. I wasn't sure if I should open a new issue because it also seemed related to the kex rewrite. Let me know if there's any additional information you need to help debugging this. |
I took a look at this , but it's puzzling.
Can you confirm that your code runs cleanly under the race detector? |
We run all our tests with the race detector and haven't had any complaints from that lately. What would happen if a client sends a kex with FirstKexFollows set? I can try to test that (using dropbear), but it's just a guess at this point. EDIT: |
good catch, yes the error check is certainly to blame for the panic, but then what error does it print? |
ok, so
looks plausible in codereview, but is actually a pretty dumb thing to do. I wonder if we should find this automatically with go vet. |
Since encryption messes up the packets, the wrongly retained packets look like noise and cause application protocol errors or panics in the SSH library. This normally triggers very rarely: the mandatory key exchange doesn't have parallel writes, so this failure condition would be setup on the first key exchange, take effect only after the second key exchange. Fortunately, the tests against openssh exercise this. This change adds also adds a unittest. Fixes golang#18850. Change-Id: I656c8b94bfb265831daa118f4d614a2f0c65d2af Reviewed-on: https://go-review.googlesource.com/36056 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Han-Wen Nienhuys <hanwen@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes golang#18850. Change-Id: Id3ae89233f9e95ec3238462bf2ecda3e0c515f88 Reviewed-on: https://go-review.googlesource.com/36051 Run-TryBot: Han-Wen Nienhuys <hanwen@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
"We've also run into a different issue during the kex/handshake.
panic: ssh: no key material for msgNewKeys
goroutine 31868301 [running]:
panic(0x7ce020, 0xc42414c260)
/usr/local/go/src/runtime/panic.go:500 +0x1a1
bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh.(*connectionState).writePacket(0xc422774068, 0xc4218a8480, 0xa90080, 0xc42000c2d0, 0xc420010790, 0x9, 0x9, 0x0, 0x0)
/go/src/bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh/transport.go:163 +0x226
bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh.(*transport).writePacket(0xc422774000, 0xc420010790, 0x9, 0x9, 0x0, 0x0)
/go/src/bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh/transport.go:144 +0x77
bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh.(*handshakeTransport).pushPacket(0xc422326140, 0xc420010790, 0x9, 0x9, 0x0, 0x0)
/go/src/bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh/handshake.go:211 +0x51
bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh.(*handshakeTransport).kexLoop(0xc422326140)
/go/src/bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh/handshake.go:291 +0x303
created by bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh.newServerTransport
/go/src/bitbucket.org/bitbucket/conker/vendor/golang.org/x/crypto/ssh/handshake.go:126 +0x227
This is on a server running on golang/crypto@41d678d
We were previously running golang/crypto@ca7e7f1"
The text was updated successfully, but these errors were encountered: