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

[bugzilla-daemon at webkit.org]

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

--- Comment #46 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Errors initializing the sandbox are ultimately errors in the UI process, so it makes sense to me that the UI process should terminate. But I suppose it's OK to not do that so long as the secondary process is guaranteed to not start and the WebKitWebView emits the crashed/terminated signals.

(In reply to Patrick Griffis from comment #45)
> > I think all the sandbox code should be moved to its own file
> 
> Maybe, what do you think Michael?

Whatever Carlos prefers. It's iffy. I considered requesting this myself, but decided in the end that it was fine in ProcessLauncher because it's intimately tied to the process launching. It's impossible to reason about or debug process launching without reading the sandboxing code, and if we split it into a separate file, the sandboxing code is ultimately just going to return a bunch of args to pass to g_spawn_async(). So keeping it in ProcessLauncher seemed fine to me.

> > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:116
> > > +#if PLATFORM(GTK) || PLATFORM(WPE)
> > > +    WebsiteDataStore& store = processPool().websiteDataStore()->websiteDataStore();
> > > +    store.resolveDirectoriesIfNecessary();
> > > +    launchOptions.extraSandboxPaths.append(store.resolvedNetworkCacheDirectory());
> > > +
> > > +    launchOptions.sandboxEnabled = m_processPool.sandboxEnabled();
> > > +#endif
> > 
> > I think we should add a platformGetLaunchOptions or something like that
> > instead of adding platform ifdefs here. Should this depend on
> > ENABLE(BUBBLEWRAP_SANDBOX) or OS(LINUX) instead of GTK|WPE?
> 
> I don't know if `platformGetLaunchOptions()` would be much better. Could
> change the ifdef though.

If it's easy to split the code into a platformGetLaunchOptions(), that would be nice/idiomatic.

P.S. I'd encourage you to test with asan ('set-webkit-configuration --debug --asan') to catch those leaks you introduced. It's extremely difficult to avoid introducing leaks if you're not developing with asan.

-- 
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/20180917/eadc3668/attachment.html>