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

[bugzilla-daemon at webkit.org]

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

--- Comment #9 from Michael Catanzaro <mcatanzaro at igalia.com> ---
(In reply to Patrick Griffis from comment #7)
> (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.

Ah yes, TestWebKitWebContext would be the right place: you can add it there instead.

> `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.

I suggest copypasting these calls in the same places with an added #if: that would probably be more clear and hopefully less-confusing. Try it and we'll see what the result looks like. I know it would be more code, but I think it's important to avoid using SandboxExtensions here.

> > > 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.

Hm good point. This seems fine, then. I wouldn't give access to all of /usr/libexec for the same reason we don't give access to /usr/bin.

> > > 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]

Oh wow, shows what I know....

-- 
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/3e4d3f89/attachment.html>