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

[bugzilla-daemon at webkit.org]

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

--- Comment #48 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #46)
> 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.

I think we could have a file with a BwrapLauncher class or something like that, used by the process launcher.

> > > > 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/20180918/5ea4379f/attachment-0001.html>