From 6119528deef5a73f6a2b18bab3ad4ec69fdec74a Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Tue, 28 Mar 2017 19:14:42 +0200 Subject: [PATCH] ssh: set rekeying thresholds on construction The normal handshake kicks off with a waitSession(), which guarantees that we never attempt to send data before the first kex is completed, but ensuring readPacketsLeft > 0 and writePacketsLeft > 0 helps understand that thresholds can never cause spurious rekeying at the start of a connection. Change-Id: If5bcafcda0c7d16fd21f22c664101ac5f5b487d7 Reviewed-on: https://go-review.googlesource.com/38696 Reviewed-by: Adam Langley Run-TryBot: Adam Langley TryBot-Result: Gobot Gobot --- ssh/handshake.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/ssh/handshake.go b/ssh/handshake.go index b9a2071..85b4013 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -107,6 +107,8 @@ func newHandshakeTransport(conn keyingTransport, config *Config, clientVersion, config: config, } + t.resetReadThresholds() + t.resetWriteThresholds() // We always start with a mandatory key exchange. t.requestKex <- struct{}{} @@ -237,6 +239,15 @@ func (t *handshakeTransport) requestKeyExchange() { } } +func (t *handshakeTransport) resetWriteThresholds() { + t.writePacketsLeft = packetRekeyThreshold + if t.config.RekeyThreshold > 0 { + t.writeBytesLeft = int64(t.config.RekeyThreshold) + } else if t.algorithms != nil { + t.writeBytesLeft = t.algorithms.w.rekeyBytes() + } +} + func (t *handshakeTransport) kexLoop() { write: @@ -285,12 +296,8 @@ write: t.writeError = err t.sentInitPacket = nil t.sentInitMsg = nil - t.writePacketsLeft = packetRekeyThreshold - if t.config.RekeyThreshold > 0 { - t.writeBytesLeft = int64(t.config.RekeyThreshold) - } else if t.algorithms != nil { - t.writeBytesLeft = t.algorithms.w.rekeyBytes() - } + + t.resetWriteThresholds() // we have completed the key exchange. Since the // reader is still blocked, it is safe to clear out @@ -344,6 +351,15 @@ write: // key exchange itself. const packetRekeyThreshold = (1 << 31) +func (t *handshakeTransport) resetReadThresholds() { + t.readPacketsLeft = packetRekeyThreshold + if t.config.RekeyThreshold > 0 { + t.readBytesLeft = int64(t.config.RekeyThreshold) + } else { + t.readBytesLeft = t.algorithms.r.rekeyBytes() + } +} + func (t *handshakeTransport) readOnePacket(first bool) ([]byte, error) { p, err := t.conn.readPacket() if err != nil { @@ -391,12 +407,7 @@ func (t *handshakeTransport) readOnePacket(first bool) ([]byte, error) { return nil, err } - t.readPacketsLeft = packetRekeyThreshold - if t.config.RekeyThreshold > 0 { - t.readBytesLeft = int64(t.config.RekeyThreshold) - } else { - t.readBytesLeft = t.algorithms.r.rekeyBytes() - } + t.resetReadThresholds() // By default, a key exchange is hidden from higher layers by // translating it into msgIgnore.