[webkit-changes] [WebKit/WebKit] 86ae23: Merge 255218 at main - [GLib] D-Bus proxy quietly fai...

Sat Oct 15 13:12:01 PDT 2022

  Branch: refs/heads/webkitglib/2.38
  Home:   https://github.com/WebKit/WebKit
  Commit: 86ae2361a05a5cd71ec9fe3f8c70d5baff2c439e
      https://github.com/WebKit/WebKit/commit/86ae2361a05a5cd71ec9fe3f8c70d5baff2c439e
  Author: Michael Catanzaro <mcatanzaro at redhat.com>
  Date:   2022-10-14 (Fri, 14 Oct 2022)

  Changed paths:
    M Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
    M Source/WebKit/UIProcess/Launcher/glib/XDGDBusProxy.cpp
    M Source/WebKit/UIProcess/Launcher/glib/XDGDBusProxy.h

  Log Message:
  -----------
  Merge 255218 at main - [GLib] D-Bus proxy quietly fails if host session bus address is an abstract socket
https://bugs.webkit.org/show_bug.cgi?id=245843

Reviewed by Carlos Garcia Campos.

Nowadays all major Linux distros run the D-Bus session bus using a
standard Unix socket created on the filesystem, but distros that do not
use systemd still wind up using dbus-daemon's default session bus
address, which up until now has used the abstract socket namespace.

Our code here is only compatible with filesystem sockets since it
attempts to create the proxy bus socket in the sandbox at exactly the
same location within the sandbox that the real session bus socket exists
on the host system. If the host session bus uses an abstract socket, our
code just fails. There's no particular reason to do things this way, so
let's not. Instead, we'll always create the proxy bus socket in a
well-known location within the sandbox, /run/webkitgtk/bus or
/run/wpe/bus. This matches flatpak's behavior and should allow things to
work regardless.

The accessibility bus requires the same changes.

Note there are major security problems if the host session bus uses an
abstract socket. See https://gitlab.freedesktop.org/dbus/dbus/-/issues/416
for full details. While this configuration is not recommended, it's
usually safe for WebKit because our sandbox does not allow network access
(unless using a non-local X server, which is inherently insecure anyway).

* Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:
(WebKit::bindDBusSession):
(WebKit::bindA11y):
* Source/WebKit/UIProcess/Launcher/glib/XDGDBusProxy.cpp:
(WebKit::XDGDBusProxy::dbusSessionPath):
(WebKit::XDGDBusProxy::accessibilityPath):
(WebKit::XDGDBusProxy::dbusSessionProxy):
(WebKit::XDGDBusProxy::accessibilityProxy):
(WebKit::XDGDBusProxy::makePath): Deleted.
* Source/WebKit/UIProcess/Launcher/glib/XDGDBusProxy.h:

Canonical link: https://commits.webkit.org/255218@main

(cherry picked from commit 79f41316d99e6496904950be78a0015503385ec2)

  Commit: 0179af5ac8490cfae8b9e4bead87b212b9edb6d1
      https://github.com/WebKit/WebKit/commit/0179af5ac8490cfae8b9e4bead87b212b9edb6d1
  Author: Michael Catanzaro <mcatanzaro at redhat.com>
  Date:   2022-10-14 (Fri, 14 Oct 2022)

  Changed paths:
    M Source/WTF/wtf/glib/Sandbox.cpp
    M Source/WTF/wtf/glib/Sandbox.h
    M Source/WebCore/platform/graphics/PlatformDisplay.h
    M Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
    M Source/WebKit/UIProcess/Launcher/glib/XDGDBusProxy.cpp
    M Source/WebKit/UIProcess/Launcher/glib/XDGDBusProxy.h
    M Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp

  Log Message:
  -----------
  Merge 255530 at main - [GTK] D-Bus proxy quietly fails if host bus address is not mounted in xdg-dbus-proxy's sandbox
https://bugs.webkit.org/show_bug.cgi?id=246159

Reviewed by Carlos Garcia Campos.

D-Bus 1.15.2 has changed the default session bus address to a filesystem
socket that lives under /tmp. However, our xdg-dbus-proxy cannot access
this location because we assume the session bus socket will always be
mounted under /run, since that's where all major distros put it. It's OK
to be flexible and mount absolutely any directory, whatever it may be,
since we're not actually trying to create a sandbox that the
xdg-dbus-proxy cannot break out of. It's a trusted process, and the
sandbox exists solely so that portals can verify the app ID of the
process that is using the proxy, which is done by inspecting
/.flatpak-info in its mount namespace's filesystem root. So let's mount
whatever directory is in use and move on. Credit to oreo639 for
investigating the problem and proposing a fix in WebKit#5011.

The a11y bus has the same theoretical problem, although it's not an
issue today because currently it will always be under /run in
practice. Still, we should fix it. There is one complication:
PlatformDisplay currently uses just one variable for both the host a11y
bus address and the proxy bus address, relying on XDGDBusProxy to change
it from the host address to the proxy address. This is fragile and it's
easier to fix it than to work around it by caching the value before it
changes, so at Carlos's suggestion, I have removed the ability to
overwrite the value in PlatformDisplay, and added a separate variable to
track the proxy address in WTF's Sandbox helpers.

I have snuck in a drive-by cleanup to avoid duplicating BASE_DIRECTORY
between two files, a problem that I introduced in 255218 at main.
Additionally, I remove a stale declaration for XDGDBusProxy::makePath,
which I forgot to delete after removing the function in the same commit.

Finally, always add the extra sandbox paths to the sandbox. These were
originally extra paths for the web process only, but changed to be extra
paths for both web process and D-Bus proxy. It's no longer needed except
for the web process, but there's no particular reason to limit it
either. I'm changing this here only because it's right next to the code
I'm editing anyway, and it's odd to be adding extra sandbox paths
specifically for the D-Bus proxy process.

Canonical link: https://commits.webkit.org/255530@main

(cherry picked from commit 67cda4acff9b343c48a2b24c326a2be08642ccee)

  Commit: 0c91a218fbc03ef61aa21bd72dd530233da6c867
      https://github.com/WebKit/WebKit/commit/0c91a218fbc03ef61aa21bd72dd530233da6c867
  Author: Michael Catanzaro <mcatanzaro at redhat.com>
  Date:   2022-10-15 (Sat, 15 Oct 2022)

  Changed paths:
    M Source/bmalloc/libpas/CMakeLists.txt

  Log Message:
  -----------
  Merge 255548 at main - [libpas] libpas is unconditionally built with -Werror
https://bugs.webkit.org/show_bug.cgi?id=246520

Reviewed by Yusuke Suzuki.

We cannot use -Werror unconditionally because it will cause builds to
fail spuriously for end users. Get rid of it.

* Source/bmalloc/libpas/CMakeLists.txt:

Canonical link: https://commits.webkit.org/255548@main

(cherry picked from commit 0d78e3342b26fbe519d44a1d97763ad5b06305a4)

Compare: https://github.com/WebKit/WebKit/compare/236b0b56aa6e...0c91a218fbc0