remove the need to choose an unused DISPLAY when starting xpra

[totaam]

Issue migrated from trac ticket # 172

component: server | priority: minor | resolution: fixed

2012-08-05 15:07:38: antoine created the issue

---

I am not aware of any solution that isn't racy or just plain wrong (making assumptions about where sockets should be and who can list/access them)

What we can do however, is try to get this patch merged into Xorg so we can just make our own randomly generated socket each time (patch from lindi):

diff --git a/src/xcb_util.c b/src/xcb_util.c
index 996ff25..fb0cd3d 100644
--- a/src/xcb_util.c
+++ b/src/xcb_util.c
@@ -139,6 +139,12 @@ static int _xcb_open(char *host, char *protocol, const int display)
     char file[sizeof(base) + 20];
     int filelen;

+    if(protocol && strcmp("unix-with-path", protocol) == 0)
+    {
+        /* display specifies Unix socket with explicit path */
+        return  _xcb_open_unix("unix", host);
+    }
+
     if(*host)
     {
 #ifdef DNETCONN

[totaam]

2012-08-05 15:08:06: antoine uploaded file X11-unix-domain-socket.patch (0.5 KiB)

X11 unix-with-path protocol patch

[totaam]

2012-10-23 15:26:13: grumpfl commented

---

How does this work with the ssh -X command? Is it possible to implement something similar here?

[totaam]

2012-10-23 17:08:56: antoine commented

---

Err no, ssh -X uses your local display, and we also connect to that with the client.

What we need is a virtual display on the server end, something that ssh does not, and almost certainly will not ever do.

[totaam]

2012-10-25 14:11:07: grumpfl commented

---

Right. But when I ssh -X into another machine, the DISPLAY environment variable is set to something local, like :3 in my test case. How does the server-side sshd obtain this display number? Perhaps xpra could do just the same?

[totaam]

2012-10-25 16:25:39: antoine commented

---

No it cannot, the "fake" display number used by ssh is forwarded via the ssh tunnel to the real display on your client box. One of the most important features of xpra is its ability to detach and re-attach later, this sort of connection forwarding would preclude that.

[totaam]

2013-08-09 22:52:05: krlmlr commented

---

This is unsolved for a year now. What about picking a display number at random, initializing the random number generator

• with "truly random" data

• with a hash computed from $USER

?

Collision probability is very low, and a collision could perhaps be detected and then the next random display number is tried. In the second case (user name hash), the output of xpra list could be analyzed to avoid useless probing.

[totaam]

2013-08-10 03:42:41: totaam commented

---

This is unsolved for a year now
This is unsolved for much longer than that.

It is not as simple as you think it is.

[totaam]

2013-08-10 21:59:20: krlmlr commented

---

Well, I'm ready to learn something new.

Let's put it another way: Assume I already have an xpra session running for display :5557 on my local machine. When I do

xpra start :5557 --no-daemon

I receive the following error message:

xpra: error: You already have an xpra server running at :5557
  (did you want 'xpra upgrade'?)

Apparently, xpra internally executes the equivalent of xpra list and sees that there's another session running. Now, if we cheat xpra to use another socket-dir so that xpra list will be empty:

xpra start :5557 --no-daemon --socket-dir=/tmp/test-xpra

the output changes:

Fatal server error:
Server is already active for display 5557
        If this server is no longer running, remove /tmp/.X5557-lock
        and start again.

This is what I call a "collision" in my previous post. To me, it seems that such collisions can be detected very well, and xpra could handle them by simply trying another display.

What am I missing?

[totaam]

2013-08-10 22:35:39: krlmlr commented

---

Also, following http://stackoverflow.com/a/12169545/946850, recent X servers (1.13) support the -displayfd <fd> command line option: It will make the X server choose the display itself and write the display number back to the file descriptor <fd>.

On my Ubuntu 13.04 machine, with an active display 0, the command

Xvfb -displayfd 1

executes without error and writes 1 to stdout.

Corresponding SO answer: http://stackoverflow.com/a/18166600/946850

[totaam]

2013-08-10 22:35:39: krlmlr

[totaam]

2013-08-10 22:35:39: krlmlr

[totaam]

2013-08-11 07:26:11: antoine commented

---

Letting the Xorg server choose the display number using the

-displayfd

switch is totally the right way of doing it (the same process that creates the lock is also the one that checks it), it's a shame they haven't bothered to document it:

$ Xorg -version |& grep X.Org
X.Org X Server 1.14.2
$ Xorg -h |& grep -i displayfd | wc -l
0
$ man Xorg |& grep -i displayfd | wc -l
0

[[BR]]

---

As for the difference between the xpra socket (which is used as some sort of lock) and the Xorg lockfile, they serve slightly different purpose unfortunately. One could conceivably run an xpra server on display NN and also an xpra shadow of the same display for example.

---

Looking at the implementation hurdles, OTOH:

• command line parsing will need to change as the display value becomes optional - unless we define some sort of magic value to imply this "auto" mode
• potential problems/conflicts with --use-display? upgrade? and xpra shadow?
• we currently daemonize before we start the display - not sure how tricky that is to re-order
• we run the vfb via setsid and close_fds=True so passing the file descriptor is going to be more complicated
• man page updates, "xpra --help" updates, wiki updates
• dealing with platforms without the feature: probably via a build time switch to disable the feature (ie: CentOS 6.3 and older, Debian wheezy, Ubuntu precise, etc)
  etc..

So really, it looks doable, but I don't think I'll have the time to deal with it in the near future - so I am keeping the milestone set to "future", but feel free to submit a patch for merging sooner.

[totaam]

2013-08-11 07:26:11: antoine

[totaam]

2013-08-11 07:26:11: antoine

[totaam]

2013-08-11 15:06:30: krlmlr commented

---

Replying to [comment:9 antoine]:

it's a shame they haven't bothered to document it:
[[BR]]
They did, actually -- perhaps not everywhere where it would be relevant?

$ man Xserver |% grep displayfd
       -displayfd fd
               finding a free one, will write the display number back on this file descriptor as a newline-terminated string. \
               The -pn option is ignored when using -displayfd.

[[BR]]

Looking at the implementation hurdles, OTOH:

• command line parsing will need to change as the display value becomes optional - unless we define some sort of magic value to imply this "auto" mode
  [[BR]]
  Just a matter of checking the length of args in run_remote_server?
  [[BR]]
• potential problems/conflicts with --use-display? upgrade? and xpra shadow?
  [[BR]]
  Mutually exclusive with --use-display, of course. Should be supported with upgrade as well. Why would shadow be a problem?
  [[BR]]
• we currently daemonize before we start the display - not sure how tricky that is to re-order
  [[BR]]
  Why is this important? (On a side note: Currently, xpra will happily daemonize (and immediately terminate) when I run xpra start :0 on a graphical work station where display 0 is used.)
  [[BR]]
• we run the vfb via setsid and close_fds=True so passing the file descriptor is going to be more complicated
  [[BR]]
  Indeed.
  [[BR]]
• man page updates, "xpra --help" updates, wiki updates
• dealing with platforms without the feature: probably via a build time switch to disable the feature (ie: CentOS 6.3 and older, Debian wheezy, Ubuntu precise, etc)
  etc..

So really, it looks doable, but I don't think I'll have the time to deal with it in the near future - so I am keeping the milestone set to "future", but feel free to submit a patch for merging sooner.
[[BR]]
Currently, my racy hack works for me, and I'll need to wait until Wheezy's successor to benefit from an implementation. Still, good to know that a "proper" solution is possible after all.

[totaam]

2013-08-11 15:06:30: totaam

[totaam]

2013-08-12 07:14:16: totaam commented

---

we currently daemonize before we start the display - not sure how tricky that is to re-order
Why is this important?
[[BR]]
Because the log filename (and socket filename) contain the DISPLAY value in them, and it is created when we daemonize.

[totaam]

2013-08-12 07:14:16: totaam

[totaam]

2013-11-15 14:10:31: totaam changed status from new to assigned

[totaam]

2013-11-15 14:10:31: totaam changed owner from antoine to totaam

[totaam]

2013-11-15 14:10:31: totaam

[totaam]

2013-11-15 14:10:31: totaam commented

---

Seems worth doing, will try for 0.12

[totaam]

2014-03-16 09:07:15: totaam

[totaam]

2014-03-16 09:07:15: totaam commented

---

Too late for this release, will try for the next one, sorry.

[totaam]

2014-05-09 15:19:43: totaam

[totaam]

2014-05-09 15:19:43: totaam commented

---

out of time...

[totaam]

2014-05-24 13:11:45: gschwind uploaded file 0001-enable-xpra-to-select-display-automaticaly.patch (33.3 KiB)

patch enabling selection of DISPLAY automaticaly

[totaam]

2014-05-24 14:27:46: gschwind uploaded file 0002-make-xpra-backward-compatible.patch (6.4 KiB)

make xpra backward compatible

[totaam]

2014-05-26 12:22:37: totaam changed status from assigned to new

[totaam]

2014-05-26 12:22:37: totaam changed owner from totaam to gschwind

[totaam]

2014-06-14 14:14:07: gschwind commented

---

for ssh string maybe :

• ssh:host:port:
  => select display automatically

• ssh:host:display
• ssh:host:port:display

I do not like this syntax but at less it resolve the issue

[totaam]

2014-06-14 16:54:41: gschwind uploaded file 0003-draft-of-template-file-build-and-install.patch (5.9 KiB)

[totaam]

2014-06-14 16:57:06: gschwind commented

---

This last patch is a draft of a build and install template.

It may be usefull to submit a proper build_tpl and install_tpl module to setuptools but I'm not an expert of setuptools

[totaam]

2014-06-16 09:53:37: totaam commented

---

Awesome stuff! So this is how you override the build or install steps (which I could not figure out how to do).

Merged in r6814 with the following tweaks:

• merged recent changes to xpra.conf
• simplified xpra.conf, tried to separate client and server parts, and added more comments to it, also removed client only config (rarely ever used anyway)
• detect_xorg_setup returns a boolean, transformed into a string later instead
• removed xpra_conf = None - did nothing
• removed shutil import in opengl codepath (now global import)
• lowered requirement for displayfd support to Xorg server version 1.12 onwards (tested with Fedora 17 / Xorg version 1.12)
• fixed install dir (/etc/xpra not /etc/
• use os.path.join rather than +

The remaining TODO items are nice-to-have, but not showstoppers.

[totaam]

2014-06-17 03:25:16: totaam commented

---

bsd path fix in r6816.

[totaam]

2014-06-17 14:18:31: totaam commented

---

build fixes for rpmbuild / debuild in r6823.

[totaam]

2014-06-20 10:45:27: totaam commented

---

(ugly) build fix for win32 in r6853

[totaam]

2014-07-10 22:33:24: totaam changed status from new to closed

[totaam]

2014-07-10 22:33:24: totaam changed resolution from ** to fixed

[totaam]

2014-07-10 22:33:24: totaam commented

---

This will do for this release. Moving the ssh bits to #612.

[totaam]

2014-07-13 17:06:36: totaam commented

---

The bug that keeps on giving: I needed r6895 to get the xpra.conf installed into /etc/xpra/ when doing a simple: sudo ./setup.py install

[totaam]

2014-08-29 07:01:40: totaam commented

---

Yet another bug: Debian Wheezy ships with Xorg 1.12, but does not support -displayfd. We cannot assume Xdummy support implies displayfd support.

[totaam]

2014-11-20 02:18:01: antoine commented

---

Yet another missing case: r8126, backport in 8129.

[totaam]

2014-11-20 02:18:01: totaam

[totaam]

2015-05-18 05:18:17: antoine commented

---

Yet another missing case: r9401, backport in 9402.

[totaam]

2015-06-02 15:54:10: antoine commented

---

Forgot to fix Debian Wheezy, now done in 9574.

Note: we just bump the version check to Xorg 1.13, Fedora supported it with 1.12 but that must have been using some patching. And that Fedora version is EOL, so we don't care.

[totaam]

2016-05-02 06:03:14: antoine commented

---

This has been broken by the 1.18.1 xorg update due to a bug in their new automatic log file renaming which duplicates what we already do, but gets it wrong - see #1192.

[totaam]

2016-08-30 05:36:12: antoine commented

---

r13502 removes the displayfd option since all the distros we support have it (also simplifies the code)

[totaam]

2018-11-12 11:41:17: antoine commented

---

From the ticket that keeps on giving, another bug: #2032.

[totaam]

2020-11-16 09:59:07: antoine commented

---

8 years later and it turns out that we were 100% correct: Allow display names to be arbitrary paths: This lets the local listening socket live in a protected directory where other users cannot spoof it.