-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
Consider replacing OpenSSL with pure-rust alternative #225
Comments
Looking at the existing pull-requests, it's somewhat the opposite of #52, that wants to rely only on OpenSSL cryptography. I think having both options would solve opposite requirements (those wanting a pure-Rust implementation, and those wanting OpenSSL). However, implementing both, I think would complicate the code a bit, as it would litter the code with Perhaps, a sub-crate should be created, say Interestingly, comment #50 (comment) on #50, states that |
Yep I'd like to integrate rust-crypto's RSA impl as well and have it as the default set. Cipher modules are actually fairly pluggable in the way how russh is structured so it'd just be a single |
@Eugeny although I don't promise anything (my "hobby budget" is already in the red), if I have the time I'll try to see if maybe I can provide a pull request. One question though: should I remove the need for OpenSSL, or should pure-Rust RSA implementation be an alternative to OpenSSL? (If there isn't a good motive to keep OpenSSL in, I would suggest removing it, as it would keep the code simpler.) |
That would be amazing! I want to keep openSSL in, as currently russh is used in VSCode where Microsoft maintains their own fork that uses OpenSSL RSA and also adds OpenSSL AES on top of russh for FIPS compliance. To add native rsa, you'd basically need to add alternative |
I'm accustomed with conditional compilation (many of my Rust projects heavily use this to keep the code "flexible"). And it's from that experience that I know that too many features might lead to heavy to follow code.
I was looking for suitable pure Rust alternatives. Would one of the following be acceptable?
|
I wasn't trying to be condescending, just pointing out the Using |
Oh, don't worry, I didn't interpret it as such (i.e. condescending). :) Perhaps my initial reply was a bit too blunt in this regard (as is usually the case with written internet communication). With regard the places where Also, if you have any other suggestions (especially with regard to code style and other requirements), please let me know. Although I expect the changes to be quite minimal and in a similar style to what already exists in the current code.
OK, I'll try to use the |
I considered using |
Hey @cipriancraciun, would you like some help here? I have some spare time this week and more in the next, happy to help :) |
@darleybarreto thanks for the offer, but unfortunately I got swamped in other tasks / projects, and this remained at the bottom of the stack. Let me check my schedule and I'll let you know. Would you be available for a call (Meet / Discord / etc.) so we can perhaps try pair-programming on this one? |
Thanks for you availability! I was thinking something more asynchronous, i.e., I would open a draft PR and you could follow the progress and review it in your own time. What do you think? |
@darleybarreto, you can create a patch and open a pull-request, and I (or anyone following this issue) could take a look. However, I can' promise anything. :) (Also, since I'm not the maintainer of this repository, it also depends on the maintainers to finally approve and merge it.) In essence I think we need to create two patches:
|
Yes, openssl is already optional (a feature enabled by default) |
I opened a PR so you can check it out, but some tests are failing. |
I have an separate implementation ported from my other fork of thrussh. See this commit: robertabcd@a63401f |
* SSH encoding of RSA keys is moved into `protocol` module. * Decouple the SSH encoding into traits. * When `openssl` feature is not enabled, the pure-rust RSA impl is used. Alternative implementation for #225
Released in 0.44 |
At the moment, from what I gather, the only requirement for the OpenSSL dependency is to support the RSA algorithms, mainly in the
russh-keys
crate, and a bit inrussh
itself.However, the main issue with OpenSSL is cross-compilation, as for example in my own project the most cross compilation issues I had was related with the OpenSSL library.
Thus, it would be nice to either replace OpenSSL with a pure Rust implementation, or have it as an alternative. For example RusTLS depends on the https://crates.io/crates/rsa crate which is written in pure Rust.
Now, given that for Ed25519 keys this project uses Rust native implementations, my assumption is that OpenSSL was an initial dependency, and thus it remains to this day until the code in question is rewritten for an alternative.
The text was updated successfully, but these errors were encountered: