ssh: don't assume packet plaintext size

When reading GCM and ChaChaPoly1305 packets, don't make assumptions
about the size of the enciphered plaintext. This fixes two panics
caused by standards non-compliant malformed packets.

Thanks to Rod Hynes, Psiphon Inc. for reporting this issue.

Fixes golang/go#49932
Fixes CVE-2021-43565

Change-Id: I660cff39d197e0d04ec44d11d792b22d954df2ef
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1262659
Reviewed-by: Katie Hockman <[email protected]>
Reviewed-by: Julie Qiu <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/368814
Trust: Roland Shoemaker <[email protected]>
Trust: Katie Hockman <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Julie Qiu <[email protected]>
Reviewed-by: Katie Hockman <[email protected]>

Author: rolandshoemaker

ssh/cipher.go

@@ -394,6 +394,10 @@ func (c *gcmCipher) readCipherPacket(seqNum uint32, r io.Reader) ([]byte, error)
 	}
 	c.incIV()
 
+	if len(plain) == 0 {
+		return nil, errors.New("ssh: empty packet")
+	}
+
 	padding := plain[0]
 	if padding < 4 {
 		// padding is a byte, so it automatically satisfies
@@ -710,6 +714,10 @@ func (c *chacha20Poly1305Cipher) readCipherPacket(seqNum uint32, r io.Reader) ([
 	plain := c.buf[4:contentEnd]
 	s.XORKeyStream(plain, plain)
 
+	if len(plain) == 0 {
+		return nil, errors.New("ssh: empty packet")
+	}
+
 	padding := plain[0]
 	if padding < 4 {
 		// padding is a byte, so it automatically satisfies

ssh/cipher_test.go

@@ -8,7 +8,12 @@ import (
 	"bytes"
 	"crypto"
 	"crypto/rand"
+	"encoding/binary"
+	"io"
 	"testing"
+
+	"golang.org/x/crypto/chacha20"
+	"golang.org/x/crypto/internal/poly1305"
 )
 
 func TestDefaultCiphersExist(t *testing.T) {
@@ -129,3 +134,98 @@ func TestCBCOracleCounterMeasure(t *testing.T) {
 		lastRead = bytesRead
 	}
 }
+
+func TestCVE202143565(t *testing.T) {
+	tests := []struct {
+		cipher          string
+		constructPacket func(packetCipher) io.Reader
+	}{
+		{
+			cipher: gcmCipherID,
+			constructPacket: func(client packetCipher) io.Reader {
+				internalCipher := client.(*gcmCipher)
+				b := &bytes.Buffer{}
+				prefix := [4]byte{}
+				if _, err := b.Write(prefix[:]); err != nil {
+					t.Fatal(err)
+				}
+				internalCipher.buf = internalCipher.aead.Seal(internalCipher.buf[:0], internalCipher.iv, []byte{}, prefix[:])
+				if _, err := b.Write(internalCipher.buf); err != nil {
+					t.Fatal(err)
+				}
+				internalCipher.incIV()
+
+				return b
+			},
+		},
+		{
+			cipher: chacha20Poly1305ID,
+			constructPacket: func(client packetCipher) io.Reader {
+				internalCipher := client.(*chacha20Poly1305Cipher)
+				b := &bytes.Buffer{}
+
+				nonce := make([]byte, 12)
+				s, err := chacha20.NewUnauthenticatedCipher(internalCipher.contentKey[:], nonce)
+				if err != nil {
+					t.Fatal(err)
+				}
+				var polyKey, discardBuf [32]byte
+				s.XORKeyStream(polyKey[:], polyKey[:])
+				s.XORKeyStream(discardBuf[:], discardBuf[:]) // skip the next 32 bytes
+
+				internalCipher.buf = make([]byte, 4+poly1305.TagSize)
+				binary.BigEndian.PutUint32(internalCipher.buf, 0)
+				ls, err := chacha20.NewUnauthenticatedCipher(internalCipher.lengthKey[:], nonce)
+				if err != nil {
+					t.Fatal(err)
+				}
+				ls.XORKeyStream(internalCipher.buf, internalCipher.buf[:4])
+				if _, err := io.ReadFull(rand.Reader, internalCipher.buf[4:4]); err != nil {
+					t.Fatal(err)
+				}
+
+				s.XORKeyStream(internalCipher.buf[4:], internalCipher.buf[4:4])
+
+				var tag [poly1305.TagSize]byte
+				poly1305.Sum(&tag, internalCipher.buf[:4], &polyKey)
+
+				copy(internalCipher.buf[4:], tag[:])
+
+				if _, err := b.Write(internalCipher.buf); err != nil {
+					t.Fatal(err)
+				}
+
+				return b
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		mac := "hmac-sha2-256"
+
+		kr := &kexResult{Hash: crypto.SHA1}
+		algs := directionAlgorithms{
+			Cipher:      tc.cipher,
+			MAC:         mac,
+			Compression: "none",
+		}
+		client, err := newPacketCipher(clientKeys, algs, kr)
+		if err != nil {
+			t.Fatalf("newPacketCipher(client, %q, %q): %v", tc.cipher, mac, err)
+		}
+		server, err := newPacketCipher(clientKeys, algs, kr)
+		if err != nil {
+			t.Fatalf("newPacketCipher(client, %q, %q): %v", tc.cipher, mac, err)
+		}
+
+		b := tc.constructPacket(client)
+
+		wantErr := "ssh: empty packet"
+		_, err = server.readCipherPacket(0, b)
+		if err == nil {
+			t.Fatalf("readCipherPacket(%q, %q): didn't fail with empty packet", tc.cipher, mac)
+		} else if err.Error() != wantErr {
+			t.Fatalf("readCipherPacket(%q, %q): unexpected error, got %q, want %q", tc.cipher, mac, err, wantErr)
+		}
+	}
+}