Skip to content

Commit

Permalink
ssh: use io.ReadFull() for reading chacha20-poly1305 packets.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
hanwen committed Jan 22, 2018
1 parent 8418138 commit ff48eba
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
8 changes: 4 additions & 4 deletions ssh/cipher.go
Expand Up @@ -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
}

Expand All @@ -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")
}
Expand All @@ -720,6 +719,7 @@ func (c *chacha20Poly1305Cipher) readPacket(seqNum uint32, r io.Reader) ([]byte,
}

plain = plain[1 : len(plain)-int(padding)]

return plain, nil
}

Expand Down
3 changes: 2 additions & 1 deletion ssh/common.go
Expand Up @@ -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
Expand Down

0 comments on commit ff48eba

Please sign in to comment.