Hello Jack, Thanks a lot for this work.
Jack Hill <jackh...@jackhill.us> writes:
> Some additional observations:
>
> With my patched webkitgtk, if I set:
>
> PULSE_CLIENTCONFIG=/gnu/store/zc4dsmvdabi00nvisrjhi9w00ff4igs7-client.conf
>
> it does work, which is an improvement compared to without the patch.
Great. I have attached a patch for Guix that stops using /etc for these
variables.
> I notice that Nix [0] has a similar patch:
>
> """
> diff -ru
> old/webkitgtk-2.26.0/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> webkitgtk-2.26.0/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> ---
> old/webkitgtk-2.26.0/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> 2019-09-09 04:47:07.000000000 -0400
> +++
> webkitgtk-2.26.0/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> 2019-09-20 21:14:10.537921173 -0400
> @@ -585,7 +585,7 @@
> { SCMP_SYS(keyctl), nullptr },
> { SCMP_SYS(request_key), nullptr },
>
> - // Scary VM/NUMA ops
> + // Scary VM/NUMA ops
> { SCMP_SYS(move_pages), nullptr },
> { SCMP_SYS(mbind), nullptr },
> { SCMP_SYS(get_mempolicy), nullptr },
> @@ -724,6 +724,10 @@
> "--ro-bind-try", "/usr/local/lib64", "/usr/local/lib64",
>
> "--ro-bind-try", PKGLIBEXECDIR, PKGLIBEXECDIR,
> +
> + // Nix Directories
> + "--ro-bind", "@storeDir@", "@storeDir@",
> + "--ro-bind", "/run/current-system", "/run/current-system",
> };
> // We would have to parse ld config files for more info.
> bindPathVar(sandboxArgs, "LD_LIBRARY_PATH");
> """
>
> [0]
> https://github.com/NixOS/nixpkgs/blob/465566948393cf533e3617704d1c4ccc34cf3753/pkgs/development/libraries/webkitgtk/fix-bubblewrap-paths.patch
>
> so I wonder if I didn't do the mounts in the right place and or if it is
> becasue I missed /run/current-system.
>
> I'm going to try to adapt the Nix patch to see if that helps.
Were you able to verify whether /run/current-system is required inside
the sandbox?
I cleaned up your patch a bit and rebased it on the latest master
branch, available as patch 2/2 below. Currently building it on
'core-updates' to verify that it works. It takes a while on my dinky
quad-core server though. :-)
It does not bind /run/current-system, and I think we should avoid it if
possible. Ideally we would only mount the store paths required by the
consumers instead of all of /gnu/store, but not sure how to achieve
that.
From a2607c8246456460a6bbed62144daf7196a5c9bd Mon Sep 17 00:00:00 2001 From: Marius Bakke <mba...@fastmail.com> Date: Wed, 6 May 2020 17:48:42 +0200 Subject: [PATCH 1/2] services: Do not use symbolic links in PulseAudio variables. This addresses <https://bugs.gnu.org/40837> by making these configuration files more easily accessible within the WebKitGTK+ sandbox. * gnu/services/sound.scm (pulseaudio-environment): Move below PULSEAUDIO-CONF-ENTRY. Create PULSE_CONFIG and PULSE_CLIENTCONFIG entries directly instead of referring to /etc/pulse. (pulseaudio-etc): Do not create /etc/pulse/client.conf and /etc/pulse/daemon.conf. --- gnu/services/sound.scm | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm index a1c928222a..bdf819b422 100644 --- a/gnu/services/sound.scm +++ b/gnu/services/sound.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2018, 2020 Oleg Pykhalov <go.wig...@gmail.com> ;;; Copyright © 2020 Leo Prikler <leo.prik...@student.tugraz.at> +;;; Copyright © 2020 Marius Bakke <mba...@fastmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -127,11 +128,6 @@ ctl.!default { (default (file-append pulseaudio "/etc/pulse/system.pa")))) -(define (pulseaudio-environment config) - `(;; Define these variables, so that pulseaudio honors /etc. - ("PULSE_CONFIG" . "/etc/pulse/daemon.conf") - ("PULSE_CLIENTCONFIG" . "/etc/pulse/client.conf"))) - (define (pulseaudio-conf-entry arg) (match arg ((key . value) @@ -139,21 +135,22 @@ ctl.!default { ((? string? _) (string-append arg "\n")))) +(define pulseaudio-environment + (match-lambda + (($ <pulseaudio-configuration> client-conf daemon-conf default-script-file) + `(("PULSE_CONFIG" . ,(apply mixed-text-file "daemon.conf" + "default-script-file = " default-script-file "\n" + (map pulseaudio-conf-entry daemon-conf))) + ("PULSE_CLIENTCONFIG" . ,(apply mixed-text-file "client.conf" + (map pulseaudio-conf-entry client-conf))))))) + (define pulseaudio-etc (match-lambda - (($ <pulseaudio-configuration> client-conf daemon-conf - default-script-file system-script-file) + (($ <pulseaudio-configuration> _ _ default-script-file system-script-file) `(("pulse" ,(file-union "pulse" - `(("client.conf" - ,(apply mixed-text-file "client.conf" - (map pulseaudio-conf-entry client-conf))) - ("daemon.conf" - ,(apply mixed-text-file "daemon.conf" - "default-script-file = " default-script-file "\n" - (map pulseaudio-conf-entry daemon-conf))) - ("default.pa" ,default-script-file) + `(("default.pa" ,default-script-file) ("system.pa" ,system-script-file)))))))) (define pulseaudio-service-type -- 2.26.2
From 3864b54f4aadefc600433d3654b0a1a73ab6fa98 Mon Sep 17 00:00:00 2001 From: Jack Hill <jackh...@jackhill.us> Date: Sat, 25 Apr 2020 22:03:48 -0400 Subject: [PATCH 2/2] gnu: webkitgtk: Patch to share store via Bubblewrap. Fixes <https://bugs.gnu.org/40837>. * gnu/packages/patches/webkitgtk-share-store.patch: New file. * gnu/local.mk (dist_patch_DATA): Add it. * gnu/packages/webkit.scm (webkitgtk)[source](patches): Use it. Co-authored-by: Marius Bakke <mba...@fastmail.com> --- gnu/local.mk | 1 + .../patches/webkitgtk-share-store.patch | 20 +++++++++++++++++++ gnu/packages/webkit.scm | 12 ++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 gnu/packages/patches/webkitgtk-share-store.patch diff --git a/gnu/local.mk b/gnu/local.mk index 62eeb39ece..5c06415205 100644 --- a/gnu/local.mk +++ b/gnu/local.mk @@ -1542,6 +1542,7 @@ dist_patch_DATA = \ %D%/packages/patches/vte-CVE-2012-2738-pt2.patch \ %D%/packages/patches/warsow-qfusion-fix-bool-return-type.patch \ %D%/packages/patches/weasyprint-library-paths.patch \ + %D%/packages/patches/webkitgtk-share-store.patch \ %D%/packages/patches/websocketpp-fix-for-boost-1.70.patch \ %D%/packages/patches/wicd-bitrate-none-fix.patch \ %D%/packages/patches/wicd-get-selected-profile-fix.patch \ diff --git a/gnu/packages/patches/webkitgtk-share-store.patch b/gnu/packages/patches/webkitgtk-share-store.patch new file mode 100644 index 0000000000..4174e73b6c --- /dev/null +++ b/gnu/packages/patches/webkitgtk-share-store.patch @@ -0,0 +1,20 @@ +Author: Jack Hill <jackh...@jackhill.us> +Tell bubblewrap to share the store. + +See <https://bugs.gnu.org/40837>. + +--- +diff --git a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp +index ad301ab2..d53b680e 100644 +--- a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp ++++ b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp +@@ -737,6 +737,9 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces + "--ro-bind-try", "/usr/local/share", "/usr/local/share", + "--ro-bind-try", DATADIR, DATADIR, + ++ // Bind mount the store inside the WebKitGTK sandbox. ++ "--ro-bind", "@storedir@", "@storedir@", ++ + // We only grant access to the libdirs webkit is built with and + // guess system libdirs. This will always have some edge cases. + "--ro-bind-try", "/lib", "/lib", diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm index e52536c279..6035d6c59d 100644 --- a/gnu/packages/webkit.scm +++ b/gnu/packages/webkit.scm @@ -128,7 +128,8 @@ engine that uses Wayland for graphics output.") "webkitgtk-" version ".tar.xz")) (sha256 (base32 - "1g9hik3bprki5s9d7y5288q5irwckbzajr6rnlvjrlnqrwjkblmr")))) + "1g9hik3bprki5s9d7y5288q5irwckbzajr6rnlvjrlnqrwjkblmr")) + (patches (search-patches "webkitgtk-share-store.patch")))) (build-system cmake-build-system) (outputs '("out" "doc")) (arguments @@ -156,6 +157,15 @@ engine that uses Wayland for graphics output.") "-DUSE_WOFF2=OFF") #:phases (modify-phases %standard-phases + (add-after 'unpack 'configure-bubblewrap-store-directory + (lambda _ + ;; This phase is a corollary to 'webkitgtk-share-store.patch' to + ;; avoid hard coding /gnu/store, for users with other prefixes. + (let ((store-directory (%store-directory))) + (substitute* + "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp" + (("@storedir@") store-directory)) + #t))) (add-after 'unpack 'patch-gtk-doc-scan (lambda* (#:key inputs #:allow-other-keys) (for-each (lambda (file) -- 2.26.2
signature.asc
Description: PGP signature
