ssh: fix public key and cert authentication compatibility with old clients

ssh/client_auth_test.go

@@ -955,3 +955,91 @@ func TestAuthMethodGSSAPIWithMIC(t *testing.T) {
 		}
 	}
 }
+
+func TestCompatibleAlgoAndSignatures(t *testing.T) {
+	type testcase struct {
+		algo       string
+		sigFormat  string
+		compatible bool
+	}
+	testcases := []*testcase{
+		{
+			KeyAlgoRSA,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			KeyAlgoRSA,
+			KeyAlgoRSASHA256,
+			false,
+		},
+		{
+			KeyAlgoRSA,
+			KeyAlgoRSASHA512,
+			false,
+		},
+		{
+			KeyAlgoRSASHA256,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			KeyAlgoRSASHA512,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			KeyAlgoRSASHA512,
+			KeyAlgoRSASHA256,
+			false,
+		},
+		{
+			KeyAlgoRSASHA256,
+			KeyAlgoRSASHA512,
+			false,
+		},
+		{
+			KeyAlgoRSASHA512,
+			KeyAlgoRSASHA512,
+			true,
+		},
+		{
+			CertAlgoRSAv01,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			CertAlgoRSAv01,
+			KeyAlgoRSASHA256,
+			true,
+		},
+		{
+			CertAlgoRSAv01,
+			KeyAlgoRSASHA512,
+			true,
+		},
+		{
+			CertAlgoRSASHA256v01,
+			KeyAlgoRSASHA512,
+			false,
+		},
+		{
+			CertAlgoRSASHA512v01,
+			KeyAlgoRSASHA512,
+			true,
+		},
+		{
+			CertAlgoRSASHA512v01,
+			KeyAlgoRSASHA256,
+			false,
+		},
+	}
+
+	for _, c := range testcases {
+		if isAlgoCompatible(c.algo, c.sigFormat) != c.compatible {
+			t.Errorf("algorithm %q, signature format %q, expected compatible to be %t", c.algo, c.sigFormat, c.compatible)
+		}
+
+	}
+
+}

ssh/server.go

@@ -370,6 +370,28 @@ func gssExchangeToken(gssapiConfig *GSSAPIWithMICConfig, firstToken []byte, s *c
 	return authErr, perms, nil
 }
 
+// isAlgoCompatible check if the signature format is
+// compatible with the selected algorithm taking into account
+// edge cases that occur with old clients
+func isAlgoCompatible(algo, sigFormat string) bool {
+	if underlyingAlgo(algo) == sigFormat {
+		return true
+	}
+	rsaCompatibleAlgos := []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}
+	// For certificate authentication with OpenSSH 7.2-7.7
+	// signature format can be rsa-sha2-256 or rsa-sha2-512
+	// for the algorithm [email protected]
+	if algo == CertAlgoRSAv01 {
+		return contains(rsaCompatibleAlgos, sigFormat)
+	}
+	// With gpg-agent < 2.2.6 the algorithm can be rsa-sha2-256
+	// or rsa-sha2-512 for signature format ssh-rsa
+	if sigFormat == KeyAlgoRSA {
+		return contains(rsaCompatibleAlgos, algo)
+	}
+	return false
+}
+
 // ServerAuthError represents server authentication errors and is
 // sometimes returned by NewServerConn. It appends any authentication
 // errors that may occur, and is returned if all of the authentication
@@ -567,7 +589,18 @@ userAuthLoop:
 					authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
 					break
 				}
-				if underlyingAlgo(algo) != sig.Format {
+				// Ensure the declared public key algo is compatible
+				// with the decoded one.
+				// This check will ensure we don't accept e.g.
+				// [email protected] algorithm with ssh-rsa
+				// public key type.
+				// The algorithm and public key type must be consistent:
+				// both must point to a certificate or none
+				if !contains(algorithmsForKeyFormat(pubKey.Type()), algo) {
+					authErr = fmt.Errorf("ssh: type mismatch for decoded key, received %q, expected %q", pubKey.Type(), algo)
+					break
+				}
+				if !isAlgoCompatible(algo, sig.Format) {
 					authErr = fmt.Errorf("ssh: signature %q not compatible with selected algorithm %q", sig.Format, algo)
 					break
 				}