ssh: relax RSA signature check in SSH_MSG_USERAUTH_REQUEST

Buggy SSH clients, such as gpg-agent v2.2.4 and OpenSSH v7.6 shipped
in Ubuntu 18.04, may send `ssh-rsa-512` as the public key algorithm
but actually include an `rsa-sha` signature.

If RFC 3808 (extension negotiation) is implemented, these clients will
fail to authenticate with the error:

```
ssh: signature "ssh-rsa" came in for selected algorithm "rsa-sha2-512", public key is type ssh-rsa
```

According to RFC 8332 section 3.2:

If the client includes the signature field, the client MUST encode the
same algorithm name in the signature as in SSH_MSG_USERAUTH_REQUEST --
either "rsa-sha2-256" or "rsa-sha2-512".  If a server receives a
mismatching request, it MAY apply arbitrary authentication penalties,
including but not limited to authentication failure or disconnect.

...A server MAY, but is not required to, accept this variant or another
variant that corresponds to a good-faith implementation and is
considered safe to accept.

While the client is expected to do the right thing, in practice older
clients may not fully support `ssh-rsa-256` and `ssh-rsa-512`. For
example, gpg-agent v2.2.6 added support for these newer signature
types.

To accomodate these clients, relax the matching constraint: if the
`SSH_MSG_USERAUTH_REQUEST` message specifies an RSA public key
algorithm and includes an RSA public key, then allow any of the
following signature types:

- `rsa-sha-512`
- `rsa-sha-256`
- `rsa-sha`

This emulates what OpenSSH does. OpenSSH only considers that the RSA
family is specified and then verifies if the signature and public key
match.

Closes golang/go#53391

Author: stanhu

ssh/client_auth_test.go

@@ -957,3 +957,87 @@ func TestAuthMethodGSSAPIWithMIC(t *testing.T) {
 		}
 	}
 }
+
+func TestCompatiblePublicKeyAndSignatureAlgorithms(t *testing.T) {
+	type testcase struct {
+		algo         string
+		pubKeyFormat string
+		sigFormat    string
+		compatible   bool
+	}
+
+	testcases := []*testcase{
+		{
+			KeyAlgoRSA,
+			KeyAlgoRSA,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			KeyAlgoRSA,
+			KeyAlgoRSASHA256,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			KeyAlgoRSA,
+			KeyAlgoRSASHA256,
+			KeyAlgoRSASHA512,
+			false,
+		},
+		{
+			KeyAlgoECDSA384,
+			KeyAlgoECDSA256,
+			KeyAlgoECDSA256,
+			false,
+		},
+		{
+			KeyAlgoDSA,
+			KeyAlgoRSA,
+			KeyAlgoRSA,
+			false,
+		},
+		{
+			CertAlgoRSAv01,
+			CertAlgoRSAv01,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			CertAlgoRSAv01,
+			CertAlgoRSASHA256v01,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			CertAlgoRSAv01,
+			CertAlgoRSASHA256v01,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			CertAlgoRSAv01,
+			CertAlgoRSASHA512v01,
+			KeyAlgoRSA,
+			true,
+		},
+		{
+			KeyAlgoDSA,
+			CertAlgoRSASHA512v01,
+			KeyAlgoRSA,
+			false,
+		},
+		{
+			KeyAlgoSKECDSA256,
+			CertAlgoRSASHA512v01,
+			KeyAlgoRSA,
+			false,
+		},
+	}
+
+	for _, c := range testcases {
+		if isAlgoCompatible(c.algo, c.pubKeyFormat, c.sigFormat) != c.compatible {
+			t.Errorf("algorithm %s, public key format %s, signature format %s, expected compatible to be %v", c.algo, c.pubKeyFormat, c.sigFormat, c.compatible)
+		}
+	}
+}

ssh/server.go

@@ -404,6 +404,26 @@ func (l ServerAuthError) Error() string {
 // It is returned in ServerAuthError.Errors from NewServerConn.
 var ErrNoAuth = errors.New("ssh: no auth passed yet")
 
+func isAlgoCompatible(algo string, pubKeyFormat string, sigFormat string) bool {
+	algo = underlyingAlgo(algo)
+	if algo == sigFormat {
+		return true
+	}
+
+	// Buggy SSH clients may send ssh-rsa2-512 as the public key algorithm but
+	// actually include a rsa-sha signature.
+	// According to RFC 8332 Section 3.2:
+	// A server MAY, but is not required to, accept this variant or another variant that
+	// corresponds to a good-faith implementation and is considered safe to
+	// accept.
+	compatibleAlgos := algorithmsForKeyFormat(pubKeyFormat)
+	if contains(compatibleAlgos, algo) && contains(compatibleAlgos, sigFormat) {
+		return true
+	}
+
+	return false
+}
+
 func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, error) {
 	sessionID := s.transport.getSessionID()
 	var cache pubKeyCache
@@ -576,7 +596,7 @@ userAuthLoop:
 					authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
 					break
 				}
-				if underlyingAlgo(algo) != sig.Format {
+				if !isAlgoCompatible(algo, pubKey.Type(), sig.Format) {
 					authErr = fmt.Errorf("ssh: signature %q not compatible with selected algorithm %q", sig.Format, algo)
 					break
 				}