Skip to content

Commit

Permalink
ssh: fix RSA certificate and public key authentication with older cli…
Browse files Browse the repository at this point in the history
…ents

After adding support for rsa-sha2-256/512 on the 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 patch is based on CL 412854 and has been tested with every version
of OpenSSH from 7.1 to 7.9 and OpenSSH 9.3.

Fixes golang/go#53391

Change-Id: Id71f596f73d84efb5c76d6d5388432cccad3e3b1
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/506835
Auto-Submit: Filippo Valsorda <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
Run-TryBot: Filippo Valsorda <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
  • Loading branch information
drakkan authored and gopherbot committed Jul 10, 2023
1 parent f1db026 commit bf4db5a
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 1 deletion.
121 changes: 121 additions & 0 deletions ssh/client_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -955,3 +955,124 @@ 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,
true,
},
{
KeyAlgoRSA,
KeyAlgoRSASHA512,
true,
},
{
KeyAlgoRSASHA256,
KeyAlgoRSA,
true,
},
{
KeyAlgoRSASHA512,
KeyAlgoRSA,
true,
},
{
KeyAlgoRSASHA512,
KeyAlgoRSASHA256,
true,
},
{
KeyAlgoRSASHA256,
KeyAlgoRSASHA512,
true,
},
{
KeyAlgoRSASHA512,
KeyAlgoRSASHA512,
true,
},
{
CertAlgoRSAv01,
KeyAlgoRSA,
true,
},
{
CertAlgoRSAv01,
KeyAlgoRSASHA256,
true,
},
{
CertAlgoRSAv01,
KeyAlgoRSASHA512,
true,
},
{
CertAlgoRSASHA256v01,
KeyAlgoRSASHA512,
true,
},
{
CertAlgoRSASHA512v01,
KeyAlgoRSASHA512,
true,
},
{
CertAlgoRSASHA512v01,
KeyAlgoRSASHA256,
true,
},
{
CertAlgoRSASHA256v01,
CertAlgoRSAv01,
true,
},
{
CertAlgoRSAv01,
CertAlgoRSASHA512v01,
true,
},
{
KeyAlgoECDSA256,
KeyAlgoRSA,
false,
},
{
KeyAlgoECDSA256,
KeyAlgoECDSA521,
false,
},
{
KeyAlgoECDSA256,
KeyAlgoECDSA256,
true,
},
{
KeyAlgoECDSA256,
KeyAlgoED25519,
false,
},
{
KeyAlgoED25519,
KeyAlgoED25519,
true,
},
}

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)
}
}
}
7 changes: 7 additions & 0 deletions ssh/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ func algorithmsForKeyFormat(keyFormat string) []string {
}
}

// isRSA returns whether algo is a supported RSA algorithm, including certificate
// algorithms.
func isRSA(algo string) bool {
algos := algorithmsForKeyFormat(KeyAlgoRSA)
return contains(algos, underlyingAlgo(algo))
}

// supportedPubKeyAuthAlgos specifies the supported client public key
// authentication algorithms. Note that this doesn't include certificate types
// since those use the underlying algorithm. This list is sent to the client if
Expand Down
21 changes: 20 additions & 1 deletion ssh/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,25 @@ func gssExchangeToken(gssapiConfig *GSSAPIWithMICConfig, firstToken []byte, s *c
return authErr, perms, nil
}

// isAlgoCompatible checks 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 {
// Compatibility for old clients.
//
// 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].
//
// With gpg-agent < 2.2.6 the algorithm can be rsa-sha2-256 or rsa-sha2-512
// for signature format ssh-rsa.
if isRSA(algo) && isRSA(sigFormat) {
return true
}
// Standard case: the underlying algorithm must match the signature format.
return underlyingAlgo(algo) == sigFormat
}

// 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 +586,7 @@ userAuthLoop:
authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
break
}
if underlyingAlgo(algo) != sig.Format {
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 bf4db5a

Please sign in to comment.