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

@@ -252,8 +252,8 @@ func TestClientUnsupportedKex(t *testing.T) {
 			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

@@ -85,27 +85,15 @@ func parseError(tag uint8) error {
 	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 @@ type algorithms struct {
 	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

@@ -332,9 +332,9 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {
 		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.