Use channels and a dedicated write loop for managing the rekeying
process. This lets us collect packets to be written while a key
exchange is in progress.
Previously, the read loop ran the key exchange, and writers would
block if a key exchange was going on. If a reader wrote back a packet
while processing a read packet, it could block, stopping the read
loop, thus causing a deadlock. Such coupled read/writes are inherent
with handling requests that want a response (eg. keepalive,
opening/closing channels etc.). The buffered channels (most channels
have capacity 16) papered over these problems, but under load SSH
connections would occasionally deadlock.
Fixes#18439.
Change-Id: I7c14ff4991fa3100a5d36025125d0cf1119c471d
Reviewed-on: https://go-review.googlesource.com/35012
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
This is done by running the key exchange and setting the session ID
under mutex. If the first exchange encounters an already set session
ID, then do nothing.
This fixes a race condition:
On setting up the connection, both sides sent a kexInit to initiate
the first (mandatory) key exchange. If one side was faster, the
faster side might have completed the key exchange, before the slow
side had a chance to send a kexInit. The slow side would send a
kexInit which would trigger a second key exchange. The resulting
confirmation message (msgNewKeys) would confuse the authentication
loop.
This fix removes sessionID from the transport struct.
This fix also deletes the unused interface rekeyingTransport.
Fixes#15066
Change-Id: I7f303bce5d3214c9bdd58f52d21178a185871d90
Reviewed-on: https://go-review.googlesource.com/21606
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Unblock writers if a read error occurs while writers are blocked on a
pending key change.
Add test to check for deadlocks in error paths in handshake.go
Fixesgolang/go#11992.
Change-Id: Id098bd9fec3d4fe83daeb2b7f935e5647c19afd3
Reviewed-on: https://go-review.googlesource.com/13594
Reviewed-by: Adam Langley <agl@golang.org>
Han-Wen Nienhuys
David du Colombier
Adam Langley