ssh: use io.ReadFull() for reading chacha20-poly1305 packets.
Incomplete reads leave (part of) the verification tag zeroed, leading
to a failing MAC, and this is more likely to happen with larger
packets. The test added in the previous commit should stop this from
regressing.
Reinstate chacha20-poly1305 as a default cipher and prefer it over AES
CTR flavors.
Fixes golang/go#23510
Change-Id: I7599897e59448edb7b814eebcc8226ea15b365d6
Reviewed-on: https://go-review.googlesource.com/89075
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/ssh/cipher.go b/ssh/cipher.go
index f2ba117..6f25076 100644
--- a/ssh/cipher.go
+++ b/ssh/cipher.go
@@ -671,7 +671,7 @@
chacha20.XORKeyStream(polyKey[:], chacha20PolyKeyInput[:], &counter, &c.contentKey)
encryptedLength := c.buf[:4]
- if _, err := r.Read(encryptedLength); err != nil {
+ if _, err := io.ReadFull(r, encryptedLength); err != nil {
return nil, err
}
@@ -692,13 +692,12 @@
c.buf = c.buf[:packetEnd]
}
- if _, err := r.Read(c.buf[4:packetEnd]); err != nil {
- return nil, err
+ if _, err := io.ReadFull(r, c.buf[4:packetEnd]); err != nil {
+ return nil, errors.New("ssh: MAC failure")
}
var mac [poly1305.TagSize]byte
copy(mac[:], c.buf[contentEnd:packetEnd])
-
if !poly1305.Verify(&mac, c.buf[:contentEnd], &polyKey) {
return nil, errors.New("ssh: MAC failure")
}
@@ -720,6 +719,7 @@
}
plain = plain[1 : len(plain)-int(padding)]
+
return plain, nil
}
diff --git a/ssh/common.go b/ssh/common.go
index abde5fc..04f3620 100644
--- a/ssh/common.go
+++ b/ssh/common.go
@@ -36,8 +36,9 @@
// preferredCiphers specifies the default preference for ciphers.
var preferredCiphers = []string{
- "aes128-ctr", "aes192-ctr", "aes256-ctr",
"aes128-gcm@openssh.com",
+ chacha20Poly1305ID,
+ "aes128-ctr", "aes192-ctr", "aes256-ctr",
}
// supportedKexAlgos specifies the supported key-exchange algorithms in
