Skip to content

Commit 4abd197

Browse files
committed
ssh: fix certificate and public key authentication with older clients
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
1 parent 0ff6005 commit 4abd197

File tree

2 files changed

+122
-1
lines changed

2 files changed

+122
-1
lines changed

ssh/client_auth_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,3 +955,91 @@ func TestAuthMethodGSSAPIWithMIC(t *testing.T) {
955955
}
956956
}
957957
}
958+
959+
func TestCompatibleAlgoAndSignatures(t *testing.T) {
960+
type testcase struct {
961+
algo string
962+
sigFormat string
963+
compatible bool
964+
}
965+
testcases := []*testcase{
966+
{
967+
KeyAlgoRSA,
968+
KeyAlgoRSA,
969+
true,
970+
},
971+
{
972+
KeyAlgoRSA,
973+
KeyAlgoRSASHA256,
974+
false,
975+
},
976+
{
977+
KeyAlgoRSA,
978+
KeyAlgoRSASHA512,
979+
false,
980+
},
981+
{
982+
KeyAlgoRSASHA256,
983+
KeyAlgoRSA,
984+
true,
985+
},
986+
{
987+
KeyAlgoRSASHA512,
988+
KeyAlgoRSA,
989+
true,
990+
},
991+
{
992+
KeyAlgoRSASHA512,
993+
KeyAlgoRSASHA256,
994+
false,
995+
},
996+
{
997+
KeyAlgoRSASHA256,
998+
KeyAlgoRSASHA512,
999+
false,
1000+
},
1001+
{
1002+
KeyAlgoRSASHA512,
1003+
KeyAlgoRSASHA512,
1004+
true,
1005+
},
1006+
{
1007+
CertAlgoRSAv01,
1008+
KeyAlgoRSA,
1009+
true,
1010+
},
1011+
{
1012+
CertAlgoRSAv01,
1013+
KeyAlgoRSASHA256,
1014+
true,
1015+
},
1016+
{
1017+
CertAlgoRSAv01,
1018+
KeyAlgoRSASHA512,
1019+
true,
1020+
},
1021+
{
1022+
CertAlgoRSASHA256v01,
1023+
KeyAlgoRSASHA512,
1024+
false,
1025+
},
1026+
{
1027+
CertAlgoRSASHA512v01,
1028+
KeyAlgoRSASHA512,
1029+
true,
1030+
},
1031+
{
1032+
CertAlgoRSASHA512v01,
1033+
KeyAlgoRSASHA256,
1034+
false,
1035+
},
1036+
}
1037+
1038+
for _, c := range testcases {
1039+
if isAlgoCompatible(c.algo, c.sigFormat) != c.compatible {
1040+
t.Errorf("algorithm %q, signature format %q, expected compatible to be %t", c.algo, c.sigFormat, c.compatible)
1041+
}
1042+
1043+
}
1044+
1045+
}

ssh/server.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,28 @@ func gssExchangeToken(gssapiConfig *GSSAPIWithMICConfig, firstToken []byte, s *c
370370
return authErr, perms, nil
371371
}
372372

373+
// isAlgoCompatible check if the signature format is
374+
// compatible with the selected algorithm taking into account
375+
// edge cases that occur with old clients
376+
func isAlgoCompatible(algo, sigFormat string) bool {
377+
if underlyingAlgo(algo) == sigFormat {
378+
return true
379+
}
380+
rsaCompatibleAlgos := []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}
381+
// For certificate authentication with OpenSSH 7.2-7.7
382+
// signature format can be rsa-sha2-256 or rsa-sha2-512
383+
// for the algorithm [email protected]
384+
if algo == CertAlgoRSAv01 {
385+
return contains(rsaCompatibleAlgos, sigFormat)
386+
}
387+
// With gpg-agent < 2.2.6 the algorithm can be rsa-sha2-256
388+
// or rsa-sha2-512 for signature format ssh-rsa
389+
if sigFormat == KeyAlgoRSA {
390+
return contains(rsaCompatibleAlgos, algo)
391+
}
392+
return false
393+
}
394+
373395
// ServerAuthError represents server authentication errors and is
374396
// sometimes returned by NewServerConn. It appends any authentication
375397
// errors that may occur, and is returned if all of the authentication
@@ -567,7 +589,18 @@ userAuthLoop:
567589
authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
568590
break
569591
}
570-
if underlyingAlgo(algo) != sig.Format {
592+
// Ensure the declared public key algo is compatible
593+
// with the decoded one.
594+
// This check will ensure we don't accept e.g.
595+
// [email protected] algorithm with ssh-rsa
596+
// public key type.
597+
// The algorithm and public key type must be consistent:
598+
// both must point to a certificate or none
599+
if !contains(algorithmsForKeyFormat(pubKey.Type()), algo) {
600+
authErr = fmt.Errorf("ssh: type mismatch for decoded key, received %q, expected %q", pubKey.Type(), algo)
601+
break
602+
}
603+
if !isAlgoCompatible(algo, sig.Format) {
571604
authErr = fmt.Errorf("ssh: signature %q not compatible with selected algorithm %q", sig.Format, algo)
572605
break
573606
}

0 commit comments

Comments
 (0)