commit 6068a2e6cfdc895ce524b6d2bdc8ea0cea8ea0e8
author Roland Shoemaker <roland@golang.org> Wed Mar 02 08:24:15 2022 -0800
committer Filippo Valsorda <filippo@golang.org> Sat Mar 12 13:11:42 2022 +0000
tree fed73256d9872634836f5ad94e8e3ff1a11cc52f
parent efcb8507fb706dc09498bdd32b7aafa38228fc86

ssh: ignore MAC if AEAD ciphers negotiated

If the server/client cipher chosen is one of the two AEAD ciphers that
we support (aes128-gcm@openssh.com and chacha20-poly1305@openssh.com),
don't attempt to find a common MAC algorithm in findAgreedAlgorithms.
Similarly in newPacketCipher, don't attempt to generate a MAC key if we
are using a AEAD cipher.

Fixes golang/go#51406

Change-Id: Id48ae72f052cb0a0c597b32e9901a0f218e4161f
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/389214
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

ssh/common.go

diff --git a/ssh/common.go b/ssh/common.go
index 5ae2275..ec1f839 100644
--- a/ssh/common.go
+++ b/ssh/common.go
@@ -152,6 +152,11 @@
 	return 1 << 30
 }
 
+var aeadCiphers = map[string]bool{
+	gcmCipherID:        true,
+	chacha20Poly1305ID: true,
+}
+
 type algorithms struct {
 	kex     string
 	hostKey string
@@ -187,14 +192,18 @@
 		return
 	}
 
-	ctos.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer)
-	if err != nil {
-		return
+	if !aeadCiphers[ctos.Cipher] {
+		ctos.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer)
+		if err != nil {
+			return
+		}
 	}
 
-	stoc.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient)
-	if err != nil {
-		return
+	if !aeadCiphers[stoc.Cipher] {
+		stoc.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient)
+		if err != nil {
+			return
+		}
 	}
 
 	ctos.Compression, err = findCommon("client to server compression", clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer)

ssh/handshake_test.go

diff --git a/ssh/handshake_test.go b/ssh/handshake_test.go
index 02fbe83..46bfd6d 100644
--- a/ssh/handshake_test.go
+++ b/ssh/handshake_test.go
@@ -560,3 +560,26 @@
 		t.Errorf("got rekey after %dG write, want 64G", wgb)
 	}
 }
+
+func TestHandshakeAEADCipherNoMAC(t *testing.T) {
+	for _, cipher := range []string{chacha20Poly1305ID, gcmCipherID} {
+		checker := &syncChecker{
+			called: make(chan int, 1),
+		}
+		clientConf := &ClientConfig{
+			Config: Config{
+				Ciphers: []string{cipher},
+				MACs:    []string{},
+			},
+			HostKeyCallback: checker.Check,
+		}
+		trC, trS, err := handshakePair(clientConf, "addr", false)
+		if err != nil {
+			t.Fatalf("handshakePair: %v", err)
+		}
+		defer trC.Close()
+		defer trS.Close()
+
+		<-checker.called
+	}
+}

ssh/transport.go

diff --git a/ssh/transport.go b/ssh/transport.go
index 49ddc2e..acf5a21 100644
--- a/ssh/transport.go
+++ b/ssh/transport.go
@@ -238,15 +238,19 @@
 // (to setup server->client keys) or clientKeys (for client->server keys).
 func newPacketCipher(d direction, algs directionAlgorithms, kex *kexResult) (packetCipher, error) {
 	cipherMode := cipherModes[algs.Cipher]
-	macMode := macModes[algs.MAC]
 
 	iv := make([]byte, cipherMode.ivSize)
 	key := make([]byte, cipherMode.keySize)
-	macKey := make([]byte, macMode.keySize)
 
 	generateKeyMaterial(iv, d.ivTag, kex)
 	generateKeyMaterial(key, d.keyTag, kex)
-	generateKeyMaterial(macKey, d.macKeyTag, kex)
+
+	var macKey []byte
+	if !aeadCiphers[algs.Cipher] {
+		macMode := macModes[algs.MAC]
+		macKey = make([]byte, macMode.keySize)
+		generateKeyMaterial(macKey, d.macKeyTag, kex)
+	}
 
 	return cipherModes[algs.Cipher].create(key, iv, macKey, algs)
 }