TurboVNC / turbovnc

Main TurboVNC repository
https://TurboVNC.org
GNU General Public License v2.0
763 stars 138 forks source link

Handling of orthaned sessions in a multiuser environment #234

Closed Robo8920 closed 3 years ago

Robo8920 commented 4 years ago

https://github.com/TurboVNC/turbovnc/blob/fb7f6863d7cc10c1dcfbd2936725d3f4beec6259/unix/vncserver.in#L924

In a multiuser environment the check for orthaned sessions may not be sufficient. It's possible that someone else started a new session with the display of one of my orthaned sessions and if I try to logon to that session it gets possibly locked due to the wrong authentication.

A check for the combination of display and process id would possibly be the better option. (just an idea for a minimal check - check if "/proc/[pid] exists for by userid).

dcommander commented 4 years ago

I need to understand the mechanics of why the issue is occurring. If a session is "orphaned", that means that it shut down unexpectedly (without invoking vncserver -kill), so the X socket files should still be there. It shouldn't be possible to start another session on the orphaned session's display as long as the socket files are there, so if that is in fact occurring, then that's the root of the issue. The rest is expected behavior. If you intentionally kill a session, then that session's display number becomes available for other users, and if one of those users starts a session on that display and you attempt to connect, you won't be able to authenticate. Note that, as currently implemented, the TurboVNC Server will temporarily lock the session for 10 seconds if anyone fails to authenticate more than 5 times. It will double that time period for every subsequent failed attempt until authentication is successful. As mentioned in #218, this mechanism should really be modified such that only the IP address from which the failed authentication occurs is locked out.

Thus, it seems that the main issue is that, for reasons unexplained, another session is able to start on the same display number as an orphaned session, and that leads to end user confusion (but also, I am curious as to how the session became orphaned in the first place.)

Robo8920 commented 4 years ago

Thanks for your (very) fast response. You may be right - It's possibly my fault. I have killed a session of one of my users (by "killing" the Xvnc process) due to a "blackscreen issues" and that led to described issue.

Have done now a workaround for my purpose: vncserver -list | grep ^: | awk -v my_user=$USER '{ if ((system("test -d " "/proc/"$2)==0 ) && (system("test $(stat -c '%U' /proc/"$2") == my_user")) ) {print $1}}'

Does not look nice put does the job.... :-)

dcommander commented 3 years ago

Closing for now. If you have further information regarding how to reproduce the problem on my end, feel free to add it, but absent such information, there is nothing actionable here.

Robo8920 commented 3 years ago

It's quit easy to reproduce it and root cause is that running sessions is maintained in "~./vnc".

Just imagine the following situation in a multiuser environment: User A with Session ":1" contacts the Admin that he can not reach his session anymore Admin kills session ":1" User B opens a new session ":1" User A still sees session ":1" as his session because of the remaining entry in his ~/.vnc and tries to connect - after 3 tries session gets locked

dcommander commented 3 years ago

What do you mean by "User A still sees :1 as his session"? Does it show up when User A runs /opt/TurboVNC/bin/vncserver -list? If so, then I concur that that's a bug, as the -list option should not list sessions that no longer have a running process associated. The vncserver script already has the capability to check whether a PID corresponds to a running process, and it should be doing that with -list.

If -list is behaving as expected, however, then I'm not sure what else you expect in terms of TurboVNC's behavior. If the session is not killed normally, using /opt/TurboVNC/bin/vncserver -kill, then the PID entry will still be present. And as soon as a session is no longer listening on a particular display number, another session is free to do so. In short, you should not rely on the presence or absence of a PID file to determine whether a session is running. Use /opt/TurboVNC/bin/vncserver -list instead, and I'll fix that feature if it is still listing orphaned sessions (I'm not in my computer lab right now, so I can't test whether that's the case.) The other issue involving the session being locked out can be addressed by making the lockout feature IP-address-specific, which I already intend to do.

I guess what I'm looking for is specific, targeted suggestions for improvement, because at the moment, it isn't clear whether what you're asking for is even possible with any VNC server. They will all similarly allow another session to start on a particular display number as soon as the session previously occupying that display number shuts down. There is no way for TurboVNC, running under User B, to know whether User A's session shut down normally or not. All it can do is probe for the next available display number.

dcommander commented 3 years ago

I have confirmed that /opt/TurboVNC/bin/vncserver -list works as expected.

dcommander commented 3 years ago

Also, referring to #218, the automatic lockout mechanism for authentication failures is now configurable, and in the dev branch (3.0 evolving), it has been made IP-address-specific.