Skip to content

Commit

Permalink
ssh: fix certificate and public key authentication with older clients
Browse files Browse the repository at this point in the history
After adding support for rsa-sha2-256/512 on server side some edge
cases started to arise with old clients:

1) public key authentication with gpg-agent < 2.2.6 fails because we receive
   ssh-rsa as signature format and rsa-sha2-256 or rsa-sha2-512 as algorithm.
   This is a bug in gpg-agent fixed in this commit:

   gpg/gnupg@80b775b

2) certificate authentication fails with OpenSSH 7.2-7.7 because we receive
   [email protected] as algorithm and rsa-sha2-256 or rsa-sha2-512
   as signature format.

This is a more scoped version of CL 412854, with this patch we only allow
the edge cases observed and not any possible variant.

This patch is tested with every version of OpenSSH from 7.1 to 7.9.

Fixes golang/go#53391
  • Loading branch information
drakkan committed Jun 24, 2023
1 parent 0ff6005 commit 4abd197
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 1 deletion.
88 changes: 88 additions & 0 deletions ssh/client_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

}

}
35 changes: 34 additions & 1 deletion ssh/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 4abd197

Please sign in to comment.