[Webkit-unassigned] [Bug 188568] [GTK][WPE] Implement subprocess sandboxing

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188568

--- Comment #7 from Patrick Griffis <pgriffis at igalia.com> ---
(In reply to Michael Catanzaro from comment #4)
> Please add a very simple test in
> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp, at the bottom
> of testWebKitSettings(), to verify that the setting getter/setters work.

This API is for web context. I don't see similar tests there.

> > Source/WebKit/Shared/SandboxExtension.h:145
> > +#if !ENABLE(SANDBOX_EXTENSIONS) && !PLATFORM(GTK) && !PLATFORM(WPE)
> 
> I don't like that we're building without ENABLE(SANDBOX_EXTENSIONS) but
> nevertheless implementing SandboxExtensionGLib. Please look into this a bit
> more. You might want to turn on ENABLE(SANDBOX_EXTENSIONS) and just make
> most of the functionality dependent on PLATFORM(COCOA). Or you might want to
> just not use SandboxExtension.

`ENABLE(SANDBOX_EXTENSIONS)` doesn't make any sense as the rest of their API is
a complete mismatch to ours *except* this group of standalone functions that
do exactly what we want, which is at resolve time of paths and before launching
processes ensure the directories exist. It is a perfect fit for our needs.
Not using it means copy pasting these calls in the same places with an added
#if.

> > Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:269
> > +    bindIfExists(args, "/usr/libexec/gst-install-plugins-helper");
> > +    bindIfExists(args, "/usr/local/libexec/gst-install-plugins-helper");
> 
> This only works on Fedora... is it really like this in flatpak? :( Check
> where it is installed on Debian, it will be under /usr/lib/ somewhere and
> likely all other distros will have it there. (Fortunately, it won't be
> multiarch, since it's an executable, so that helps.)

We already grant read access to /usr/lib, so that is fine. We could do
blanket read access to /usr/libexec also.

> > Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:464
> > +    if (processType == ProcessLauncher::ProcessType::Plugin64
> > +        || processType == ProcessLauncher::ProcessType::Plugin32)
> 
> Hm, I'm surprised style checker didn't complain about this... the || goes at the end of the previous line in WebKit, not the beginning of the new line.


    ERROR: Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:461:  Boolean expressions thatspan multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]

---

Everything else was resolved.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180815/518722dc/attachment.html>