ssh: add support for unpadded RSA signatures
The original SSH RFC 4253 explicitly disallows padding. This applies to
ssh-rsa signatures.
The updated SSH RFC 8332 which defines the SHA2 RSA signature variants
explicitly calls out the existence of signers who produce short
signatures and specifies that verifiers may allow this behavior.
In practice, PuTTY 0.81 and prior versions, as well as SSH.NET prior to
2024.1.0 always generated short signatures. Furthermore, PuTTY is
embedded in other software like WinSCP and FileZilla, which are updated
on their own schedules as well. This leads to occasional unexplained
login errors, when using RSA keys.
OpenSSH server allows these short signatures for all RSA algorithms.
Fixes golang/go#68286
Change-Id: Ia60ece21bf9c111c490fac0c066443ed5ff7dd29
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/598534
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/ssh/keys.go b/ssh/keys.go
index 7967665..98e6706 100644
--- a/ssh/keys.go
+++ b/ssh/keys.go
@@ -488,7 +488,49 @@
h := hash.New()
h.Write(data)
digest := h.Sum(nil)
- return rsa.VerifyPKCS1v15((*rsa.PublicKey)(r), hash, digest, sig.Blob)
+
+ // Signatures in PKCS1v15 must match the key's modulus in
+ // length. However with SSH, some signers provide RSA
+ // signatures which are missing the MSB 0's of the bignum
+ // represented. With ssh-rsa signatures, this is encouraged by
+ // the spec (even though e.g. OpenSSH will give the full
+ // length unconditionally). With rsa-sha2-* signatures, the
+ // verifier is allowed to support these, even though they are
+ // out of spec. See RFC 4253 Section 6.6 for ssh-rsa and RFC
+ // 8332 Section 3 for rsa-sha2-* details.
+ //
+ // In practice:
+ // * OpenSSH always allows "short" signatures:
+ // https://github.com/openssh/openssh-portable/blob/V_9_8_P1/ssh-rsa.c#L526
+ // but always generates padded signatures:
+ // https://github.com/openssh/openssh-portable/blob/V_9_8_P1/ssh-rsa.c#L439
+ //
+ // * PuTTY versions 0.81 and earlier will generate short
+ // signatures for all RSA signature variants. Note that
+ // PuTTY is embedded in other software, such as WinSCP and
+ // FileZilla. At the time of writing, a patch has been
+ // applied to PuTTY to generate padded signatures for
+ // rsa-sha2-*, but not yet released:
+ // https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a5bcf3d384e1bf15a51a6923c3724cbbee022d8e
+ //
+ // * SSH.NET versions 2024.0.0 and earlier will generate short
+ // signatures for all RSA signature variants, fixed in 2024.1.0:
+ // https://github.com/sshnet/SSH.NET/releases/tag/2024.1.0
+ //
+ // As a result, we pad these up to the key size by inserting
+ // leading 0's.
+ //
+ // Note that support for short signatures with rsa-sha2-* may
+ // be removed in the future due to such signatures not being
+ // allowed by the spec.
+ blob := sig.Blob
+ keySize := (*rsa.PublicKey)(r).Size()
+ if len(blob) < keySize {
+ padded := make([]byte, keySize)
+ copy(padded[keySize-len(blob):], blob)
+ blob = padded
+ }
+ return rsa.VerifyPKCS1v15((*rsa.PublicKey)(r), hash, digest, blob)
}
func (r *rsaPublicKey) CryptoPublicKey() crypto.PublicKey {
diff --git a/ssh/keys_test.go b/ssh/keys_test.go
index 7b14429..7d5b86f 100644
--- a/ssh/keys_test.go
+++ b/ssh/keys_test.go
@@ -154,6 +154,44 @@
}
}
+func TestKeySignWithShortSignature(t *testing.T) {
+ signer := testSigners["rsa"].(AlgorithmSigner)
+ pub := signer.PublicKey()
+ // Note: data obtained by empirically trying until a result
+ // starting with 0 appeared
+ tests := []struct {
+ algorithm string
+ data []byte
+ }{
+ {
+ algorithm: KeyAlgoRSA,
+ data: []byte("sign me92"),
+ },
+ {
+ algorithm: KeyAlgoRSASHA256,
+ data: []byte("sign me294"),
+ },
+ {
+ algorithm: KeyAlgoRSASHA512,
+ data: []byte("sign me60"),
+ },
+ }
+
+ for _, tt := range tests {
+ sig, err := signer.SignWithAlgorithm(rand.Reader, tt.data, tt.algorithm)
+ if err != nil {
+ t.Fatalf("Sign(%T): %v", signer, err)
+ }
+ if sig.Blob[0] != 0 {
+ t.Errorf("%s: Expected signature with a leading 0", tt.algorithm)
+ }
+ sig.Blob = sig.Blob[1:]
+ if err := pub.Verify(tt.data, sig); err != nil {
+ t.Errorf("publicKey.Verify(%s): %v", tt.algorithm, err)
+ }
+ }
+}
+
func TestParseRSAPrivateKey(t *testing.T) {
key := testPrivateKeys["rsa"]