Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix improper RSA key conversion from ssh_key crate #400

Merged
merged 5 commits into from
Dec 1, 2024

Conversation

EpicEric
Copy link
Contributor

rsa 0.9.7 introduces an error when using a key to sign (likely due to overeager validation during conversion). You can see the changes here: https://diff.rs/rsa/0.9.6/0.9.7/

This commit fixes the version back to 0.9.6 as a quick fix, although this may not be the desired solution.

`rsa` 0.9.7 introduces an error when using a key to sign (likely due to
overeager validation during conversion). You can see the changes here:
https://diff.rs/rsa/0.9.6/0.9.7/

This commit fixes the version back to 0.9.6 as a quick fix, although
this may not be the desired solution.
@EpicEric
Copy link
Contributor Author

EpicEric commented Dec 1, 2024

Tracking issue: RustCrypto/RSA#463

This solves signatures not working in the current version of russh_keys,
which were previously incorrect due to an upstream bug
(see RustCrypto/SSH#318)
@EpicEric
Copy link
Contributor Author

EpicEric commented Dec 1, 2024

I've updated this with a proper workaround while RustCrypto/SSH#318 is not merged.

It turns out that the issue is in ssh_key, not rsa. I've adapted this from the following code: https://github.com/RustCrypto/SSH/blob/6ae2c0850c0febb485fa53678cd622aa0e5f3db5/ssh-key/src/signature.rs#L665-L679

@EpicEric EpicEric changed the title Fix against regression from RSA crate Fix improper RSA key conversion from ssh_key crate Dec 1, 2024
@Eugeny
Copy link
Owner

Eugeny commented Dec 1, 2024

Thank you for investigating this!

@Eugeny Eugeny merged commit 2a4f451 into Eugeny:main Dec 1, 2024
6 checks passed
@@ -26,7 +26,7 @@ futures = "0.3"
hmac = "0.12"
log = "0.4"
rand = "0.8"
rsa = "0.9"
rsa = "=0.9.6"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend not pinning to 0.9.6 to make sure this issue doesn't occur in other code paths, given that you have a workaround now

@EpicEric EpicEric deleted the rsa-regression branch December 2, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants