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

[bugzilla-daemon at webkit.org]

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

--- Comment #50 from Patrick Griffis <pgriffis at igalia.com> ---
(In reply to Carlos Garcia Campos from comment #47)
> (In reply to Patrick Griffis from comment #45)
> > (In reply to Carlos Garcia Campos from comment #40)
> > > > Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:127
> > > > +    void appendPermissions(std::initializer_list<CString> permissions)
> > > > +    {
> > > > +        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_isRunning);
> > > > +
> > > > +        Vector<CString> newPermissions(permissions);
> > > 
> > > Why doesn't this receive a Vector<CString> directly?
> > 
> > I just found it cleaner to pass {"initializer", "lists"} around than create
> > vectors.
> 
> Can't you use { } if the function receives a Vector<>&& ?

Last I tried no.

> > Sure we could. Same FD comment as a above, the `bwrap` command passes an fd
> > to its child `xdg-dbus-proxy`.
> > 
> > We could just not sandbox `xdg-dbus-proxy` and clean this up a bit. The
> > extra layer of sandboxing surely
> > can't hurt but its probably safe and `flatpak` doesn't do it.
> 
> I still don't understand what xdg-dbus-proxy is for :-P
> 
> > > > Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:287
> > > > +        proxy.launch();
> > > 
> > > So, this is the one launching the bwrap executable. Should this be
> > > launchWhateverIfNeeded instead? or ensureWhatever?
> > 
> > `proxy.launchProxy()` ? I'm not sure the duplication is needed.
> 
> I don't know, I don't understand how it works to decide on the API or name.

So normally you would just mount `$rundir/bus` in the sandbox for dbus to work but that means
you have access to every dbus service on the host which is obviously bad. `xdg-dbus-proxy` opens
a connection to the host bus, then creates a proxy bus which is put into the sandbox. The proxy
allows you to filter which bus names, interfaces, and paths that are allowed through and are otherwise
rewritten.

> > > > Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:413
> > > > +    static DBusProxy proxy;
> > > 
> > > So, the proxy is only for a11y?
> > 
> > There are 3 proxies per UI process. 1 for a11y and 2 for session bus for web
> > and network processes.
> > (At a later point another for system bus probably.)
> 
> But there can be more than one web process. What happens when a process
> finishes or crashes? Theses proxies are all static.

The web processes all have the same permissions so always share the same proxy, I don't *think* that is a problem.(In reply to Carlos Garcia Campos from comment #49)

> > Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:66
> > +    return syscall(__NR_memfd_create, name, flags);
> 
> Is this always available? doesn't it depend on a kernel version or libc?

Linux 3.17 (~4 years ago)

> > Source/cmake/OptionsGTK.cmake:215
> > +    find_program(BWRAP_EXECUTABLE bwrap)
> > +    if (NOT BWRAP_EXECUTABLE)
> > +        message(FATAL_ERROR "bwrap executable is needed for ENABLE_BUBBLEWRAP_SANDBOX")
> > +    endif ()
> > +    add_definitions(-DBWRAP_EXECUTABLE="${BWRAP_EXECUTABLE}")
> 
> hmm, this executable is not actually a build dep, but a runtime one, no?
> Wouldn't it be possible to not have the executable at build time, for
> example when cross-compiling?

Yes it is a runtime dep. The value of it being a build time check is we get an absolute path and we get to check the version (0.3.1 isn't released, so it would commonly fail atm).

-- 
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/99b9ed48/attachment.html>