diff mbox series

[bug#39127] fixing icecat's multimedia

Message ID 20200114015819.713f4e4f@tachikoma.lepiller.eu
State Accepted
Headers show
Series [bug#39127] fixing icecat's multimedia | expand

Checks

Context Check Description
cbaines/applying patch fail Apply failed

Commit Message

Julien Lepiller Jan. 14, 2020, 12:58 a.m. UTC 
From IRC yesterday, I found that icecat was still missing something to
properly read multimedia streams, like mp3/mp4. In the current version,
it now tries to open ffmpeg's library dynamically, by looking in the
store, instead of standard locations (/usr/lib etc). But this is not
enough: even if icecat can properly find the library, it cannot load it
because it uses a sandboxing feature that only allows it to read and
write files from/to specific locations. /gnu/store is not part of them.

Since icecat has access to /lib and /usr/lib, I think we can also give
it read access (not write) to /gnu/store. This patch attempts to do
just that, but I couldn't build icecat because of a lack of space. It
sets the default security.sandbox.content.read_path_whitelist to
/gnu/store/, the leading / meaning "and everything under it,
recursively").

Wdyt?

Comments

ashish.is--- via Guix-patches" via Jan. 14, 2020, 1:29 a.m. UTC | #1 
Julien,

Thanks!  For anything with ‘security’ *and* ‘sandbox’ in the name 
we should definitely involve IceCat upstream.

Julien Lepiller 写道：
> (substitute* "browser/app/profile/icecat.js"
>   (("\"security.sandbox.content.read_path_whitelist\", \"\"")
>    (string-append
>      "\"security.sandbox.content.read_path_whitelist\", \""
>      (%store-directory) "/\"")))

When I asked bandali on IRC a few weeks(?) ago about this exact 
patch, they didn't sound convinced.  But we were both quite unsure 
:-)  Have things changed?  Have you talked to Mark?

> Since icecat has access to /lib and /usr/lib, I think we can 
> also give
> it read access (not write) to /gnu/store.

That sounds reasonable, if you're certain that it's read-only.

> Wdyt?

LGTM from the Guix side.

Kind regards,

T G-R
Julien Lepiller Jan. 14, 2020, 1:36 a.m. UTC | #2 
Le Tue, 14 Jan 2020 02:29:20 +0100,
Tobias Geerinckx-Rice <me@tobias.gr> a écrit :

> Julien,
> 
> Thanks!  For anything with ‘security’ *and* ‘sandbox’ in the name 
> we should definitely involve IceCat upstream.
> 
> Julien Lepiller 写道：
> > (substitute* "browser/app/profile/icecat.js"
> >   (("\"security.sandbox.content.read_path_whitelist\", \"\"")
> >    (string-append
> >      "\"security.sandbox.content.read_path_whitelist\", \""
> >      (%store-directory) "/\"")))  
> 
> When I asked bandali on IRC a few weeks(?) ago about this exact 
> patch, they didn't sound convinced.  But we were both quite unsure 
> :-)  Have things changed?  Have you talked to Mark?

I haven't talked to Mark, but here's how you can check:

set security.sandbox.content.read_path_whitelist in about:config to an
empty string (the default) and restart icecat. It cannot play the video
from https://harmonist.tuxfamily.org/. It doesn't work. Set it to
/gnu/store/ (with a trailing /) and restart the browser. Now the video
works. This patch attempts to make the working scenario the default :)

> 
> > Since icecat has access to /lib and /usr/lib, I think we can 
> > also give
> > it read access (not write) to /gnu/store.  
> 
> That sounds reasonable, if you're certain that it's read-only.
> 
> > Wdyt?  
> 
> LGTM from the Guix side.
> 
> Kind regards,
> 
> T G-R
ashish.is--- via Guix-patches" via Jan. 14, 2020, 1:42 a.m. UTC | #3 
Julien,

Julien Lepiller 写道：
> I haven't talked to Mark, but here's how you can check:

[…]

I meant about any potential security issues or alternative 
solutions (e.g. restricting access to less than the entire store).

I was already aware of the problem and this work-around, and can 
confirm that it works.

Kind regards,

T G-R
ashish.is--- via Guix-patches" via Jan. 16, 2020, 9:04 a.m. UTC | #4 
Fixed by mhw[0] in commit 
429c8284d232c3f9fbe3dc87a3da323f3a864c03, so closing this one.

Thanks!

T G-R

[0]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38831#5
diff mbox series

Patch

From adf7fdeffaa806edcd8abdac0746c06dad52c495 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Tue, 14 Jan 2020 01:48:42 +0100
Subject: [PATCH] gnu: icecat: Give access to the store to the sandbox.

* gnu/packages/gnuzilla.scm (icecat): Add punch-hole-in-sandbox phase.
---
 gnu/packages/gnuzilla.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index 62b4390eab..14f446ee0a 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -971,6 +971,14 @@  from forcing GEXP-PROMISE."
                (("libavcodec\\.so")
                 (string-append (assoc-ref inputs "ffmpeg") "/lib/libavcodec.so")))
              #t))
+         (add-after 'fix-ffmpeg-runtime-linker 'punch-hole-in-sandbox
+           (lambda _
+             (substitute* "browser/app/profile/icecat.js"
+               (("\"security.sandbox.content.read_path_whitelist\", \"\"")
+                (string-append
+                  "\"security.sandbox.content.read_path_whitelist\", \""
+                  (%store-directory) "/\"")))
+             #t))
          (replace 'bootstrap
            (lambda _
              (invoke "sh" "-c" "autoconf old-configure.in > old-configure")
-- 
2.24.0