feat: rust-native ssh support

[plaflamme]

This is a very basic implementation of a rust-native SSH transport (#1246). I've got as far as a successful handshake method call.

The approach used is:

• depend on russh and declare a new feature to enable it
• implement the async version of the Transport trait
• bridge futures_io AsyncRead/Write traits to Tokio's AsyncRead/Write (this was pretty involved, maybe there's a simpler way)
• trivially supports SSH Agent-based identities by iterating over all of them and trying each one

The PR lacks the following:

• tests: we could spin up a local SSH daemon or tests, but this will be pretty involved, so perhaps it can be deferred to another PR
• more auth methods: we'd want to be able to specify an ssh private key from a file or from memory
• configuration: there's no way to configure any of the many configuration options one would one to set on their SSH connection, we could wire up an (mostly empty) Option struct for now and let people add options as they need them
• error / channel updates handling: error handling is pretty trivial and I'm not quite sure if were supposed to handle certain messages in the Handler implementation, things like detecting disconnects or channel failures of some kind

[Byron]

This is just WOW! I am truly amazed by this being tackled as a contribution, and hope to have some time to take a look at the code soon.Also thanks so much for the detailed PR notes, I found them very helpful to get a first understanding on what's planned and how.

First of all, I agree that it might be best to get this to work as proof-of-concept, but also think that this means it has to be wired up with gix so it can be tested on the command-line by hand.

Regarding futures_io and tokio integration, it seems odd that this isn't already available via a crate or a feature toggle. I'd think plenty of people have a similar problem.

[plaflamme]

This is just WOW! I am truly amazed by this being tackled as a contribution, and hope to have some time to take a look at the code soon.Also thanks so much for the detailed PR notes, I found them very helpful to get a first understanding on what's planned and how.

Super happy to help. gix is a huge effort, you'll need all the help you can get!

Regarding futures_io and tokio integration, it seems odd that this isn't already available via a crate or a feature toggle. I'd think plenty of people have a similar problem.

It's available in tokio-utils, and I mostly copy-pasted the code from there, but there were complications with the ownership model:

• GitConnection requires ownership of one AsyncRead and one AsyncWrite (separately)
• This requires that we pass something like Compat(tokio_async_read) and Compat(tokio_async_write)
• This requires that russh provides an "owned" AsyncRead / AsyncWrite split, which it does here
• but, the make_reader call captures the Channel's lifetime which means we cannot call make_writer without cloning, but russh's types are not clone
• so we'd have to wrap the read/write halves in Arc<Mutex> anyway and then re-implement tokio::io::AsyncRead/Write for them
• so instead, I used the ChannelStream which is both AsyncRead and AsyncWrite and then implemented futures_io::AsyncRead/Write directly for that

I think the solution would be to have GitConnection take a single parameter that is both AsyncRead and AsyncWrite, but I'm not 100% sure that makes sense.

First of all, I agree that it might be best to get this to work as proof-of-concept, but also think that this means it has to be wired up with gix so it can be tested on the command-line by hand.

Nice, yeah I'd be happy to do that... if you can provide some guidance and I can try to tackle that.

[Byron]

Thanks for elaborating.

I think the solution would be to have GitConnection take a single parameter that is both AsyncRead and AsyncWrite, but I'm not 100% sure that makes sense.

It's generally fine to adjust the API to be more suitable. The reason it is what it is is merely that the requirements allows for it, and it was easiest to do it that way. Having just one argument there would probably shift the complexity elsewhere in the code, but it might still be preferable if mostly sync code is affected (i.e. less complexity overall).

Besides that, I don't know if anything can be done about the cargo-deny error, pulling in a 'broken' dependency without mitigation doesn't seem great. Maybe there is a Cargo feature to toggle it off?

Nice, yeah I'd be happy to do that... if you can provide some guidance and I can try to tackle that.

it looks like in order to use this transport, the caller would have to pass in a custom one to the gix::Remote, which is cumbersome and very special.
Instead, the is a system to create a connection automatically by selecting it via the scheme, so a custom scheme could be made-up that can select the new transport. That way, it should just work as PoC, without any configuration support.