Regression in key conversion in 0.9.7

[EpicEric]

TL;DR Regression pertaining to 2edd479.

With the 0.9.7 release, this created breakage when converting from an RsaKeypair to a SigningKey. More specifically, in the context of ssh_key::Signer::try_sign, which has caused tests with a dependency on rsa = "0.9" to fail in russh due to this call. I've tried refreshing the key used for tests, but it still results in an error due to this change.

The only fix I found was to pin the dependency on rsa to version 0.9.6, as seen in Eugeny/russh#400.

[dignifiedquire]

This sounds like you are trying to sign using an invalid key, or we have an underlying bug in key validation. Do you have a key that can reproduce the issue?

[EpicEric]

Here's a minimal reproduction using a PKCS8 encrypted key: https://github.com/EpicEric/rsa-regression-example

As it shows, RsaPrivateKey::from_components() succeeds, but converting to an ssh_key::KeypairData and then trying to sign fails when it shouldn't.

[dignifiedquire]

Thanks for the repo, I just looked into it, and the failure happens in the key conversion, and not in actual signature generation here:

https://github.com/RustCrypto/SSH/blob/ssh-key/v0.6.7/ssh-key/src/signature.rs#L668

let data = rsa::pkcs1v15::SigningKey::<Sha512>::try_from(self)?

I don't know yet if the key itself is invalid, or there is an issue in the conversion.

[EpicEric]

I've generated a new key with the following command, and the conversion error persists:

openssl genrsa -aes128 -passout pass:blabla 3072

[dignifiedquire]

What a labyring of traits..but I found the bug that was revealed by the validation: https://github.com/RustCrypto/SSH/blob/ssh-key/v0.6.7/ssh-key/src/private/rsa.rs#L201-L202

  rsa::BigUint::try_from(&key.private.p)?,
  rsa::BigUint::try_from(&key.private.p)?,

This is passing the p prime twice, instead of p and q creating an invalid key. Before this was left silently and invalid signatures were generated.