Skip to content

Commit

Permalink
ssh: relax RSA signature check in SSH_MSG_USERAUTH_REQUEST
Browse files Browse the repository at this point in the history
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
  • Loading branch information
stanhu committed Jun 7, 2023
1 parent 1622238 commit 9c9ba6a
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 1 deletion.
90 changes: 90 additions & 0 deletions ssh/client_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
22 changes: 21 additions & 1 deletion ssh/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 9c9ba6a

Please sign in to comment.