Skip to content

Commit 64e0e99

Browse files
drakkangopherbot
authored andcommitted
ssh: fix RSA certificate and public key authentication with older clients
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]>
1 parent 23b1b90 commit 64e0e99

File tree

3 files changed

+148
-1
lines changed

3 files changed

+148
-1
lines changed

ssh/client_auth_test.go

+121
Original file line numberDiff line numberDiff line change
@@ -955,3 +955,124 @@ 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+
true,
975+
},
976+
{
977+
KeyAlgoRSA,
978+
KeyAlgoRSASHA512,
979+
true,
980+
},
981+
{
982+
KeyAlgoRSASHA256,
983+
KeyAlgoRSA,
984+
true,
985+
},
986+
{
987+
KeyAlgoRSASHA512,
988+
KeyAlgoRSA,
989+
true,
990+
},
991+
{
992+
KeyAlgoRSASHA512,
993+
KeyAlgoRSASHA256,
994+
true,
995+
},
996+
{
997+
KeyAlgoRSASHA256,
998+
KeyAlgoRSASHA512,
999+
true,
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+
true,
1025+
},
1026+
{
1027+
CertAlgoRSASHA512v01,
1028+
KeyAlgoRSASHA512,
1029+
true,
1030+
},
1031+
{
1032+
CertAlgoRSASHA512v01,
1033+
KeyAlgoRSASHA256,
1034+
true,
1035+
},
1036+
{
1037+
CertAlgoRSASHA256v01,
1038+
CertAlgoRSAv01,
1039+
true,
1040+
},
1041+
{
1042+
CertAlgoRSAv01,
1043+
CertAlgoRSASHA512v01,
1044+
true,
1045+
},
1046+
{
1047+
KeyAlgoECDSA256,
1048+
KeyAlgoRSA,
1049+
false,
1050+
},
1051+
{
1052+
KeyAlgoECDSA256,
1053+
KeyAlgoECDSA521,
1054+
false,
1055+
},
1056+
{
1057+
KeyAlgoECDSA256,
1058+
KeyAlgoECDSA256,
1059+
true,
1060+
},
1061+
{
1062+
KeyAlgoECDSA256,
1063+
KeyAlgoED25519,
1064+
false,
1065+
},
1066+
{
1067+
KeyAlgoED25519,
1068+
KeyAlgoED25519,
1069+
true,
1070+
},
1071+
}
1072+
1073+
for _, c := range testcases {
1074+
if isAlgoCompatible(c.algo, c.sigFormat) != c.compatible {
1075+
t.Errorf("algorithm %q, signature format %q, expected compatible to be %t", c.algo, c.sigFormat, c.compatible)
1076+
}
1077+
}
1078+
}

ssh/common.go

+7
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ func algorithmsForKeyFormat(keyFormat string) []string {
119119
}
120120
}
121121

122+
// isRSA returns whether algo is a supported RSA algorithm, including certificate
123+
// algorithms.
124+
func isRSA(algo string) bool {
125+
algos := algorithmsForKeyFormat(KeyAlgoRSA)
126+
return contains(algos, underlyingAlgo(algo))
127+
}
128+
122129
// supportedPubKeyAuthAlgos specifies the supported client public key
123130
// authentication algorithms. Note that this doesn't include certificate types
124131
// since those use the underlying algorithm. This list is sent to the client if

ssh/server.go

+20-1
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,25 @@ func gssExchangeToken(gssapiConfig *GSSAPIWithMICConfig, firstToken []byte, s *c
370370
return authErr, perms, nil
371371
}
372372

373+
// isAlgoCompatible checks if the signature format is compatible with the
374+
// selected algorithm taking into account edge cases that occur with old
375+
// clients.
376+
func isAlgoCompatible(algo, sigFormat string) bool {
377+
// Compatibility for old clients.
378+
//
379+
// For certificate authentication with OpenSSH 7.2-7.7 signature format can
380+
// be rsa-sha2-256 or rsa-sha2-512 for the algorithm
381+
382+
//
383+
// With gpg-agent < 2.2.6 the algorithm can be rsa-sha2-256 or rsa-sha2-512
384+
// for signature format ssh-rsa.
385+
if isRSA(algo) && isRSA(sigFormat) {
386+
return true
387+
}
388+
// Standard case: the underlying algorithm must match the signature format.
389+
return underlyingAlgo(algo) == sigFormat
390+
}
391+
373392
// ServerAuthError represents server authentication errors and is
374393
// sometimes returned by NewServerConn. It appends any authentication
375394
// errors that may occur, and is returned if all of the authentication
@@ -567,7 +586,7 @@ userAuthLoop:
567586
authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
568587
break
569588
}
570-
if underlyingAlgo(algo) != sig.Format {
589+
if !isAlgoCompatible(algo, sig.Format) {
571590
authErr = fmt.Errorf("ssh: signature %q not compatible with selected algorithm %q", sig.Format, algo)
572591
break
573592
}

0 commit comments

Comments
 (0)