diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index 35b62e3311..cf917375cb 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -955,3 +955,93 @@ 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, + CertAlgoRSAv01, + KeyAlgoRSASHA512, + 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) + } + } +} diff --git a/ssh/server.go b/ssh/server.go index 9e3870292f..f8eeb0d4de 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -395,6 +395,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(underlyingAlgo(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 @@ -567,7 +587,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 }