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

X11Input: Support Xvnc #524

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

X11Input: Support Xvnc #524

wants to merge 1 commit into from

Conversation

siro20
Copy link

@siro20 siro20 commented Mar 1, 2017

Xvnc requires extended shared memory permissions.
Allows to capture screen sessions when running under Xvnc.

Tested on SuSe Linux Enterprise 12 Xvnc Server.

Signed-off-by: Patrick Rudolph siro@das-labor.org

Xvnc requires extended shared memory permissions.
Allows to capture screen sessions when running under Xvnc.

Signed-off-by: Patrick Rudolph <siro@das-labor.org>
@MaartenBaert
Copy link
Owner

MaartenBaert commented Aug 28, 2017

Huh? Is that really required? Xvnc runs under the same user as SSR, so why would SSR need to allow random other users to write to shared memory?

I'm asking because this is totally insecure and creates a serious privacy risk on shared servers (i.e. the type of environment where Xvnc would be used). At the very least this needs to be optional, disabled by default, and with a clear warning that it isn't secure (like cross-user OpenGL recording).

Edit: I now realize this is several months old, somehow I must have missed the original notification.

@siro20
Copy link
Author

siro20 commented Aug 28, 2017

It's a possible solution, not sure if there's a better one.

@MaartenBaert
Copy link
Owner

Do you know why the problem exists in the first place? Are you running Xvnc as a different user?

@siro20
Copy link
Author

siro20 commented Aug 29, 2017

Yes it runs as user vnc . Is there a xorg.conf entry to allow cross user shared memory ?

@MaartenBaert
Copy link
Owner

MaartenBaert commented Sep 2, 2017

This is not something that can be fixed in the VNC server. The fundamental problem is that there is no secure way to send data from one user to another using the SysV shared memory API. The secure way to do this (which is used by Wayland AFAIK) is to avoid SysV, and instead use /dev/shm. The procedure is as follows:

  • Create a file in /dev/shm (or other tmpfs like /run/user/1000).
  • Resize the file to the required shared memory size.
  • Use ancillary data to transfer the open file descriptor with sendmsg() to the other process (only works with UNIX-domain sockets, not TCP).
  • mmap() the file with MAP_SHARED on both sides.

This strategy has some robustness problems though. The client can resize the file while the server is trying to read it, which results in a server crash (SIGBUS) which is very hard to recover from. As of Linux 3.17 there is a solution for this based on memfd_create and F_ADD_SEALS.

The X11 protocol predates these mechanisms, so it uses SysV shared memory. This means there is no secure way to share data between two different users. So instead, you need to run the VNC server as the same user. I don't really see why this is a problem though, I've always used VNC like that. Log in with SSH while tunneling the correct VNC port, then launch the VNC server through SSH. The VNC server will detach itself from your terminal, so it will continue to run after you disconnect your SSH session.

@siro20
Copy link
Author

siro20 commented Sep 7, 2017

I'm using xinetd to launch the Xvnc on demand and present a kdm to the remote end. As at this point nobody is logged in I'm using the vnc user to run the Xvnc. The user chooses a username and password and logins as usual. I don't think that it's possible to switch the Xvnc to another user.

Would it be OK to add this insecure feature to ssr config file and disable it by default ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants