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

Built-in SSH transport #1246

Open
Tracked by #106
Byron opened this issue Jan 12, 2024 · 8 comments
Open
Tracked by #106

Built-in SSH transport #1246

Byron opened this issue Jan 12, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@Byron
Copy link
Member

Byron commented Jan 12, 2024

Summary 💡

A built-in transport that allows to manually configure an SSH connection without the need for the ssh program.

Candidates for the ssh part are:

  • russh - Rust
    • even with limited support for ssh-agent, which makes it something I'd seriously look at
  • ssh2 - Rust bindings to libssh2
  • libssh2 - C library
    • used by libgit2
    • desirable license, but different feature set, maybe less mature than libssh
  • libssh - C library
    • Maybe more mature than libssh2, but has undesirable license

Motivation 🔦

Cargo should be standalone, and right now it needs the ssh-program to clone ssh:// URLs. Even though this is beneficial on Linux, it's usually not very portable and often won't work at all on Windows. git2 has ssh support built-in using libssh which works, but has a less desirable license than libssh2.

References

Suggestions

  • Try to add an actual test using the existing ssh transport to create a connection to a local (and controlled) ssh daemon. This could be very useful when testing a new implementation.

Requirement

  • Needs a feature toggle, and added dependencies are optional (but pulled in by the feature)
@Byron Byron added the enhancement New feature or request label Jan 12, 2024
@Byron Byron mentioned this issue Jan 12, 2024
47 tasks
@joshtriplett
Copy link
Contributor

@Byron libssh2 is what libgit2 uses, so I'd expect it to be sufficiently mature.

ssh2 appears to just be bindings to libssh2.

So it seems like the choice is between libssh2 (mature and used elsewhere) or russh (pure Rust).

@NobodyXu
Copy link
Contributor

So it seems like the choice is between libssh2 (mature and used elsewhere) or russh (pure Rust).

Maybe gitoxide can add new features for:

  • using the external ssh cmd
  • using libssh2 (and linked statically?)
  • using russh

so that cargo can always use libssh2 as dylib, while other who want to avoid dynamic dep on external C lib can either use external ssh cmd or use russh?

@Byron
Copy link
Member Author

Byron commented Jan 14, 2024

Thanks @joshtriplett for pointing that out - the issue has been updated for correctness, and I agree about the choice, particularly if it's true that libssh doesn't have Rust bindings yet.

so that cargo can always use libssh2 as dylib, while other who want to avoid dynamic dep on external C lib can either use external ssh cmd or use russh?

There could definitely be multiple implementations, even though I don't know how much effort it is to integrate them correctly (usually, how much configuration git applies or libgit2 allows to pass so gitoxide would have to match it for Cargo-compatibility).
It was always my thought that the current ssh transport (based on the ssh program) can remain as basis that is the default on linux (as per the gitoxide.ssh.transport configuration or something like that), and is changed to a built-in implementation in Windows by default while still allowing the user to change it to a built-in variant provided it was compiled in.

@NobodyXu
Copy link
Contributor

even though I don't know how much effort it is to integrate them correctly

I think the libssh2 is definitely the easier one to implement given that cargo also uses it.

russh might be missing some configuration/support for some ciphers, though I think it would still be great to add support for it, I'm hoping for a mature ssh implementation in (pure) Rust.

It was always my thought that the current ssh transport (based on the ssh program) can remain as basis that is the default on linux (as per the gitoxide.ssh.transport configuration or something like that), and is changed to a built-in implementation in Windows by default while still allowing the user to change it to a built-in variant provided it was compiled in.

I agree, though AFAIK Windows also provides ssh from a certain windows 10 version.

I would definitely want them to be gated behind feature flags since for cargo-binstall I might choose to use external ssh cmd only.

@Byron Byron mentioned this issue Apr 11, 2024
56 tasks
@NobodyXu
Copy link
Contributor

NobodyXu commented Apr 11, 2024

russh currently requires openssl for RSA key support, which is a bit unfortunate since it takes away ability to use pure-rust alternatives such as ring or rust-crypto.

On the plus side, there's async-ssh2-tokio, a high level wrapper for russh, providing an async API compatible with tokio.

BTW, one thing I always desire is the support of async in high-level API, so that:

  • I can easily cancel a task without using an AtomicBool for cancellation
  • can reuse my existing reqwest::Client, which contains a conn pool, and the tokio runtime
  • easier to use in async functions, so I don't have to use tokio::task::spawn_blocking or tokio::task::block_in_place when using gix

I understand why it is structured as is, because using async can be painful, its future size returned might be huge and IIRC compiler can't return huge future without copying yet (there's some missed optimization opportunities) and async-trait is still not supported very well, plus using async would immediately cause all high-level API to switch to async.

And most of the operation is synchronous for now (filesystem operations) and it's only fetching that uses network operations.

Though in the future - with the io-uring being adopted by runtime, it would also help reducing I/O for gitoxide, especially if the repository is large.

Tokio is already working on it in crate tokio-uring and they might eventually use it as a backend in tokio tokio-rs/tokio#2411

@darleybarreto
Copy link

Hey folks, I think russh supports a pure rust RSA now after #273

@NobodyXu
Copy link
Contributor

NobodyXu commented May 5, 2024

That's good news! it's time to start adopting russh in gitoxide once 0.44 comes out.

@EliahKagan
Copy link
Member

I just noticed that russh 0.44.0 has been released today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants