bug#40837: core-updates: webkitgtk web process sandbox incomplete

[Marius Bakke <mbakke@fastmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2626 bytes --]

Jack Hill <jackhill@jackhill.us> writes:

> On Wed, 6 May 2020, Marius Bakke wrote:
>
>> Hello Jack,
>>
>> Thanks a lot for this work.
>
> You're welcome. I'm happy that we seem to be making good progress.
>
>> Jack Hill <jackhill@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.
>
> Good idea! That way we won't have to wait for WebKitGTK to canonicalize 
> all paths :)
>
>>> [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 don't think /run/current-system is needed.

Excellent.  I tested Epiphany with these patches on a popular video
streaming site and everything seemed fine.

>> 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.
>
> I've tested the updated patch by applying it to master and merging into 
> core-updates. I'm happy to report that everything seems to be working for 
> me after doing so!
>
> Sharing less than the whole store sounds like a great aspiration, but I 
> think we'd have to teach WebKitGTK how to ask Guix for its closure to do 
> so. On FHS-compliant systems, all of the various /usr/lib and /usr/share 
> directories are bind-mounted into the new namespace, so I don't think 
> we're providing too much more. It's nice that our setuid binaries reside 
> outside of the store :)

Indeed, thanks for testing and confirming.

I added a little more context in the patch description and finally
pushed it as a6919866b07e9ed3986abde7ae48d0c69ff3deed.

Again, thank you very much for taking care of this.  :-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]