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

xpra 5.0 overrides ~/.ssh/config, legacy format conflates port & display #3778

Closed
adamhotep opened this issue Mar 1, 2023 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@adamhotep
Copy link
Contributor

adamhotep commented Mar 1, 2023

Consider a case in which the local system specifies a different user and/or port in ~/.ssh/config like this:

Host remotehost
  user remoteuser
  port 1234

Given xpra attach ssh:remotehost:51, Xpra 4.4.3 would connect to remotehost on display 51 with SSH assembling the username and port on its own. Xpra 5.0 beta tries to connect to remotehost on the default display with the client's username on port 51. I expected the old behavior. Workaround: use the new syntax with an explicit username: xpra attach ssh://remoteuser@remotehost/51

Xpra 5.0 beta's error output helped diagnose the issue:

$ xpra attach ssh:remotehost:51
…
2023-03-01 20:59:12,674 Error: SSH connection to the xpra server failed
2023-03-01 20:59:12,675  check your username, hostname, display number, firewall, etc
2023-03-01 20:59:12,675  for server: ssh://remotehost:51
2023-03-01 20:59:12,675  the command line used was:
2023-03-01 20:59:12,675  ssh -x -l localuser -p 51 -T remotehost
2023-03-01 20:59:12,686 removing unix domain socket '/run/user/1000/xpra/clients/localsystem-1234567'
$ xpra attach ssh://remotehost/51
…
localuser@remotehost's password:
^C
2023-03-01 21:04:35,076 Python/GTK3 Client got signal SIGINT
2023-03-01 21:04:35,076 exiting
2023-03-01 21:04:35,079 The SSH process has terminated with exit code -2
2023-03-01 21:04:35,079  the command line used was:
2023-03-01 21:04:35,079  ssh -x -l localuser -T remotehost
2023-03-01 21:04:35,115 removing unix domain socket '/run/user/1000/xpra/clients/habanero-1234568'
$ xpra attach ssh://remoteuser@remotehost/51
(works)

The first command uses the legacy connection string format, a command I used to run all the time. The second command updates it to use the new format. The port issue goes away but the user issue is still present. The third command explicitly states the remote user so Xpra doesn't provide the local username.

Issue 1: Xpra is unnecessarily supplying a username to SSH

This is true with either the legacy connection string format or the newer :// format.

Explicitly stating -l username overrides a username specified via SSH configuration. Don't do it unless the connection string explicitly names a user (ssh:USERNAME[:PASSWORD]@HOST:DISPLAY or ssh://USERNAME[:PASSWORD]@HOST[:SSH_PORT][/DISPLAY][?OPTIONS]). SSH will default to the current username on its own.

(Documentation bug: Since a colon is only needed given a password, the man page needs to put that colon inside the password's square brackets as I have. This affects all connection string templates in the man page.)

Issue 2: Xpra's legacy SSH connection string format favors ports over displays

(This only happens with the legacy connection string format.)

Xpra has wrongly concluded that 51 is a port number rather than a display number, but this connection string format doesn't actually support specifying a port.

The man page (identical for xpra 4.4.3 & 5.0) could be read as supporting ports in the legacy form:

For backwards compatibility, SSH mode also supports the syntax: ssh:[USERNAME[:PASSWORD]@]HOST:DISPLAY but this form does not support specifying the SSH port number. Older versions also used the form protocol:host:port, but users are encouraged to move to a more standard URI format using :// as separator.

If I read this correctly, the only way to specify a port is to use the new format, though #3599 appears to have broken that, reverting Xpra back to the protocol:host:port form.

Xpra should not extract a port with this legacy syntax.

After fixing this bug, please update the documentation to be more clear, maybe something like:

For backwards compatibility, SSH mode also supports the ssh:[USERNAME[:PASSWORD]@]HOST:DISPLAY syntax. While older versions used the form protocol:host:port, this is no longer supported. To specify a port number, you must use the standard URI format using :// as the protocol separator.

System Information

  • Server: Ubuntu 20.04.5 LTS (Focal)
  • Xpra Server: 4.4.3-r0-1 (the latest focal/main from xpra.org)
  • Client: Debian Testing (Bookworm)
  • Xpra Client: 5.0-r32781-1 (the latest unstable/main from xpra.org/beta)

(This issue was originally mentioned in #3755.)

@adamhotep adamhotep added the bug Something isn't working label Mar 1, 2023
@taranu
Copy link

taranu commented Apr 27, 2023

With previous versions I was able to connect to a server through a gateway (specified in ~/.ssh/config) like so:
xpra attach --ssh="ssh" ssh:server:display
(There was at some point a bug, possibly in a dependency or in my conda env, where it used the wrong ssh executable, hence my specifying it directly. But you can also specify ssh -vvv that way)

With the latest master, I had success like so:
xpra attach ssh:username@server
Evidently the ssh config settings for the gateway are parsed correctly except for the username, and the ssh arg is unnecessary, but somehow I didn't have to specify the display or port either despite it not being the default - is that a new-ish feature?

@totaam
Copy link
Collaborator

totaam commented Apr 28, 2023

xpra attach ssh:remotehost:51
Xpra has wrongly concluded that 51 is a port number rather than a display number, but this connection string format doesn't actually support specifying a port.

FYI: This syntax has been deprecated for a very long time.
Use protocol://[username[:password]]@host:port/display, ie: ssh://server/display.

Xpra is unnecessarily supplying a username to SSH

Sounds simple, it's not.

Since a colon is only needed given a password, the man page needs to put that colon inside the password's square brackets as I have. This affects all connection string templates in the man page.

A PR would be most welcome, otherwise I don't have time for things like this, of which there are many.

After fixing this bug, please update the documentation to be more clear, maybe something like:

The username issue perhaps, I don't see another issue to fix, just use the new syntax.
As for documentation updates, please submit PRs.

@totaam
Copy link
Collaborator

totaam commented Jul 12, 2023

Fixed in the commits above.

@totaam totaam closed this as completed Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants