Various development questions

[basilgello]

@totaam Let's keep this issue for various ongoing questions which don't deserve the seaprate issue instead of using email.

[basilgello]

1. Is Xpra server intentionally creating Unix domain sockets in every dir from socket-dirs?

From Xpra manual I understood that the socket is expected to be created to either all places specified by bind, and if bind is set to auto, then the socket is created in a directory specified by socket-dir or, if this option is omitted, the first element of socket_dirs (aka socket_dirs[0]).

However, if bind is set to auto, setup_local_sockets populates sockets in every directory from socket_dirs list.

If it is an intended behaviour, then manpage must be clarified. If it is a bug, the fix is trivial but I don't want to push it as PR without clear understanding of your intent first.

2. For my scenario to access containerized Xpra servers from the host (either via xpra proxy or xpra list / xpra attach) I will need the socket directory which is searched by DotXpra.socket_details but is not filled with sockets created on host by setup_local_sockets (i.e the directory for "foreign" sockets). As a workaround for previous question I overrode bind and socket-dirs in /etc/xpra/xpra.conf as follows:

bind = $XDG_RUNTIME_DIR/xpra/
socket-dirs = ~/.xpra/test

but I encountered another bunch of logic flaws.

First issue is that SysAuthenticatorBase does not honor options.socket_dirs, instead expecting it as part of conn.options:

https://github.com/Xpra-org/xpra/blob/master/xpra/server/auth/sys_auth_base.py#L67

So does get_socket_dirs: https://github.com/Xpra-org/xpra/blob/master/xpra/platform/xposix/paths.py#L136

The result is that no sockets can be found in ~/.xpra/test that I declared in configuration file. As a debugging measure, I appended ~/.xpra/test to https://github.com/Xpra-org/xpra/blob/master/xpra/platform/xposix/paths.py#L150 and Xpra proxy could find the foreign socket.

Second issue is that DotXpra does expand ~, $USERNAME etc as root even if uid and gid are not 0 (and if username is empty). for example, in get_sockpath:

https://github.com/Xpra-org/xpra/blob/master/xpra/scripts/main.py#L1191

despite uid and gid are 1000, username is None and is expanded to root. I think that if Xpra proxy server authenticated the local user, it is required to retrieve its username corresponding to authenticated uid like this:

diff -ru --color /tmp/xpra/xpra/scripts/main.py /usr/lib/python3/dist-packages/xpra/scripts/main.py
--- /tmp/xpra/xpra/scripts/main.py  2021-09-21 10:29:37.875777899 +0300
+++ /usr/lib/python3/dist-packages/xpra/scripts/main.py 2021-09-21 18:52:34.442602993 +0300
@@ -1188,11 +1188,12 @@
     sockpath = display_desc.get("socket_path")
     if not sockpath:
         #find the socket using the display:
+        uid = display_desc.get("uid", 0)
         dotxpra = DotXpra(
             display_desc.get("socket_dir"),
             display_desc.get("socket_dirs"),
-            display_desc.get("username", ""),
-            display_desc.get("uid", 0),
+            display_desc.get("username", get_username_for_uid(uid)),
+            uid,
             display_desc.get("gid", 0),
             )
         display = display_desc["display"]

and if uid, gid and username of a containerized Xpra server don't have a local equivalent, then use local authenticated user's uid, gid and username. It also may need to push proper check_uid here: https://github.com/Xpra-org/xpra/blob/master/xpra/scripts/main.py#L1200 but everything works for me without it for now.

For industrial adoption of this scenario, we need either a separate option like socket-dirs for foreign sockets destination or to modify socket_details so it can recursively scan the directory in search of sockets (and probably, the ability to specify socket prefix like xpra-<hostname>-<display-number> instead of current <hostname>-<display-number>). This is needed because bind-mount cannot mount sockets, only directories containing sockets.

[totaam]

Is Xpra server intentionally creating Unix domain sockets in every dir from socket-dirs?

Yes.

If it is an intended behaviour, then manpage must be clarified.

Where is the part that is confusing or unclear?

As for the rest, I will take a look tomorrow when my eyes line up better with my eye sockets!

[totaam]

First issue is that SysAuthenticatorBase does not honor options.socket_dirs, instead expecting it as part of conn.options

The commit above should fix that (untested). But specifying it as part of the socket is not a bad way of doing things. OTOH, something like: ---bind-tcp=0.0.0.0:14500,auth=sys:socket-dirs=/your/path

The only problem is that you can only specify a single value in socket-dirs this way because of the syntax used for bind options - this is something that I intend to change so that we can specify the same attribute multiple times with different values for each option. I am thinking of using brackets to group attributes: --bind-tcp=0.0.0.0:14500,auth=password(value=password1,prompt=password1),auth=password(value=password2,prompt=password2) Unless you have a better idea? The prompt value is shown to the client in the password dialog, allowing us to differentiate authenticators. (and eventually, differentiate authentication with the proxy from authentication with the actual server)

So does get_socket_dirs

get_socket_dirs is where we get the defaults from. It is not meant to honour any options.

Second issue is that ...

Your change looks good. do you want to send a PR or do you want me to just commit it?

and if uid, gid and username of a containerized Xpra server don't have a local equivalent, then use local authenticated user's uid, gid and username

Isn't that root in this case?

It also may need to push proper check_uid

I'm lost now..

we need either a separate option like socket-dirs for foreign sockets destination

What is "foreign sockets destination"?

or to modify socket_details so it can recursively scan the directory in search of sockets

I'm really not keen on a full recursive search. But a one-level search would be great to have: we could move the socket into the session directory, ie: /run/user/$UID/xpra/$DISPLAY_NAME/<hostname>-$DISPLAY And perhaps even drop the $DISPLAY from there since the path already has the $DISPLAY_NAME. Also, since containers can re-use the same $DISPLAY value, it would make sense to dissociate the two: you could run all your containers with the same $DISPLAY value if you wanted to, and just bind mount under a unique $DISPLAY_NAME on the host. (it's easier to ensure uniqueness from the host I think)

And the directory /run/user/$UID/xpra/ would only contain session directories. (which you can bind mount)

But initially, sockets would be created in both locations in auto mode, for backwards compatibility.

[basilgello]

The commit above should fix that (untested).

Close but no cigar :)

2021-09-23 11:44:47,017 authenticator 0: sys(user, {'exec_cwd': '/home/user', 'connection': wss socket: ::ffff:127.0.0.1, 14500, 0, 0 <- ::ffff:127.0.0.1, 44018, 0, 0, 'socket-dirs': ['/run/xpra', '/run/user/0/xpra/clients']})=PAM

where /run/xpra is from command-line (correct) and /run/user/0/xpra/clients is likely from proxy server's client side? The desired output is socket-dirs': ['/run/xpra', '/run/user/1000/xpra', '/home/user/.xpra', '/home/user/.xpra/test'] in this case.

AFAIK the former solution worked because get_socket_dirs returned a set of unexpanded default paths that were later expanded after authentication succeeded with the proper $USERNAME, $UID, $GID and $HOME / ~.

Unless you have a better idea?

Let me think of it. Currently grouping is quite intuitive.

Your change looks good. do you want to send a PR or do you want me to just commit it?

I will PR it with fallback for uid/gid/username that I mentioned and you asked if it is not the root cause

Isn't that root in this case?

It can be caused by either missing username or uid/gid/username that has no local equivalent, so we need a fallback on "proxy" side

What is "foreign sockets destination"?

I call the directory that is searchable by socket_details but local servers do not create sockets in it. So we can use this directory to place sub-directories bound-mount from containers / VMs etc.

But a one-level search would be great to have: we could move the socket into the session directory, ie:

Perfect! You got my point!!!

[basilgello]

use the unexpanded value

You were faster than I editing the comment above in re unexpanded set of values :)

[totaam]

Close but no cigar :) (..) The desired output is socket-dirs': ['/run/xpra', '/run/user/1000/xpra', '/home/user/.xpra', '/home/user/.xpra/test'] in this case.

The commit above should give you /run/user/$UID/xpra and /home/$USERNAME/.xpra which should expand to the correct path given the correct uid and username. But I'm not sure about /home/user/.xpra/test - where would test come from?

Perfect! You got my point!!!

Cool. I'll have a go at it as soon as I find the time.

[basilgello]

The commit above should give you /run/user/$UID/xpra and /home/$USERNAME/.xpra which should expand to the correct path given the correct uid and username.

Nope :(

authenticator 0: sys(user, {'exec_cwd': '/home/user', 'connection': wss socket: ::ffff:127.0.0.1, 14500, 0, 0 <- ::ffff:127.0.0.1, 44212, 0, 0, 'socket-dirs': ['/run/xpra']})=PAM

But I'm not sure about /home/user/.xpra/test - where would test come from?

I added it as the last socket-dirs in /etc/xpra/conf.d/10_network.conf: socket-dirs = ~/.xpra/test

[totaam]

@basilgello can you give me the simplest command line possible to reproduce? Something like xpra proxy --bind-tcp=... --socket-dirs=... -d auth

[basilgello]

sudo /usr/bin/xpra proxy :14500 --daemon=no --tcp-auth=sys --ssl-cert=/etc/xpra/ssl-cert.pem --ssl=on --bind-tcp=:::14500 --auth=sys --socket-dirs=/run/xpra --socket-permissions=666 --log-dir=/var/log --pidfile=/run/xpra/proxy.pid --debug=server,auth

/etc/xpra/conf.d/10_network.conf:

# Socket directories (may be specified more than once):
#socket-dirs = /tmp
#socket-dirs = ~/.xpra
#socket-dirs = /run/xpra
socket-dirs = $XDG_RUNTIME_DIR/xpra
socket-dirs = /run/xpra
socket-dirs = ~/.xpra
socket-dirs = ~/.xpra/test

# Where to create new sockets
# (otherwise the first "socket-dirs" is used)
#socket-dir = /tmp
#socket-dir = ~/.xpra

/etc/xpra/conf.d/50_server_network.conf:

# Where to create the sockets:
# (can be specified multiple times to create multiple sockets,
#  either a directory or a socket filename)
#bind=none
#bind=auto
#bind=~/.xpra/
#bind=FILENAME
#bind=/path/to/socketfilename
#bind=/run/user/$UID/xpra/
#bind = auto
bind = $XDG_RUNTIME_DIR/xpra/

[basilgello]

If it is an intended behaviour, then manpage must be clarified.

Where is the part that is confusing or unclear?

From https://github.com/Xpra-org/xpra/blob/master/fs/share/man/man1/xpra.1#L502

Location where to write and look for the Xpra socket files.
The default location varies from platform to platform
("~/.xpra" on most Posix systems).
If unspecified, the first value from \fBsocket-dirs\fP will be used.

This reads like Unix domain socket will be created only in socket-dir if explicitly specified, or first entry of socket_dirs if not specified. In fact, sockets are created in socket-dir + all socket-dirs.

[basilgello]

@totaam If command-line options like --socket-dirs override the config file and not append to them, then your latest fix is perfectly vaid and it is the command line which must be edited to remove --socket-dirs=/run/xpra:

2021-09-23 13:19:10,129 authenticator 0: sys(user, {'exec_cwd': '/tmp', 'connection': wss socket: ::ffff:127.0.0.1, 14500, 0, 0 <- ::ffff:127.0.0.1, 45212, 0, 0, 'socket-dirs': ['$XDG_RUNTIME_DIR/xpra', '/run/xpra', '~/.xpra', '~/.xpra/test']})=PAM

[totaam]

If command-line options like --socket-dirs override the config file and not append to them, then your latest fix is perfectly vaid

Yes, it overrides. Otherwise there would be no way to remove values. ie --keyboard-shortcuts=none.

[basilgello]

OK, so now the only thing to check why paths not get expanded in authenticators :)

Hm...

kwargs =  {}

printing print("kwargs = ", kwargs) here: https://github.com/Xpra-org/xpra/blob/master/xpra/server/auth/sys_auth_base.py#L68

[basilgello]

@totaam There was an easy fix:

https://github.com/Xpra-org/xpra/blob/master/xpra/server/auth/sys_auth_base.py#L217

super().__init__(username, **kwargs)

[totaam]

Good catch!

[basilgello]

Your change looks good. do you want to send a PR or do you want me to just commit it?

I refactored the patch as follows:

diff -ru --color /tmp/xpra/xpra/scripts/main.py /usr/lib/python3/dist-packages/xpra/scripts/main.py
--- /tmp/xpra/xpra/scripts/main.py  2021-09-23 14:20:56.464094928 +0300
+++ /usr/lib/python3/dist-packages/xpra/scripts/main.py 2021-09-23 14:19:22.342702599 +0300
@@ -1188,12 +1188,21 @@
     sockpath = display_desc.get("socket_path")
     if not sockpath:
         #find the socket using the display:
+        # if uid, gid or username are missing or not found on
+        # local system, use uid, gid, username of authenticated user
+        uid = display_desc.get("uid", getuid())
+        gid = display_desc.get("gid", getgid())
+        username = display_desc.get("username", get_username_for_uid(uid))
+        if not username:
+            uid = getuid()
+            gid = getgid()
+            username = get_username_for_uid(uid)
         dotxpra = DotXpra(
             display_desc.get("socket_dir"),
             display_desc.get("socket_dirs"),
-            display_desc.get("username", ""),
-            display_desc.get("uid", 0),
-            display_desc.get("gid", 0),
+            username,
+            uid,
+            gid,
             )
         display = display_desc["display"]
         def socket_details(state=DotXpra.LIVE):

so that if there is no uid, gid or username returned from remote Xpra server, we just use local ones. And we explicitly use local ones if we got everything from remote display but we can not find the same user on local system.

If you see no caveats, just commit it :)

[basilgello]

Also, again we have an issue with Firefox in HTML5 client:

TypeError: WorkerGlobalScope.createImageBitmap: 2 is not a valid argument count for any overload.

You asked me in xpra-org/xpra-html5#92 if it is related to image file itself. No, it is a Firefox issue that will be likely patched in FF93: https://bugzilla.mozilla.org/show_bug.cgi?id=1533680 and https://github.com/mrdoob/three.js/issues/21003

Till that time, I think we'll need to pass a minimal image to createImageBitmap with different syntax and catch a TypeError then try another, "foxy" syntax. Matching by user-agent is useless like pointed here: https://github.com/mrdoob/three.js/issues/21003#issuecomment-754546749

[totaam]

Also, again we have an issue with Firefox in HTML5 client:

I'm seeing this too! (I was certain I had tested with Firefox but perhaps not with png or jpeg encoding) - fixed: https://github.com/Xpra-org/xpra-html5/issues/20#issuecomment-925757769

[basilgello]

Also another minor :bug: in HTML5: despite the Menu position is Top the actual menu is placed top-left on both FF and Chrome.

Fix: xpra-org/xpra-html5#99

[totaam]

Also another minor :bug: in HTML5: despite the Menu position ...

https://github.com/Xpra-org/xpra-html5/pull/99

[basilgello]

@totaam check our email conversation with Dmitry... He is considering upload of 4.x to Debian testing but he asks if there are any other benefits of LTS tag compared to trunk...

[basilgello]

Also, why do we need --socket-dirs=/run/xpra in systemd units? AFAIK /run/xpra is in socket-dirs option inside fs/etc/xpra/con.d/10_network.conf anyway. Do you override socket-dirs here to let only one socket to be created?

Since socket activation unit (fs/lib/systemd/system/xpra.service) has --bind=none anyway, and no-socket-activation unit (fs/lib/systemd/system/xpra-nosocketactivation.service) has --bind=/run/xpra/system, this override is not necessary and even more, it breaks connection to estabilished Xpra serverswith recent commits fixing socket search.

[totaam]

check our email conversation with Dmitry..

I don't see anything. Nothing in the spam folder either.

Do you override socket-dirs here to let only one socket to be created?

Yes.

... this override is not necessary and even more, it breaks connection to estabilished Xpra servers with recent commits fixing socket search

How so?

[basilgello]

I don't see anything. Nothing in the spam folder either.

I forwarded that one to you again

How so?

First of all, specifying /run/xpra as the only socket-dirs results in https://github.com/Xpra-org/xpra/issues/3276#issuecomment-925684358

Second, in the morning I found that with recent commits AND socket-dirs overridden to /run/xpra in systemd unit, you can start Xpra seamless server from scratch (either by "Connect" or "Start" action in HTML5 client) but connecting to already running Xpra server fails because /run/user/1000/xpra/ is not on socket-dirs path. So I checked the systemd units and found that --socket-dirs can be safely dropped from both units, restoring the correct functionality.

Do you override socket-dirs here to let only one socket to be created? Yes.

As I said, our new commits made this precaution unnecessary: systemd unit for socket activation binds no sockets (--bind=none) and nosocketactivation systemd unit binds to /run/xpra/system (--bind=/run/zpra/system). And as we know, bind has precedence over socket-dir(s).

[totaam]

I forwarded that one to you again

I can see the email in the smtp log, but not the email in my mailbox! (and I can see the one I just sent from my own gmail account) Can you try just one more time and CC totaam (at gmail).

.. results in ..

That comment link is broken. Can you edit it?

.. can be safely dropped from both units, restoring the correct functionality

Sounds reasonable and I can't find a valid reason against it. (though I am feeling slightly uncomfortable - like I am missing something obvious) Can you send a PR or a patch?

[basilgello]

Can you try just one more time and CC totaam (at gmail).

Done

That comment link is broken. Can you edit it?

Done

Can you send PR as patch?

Will do :)

[basilgello]

Xpra-org/xpra#3281

[basilgello]

like I am missing something obvious

The only thing that looks like something buzzing from the back is that you will need to propagate #3281 to 4.2.x and 4.1.x, where you backported "use socket-dirs from servers in authenticators". 3.x is likely not affected by regression.

[totaam]

v4.2.x: ef6688f4e91a176ed0f9e56d956756d116f14066 (v4.1.x is EOL - I only made some commits there today by mistake)

@basilgello Why would 3.x not be affected? It has 6a464c4b9f8d8cc0618a80a9348163870a2b37a9

[basilgello]

Then 3.x is affected, too!

[totaam]

v3.1.x: b0f14d3e87b380dc654230324ec262c20f9f8bd0

I think we can close this ticket?

[basilgello]

I think we can close this ticket?

Uhm, I created this ticket like a chat to discuss questions that don't deserve a separate ticket :) Those who deserve one will be spawned into separate issues. Or you feel uncomfortable with such "chatty" issue?

[totaam]

I don't mind. I just like to close tickets as fast as I can! (in the foolish hope that one day I will reach zero open tickets)

[basilgello]

(in the foolish hope that one day I will reach zero open tickets)

"Zarro Boogs Encountered" :D

[basilgello]

Meh...

2021-09-24 19:44:22,112 Error moving the session directory
2021-09-24 19:44:22,112  from '/run/xpra' to '/run/user/1000/xpra/1'
2021-09-24 19:44:22,112  [Errno 18] Invalid cross-device link: '/run/xpra' -> '/run/user/1000/xpra/1'

and all subsequent directory operations spill errors.

Source is likely this commit: Xpra-org/xpra@e41a801c4f12cfb026af49c245a1199b0560120f

Solution is to create proto-directory, try moving it and if the smoke test fails, try next destination, or just create directory where it should be moved.

[totaam]

Source is likely this commit: e41a801

This is just fallout from #3217. The commit you pointed to is actually supposed to make it less likely that we'll choose a different session directory once we have the actual display number, by re-using the same function to get the directory path.

I don't really understand how you got different paths here. The first call to:

session_dir = make_session_dir(opts.sessions_dir, display_name, uid, gid)

Should have also used $XDG_RUNTIME_DIR/xpra/S$PID as a temporary folder until the display name was known.

---

We cannot move files across filesystems.. and copying then removing is tricky / dangerous. Especially as root.

[basilgello]

I don't really understand how you got different paths here.

Let me investigate. The issue appears only when Xpra backend server is started by Xpra proxy server.

[basilgello]

I don't really understand how you got different paths here.

First call on make_session_dir yields:

do_run_server: session_dir #1 =  /run/xpra opts.sessions_dir =  $XDG_RUNTIME_DIR/xpra

At this time probably $XDG_RUNTIME_DIR is not correctly expanded because authentication has not taken place yet (?)

Second call to get_session_dir yields:

do_run_server: session_dir #2 =  /run/user/1000/xpra/1 opts.sessions_dir =  $XDG_RUNTIME_DIR/xpra

Fix: https://github.com/Xpra-org/xpra/blob/master/xpra/scripts/server.py#L364

session_dir = get_session_dir(sessions_dir, display_name, uid)

[basilgello]

3284 fixes a data race in XDG menu sending from Xpra backend server to clients.

[basilgello]

Did you notice the fix above for directory paths? :)

[totaam]

Where?

[basilgello]

Fix: https://github.com/Xpra-org/xpra/blob/master/xpra/scripts/server.py#L364

session_dir = get_session_dir(sessions_dir, display_name, uid)

[totaam]

Oh, so it's just the uid missing. Will commit when I get back.

[basilgello]

Cool. I'll have a go at it as soon as I find the time.

I see you started doing that closing #3217 right? Ping me please when you need a test build.

Also, I think it is perfect time to convince Dmitry to upload 4.x to Debian

[totaam]

I see you started doing that closing #3217 right?

Yes, and now the sockets are also found in the session directory: 3ffd5de1e95738a61a6ce7c49582eb17224110dd. This should make it easier for you to bind mount those directories.

Ping me please when you need a test build.

Now would be a good time.

[basilgello]

xpra main error:                                                                                                       жов 15 00:01:14 buildbot xpra[1960395]: Traceback (most recent call last):                                                                                     жов 15 00:01:14 buildbot xpra[1960395]:   File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 140, in main                                        жов 15 00:01:14 buildbot xpra[1960395]:     return run_mode(script_file, cmdline, err, options, args, mode, defaults)                                          жов 15 00:01:14 buildbot xpra[1960395]:   File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 413, in run_mode                                    жов 15 00:01:14 buildbot xpra[1960395]:     return do_run_mode(script_file, cmdline, error_cb, options, args, mode, defaults)                                  жов 15 00:01:14 buildbot xpra[1960395]:   File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 443, in do_run_mode                                 жов 15 00:01:14 buildbot xpra[1960395]:     return run_server(script_file, cmdline, error_cb, options, args, mode, defaults)                                   жов 15 00:01:14 buildbot xpra[1960395]:   File "/usr/lib/python3/dist-packages/xpra/scripts/main.py", line 1743, in run_server                                 жов 15 00:01:14 buildbot xpra[1960395]:     return do_run_server(script_file, cmdline, error_cb, options, args, mode, display, defaults)                       жов 15 00:01:14 buildbot xpra[1960395]:   File "/usr/lib/python3/dist-packages/xpra/scripts/server.py", line 651, in do_run_server
жов 15 00:01:14 buildbot xpra[1960395]:     return _do_run_server(script_file, cmdline,                                                                        жов 15 00:01:14 buildbot xpra[1960395]:   File "/usr/lib/python3/dist-packages/xpra/scripts/server.py", line 887, in _do_run_server
жов 15 00:01:14 buildbot xpra[1960395]:     if x=="/run/user/%i" % uid:

s/x/xrd/

[basilgello]

OK, so podman run --rm --userns keep-id -v $HOME/.xpra/test/234:/run/user/1000/xpra/0:rw debian-xpra spawns the container accessible by xpra list. Good job!!!

Now two unrelated questions.

First: how does one list and switch between windows (Alt-Tab like behavior) in HTML5 client?

Second: I am planning to add not only proper keyboard to HTML5 client, but also mouse buttons to have long press of right/center button/scroll.

What do you think on it?

[totaam]

xpra main error

Oops. I had not tested the proxy as root... because I knew you would! (fixed in 1a0adc6)

First: how does one list and switch between windows (Alt-Tab like behavior) in HTML5 client?

The top bar is one way. Alt-Tab is more difficult because the OS will intercept this shortcut. We could use another one but it wouldn't be obvious. Or perhaps we can grab the keyboard, but this prompts the user to enable and then they can't switch away easily.

Second: I am planning to add not only proper keyboard to HTML5 client

That would be awesome!

but also mouse buttons to have long press of right/center button/scroll

I'm not sure what's missing there, can you clarify? "long press"?

[basilgello]

The top bar is one way.

Oops I thought it is screenshot maker!!!

mouse buttons emulation

Splashtop Android client has an option to spawn left, right, center button and scroll into a separate pane that can be placed for example on right border of screen. The virtual mouse buttons can "stick" emulating drag with mouse button pressed. Also it is super useful for right clicks :) Ideally, trackball mode is mandatory to have, too. CAD app users will say thank you to us ;)

[totaam]

This ticket is becoming too hard to follow, please open new issues instead.