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>

Author: imirkin

ssh/keys.go

@@ -488,7 +488,49 @@ func (r *rsaPublicKey) Verify(data []byte, sig *Signature) error {
 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 {

ssh/keys_test.go

@@ -154,6 +154,44 @@ func TestKeySignWithAlgorithmVerify(t *testing.T) {
 }
 }
 
+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"]