Add ability to drop privileges when launching X server

[twhitehead]

We recently experienced a local root escalation issue, and our security team identified one possible avenue for the issue having been one of these security bugs in the tigervnc server along with the fact that it was running as root despite just being a glorified in memory frame buffer

https://access.redhat.com/errata/RHSA-2020:1497

We've patched it, but, to protect against any further such attacks in the future, I also went ahead and added the ability for lightdm to drop privileges when forking local X servers

$ ps -HA -o pid,user,group,args
...
3896 root     root       /usr/sbin/lightdm
 3906 vnc      vnc          /usr/bin/Xvnc -securitytypes vencrypt ...
 3925 root     root         lightdm --session-child 19 22
 3928 lightdm  lightdm        /usr/sbin/slick-greeter
 3965 root     root         lightdm --session-child 15 22
...

The option is exposed as a [VNCServer] user setting, it's not VNC specific. The mechanism was added entirely to the x-server-local and process infrastructures, so it would be pretty trivial to apply it to the X sever proper too (which I've heard can now run without root in some cases too).

I did have to rebase my original commits (which I tested) due to a recent commit that removed the cached priv pointers x-server-local and process, changing many lines of sever->priv->... to just priv->.... It was a pretty trivial fix, so I haven't verified it beyond that fact that it compiles.

[twhitehead]

Just a note that it doesn't look like that CI build test failure was an issue with this pull request. The Fedora build was fine. The Ubuntu build seems to have hung configuring the build environment without ever actually reaching the build step.

[robert-ancell]

The polling for the socket is a bit flaky, there's actually another method supported by Xorg:

-displayfd fd          file descriptor to write display number to when ready to connect

Will this work in your case? I didn't implement this originally because the feature was relatively new and not necessarily supported. But it might be reasonable to expect if you're already requiring X to work with dropped privileges.

This change should also have a test case, but I'm happy to help out with that if you have trouble working out how to do that.

[robert-ancell]

Extra whitespace here doesn't match existing code.

[twhitehead]

I believe you mean blank like 405. I've deleted it. Please let me know if this wasn't it.

[robert-ancell]

I mean it should change from if ( g_stat (socketpath, &statbuf) == 0 ) to if (g_stat (socketpath, &statbuf) == 0)

[corsac-s]

Hi, as a Debian maintainer I'll try to test the patch and report back, not for the XVnc part but rather for the whole “deprivilege X” thing. We have https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=809067 opened which I fowarded to https://bugs.launchpad.net/lightdm/+bug/1292324 on Launchpad but that bug was never migrated to github I think (although there is #18 )

[twhitehead]

@corsac-s Although most of the changes were made at the process.c and xserver-local.c levels, currently only the seat-xvnc.c code has been changed to call the new x_server_local_set_user function when the [VNCServer] user option is set that triggers the privilege drop.

A similar configuration option and x_server_local_set_user call would have to be made for the case of non-xvnc local seat to enable rootless regular X. From the issues you link though, it does sound like there may also be some VT ownership complications that would need be resolved (it isn't clear to me how exactly the Xorg VT stuff works).

[twhitehead]

@robert-ancell I think it is reasonable if this requires a newer Xorg. I'll see what I can do with regard to the -display-fd method of getting the display.

[twhitehead]

Just a note that the Xserver man page says that -displayfd means that the X server chooses a display number for you (not sure what that might mean for lightdm).

-displayfd fd

specifies a file descriptor in the launching process. Rather than specify a display number, the X server will attempt to listen on successively higher display numbers, and upon finding a free one, will write the display numberback on this file descriptor as a newline-terminated string. The -pn option is ignored when using -displayfd.

[robert-ancell]

Just a note that the Xserver man page says that -displayfd means that the X server chooses a display number for you (not sure what that might mean for lightdm).

-displayfd fd
specifies a file descriptor in the launching process. Rather than specify a display number, the X server will attempt to listen on successively higher display numbers, and upon finding a free one, will write the display numberback on this file descriptor as a newline-terminated string. The -pn option is ignored when using -displayfd.

Looking at the xorg source:

       else if (argv[i][0] == ':') {
            /* initialize display */
            display = argv[i];
            explicit_display = TRUE;

    else if ((displayfd < 0) || explicit_display) {
        if (TryCreateSocket(atoi(display), &partial) &&
            ListenTransCount >= 1)
            if (!PartialNetwork && partial)
                FatalError ("Failed to establish all listening sockets");
    }
    else { /* -displayfd and no explicit display number */
        Bool found = 0;
        for (i = 0; i < 65536 - X_TCP_PORT; i++) {
            if (TryCreateSocket(i, &partial) && !partial) {
                found = 1;
                break;
            }

If you pass a display number (as we do in LightDM) it won't try to pick it's own. I'm pretty sure I got this to work in the past so it should work fine.

[twhitehead]

Thanks for looking into that Robert. I will work to incorporate this as the display-server-has-started check instead of polling for the socket file.

Just a heads up that it may take awhile as I am now heavily involved in another project, but I will push the results here when done. 😄

[twhitehead]

@corsac-s I spent a bit of time reading up on how the whole seat thing and such is all done. The best resource I found was this

https://dvdhrm.wordpress.com/2013/08/24/session-management-on-linux/

and combined with the fact that I believe I saw session dbus calls in the lightdm code, I think there is a reasonable chance that my device ownership and such concerns are null and void.

[corsac-s]

@corsac-s I spent a bit of time reading up on how the whole seat thing and such is all done. The best resource I found was this

https://dvdhrm.wordpress.com/2013/08/24/session-management-on-linux/

and combined with the fact that I believe I saw session dbus calls in the lightdm code, I think there is a reasonable chance that my device ownership and such concerns are null and void.

So that would only leave the specific option and the x_server_local_set_user stuff?

[montvid]

When is this shipping? :( I had to uninstall lightdm to make xorg run as nonroot.

[eylles]

is it possible to use the xserver-command option form the config along xserver-xorg-legacy to force x11 to drop privileges even when started by lightdm or trying that is just an exercise in futility?

[twhitehead]

When is this shipping? :( I had to uninstall lightdm to make xorg run as nonroot.

The developers wanted a better detection for when X11 is ready for connections than socket polling (see above comment about -displayfd) and I haven't got around to implementing that yet.

Hopefully I'll have some time at some point in the future here. It likely won't be in the nearish future, though, so, if anyone else is feeling up for it, go for it!

[zWhdmB5T]

@twhitehead Hi! Is there still any effort on this?

[twhitehead]

@zWhdmB5T it is something I would like to do sometime, not sure if it will ever get up high enough on my priority list to get done though...