Bug 140064

Summary: [Linux] SeccompFilters: improve the port-agnostic whitelist
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKit2Assignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Enhancement CC: cgarcia, commit-queue, mcatanzaro, tmpsantos, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 110014, 140065    
Attachments:
Description Flags
Patch
none
[Linux] SeccompFilters: improve the port-agnostic whitelist
none
[Linux] SeccompFilters: improve the port-agnostic whitelist
none
[Linux] SeccompFilters: improve the port-agnostic whitelist none

Description Michael Catanzaro 2015-01-04 18:50:04 PST
I think pretty much all the files and directories that the GTK+ port web process needs to access are generic enough that they should most likely be whitelisted for EFL as well. I'd like to add several locations to the web process whitelist:

* /lib64 and /usr/lib64: nothing has any chance of working on x86_64 Linux otherwise, except on Debian which doesn't have these directories.
* /run/user/UID where UID is the result of getuid(), since at-spi2 creates lots of random directories here. (I'd rather this be more restrictive.) I think at-spi2 is not specific to GTK+.
* Specific files in /sys/fs/cgroup, for the memory pressure handler that landed last month. (These accesses will fail regardless because those files are owned by root. Not sure how the memory pressure handler is supposed to work.)
* Check $XDG_DATA_HOME before assuming that mime types are in ~/.local/share/mime
* /var/tmp -- this is unfortunate, but with recent enough mesa (for DRI3) and barring a configure-time override (which Debian wisely does), shared memory winds up here: https://bugzilla.redhat.com/show_bug.cgi?id=1172869
* mesa configuration files
* Various directories needed by udev
* ~/nv for the NVIDIA proprietary driver, suggested by Zan.
* Tempted to also put gstreamer stuff here....

We can talk about whether any of these locations should be moved to the GTK+ and/or EFL whitelists instead.
Comment 1 Michael Catanzaro 2015-01-04 18:55:36 PST
(In reply to comment #0)
> * /var/tmp -- this is unfortunate, but with recent enough mesa (for DRI3)
> and barring a configure-time override (which Debian wisely does), shared
> memory winds up here: https://bugzilla.redhat.com/show_bug.cgi?id=1172869
> * mesa configuration files
> * Various directories needed by udev

I currently have access to these directories disabled if compiled without support for mesa, but I wonder if it's wiser to just unconditionally allow them to avoid potential future breakage.
Comment 2 Michael Catanzaro 2015-01-04 18:56:05 PST
Created attachment 243951 [details]
Patch
Comment 3 Michael Catanzaro 2015-03-23 15:58:45 PDT
Created attachment 249298 [details]
[Linux] SeccompFilters: improve the port-agnostic whitelist
Comment 4 Michael Catanzaro 2015-03-23 16:03:32 PDT
Comment on attachment 249298 [details]
[Linux] SeccompFilters: improve the port-agnostic whitelist

So, this is the first patch in my series. The goal here is the same as in bug #142987, but I've spent enough time rebasing this series already that I'd rather commit them separately, especially since that patch is the last in the series and this is the first, it would just be a pain. So this one alone doesn't really accomplish much (well, it makes accelerated compositing work), it's just a bit better than what we have now....
Comment 5 Zan Dobersek 2015-03-24 05:37:28 PDT
Comment on attachment 249298 [details]
[Linux] SeccompFilters: improve the port-agnostic whitelist

View in context: https://bugs.webkit.org/attachment.cgi?id=249298&action=review

> Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:194
> +    // Needed by at-spi2.
> +    addDirectoryPermission("/run/user/" + String::number(getuid()), ReadAndWrite);

Is this truly port-agnostic, or is it specific to GNOME/GTK?

> Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:216
> +    // Try removing this permission when we can rely on a newer libxshmfence.

Put a FIXME here.
Comment 6 Michael Catanzaro 2015-03-24 08:00:14 PDT
(In reply to comment #5)
> Is this truly port-agnostic, or is it specific to GNOME/GTK?

It's used by both EFL and GTK. In general, I don't mind giving a few extra permissions even if they're not needed (e.g. I assume WPE may not use ATK) since if you don't use at-spi2 then it's pretty unlikely you'd put important data in a folder named at-spi2. However in this case it's actually bad, since at-spi2 is not in the name of the permission at all. /run/user/1000 is quite wide a permission. I'll create a new bug #143004 to narrow this permission.

> Put a FIXME here.

OK (although it's removed in one of the follow-up patches)
Comment 7 Zan Dobersek 2015-03-25 10:08:43 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > Is this truly port-agnostic, or is it specific to GNOME/GTK?
> 
> It's used by both EFL and GTK. In general, I don't mind giving a few extra
> permissions even if they're not needed (e.g. I assume WPE may not use ATK)
> since if you don't use at-spi2 then it's pretty unlikely you'd put important
> data in a folder named at-spi2. However in this case it's actually bad,
> since at-spi2 is not in the name of the permission at all. /run/user/1000 is
> quite wide a permission. I'll create a new bug #143004 to narrow this
> permission.
> 

Place a FIXME comment above this code, a note about the problem and link to the bug, and we can move forward with this.
Comment 8 Michael Catanzaro 2015-03-26 08:58:14 PDT
Created attachment 249490 [details]
[Linux] SeccompFilters: improve the port-agnostic whitelist

Added FIXMEs
Comment 9 Michael Catanzaro 2015-03-26 08:59:21 PDT
Created attachment 249491 [details]
[Linux] SeccompFilters: improve the port-agnostic whitelist

Actually added FIXMEs
Comment 10 WebKit Commit Bot 2015-03-26 12:00:48 PDT
Comment on attachment 249491 [details]
[Linux] SeccompFilters: improve the port-agnostic whitelist

Clearing flags on attachment: 249491

Committed r182021: <http://trac.webkit.org/changeset/182021>
Comment 11 WebKit Commit Bot 2015-03-26 12:00:53 PDT
All reviewed patches have been landed.  Closing bug.