Add option allowing to connect only the user owning the running session

[grulja]

Checks, whether the user who is trying to authenticate is already logged into the running session in order to allow or reject the connection. This is expected to be used with 'plain' security type in combination with 'PlainUsers=*' option allowing everyone to connect to the session.

[grulja]

Marking this as a draft as I would like to get some feedback on this first. This feature has been requested by our customer. Their workflow is to configure GDM to accept XDMCP in /etc/gdm/custom.conf:

[xdmcp]
Enable=true

And create and start a systemd unit file as /etc/systemd/system/xvnc.service with:

[Unit]
Description=VNC XDMCP Daemon

[Service]
ExecStart=-/usr/bin/Xvnc -query localhost -once -Log *:stderr:100 rfbport=5900 securitytypes=TLSPlain PlainUsers=* pam_service=gdm-password AlwaysShared=1
User=root
StandardError=syslog

[Install]
WantedBy=multi-user.target

This now allows everyone to connect and log in GDM, however, they want to have an option to only authenticate the same user who logged in through GDM.

There are some things I'm not sure about:

1. Name of this option
2. Where this is now implemented. I think it would make more sense to be implemented in XServerDesktop (maybe), but this is tricky, because it uses Makefile and I don't know how to add this optional systemd support. From there I could also get the display number and also use sd_session_get_display() for an additional check.

[CendioOssman]

I'm afraid I don't quite understand the user case. Even more so given the fact that modern systems (of which Red Hat has been a driving force) don't work properly if you are logged in locally and via TigerVNC at the same time.

I would more have expected a patch doing the opposite, blocking users that are already logged in elsewhere.

Can you explain more why the customer wants this policy, and what they expect to actually happen?

[grulja]

I probably failed at explanation. The workflow is:

1. Let's say we have a system with users vnc1 and vnc2.
2. Configure the system as mentioned above
3. Connect with vncviewer -SecurityTypes TLSPlain server
4. Authenticate as user vnc1 and log into e.g. GNOME session using GDM
5. Connect again with vncviewer -SecurityTypes TLSPlain server
6. Try to authenticate as user vnc2, where this user will be able to connect and see the session of user vnc1.

The goal is to have an option that will not allow user vnc2 to be connected (as a viewer) if there is already a different user logged into a graphical session.

[CendioOssman]

Ah, this is not an inetd setup. That's usually where XDMCP is used.

That complicates things.

I don't think the proposed implementation scales. You'd be blocked by any other user login, right? Ideally, we should check the status of this session only.

Does the XDMCP-protocol provide any feedback we might use?

[grulja]

I don't think the proposed implementation scales. You'd be blocked by any other user login, right? Ideally, we should check the status of this session only.

Right, any other running session would be also blocking you.

Does the XDMCP-protocol provide any feedback we might use?

I don't know, I will look and ask around. That's why I mentioned use of sd_session_get_display() which we could use to check this session only, but I don't know how to get the display number in VNCServerST. At least this is how I think it should be possible to check the session we want.

[CendioOssman]

The core is not aware of X11 things. And it shouldn't be. So this sounds very much like a frontend thing that should be in Xvnc somewhere. It might be XServerDesktop, as you suggested, but might be elsewhere. Probably depends on which part has access to the best information.

[grulja]

The core is not aware of X11 things. And it shouldn't be. So this sounds very much like a frontend thing that should be in Xvnc somewhere. It might be XServerDesktop, as you suggested, but might be elsewhere. Probably depends on which part has access to the best information.

I was looking into adding it to XServerDesktop, where I think it makes more sense as it's X11-aware, but I struggled with the use of Makefile there and the optional use of systemd library. I guess it can be done through CMake modifying the makefile similar to config.h.in. If you think it makes more sense to be there, I will give it a try.

[CendioOssman]

CMake cannot do much about Xorg's autoconf build. If you need to adjust things there, I'm afraid you'll need to update the patch files that modifes Xorg's configure.ac. There is some systemd support there already. Could be enough?

[grulja]

What about something like this? To enable this, Xvnc has to be built with --enable-systemd-logind and --enable-config-udev which is a dependency for systemd-logind support in Xorg (unless we patch it).

[CendioOssman]

Slightly better. But I'm concerned that this is fundamentally built on making assumptions about what the display manager will do. I would prefer if we kept to things we have control over. Or push those decisions out of TigerVNC.

Did you examine what you can get from XDMCP? I'm thinking one model would be that Xvnc keeps track of which user authenticated. Then locks that in as long as XDMCP indicates that a session is active. Once the session ends, Xvnc can forget about the user and go back to accepting everyone.

Another approach is to delegate this to something external. Perhaps using the query system. Write a custom program there that can implement the logic of which connections to accept or not.

[grulja]

Did you examine what you can get from XDMCP? I'm thinking one model would be that Xvnc keeps track of which user authenticated. Then locks that in as long as XDMCP indicates that a session is active. Once the session ends, Xvnc can forget about the user and go back to accepting everyone.

Not yet.

Another approach is to delegate this to something external. Perhaps using the query system. Write a custom program there that can implement the logic of which connections to accept or not.

Something similar to vncconfig and QueryConnect option in Xvnc? Can vncconfig be extended with this functionality? Or would you prefer a separate one, but implemented in a similar way as vnconfig.

[CendioOssman]

Something similar to vncconfig and QueryConnect option in Xvnc?

Yes. I'm hoping there is enough information there to act on.

Can vncconfig be extended with this functionality? Or would you prefer a separate one, but implemented in a similar way as vnconfig.

Might be best to make this a separate tool. I'm still unsure if it's too niche to include in the standard TigerVNC. Being a separate tool would definitely make it easier to include.

(Related, vncconfig should probably be split in to multiple tools)

[Neustradamus]

To follow this PR :)

[grulja]

Did you examine what you can get from XDMCP? I'm thinking one model would be that Xvnc keeps track of which user authenticated. Then locks that in as long as XDMCP indicates that a session is active. Once the session ends, Xvnc can forget about the user and go back to accepting everyone.

I've been looking into this and in the XDMCP implementation I can see it regularly checks whether the session is still alive, BUT I don't know how to propagate or connect this with the code I have in XServerDesktop.cc. One thing that could work would be to just add an additional public function that will simply return the state of XDMCP and use a timer to check this state regularly. Another approach might be to add a hook (pass a function pointer) which we can call once the session is no longer active and remove the locked user at that point.

Can vncconfig be extended with this functionality? Or would you prefer a separate one, but implemented in a similar way as vnconfig.

Might be best to make this a separate tool. I'm still unsure if it's too niche to include in the standard TigerVNC. Being a separate tool would definitely make it easier to include.

I've been also looking into this and it would be way too much work for something like this. I would prefer spend less time on this. I guess it's fine if you are not going to accept this upstream, still at least some feedback and review will be helpful. Even though it's not the way I prefer, we can at least carry this as a downstream patch.