Improve agent forwarding example and documentation
While libssh2 supports all the functionality needed for agent forwarding, example/ssh2_agent_forwarding.c does not actually implement this. See https://github.com/libssh2/libssh2/issues/535#issuecomment-2337341551.
This PR adds full agent forwarding functionality to the aforementioned example. The following is an example interaction with the example:
$ ./build/example/ssh2_agent_forwarding 127.0.0.1 kian
Fingerprint: [redacted]
Authentication methods: publickey,password,keyboard-interactive
Authentication with username kian and public key kian@Bagel succeeded.
Last login: Fri Dec 20 14:37:00 2024 from 127.0.0.1
(kian@Bagel) ~ $ TERM=xterm-256color
(kian@Bagel) ~ $ echo $SSH_AUTH_SOCK
/tmp/ssh-uAPYxqbFo3/agent.80484
(kian@Bagel) ~ $ ssh-add -l
256 SHA256:p4OdIQcnebPG7h1VndvkYIGpp6OE4XY+Irb97Vyugg4 kian@Bagel (ED25519)
(kian@Bagel) ~ $
I've also added a paragraph to the libssh2_channel_request_auth_agent man page to mention that the callback is needed for full functionality.
Both the example and the man page addition are heavily inspired/copied from the existing X11 example and man pages.
This needs conflict resolution before it can be merged.
This needs conflict resolution before it can be merged.
Rebased on master.
The patch makes this example require sys/un.h, which makes it non-portable
to systems without this header.
Can this proposal be updated to add the new functionality while retaining compatibility with all platforms libssh2 supports?
The patch makes this example require
sys/un.h, which makes it non-portable to systems without this header.Can this proposal be updated to add the new functionality while retaining compatibility with all platforms libssh2 supports?
Not easily. The SSH agent, with which this example communicates, listens on a UNIX domain socket (typically). See man ssh-agent:
-a bind_address
Bind the agent to the UNIX-domain socket bind_address. The
default is $TMPDIR/ssh-XXXXXXXXXX/agent.<ppid>.
On Windows, it seems like (from a quick Google search) it uses something else, but I don't have a Windows system to develop/test on. I would need someone else to contribute that logic.
Also, is there a documented list of what platforms libssh2 supports? I didn't see one on the website. I'm wondering if there are platforms other than Windows that don't have UNIX domain sockets which you had in mind. If so, those might be easy enough for me to adapt the example to support.
And FWIW, example/x11.c also relies on sys/un.h.
For supported platforms the CI is a good overview. In general libssh2 aims to be compatible with all major platforms and a broad range of Unixes. Also in general removing support for platforms is something to avoid.
X11 was always *nix specific, because X11 is.
This example worked for platforms without sys/un.h. It's also CI tested for
major platforms, and IMO would be important to keep it, by making sure that
the portable functionality remains enabled and builds cleanly in CI after this
change.
I think I understand what you mean now. Thanks for the clarification.
I removed the requirement on sys/un.h (and termios.h), and it appears to still pass CI for all non-Windows platforms, so the UNIX side of things should be good to go.
I spent some time trying to adapt the example to build on Windows as well by making changes and waiting for the CI results. It's proven slow and unproductive, so is there anyone who would be willing to add the Windows support to this example?
And regarding the last two commits: I'll rebase those down into the original commits before merging. Just leaving them as separate for now since they were trial-and-error attempts to get Windows working, and I wanted them to be easy to remove.
@kdkasad Are you still working on this update?
@kdkasad Are you still working on this update?
No; I don't have the ability to test/develop for Windows.
IMO, this update is still worth having even without Windows support (I can add back the #ifdef so it compiles). The current example claims to demonstrate agent forwarding but actually doesn't. This one at least does demonstrate agent forwarding, just doesn't support Windows.
The issue is not the lack of Windows support in the new code (adapted from x11.c), but that it makes an existing, portable, and confirmed working example into a non-portable one.
Maybe an incremental update to the existing example would be more approachable and may make it easier to keep it portable? Also to understand what are the improvements made?
Since we're stuck with this, I'd prefer not merging the example update in this form.
The documentation update looks sensible and merging that part.
If someone has a better idea or a different approach to improve the example, please feel free to chime in.
Thanks!
The issue is not the lack of Windows support in the new code (adapted from x11.c), but that it makes an existing, portable, and confirmed working example into a non-portable one.
I disagree with the "confirmed working." In my eyes, it does not work. It is supposed to be an example of SSH agent forwarding, but it does not perform SSH agent forwarding.
Also to understand what are the improvements made?
The issue with the current example is that it calls the libssh2 function to establish SSH forwarding, which tells the remote server to forward SSH agent communication on the remote end to the local libssh2 client. However, that forwarded data is never handled.
I'll use the X11 example as reference. There are two important parts. The first is enabling X11 forwarding, which is done with this function call: https://github.com/libssh2/libssh2/blob/30203f167b055206bbc76b62501981f01cbd165d/example/x11.c#L390-L398
The second important part is ~150 lines of code which handle receiving the X11 data from the remote server, passing it to the local X11 server, and passing the response back:
https://github.com/libssh2/libssh2/blob/30203f167b055206bbc76b62501981f01cbd165d/example/x11.c#L115-268
The current agent forwarding example is missing this second part. It has no logic to forward requests from the remote server to the local SSH agent and send the responses back. That is required for agent forwarding to work. The example is quite misleading, because it's called ssh2_agent_forwarding.c yet it is missing the actual forwarding logic.
Here is what I get when I run the current example. The ssh-add -l command should list the agent's keys, but instead it fails. The echo hi line is to show that other commands' outputs are returned properly.
$ ssh -A [email protected] "ssh-add -l"
256 SHA256:Ljp58PPBJbJVAFFYFKQC2fR3+PK2zeXAQ5FjuBO4hSQ kian@... (ED25519)
$ ./build/example/ssh2_agent_forwarding 127.0.0.1 kian "ssh-add -l"
Authentication with username kian and public key kian@... succeeded.
Agent forwarding request succeeded.
libssh2_channel_read returned -13
EXIT: 0 bytecount: 0
all done
$ ./build/example/ssh2_agent_forwarding 127.0.0.1 kian "echo hi"
Authentication with username kian and public key kian@... succeeded.
Agent forwarding request succeeded.
We read:
hi
libssh2_channel_read returned 0
EXIT: 0 bytecount: 3
all done
I'm not 100% sure why it returns an libssh2 error instead of just an empty response; I believe the reason is that the agent forwarding is requested on a channel and then the command execution is requested on the same channel. In either case, agent forwarding doesn't work.
The documentation update looks sensible and merging that part.
Thanks! 🎉
Since we're stuck with this, I'd prefer not merging the example update in this form.
I understand. In that case, can we either:
- Add an issue that mentions this example being incomplete and link to this PR as a starting point for others who might want to attempt to fix it, or
- Add this as an additional example? It's quite hard to actually find information on how to make agent forwarding work with libssh2, and having a working example would be a great help, even if not fully cross-platform.
I disagree with the "confirmed working." In my eyes, it does not work. It is supposed to be an example of SSH agent forwarding, but it does not perform SSH agent forwarding.
Assessments (none mine) in the original PR: https://github.com/libssh2/libssh2/pull/219#issuecomment-428649948
According to this thread, the example worked. Maybe it didn't do everything you expected, but that's a different thing.
The issue with the current example is that it calls the libssh2 function to establish SSH forwarding, which tells the remote server to forward SSH agent communication on the remote end to the local libssh2 client. However, that forwarded data is never handled.
I'll use the X11 example as reference. There are two important parts. The first is enabling X11 forwarding, which is done with this function call:
https://github.com/libssh2/libssh2/blob/30203f167b055206bbc76b62501981f01cbd165d/example/x11.c#L390-L398
The second important part is ~150 lines of code which handle receiving the X11 data from the remote server, passing it to the local X11 server, and passing the response back: https://github.com/libssh2/libssh2/blob/30203f167b055206bbc76b62501981f01cbd165d/example/x11.c#L115-268 The current agent forwarding example is missing this second part. It has no logic to forward requests from the remote server to the local SSH agent and send the responses back. That is required for agent forwarding to work. The example is quite misleading, because it's called
ssh2_agent_forwarding.cyet it is missing the actual forwarding logic.
Is there anything inherently platform-specific in these missing parts?
I'm not 100% sure why it returns an libssh2 error instead of just an empty response; I believe the reason is that the agent forwarding is requested on a channel and then the command execution is requested on the same channel. In either case, agent forwarding doesn't work.
Maybe this could be fixed as a starting point?
- Add an issue that mentions this example being incomplete and link to this PR as a starting point for others who might want to attempt to fix it, or
- Add this as an additional example? It's quite hard to actually find information on how to make agent forwarding work with libssh2, and having a working example would be a great help, even if not fully cross-platform.
Adding an extra non-portable example still adds non-portable code, along with more confusion and duplicate the code to maintain. I don't think it helps.
If you'd like, feel free to open a Discussion about this, maybe to collect feedback and have a place to record snippets and other useful information.