crypto/ssh: clearer error messages when "no ciphers in common"

The error message reported by the ssh client when it can't find a
"cipher" in common between the client and server was overly vague.  This
adds more detailed error messages to findAgreedAlgorithms so that the
user can more easily identify which of the components can't reach
agreement.

Change-Id: I4d985e92fea964793213e5600b52b3141e712000
Reviewed-on: https://go-review.googlesource.com/13817
Reviewed-by: Adam Langley <agl@golang.org>

ssh/client_auth_test.go

diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go
index c92b587..2ea4462 100644
--- a/ssh/client_auth_test.go
+++ b/ssh/client_auth_test.go
@@ -252,8 +252,8 @@
 			KeyExchanges: []string{"diffie-hellman-group-exchange-sha256"}, // not currently supported
 		},
 	}
-	if err := tryAuth(t, config); err == nil || !strings.Contains(err.Error(), "no common algorithms") {
-		t.Errorf("got %v, expected 'no common algorithms'", err)
+	if err := tryAuth(t, config); err == nil || !strings.Contains(err.Error(), "common algorithm") {
+		t.Errorf("got %v, expected 'common algorithm'", err)
 	}
 }
 

ssh/common.go

diff --git a/ssh/common.go b/ssh/common.go
index 1aef9c0..9fc739e 100644
--- a/ssh/common.go
+++ b/ssh/common.go
@@ -85,27 +85,15 @@
 	return fmt.Errorf("ssh: parse error in message type %d", tag)
 }
 
-func findCommonAlgorithm(clientAlgos []string, serverAlgos []string) (commonAlgo string, ok bool) {
-	for _, clientAlgo := range clientAlgos {
-		for _, serverAlgo := range serverAlgos {
-			if clientAlgo == serverAlgo {
-				return clientAlgo, true
+func findCommon(what string, client []string, server []string) (common string, err error) {
+	for _, c := range client {
+		for _, s := range server {
+			if c == s {
+				return c, nil
 			}
 		}
 	}
-	return
-}
-
-func findCommonCipher(clientCiphers []string, serverCiphers []string) (commonCipher string, ok bool) {
-	for _, clientCipher := range clientCiphers {
-		for _, serverCipher := range serverCiphers {
-			// reject the cipher if we have no cipherModes definition
-			if clientCipher == serverCipher && cipherModes[clientCipher] != nil {
-				return clientCipher, true
-			}
-		}
-	}
-	return
+	return "", fmt.Errorf("ssh: no common algorithm for %s; client offered: %v, server offered: %v", what, client, server)
 }
 
 type directionAlgorithms struct {
@@ -121,50 +109,50 @@
 	r       directionAlgorithms
 }
 
-func findAgreedAlgorithms(clientKexInit, serverKexInit *kexInitMsg) (algs *algorithms) {
-	var ok bool
+func findAgreedAlgorithms(clientKexInit, serverKexInit *kexInitMsg) (algs *algorithms, err error) {
 	result := &algorithms{}
-	result.kex, ok = findCommonAlgorithm(clientKexInit.KexAlgos, serverKexInit.KexAlgos)
-	if !ok {
+
+	result.kex, err = findCommon("key exchange", clientKexInit.KexAlgos, serverKexInit.KexAlgos)
+	if err != nil {
 		return
 	}
 
-	result.hostKey, ok = findCommonAlgorithm(clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos)
-	if !ok {
+	result.hostKey, err = findCommon("host key", clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos)
+	if err != nil {
 		return
 	}
 
-	result.w.Cipher, ok = findCommonCipher(clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer)
-	if !ok {
+	result.w.Cipher, err = findCommon("client to server cipher", clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer)
+	if err != nil {
 		return
 	}
 
-	result.r.Cipher, ok = findCommonCipher(clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient)
-	if !ok {
+	result.r.Cipher, err = findCommon("server to client cipher", clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient)
+	if err != nil {
 		return
 	}
 
-	result.w.MAC, ok = findCommonAlgorithm(clientKexInit.MACsClientServer, serverKexInit.MACsClientServer)
-	if !ok {
+	result.w.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer)
+	if err != nil {
 		return
 	}
 
-	result.r.MAC, ok = findCommonAlgorithm(clientKexInit.MACsServerClient, serverKexInit.MACsServerClient)
-	if !ok {
+	result.r.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient)
+	if err != nil {
 		return
 	}
 
-	result.w.Compression, ok = findCommonAlgorithm(clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer)
-	if !ok {
+	result.w.Compression, err = findCommon("client to server compression", clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer)
+	if err != nil {
 		return
 	}
 
-	result.r.Compression, ok = findCommonAlgorithm(clientKexInit.CompressionServerClient, serverKexInit.CompressionServerClient)
-	if !ok {
+	result.r.Compression, err = findCommon("server to client compression", clientKexInit.CompressionServerClient, serverKexInit.CompressionServerClient)
+	if err != nil {
 		return
 	}
 
-	return result
+	return result, nil
 }
 
 // If rekeythreshold is too small, we can't make any progress sending

ssh/handshake.go

diff --git a/ssh/handshake.go b/ssh/handshake.go
index 12633c8..1c54f75 100644
--- a/ssh/handshake.go
+++ b/ssh/handshake.go
@@ -332,9 +332,9 @@
 		magics.serverKexInit = otherInitPacket
 	}
 
-	algs := findAgreedAlgorithms(clientInit, serverInit)
-	if algs == nil {
-		return errors.New("ssh: no common algorithms")
+	algs, err := findAgreedAlgorithms(clientInit, serverInit)
+	if err != nil {
+		return err
 	}
 
 	// We don't send FirstKexFollows, but we handle receiving it.