libssh-rs icon indicating copy to clipboard operation
libssh-rs copied to clipboard

Add binding for channel_open_request_auth_agent_callback

Open Riatre opened this issue 2 months ago • 1 comments

This callback is required for implementing ssh agent forward as unlike X11 forward, there is no other way to establish a forwarding channel.

The API design looks slightly convoluted, it's because in libssh:

  1. Callback is triggered while handling protocol packets in other libssh call.
  2. The callback creates a new channel and prepare for bidirectional forwarding between it and ssh agent.
  3. The callback then returns a borrow of the newly created channel for libssh to make reply to the remote side.

To do 3 we have to somehow steal a struct ssh_channel* from the user-owned channel. We decided to do so by create channel in the binding code, keep a ref and move it to user. Due to locking issues we have to take the Channel back if the user decided to not accept forward request. See SATEFY comment in bridge_channel_open_request_auth_agent_callback for details.

Riatre avatar Apr 27 '24 11:04 Riatre

I have a draft for using this to implement ssh agent forward in wezterm: https://github.com/wez/wezterm/pull/5345

Riatre avatar Apr 27 '24 11:04 Riatre

Thanks for the review! I've addressed most comments, but have further comments on the safety discussion.

~~In addition, I wonder why do we need std::panic::catch_unwind in bridge_*_callback-s? I just followed suit here.~~

~~Edit: nomicon mentioned:~~

If you are writing Rust code that may panic, and you don't wish to abort the process if it panics, you must use catch_unwind

~~But we don't have UB here even if we don't catch it: panic will be turned into abort in extern "C" functions. My question holds: do we need to eat the panic and return error code instead of bring down the entire process?~~

Edit 2: nvm, c_unwind is not in stable yet.

Riatre avatar May 06 '24 15:05 Riatre

Thank you!

wez avatar May 08 '24 12:05 wez

Published to crates.io as 0.3.1

wez avatar May 08 '24 12:05 wez

I'd also love to see agent forwarding in WezTerm, but small side-note that the Windows OpenSSH agent (and specifically ssh-add) is broken currently for users of SSH certificates.

tomtastic avatar May 09 '24 13:05 tomtastic