Support SSH agent forwarding

[totaam]

Issue migrated from trac ticket # 2303

component: server | priority: minor

2019-05-17 20:03:20: erikjensen created the issue

---

I use a hardware token for SSH access. It would be great if Xpra could forward requests to the SSH agent on the client.

This would probably involve creating a socket and setting SSH_AUTH_SOCK in the Xpra session (only if the feature is turned on), and then forwarding connections to the socket to the client, which would in turn forward them to whatever SSH_AUTH_SOCK was set to, locally.

[totaam]

Can be added using paramiko agent.

@erikjensen: in the meantime, you can use --ssh=ssh to switch back to the openssh backend which supports agent forwarding by default.

[totaam]

paramiko rocks, adding agent forwarding support is as simple as adding adding AgentRequestHandler(session).

This should be an option, off by default, but we should honour ssh-config.
Do we want to overload the ssh=paramiko command line option for that? Maybe add a -A / -a switch to it, just like openssh?

[totaam]

2019-05-31 10:04:16: antoine uploaded file ssh-agent.patch (5.6 KiB)

try to start the AgentRequestHandler

[totaam]

According to the documentation, the code in the patch above should do the job but I can't see anything in the environment that would tell applications about the forwarded agent. (no SSH_AUTH_SOCK)

So I'm out of ideas.

[totaam]

I must have tested wrong. With the two commits above, one can enable agent forwarding with paramiko.
Either with:

XPRA_SSH_AGENT=1 xpra start ssh://host/ --start=xterm

or

xpra start --ssh=paramiko:agent=on ssh://host/ --start=xterm

---

One important caveat though: this feature works for ssh start (ie: the command lines above), but not for xpra attach because the environment for the session has already been created in that case..
We could find a way to get the ssh proxy command to communicate the agent socket path to the xpra server it connects to (adding an --env=SSH_AGENT= command line argument and somehow getting it sent to the server before we execute any new --start= arguments!) , but by that point the commands that the server runs have already been started anyway.
So this is probably not worth the effort.

[Martin-Buchholz]

The way to support ssh agent forwarding is via the symlink-to-socket technique:

• at xpra server start time, if ssh has provided a socket via $SSH_AUTH_SOCK, then create a symlink to $SSH_AUTH_SOCK in a well known location like $XDG_RUNTIME_DIR/xpra/$display_num/ssh_auth_sock and modify $SSH_AUTH_SOCK in the environment to point to that symlink
• when client ssh disconnects, this will be a dangling symlink, but nothing we can do about that
• when client re-attaches or auto-reconnects, fix up the symlink to point to the new $SSH_AUTH_SOCK, if one was provided
• a super-high-quality implementation could try to properly handle the tricky situation when multiple clients attach, e.g. direct requests to the agent that sent the most recent key or mouse events ...

I'm pretty sure the symlink-to-socket technique will work because I've seen it used successfully with other persistent remote access programs, and because I've sort-of implemented the same with xpra by wrapping programs I start with a script that fiddles with SSH_AUTH_SOCK and updates the symlink, and by keeping a separate ssh connection running that provides the ssh agent.
I would have put the symlink into $XDG_RUNTIME_DIR/xpra/$display_num/ but xpra doesn't like "foreign" files put into this directory.

It occurs to me I could try to symlink to the $SSH_AUTH_SOCK used by the current xpra session, but I don't know how to identify which socket is being used by which xpra server. A problem that xpra doesn't have!

[Martin-Buchholz]

It occurred to me that I could leverage xpra's --remote-xpra flag to implement the symlink-to-socket technique.
Seems to work well.
But this is a workaround - xpra should support this directly

#!/bin/bash
# Use this xpra wrapper script with xpra's --remote-xpra flag

set -euo pipefail

# We depend on xpra implementation details to extract the display from argv
[[ "$1" =~ ^_proxy && "$2" =~ ^:([0-9]+)$ ]] || {
  echo "FIXME: Can't parse xpra command line; format has changed:" "$@" >&2
  exit 1
}

# Use the SSH_AUTH_SOCK persistent symlink technique, if available
if [[ -n "${SSH_AUTH_SOCK-}" ]]; then
  readonly DISPLAY_NUM="${BASH_REMATCH[1]}"
  readonly PERSISTENT_SSH_AUTH_SOCK="$HOME/.xpra/ssh-agent-${HOSTNAME}-${DISPLAY_NUM}"
  ln -snf "$SSH_AUTH_SOCK" "$PERSISTENT_SSH_AUTH_SOCK"
  export SSH_AUTH_SOCK="$PERSISTENT_SSH_AUTH_SOCK"
fi

# Hand execution over to stock xpra
exec xpra "$@"
echo "Can't exec xpra" "$@" >&2
exit 1

[totaam]

@Martin-Buchholz Excellent idea!

The commit above works for local connections, even without using ssh.

Caveats:

• we only setup the ssh agent for interactive clients for now (ie: not for clients like xpra top) - not sure if this should be changed

TODO:

• the server should revert the symlink to point to another client or to the agent.default symlink (if it exists) when a client exits
• delete the symlink when xpra _proxy exits? we have no way of knowing if the ssh connection actually terminated with the xpra _proxy command (ssh connection sharing will keep it alive) - but we this symlink is bound to the ssh session
• changing dynamically to the last user to produce UI events should be easy since we have all the plumbing, but it's not: we normally never ever do IO from the GUI thread (even just changing a symlink), but in this case we have to set the correct symlink before sending the keyboard or pointer event.. (and bouncing to another thread would just be terribly messy)
• update the builtin ssh server, which does its own strict command line parsing and knows nothing about this new proxy argument

[totaam]

I forgot to test with other / older client and caused a regression: 8c52d48

[Martin-Buchholz]

Thanks!

This prompted me to finally create one of those new-fangled openssh keys that are partially stored on my yubikey, and check that indeed, those can be forwarded via ssh agent forwarding, including using my temporary but effective workaround.

I'm not quite ready to become a real xpra developer and build my own binaries - I'll wait for an upcoming release.

[totaam]

Starting a session on :10 from a terminal which already has its own ssh agent running:

 ls -la /run/user/1000/xpra/10/ssh/
total 0
drwx------. 2 antoine antoine  60 Jul 22 05:18 .
drwxr-x---. 3 antoine antoine 220 Jul 22 05:18 ..
lrwxrwxrwx. 1 antoine antoine  26 Jul 22 05:18 agent.default -> /run/user/1000/keyring/ssh

Then after connecting via ssh:

$ ls -la /run/user/1000/xpra/10/ssh/
total 0
drwx------. 2 antoine antoine 100 Jul 22 05:22 .
drwxr-x---. 3 antoine antoine 220 Jul 22 05:18 ..
lrwxrwxrwx. 1 antoine antoine  64 Jul 22 05:22 agent -> fca7cd0c711214f519f8003c8b109a91219b876ceeec934f4e3721668601c4a8
lrwxrwxrwx. 1 antoine antoine  26 Jul 22 05:18 agent.default -> /run/user/1000/keyring/ssh
lrwxrwxrwx. 1 antoine antoine  32 Jul 22 05:22 fca7cd0c711214f519f8003c8b109a91219b876ceeec934f4e3721668601c4a8 -> /tmp/ssh-XXXXcNuKn1/agent.873195

delete the symlink when xpra _proxy exits?

I tried to add code to ensure that the dangling ssh agent socket symlink is removed before the connection is closed, but the xpra _proxy script seems to get forcibly closed with the ssh channel, before we can do anything about it.

---

Im not quite ready to become a real xpra developer and build my own binaries - I'll wait for an upcoming release.

There are beta builds here: https://xpra.org/beta

Building should be easy: https://github.com/Xpra-org/xpra/tree/master/docs/Build

[Martin-Buchholz]

You talked me into it. I boldly upgraded to
xpra v4.4-r31563 (gf192c0312) beta
and reverted my --remote-xpra workaround.
My first session worked perfectly, but after shutdown, the second session failed with SSH_AUTH_SOCK undefined.

My test command was

xpra start --start-child='xterm' --exit-with-children --ssh='/usr/bin/ssh -A -Y' ssh://lempel/51

then exit xterm, then repeat the above,
My diagnosis is that the xpra session did not shut down completely.

 $ (cd /run/user/$UID/xpra/51 && find)
.
./ssh
./ssh/agent
./ssh/agent.default

After shutdown, that directory should be deleted.

In main.py, I see:

        #files we can remove safely:
        KNOWN_SERVER_FILES = [
            "cmdline", "config",
            "dbus.env", "dbus.pid",
            "server.env", "server.pid", "server.log",
            "socket", "xauthority", "Xorg.log", "xvfb.pid",
            "pulseaudio.pid",
            ]
        KNOWN_SERVER_DIRS = [
            "pulse",
            ]

If we're very lucky, all we need to do is add ssh to KNOWN_SERVER_DIRS

[Martin-Buchholz]

If we're very lucky, all we need to do is add ssh to KNOWN_SERVER_DIRS

No, we're not that lucky. A few lines further down I see

                    os.rmdir(pathname)
            except OSError as e:
                error("Error removing %r: %s" % (pathname, e))

and that will fail because ssh is non-empty

[totaam]

My first session worked perfectly, but after shutdown, the second session failed with SSH_AUTH_SOCK undefined.

What is the exact error and backtrace?
This issue with session files not getting removed should now be fixed but I didn't think it was causing any real problems and I would much prefer fixing the bug you saw rather than relying on session dirs being cleaned up properly.

[Martin-Buchholz]

The reason I believe that fixing removal of session files is key is because manually removing them fixes it!

(cd /run/user/$UID/xpra && rm -rf 51)

The first successful session ends with

2022-07-24 08:54:14,792 server requested disconnect:
2022-07-24 08:54:14,793  server shutdown

while the second session with unset SSH_AUTH_SOCK additionally prints in red

2022-07-24 09:06:12,816 The SSH process has terminated with exit code 0
2022-07-24 09:06:12,816  the command line used was:
2022-07-24 09:06:12,816  /usr/bin/ssh -A -Y -T lempel

What is the red-ness trying to tell me? I stared at ssh.py:abort_test for a while, but not enlightened.

[totaam]

while the second session with unset SSH_AUTH_SOCK additionally prints in red

What do you mean exactly by "unset SSH_AUTH_SOCK"?
I've tried unsetting it and re-connecting and I'm not seeing any difference.

What is the red-ness trying to tell me?

Red is for errors.

/usr/bin/ssh -A -Y -T lempel

Ah, so you're using the ssh backend rather than the paramiko default.
Adding -d ssh will show extra debug statements.

abort_test

This can be triggered in a number of ways, it's the end result rather than the cause.

[Martin-Buchholz]

What do you mean exactly by "unset SSH_AUTH_SOCK"?

I mean that SSH_AUTH_SOCK is not defined in the environment in the shell inside the xterm. See below:

experiment A:

( host=lempel display=51; set -eux; ssh $host "rm -rf /run/user/\$UID/xpra/$display"; xpra start --start-child='xterm' --exit-with-children --ssh='/usr/bin/ssh -A -Y' -d ssh ssh://$host/$display; )
...
debug enabled for xpra.scripts.main / ('ssh',)
setup_proxy_ssh_socket(['/usr/bin/xpra', '_proxy_start', ':51', '--ssh=/usr/bin/ssh -A -Y', '--debug=ssh', '--exit-with-children=yes', '--start-child=xterm'], '/tmp/ssh-XXXXTeICcE/agent.2622440'
setup_proxy_ssh_socket invalid XPRA_SESSION_DIR='/run/user/1000/xpra/socket:///run/user/1000/xpra/lempel-51'

Snippet from started xterm:

$ ssh-add -l | wc -l; env | grep SSH_
3
SSH_CLIENT=10.0.1.8 57504 22
SSH_CONNECTION=10.0.1.8 57504 10.0.1.169 22
SSH_AUTH_SOCK=/run/user/1000/xpra/51/ssh/agent

follow-on experiment B:

( host=lempel display=51; set -eux; xpra start --start-child='xterm' --exit-with-children --ssh='/usr/bin/ssh -A -Y' -d ssh ssh://$host/$display; )
...
debug enabled for xpra.scripts.main / ('ssh',)
setup_proxy_ssh_socket(['/usr/bin/xpra', '_proxy_start', ':51', '--ssh=/usr/bin/ssh -A -Y', '--debug=ssh', '--exit-with-children=yes', '--start-child=xterm'], '/tmp/ssh-XXXXCLFYkc/agent.2622675'
setup_proxy_ssh_socket invalid XPRA_SESSION_DIR='/run/user/1000/xpra/socket:///run/user/1000/xpra/51/socket'

Snippet from started xterm:

$ ssh-add -l | wc -l; env | grep SSH_
Could not open a connection to your authentication agent.
0
SSH_CLIENT=10.0.1.8 57506 22
SSH_CONNECTION=10.0.1.8 57506 10.0.1.169 22

Snippet from server.log:

2022-07-24 12:44:31,267 debug enabled for xpra.server.server_base / ('server', 'ssh')
2022-07-24 12:44:31,267 get_ssh_agent_path(a5d83456f0aa1e7cfa0b3e06d29684b8f1390007be2bd8cc6a41ac54715c325e)=/run/user/1000/xpra/51/ssh/a5d83456f0aa1e7cfa0b3e06d29684b8f1390007be2bd8cc6a41ac54715c325e
2022-07-24 12:44:31,267 client supplied ssh-auth-sock=/tmp/ssh-XXXXXXnjmJhC/agent.2517

I'm surprised by

client supplied ssh-auth-sock=/tmp/ssh-XXXXXXnjmJhC/agent.2517

That's the path on the client host, not the server host. server should not care about the path on the client, and the two paths to SSH_AUTH_SOCK are not expected to match.

[totaam]

That's the path on the client host, not the server host. server should not care about the path on the client, and the two paths to SSH_AUTH_SOCK are not expected to match.

That's used to make things work with other connections, like local socket connections.
In this case, the server needs to point the auth socket to what the client specifies but does not have the luxury of being started by an xpra _proxy script to set this up.
That's what I meant in #2303 (comment) when I said: The commit above works for local connections, even without using ssh

Anyway, in both cases the log shows:

setup_proxy_ssh_socket invalid XPRA_SESSION_DIR='/run/user/1000/xpra/socket:///run/user/1000/xpra/lempel-51'

And this SESSION_DIR is completely mangled. I'll need to figure out why.

[Martin-Buchholz]

I tried a different experiment - creating a server session as before, then killing the client (via Ctrl-C), then re-attaching using

( host=lempel display=51; set -eux; xpra attach --ssh='/usr/bin/ssh -A -Y' -d ssh ssh://$host/$display; )

and that worked well - i.e. the ssh agent is working in the xterm via the new socket connection

---

I noticed more than once that my xterm would stop accepting keyboard input (key events with no effect) when left idle for a long time (e.g. one hour), while mouse input continues to work fine. Probably an independent bug?

[totaam]

Probably an independent bug?

Yes, please file a separate ticket for that.
Please run both the client and server with -d keyboard,focus to see where the events get lost.
It could also be a focus issue. Does re-connecting help? Does upgrading the server help? (xpra upgrade in place)

[Martin-Buchholz]

After today's auto-update to 4.4, I noticed this:
(testing by connecting to my own machine)

xpra start  "ssh://$(uname -n)/:78" '--ssh=/usr/bin/ssh -Y -A' '--start-child=xterm' --exit-with-children=yes

This succeeds, and in the xterm I have my ssh agent, as I can see by ssh-add -l and SSH_AUTH_SOCK.

If I then terminate the session by exiting from the xterm, and then retry the exact same xpra command, then in the resulting xterm I do NOT have my agent

$ ssh-add -l; env | grep SSH
Could not open a connection to your authentication agent.
SSH_CLIENT=192.168.1.84 38432 22
SSH_CONNECTION=192.168.1.84 38432 192.168.1.84 22

I poked around and discovered lingering session config files that should probably have been cleaned up. If I delete that directory, ssh agent forwarding works again

$ ( cd /run/user/$(id -u)/xpra/78/ && find ); rm -rf /run/user/$(id -u)/xpra/78/
.
./ssh
./ssh/agent
./ssh/agent.default

[totaam]

Sorry about that, there were still a few bugs, especially with the --ssh=ssh backend you are using.
The commits above should fix that, updated beta builds will land shortly and this will be included in 4.4.1

BTW, best to drop : from the display in ssh URLs, ie: ssh://localhost/78 instead of ssh://localhost/:78.

[Martin-Buchholz]

BTW, best to drop : from the display in ssh URLs, ie: ssh://localhost/78 instead of ssh://localhost/:78.

I changed my local scripts to do that. I probably included the colon to be more consistent with the documented ssh://[USER@]HOST/DISPLAY since the DISPLAY environment variable includes the colon

[totaam]

since the DISPLAY environment variable includes the colon

Right. And it is also in the documentation...
Changing this to "the $DISPLAY without the leading :" would make the documentation unreadable. So for now, it still works.

[totaam]

This feature is much improved in v5, see changes in #3593

A major source of problems was #3664 (comment)

[totaam]

See also #4353