From d94f6bc902c2970500950e88ab1a54a0c26bec48 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Mon, 22 Jan 2018 20:45:19 +0100 Subject: [PATCH] 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 Run-TryBot: Han-Wen Nienhuys TryBot-Result: Gobot Gobot --- ssh/cipher.go | 8 ++++---- ssh/common.go | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) 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 @@ func (c *chacha20Poly1305Cipher) readPacket(seqNum uint32, r io.Reader) ([]byte, 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 @@ func (c *chacha20Poly1305Cipher) readPacket(seqNum uint32, r io.Reader) ([]byte, 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 @@ func (c *chacha20Poly1305Cipher) readPacket(seqNum uint32, r io.Reader) ([]byte, } 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 @@ var supportedCiphers = []string{ // 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