enhance sandbox on OpenBSD with unveil()
Categories
(Core :: Security: Process Sandboxing, enhancement, P5)
Tracking
()
Tracking | Status | |
---|---|---|
firefox72 | --- | fixed |
People
(Reporter: jcs, Assigned: jcs)
References
Details
Attachments
(7 files, 8 obsolete files)
Use the unveil
system call on OpenBSD to lock down filesystem access. Our OpenBSD package of Firefox will contain 3 files listing the files that the main, content, and GPU processes are allowed to access. These are loaded in at sandbox initialization time, and each passed to unveil()
. Once the pledge
call is executed after this, filesystem access is locked down.
Depends on #1580268
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Comment 3•5 years ago
|
||
See the unveil.* files in https://github.com/openbsd/ports/tree/master/www/chromium/files for the list of paths whitelisted/allowed per process type in our chromium port.
Comment 4•5 years ago
|
||
Comment 5•5 years ago
|
||
Comment 6•5 years ago
|
||
Updating unveil.main
, it also needs read access to /dev/urandom
or the new bookmark window won't draw correctly (?!)
Comment 8•5 years ago
|
||
for pledge we use about:config strings for that wont work for unveil paths lists.
Why not?
FYI we do offer that option on Linux so "weird" distros and users with unusual setups can help themselves:
https://searchfox.org/mozilla-central/source/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#494
It's a comma-separated list.
Right now, filesystem isolation is configured this way in our chromium port but i'm not comfortable with adding /etc/firefox. A default file under browser/defaults/preferences/, overridable by the user ?
Some kind of prefs file seems like it would work, but by default those are in the profile. I don't think we expect prefs in a system-wide location, but I'm far from the expert here. As far as bundling, it seems only channel-prefs is shipped, and the default prefs.js is reconstructed when we write out the profile.
I think I'll need to ask someone who knows our prefs system better to see what the best option is.
Comment 9•5 years ago
|
||
Nicholas, see above comments for context. Do you have any advice where a "system-wide" Firefox preference file should live, or if that would even be a good idea (as opposed to a per-profile config) in this case?
Comment 10•5 years ago
|
||
We don't currently have any system-wide prefs file that I know of, and I am reluctant to add one. It sounds challenging to get right.
mkaply might have opinions. Does this relate to any of our enterprise policy stuff? I don't understand that well.
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
Technically any file in default/pref is a system wide preference file.
So we know what kinds of things users will need to add for this?
Comment 13•5 years ago
|
||
If we can use an about:config string that's fine, but the current tuples (path, perms) list could get large and not easily editable in about:config. I would personally prefer that of course, just not sure if we'd hit the limit.
(In reply to Gian-Carlo Pascutto [:gcp] from comment #8)
FYI we do offer that option on Linux so "weird" distros and users with unusual setups can help themselves:
https://searchfox.org/mozilla-central/source/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#494
It's a comma-separated list.
so there's a "default list" that is hardcoded, and on top of that two prefs for extra exclusions/inclusions - good, didnt know that. Maybe we could reuse this idea, once we're comfortable with the final paths lists..
(In reply to Nicholas Nethercote [:njn] from comment #10)
We don't currently have any system-wide prefs file that I know of, and I am reluctant to add one. It sounds challenging to get right.
Totally agree with that, /etc/firefox feels deeply wrong to me. If we need a systemwide extra pref file, default/pref under the install dir is the way to go imo. And if the user wants to override it, he copies the file to his profile and edits it there.
Assignee | ||
Comment 14•5 years ago
|
||
Comment 15•5 years ago
|
||
If we can use an about:config string that's fine, but the current tuples (path, perms) list could get large and not easily editable in about:config. I would personally prefer that of course, just not sure if we'd hit the limit.
There is a hard 1 MiB limit for string prefs. Furthermore, any string pref that exceeds 4 KiB will not be sent to content processes; I don't know if that's relevant here.
Is there something specific that would be preventing that?
My first guess would be the sandbox, but it's just a guess.
Assignee | ||
Comment 16•5 years ago
|
||
Here's a rewrite of the patch that uses more of the C++ API for file IO and string manipulation, but it fails in the GPU process because the first nsCOMPtr<nsIFile> lf(do_CreateInstance(NS_LOCAL_FILE_CONTRACTID, &rv))
line returns a failure. It works fine in the main and content processes.
Updated•5 years ago
|
Comment 17•5 years ago
|
||
My first guess would be the sandbox, but it's just a guess.
They're on OpenBSD, so they don't see our Linux seccomp filter & broker. In fact, this config file that won't load contains the paths that are supposed to be loadable after they do the unveil calls.
Is the GPU process actually enabled on OpenBSD? Could it be that it's not and the second call fails because we already restricted filesystem access in this process?
Comment 18•5 years ago
|
||
If we can use an about:config string that's fine, but the current tuples (path, perms) list could get large and not easily editable in about:config. I would personally prefer that of course, just not sure if we'd hit the limit.
Would you or wouldn't you prefer that? I'm fine with either approach suggested so far (i.e. defaults/prefs, even though that seems basically empty on my Linux installs, or prefs). We can do as for Linux and take a default list that we know is going to be needed in the source too.
Assignee | ||
Comment 19•5 years ago
|
||
Is the GPU process actually enabled on OpenBSD?
It is in that it starts up (see #1580268) but I'm not sure whether you mean something else by "enabled".
Comment 20•5 years ago
|
||
(In reply to Gian-Carlo Pascutto [:gcp] from comment #18)
If we can use an about:config string that's fine, but the current tuples (path, perms) list could get large and not easily editable in about:config. I would personally prefer that of course, just not sure if we'd hit the limit.
Would you or wouldn't you prefer that? I'm fine with either approach suggested so far (i.e. defaults/prefs, even though that seems basically empty on my Linux installs, or prefs). We can do as for Linux and take a default list that we know is going to be needed in the source too.
I would be fine with going with two string prefs for user-level overrides, and a default hardcoded list in the source if we had a minimal & stable "trusted list" of paths/perms - iirc (but i might be wrong, please correct me jcs) the current lists has been taken straight from the chromium port, with some additions, and might still need refinement in some corner cases that will only be discovered once users start dogfooding the feature.
(In reply to Nicholas Nethercote [:njn] from comment #15)
There is a hard 1 MiB limit for string prefs. Furthermore, any string pref that exceeds 4 KiB will not be sent to content processes; I don't know if that's relevant here.
from my understanding the content process receives its config on the cmdline, and that should include the list of paths to unveil with which perms. How are the >4kb prefs sent then ?
Anyway, right now the paths lists with comments are 1.3kb for main/content processes so we should be fine.
Assignee | ||
Comment 21•5 years ago
|
||
The failure from do_CreateInstance
was NS_ERROR_NOT_INITIALIZED
, which lead me to find that NS_InitMinimalXPCOM
needed to be called first. This is actually done already in GPUParent::Init
(from the child, not the parent), so I just need to delay the call to StartOpenBSDSandbox
until after NS_InitMinimalXPCOM
is called. This fixed the preferences not loading in #1580268, and now the error I get from do_CreateInstance
is NS_ERROR_FACTORY_NOT_REGISTERED
which seems right if the GPU process is not supposed to be loading files.
So I can revert back to using the C FILE
API instead, unless I am supposed to be allowed to access files from the GPU child and there's something else wrong here.
Assignee | ||
Comment 22•5 years ago
|
||
I suppose if we're moving the unveil strings into a preference, I won't need the file API anyway. I'll work moving these to preferences.
Assignee | ||
Comment 23•5 years ago
|
||
3rd revision of unveil patch, now reading file lists from preferences.
Comment 24•5 years ago
|
||
from my understanding the content process receives its config on the cmdline, and that should include the list of paths to unveil with which perms. How are the >4kb prefs sent then ?
That was true at one point, but has since changed. Prefs are now all sent via proper IPC mechanisms such as shared memory.
Comment 25•5 years ago
|
||
So I can revert back to using the C FILE API instead, unless I am supposed to be allowed to access files from the GPU child and there's something else wrong here.
I think that should work but it's quite possible there's nothing else using our File APIs in GPUParent (in the child, sigh!) right now.
That was true at one point, but has since changed. Prefs are now all sent via proper IPC mechanisms such as shared memory.
FWIW this passing/setup happens here, which is close to where you originally added the pref transfer in the other bug:
https://searchfox.org/mozilla-central/source/gfx/ipc/GPUProcessImpl.cpp#67
Comment 26•5 years ago
|
||
Assignee | ||
Comment 27•5 years ago
|
||
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
Comment on attachment 9092480 [details] [diff] [review]
ff-unveil3.diffReview of attachment 9092480 [details] [diff] [review]:
rwc in ~/Downloads from content seems seriously wrong.
::: browser/app/profile/firefox.js
@@ +1137,1 @@pref("security.sandbox.pledge.content", "stdio rpath wpath cpath inet recvfd sendfd prot_exec unix drm ps");
Wonder how close our SocketProcess work brings us to being able to nuke the
inet pledge here.
I've been running without inet for many days since another OpenBSD developer mentioned it and I haven't seen any fallout from it. I'm not sure what to expect to break without it, so I didn't want to include it here, but I can open it as another bug if there is no fallout.
Aside from that, drm is the biggie to get rid of? I would think wpath isn't
so critical if unveil works?
Yeah, the unveil really makes the *path ones not so scary.
@@ +1139,5 @@
pref("security.sandbox.pledge.gpu", "stdio rpath wpath cpath ps sendfd recvfd drm dns unix prot_exec");
+
+// default file paths unveiled to each process, bug 1580271
+pref("security.sandbox.unveil.main", "/dev/urandom r,/dev/video rw,/etc/fonts r,/etc/machine-id r,/tmp rwc,/usr/bin/lpr rx,/usr/local/bin/xdg-desktop-menu rx,/usr/local/bin/xdg-open rx,/usr/local/bin/xdg-settings rx,/usr/local/lib r,/usr/local/firefox r,/usr/local/lib/firefox rx,/usr/local/share r,/usr/share/locale r,/var/cache/fontconfig r,/usr/X11R6/lib r,/usr/X11R6/share r,/var/run rw,/.XCompose r,/.Xauthority r,/.Xdefaults r,/.fontconfig r,/.fonts r,/.fonts.conf r,/.fonts.conf.d r,/.icons r,/.mozilla rwc,/.pki rwc,/.sndio rwc,/.terminfo r,/.cache/dconf rwc,/.cache/thumbnails rwc,/.config/dconf r,/.config/fontconfig r,/.config/gtk-3.0 r,/.config/mimeapps.list r,/.config/mozilla rwc,/.config/user-dirs.dirs r,/.local/share/applications rwc,/.local/share/applnk r,/.local/share/fonts r,/.local/share/glib-2.0 r,/.local/share/icons r,/.local/share/mime r,/.local/share/recently-used.xbel rwc,/.local/share/themes r,/Downloads rwc");/.XCompose r,
+pref("security.sandbox.unveil.content", "/dev/drm0 rw,/etc/fonts r,/etc/machine-id r,/tmp rwc,/usr/bin/lpr rx,/usr/local/bin/xdg-desktop-menu rx,/usr/local/bin/xdg-open rx,/usr/local/bin/xdg-settings rx,/usr/local/lib r,/usr/local/firefox r,/usr/local/lib/firefox rx,/usr/local/share r,/usr/share/locale r,/var/cache/fontconfig r,/usr/X11R6/lib r,/usr/X11R6/share r,/var/run rw,/.Xauthority r,/.Xdefaults r,/.fontconfig r,/.fonts r,/.fonts.conf r,/.fonts.conf.d r,/.icons r,/.mozilla rwc,/.pki rwc,/.sndio rwc,/.terminfo r,/.cache/dconf rwc,/.cache/thumbnails rwc,/.config/dconf r,/.config/fontconfig r,/.config/gtk-3.0 r,/.config/mimeapps.list r,/.config/mozilla rwc,/.config/user-dirs.dirs r,/.local/share/applications rwc,/.local/share/applnk r,/.local/share/fonts r,/.local/share/glib-2.0 r,/.local/share/icons r,/.local/share/mime r,/.local/share/recently-used.xbel rwc,/.local/share/themes r,/Downloads rwc");See remark below about /tmp rwc
~/.sndio rwc seems potentially problematic. No idea how easy it is to get
the audio remoting working on OpenBSD though.
~/.cache/dconf rwc similar issues.
~/Downloads rwc seems like a total no-go? I doubt it's needed?!
Changed to 'r' so downloaded files can be viewed.
@@ +1140,5 @@
+// default file paths unveiled to each process, bug 1580271
+pref("security.sandbox.unveil.main", "/dev/urandom r,/dev/video rw,/etc/fonts r,/etc/machine-id r,/tmp rwc,/usr/bin/lpr rx,/usr/local/bin/xdg-desktop-menu rx,/usr/local/bin/xdg-open rx,/usr/local/bin/xdg-settings rx,/usr/local/lib r,/usr/local/firefox r,/usr/local/lib/firefox rx,/usr/local/share r,/usr/share/locale r,/var/cache/fontconfig r,/usr/X11R6/lib r,/usr/X11R6/share r,/var/run rw,/.XCompose r,/.Xauthority r,/.Xdefaults r,/.fontconfig r,/.fonts r,/.fonts.conf r,/.fonts.conf.d r,/.icons r,/.mozilla rwc,/.pki rwc,/.sndio rwc,/.terminfo r,/.cache/dconf rwc,/.cache/thumbnails rwc,/.config/dconf r,/.config/fontconfig r,/.config/gtk-3.0 r,/.config/mimeapps.list r,/.config/mozilla rwc,/.config/user-dirs.dirs r,/.local/share/applications rwc,/.local/share/applnk r,/.local/share/fonts r,/.local/share/glib-2.0 r,/.local/share/icons r,/.local/share/mime r,/.local/share/recently-used.xbel rwc,/.local/share/themes r,/Downloads rwc");/.XCompose r,
+pref("security.sandbox.unveil.content", "/dev/drm0 rw,/etc/fonts r,/etc/machine-id r,/tmp rwc,/usr/bin/lpr rx,/usr/local/bin/xdg-desktop-menu rx,/usr/local/bin/xdg-open rx,/usr/local/bin/xdg-settings rx,/usr/local/lib r,/usr/local/firefox r,/usr/local/lib/firefox rx,/usr/local/share r,/usr/share/locale r,/var/cache/fontconfig r,/usr/X11R6/lib r,/usr/X11R6/share r,/var/run rw,/.Xauthority r,/.Xdefaults r,/.fontconfig r,/.fonts r,/.fonts.conf r,/.fonts.conf.d r,/.icons r,/.mozilla rwc,/.pki rwc,/.sndio rwc,/.terminfo r,/.cache/dconf rwc,/.cache/thumbnails rwc,/.config/dconf r,/.config/fontconfig r,/.config/gtk-3.0 r,/.config/mimeapps.list r,/.config/mozilla rwc,/.config/user-dirs.dirs r,/.local/share/applications rwc,/.local/share/applnk r,/.local/share/fonts r,/.local/share/glib-2.0 r,/.local/share/icons r,/.local/share/mime r,/.local/share/recently-used.xbel rwc,/.local/share/themes r,/Downloads rwc");
+pref("security.sandbox.unveil.gpu", "/dev/drm0 rw,/tmp rwc,/usr/local/lib/firefox r,/usr/local/lib/gdk-pixbuf-2.0 r,/usr/X11R6/lib r,/usr/share/locale r,/usr/local/share r,~/.Xauthority r");Do you need the rwc in /tmp?
The GPU process is unsandboxed now on other platforms (temporarily!), but
when it was still in content we remapped /tmp access to a specific other
dir. AFAIK the only thing that needed access to it were shader caches (which
are optional) and fontconfig that wanted to write out a random file.See for example: https://bugzilla.mozilla.org/show_bug.cgi?id=1386404
You might be able to outright block it without any real ill effects, which
is worth considering.
Yes, rwc is needed for tmp for the shm files. Without all 3 one gets:
[Child 77282, Main Thread] WARNING: failed to open shm: Permission denied: file /home/jcs/code/firefox/ipc/chromium/src/base/shared_memory_posix.cc, line 142
Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Failed to connect WebRenderBridgeChild. (t=1.44932) |[C151][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=10.8195) |[C152][GFX1]: ValidateTile failed (t=10.8195) |[C153][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=10.8711) |[C154][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=11.0112) |[C155][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=11.0814) |[C156][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=11.1069) |[C142][GFX1]: ValidateTile failed (t=10.8187) |[C143][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=10.8188) |[C144][GFX1]: ValidateTile failed (t=10.8189) |[C145][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=10.819) |[C146][GFX1]: ValidateTile failed (t=10.8191) |[C147][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=10.8192) |[C148][GFX1]: ValidateTile failed (t=10.8193) |[C149][GFX1]: [Tiling:Client] Failed to allocate a TextureClient (t=10.8193) |[C150][GFX1]: ValidateTile failed (t=10.8194) [GFX1]: [Tiling:Client] Failed to allocate a TextureClient
::: dom/ipc/ContentChild.cpp
@@ +4155,5 @@
nUnveils++;
- }
- if (!nUnveils) {
This is used as a boolean only, so you could make it one.
Ok, will change.
Assignee | ||
Comment 28•5 years ago
|
||
Comment 29•5 years ago
|
||
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
~/.sndio rwc seems potentially problematic. No idea how easy it is to get
the audio remoting working on OpenBSD though.
I had a start at it in bug #1467982 but that got stalled on bug #1467872 - now that the latter is fixed, maybe i could revisit this if i find hacking time...
(oh, and many thanks for the review/feedback on unveil paths/pledge promises !)
Comment 30•5 years ago
|
||
(In reply to jcs from comment #27)
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
@@ +1137,1 @@
pref("security.sandbox.pledge.content", "stdio rpath wpath cpath inet recvfd sendfd prot_exec unix drm ps");
Wonder how close our SocketProcess work brings us to being able to nuke the
inet pledge here.I've been running without inet for many days since another OpenBSD developer mentioned it and I haven't seen any fallout from it. I'm not sure what to expect to break without it, so I didn't want to include it here, but I can open it as another bug if there is no fallout.
I looked at bit in my git history and inet was in my first wip on pledging the content process (https://cgit.rhaalovely.net/mozilla-firefox/commit/files/all-openbsd.js?id=db0ed17a814399422112130000d8e6508ea6ea9f), and i have absolutely zero memory why.
If we're fine without it that's great. Will test that on my installs.
Comment 31•5 years ago
|
||
(In reply to jcs from comment #27)
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
~/Downloads rwc seems like a total no-go? I doubt it's needed?!
Changed to 'r' so downloaded files can be viewed.
If this is about navigating to file:///
URLs, there's a dedicated content process for those (see FILE_REMOTE_TYPE
); with the existing sandbox implementations that process gets global file read access and others don't.
[Child 77282, Main Thread] WARNING: failed to open shm: Permission denied: file /home/jcs/code/firefox/ipc/chromium/src/base/shared_memory_posix.cc, line 142
There's a per-process prefix for the shm_open
names (see base::SharedMemory::AppendPosixShmPrefix
), if that would help restrict the /tmp
access. (Also, that error message from line 142 looks like an older version of the source.)
Comment 32•5 years ago
|
||
(In reply to Landry Breuil (:gaston) from comment #30)
I looked at bit in my git history and inet was in my first wip on pledging the content process (https://cgit.rhaalovely.net/mozilla-firefox/commit/files/all-openbsd.js?id=db0ed17a814399422112130000d8e6508ea6ea9f), and i have absolutely zero memory why.
Remote X11, maybe? (Including ssh forwarding, assuming that OpenBSD ssh uses IP loopback like portable OpenSSH does.) See also bug 1376559.
Comment 33•5 years ago
|
||
There's a larger problem with unveil
: as I understand it, it builds up the allowed list non-atomically, and adding the first entry changes the policy from allow-all to default-deny, which means that if another thread is trying to access the filesystem during this process, it can get spurious denials.
This could be avoided by unveiling early in startup when the process is single-threaded (note that starting IPC creates threads), but that might require adding more permissions to the policies.
Assignee | ||
Comment 34•5 years ago
|
||
Are the preferences available for reading that early?
Assignee | ||
Comment 35•5 years ago
|
||
(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #31)
(In reply to jcs from comment #27)
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
~/Downloads rwc seems like a total no-go? I doubt it's needed?!
Changed to 'r' so downloaded files can be viewed.
If this is about navigating to
file:///
URLs, there's a dedicated content process for those (seeFILE_REMOTE_TYPE
); with the existing sandbox implementations that process gets global file read access and others don't.
Without the 'r' for ~/Downloads, the tab just shows an error page saying it can't read the file.
Comment 36•5 years ago
|
||
(In reply to jcs from comment #35)
(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #31)
(In reply to jcs from comment #27)
(In reply to Gian-Carlo Pascutto [:gcp] from comment #26)
~/Downloads rwc seems like a total no-go? I doubt it's needed?!
Changed to 'r' so downloaded files can be viewed.
If this is about navigating to
file:///
URLs, there's a dedicated content process for those (seeFILE_REMOTE_TYPE
); with the existing sandbox implementations that process gets global file read access and others don't.Without the 'r' for ~/Downloads, the tab just shows an error page saying it can't read the file.
Iirc some release ago, i tried triggerring the dedicated content process for file:// urls and failed to have anything clearly separated out from the other content processes, but maybe i tried wrong.
Assignee | ||
Comment 37•5 years ago
|
||
Here's a fifth version that honors $XDG_CONFIG_HOME
, $XDG_CACHE_HOME
, and $XDG_DATA_HOME
being set in the user's environment, replacing hard-coded paths ~/.config
, ~/.cache
, and ~/.local/share
in the unveil list with those variables (and then expanding them).
Assignee | ||
Comment 38•5 years ago
|
||
Landry pointed out that executing 3rd party apps for MIME handlers (such as mupdf for viewing PDFs) broke, so this adds a workaround.
Any handler not in the unveil list won't show up in the "Open With" dialog because at least Glib checks that the binary is accessible, so this adds a generic system default entry which executes xdg-open
on the temporary file. Since xdg-open
will be outside the unveil/pledge, it can execute whatever application is registered as the default. This is kind of confusing because Glib executes that handler (xdg-open
in this case) by way of executing gio-launch-desktop
, so only gio-launch-desktop
needs to be in the pledge list.
This also adds /etc/mailcap
and ~/.mailcap
to the unveil list, so that older-style mailcap handlers still work if the executable is explicitly added to the unveil list by the user. For example, adding application/pdf; /usr/local/bin/xpdf %s
to ~/.mailcap
and then adding /usr/local/bin/xpdf rx
to security.sandbox.unveil.main
will allow it to show up in the list and get executed when selected in the Firefox dialog.
Assignee | ||
Comment 39•5 years ago
|
||
A new revision, rebased on top of #1584839
Comment 40•5 years ago
|
||
Landry pointed out that executing 3rd party apps for MIME handlers (such as mupdf for viewing PDFs) broke, so this adds a workaround.
IIRC We had it as a task to make sure MIME remoting is enabled on all platforms. Let me nag haik to find the bug...
Iirc some release ago, i tried triggerring the dedicated content process for file:// urls and failed to have anything clearly separated out from the other content processes, but maybe i tried wrong.
I'm not sure what you mean exactly, but you can grep the source for FILE_REMOTE_TYPE to see the related logic and how to detect that you're in this process.
Comment 41•5 years ago
|
||
(In reply to Gian-Carlo Pascutto [:gcp] from comment #40)
Landry pointed out that executing 3rd party apps for MIME handlers (such as mupdf for viewing PDFs) broke, so this adds a workaround.
IIRC We had it as a task to make sure MIME remoting is enabled on all platforms. Let me nag haik to find the bug...
We have bug 1446549. My plan has been to enable bug 1452278 for all platforms to address this.
This can be tested by changing nsExternalHelperAppService::GetSingleton() to use nsOSHelperAppServiceChild for child processes like it does for Mac on other platforms.
Assignee | ||
Comment 42•5 years ago
|
||
New patch against Firefox 70
Comment 43•5 years ago
|
||
(In reply to Landry Breuil (:gaston) from comment #36)
(In reply to jcs from comment #35)
(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #31)
If this is about navigating to
file:///
URLs, there's a dedicated content process for those (seeFILE_REMOTE_TYPE
); with the existing sandbox implementations that process gets global file read access and others don't.Without the 'r' for ~/Downloads, the tab just shows an error page saying it can't read the file.
Iirc some release ago, i tried triggerring the dedicated content process for file:// urls and failed to have anything clearly separated out from the other content processes, but maybe i tried wrong.
Fwiw, outside of this unveil work, just tried in 71.0b5, navigating to a file:/// url in a new tab indeed creates a new distinct process, and about:support shows a 'local file : 1' line in 'remote processes' section, so pledge/unveil can/should be applied with distinct params (ie full filesystem access but sensitive dirs/files ? or restricted to usr/local/share where most html/doc files live ? what about opening html files in ~ ?) to those FILE_REMOTE_TYPE processes.
If i open another file:// url in a new tab, it seems the same process is reused.
Comment 44•5 years ago
|
||
Hmm..i think i'll need to retract what i just said, apparently FILE_REMOTE_TYPE is not a value of the GeckoProcessType enum in https://searchfox.org/mozilla-central/source/xpcom/build/GeckoProcessTypes.h#25 so we have no way to distinguish a 'file://' tab process from a regular tab process.. unless we use a similar construct to https://searchfox.org/mozilla-central/source/dom/ipc/ContentChild.cpp#1696, but we have no access to the ContentChild object in StartOpenBSDSandbox().
Comment 45•5 years ago
|
||
ExpandUnveilPath() takes care of expanding potentially
environment-specific XDG_DATA/CONFIG/CACHE_HOME dirs.
The unveil config files lists the allowed paths & modes.
'disable' in the files will disable the corresponding pledge/unveil
syscall.
Depends on D51386
Comment 46•5 years ago
|
||
g_app_info_get_default_for_type() will fail on OpenBSD's veiled
filesystem since we most likely don't have direct access to the binaries
that are registered as defaults for this type. Fake it up by just
executing xdg-open via gio-launch-desktop (which we do have access to)
and letting it figure out which program to execute for this MIME type.
This has the side-effect of ignoring/losing MIME types associations
registered in firefox prefs only.
Depends on D51387
Assignee | ||
Comment 47•5 years ago
|
||
New patch relative to #1580268
Comment 48•5 years ago
|
||
At that point, i think it's simpler to move the patches to phabricator as i started doing, that's the required (painful?) process for proper review/upstreaming.. and im pretty sure several ppl can work incrementally on the same patchset, creating independent revisions/diffs on phabricator. I still need to get used to it :)
I tried keeping authorship in the hg commits, but somehow bugzilla/phabricator seem to show me as author ? weird, at least that's not the case on the commits.
Comment 49•5 years ago
|
||
One thing i noted - to disable unveil, one needs to put 'disable ' (with a trailing space) otherwise it wont start up and complain about firefox: /etc/firefox/unveil.main: line 1: invalid format
.
I need to figure out what paths i need to unveil now that i have a trunk build with the complete patch stack..
Comment 50•5 years ago
|
||
And with /home/landry/firefox r
in unveil.main
& unveil.content
+ /home/landry/firefox/firefox rx
in unveil.main
, my trunk build untarred in ~/firefox runs fine, yay.
Assignee | ||
Comment 51•5 years ago
|
||
This should fix the disable problem. I'm not going to keep uploading new patches since you're working on these somewhere else.
diff --git dom/ipc/ContentChild.cpp dom/ipc/ContentChild.cpp
index e42c4453b5..91bd60fa64 100644
--- dom/ipc/ContentChild.cpp
+++ dom/ipc/ContentChild.cpp
@@ -4223,17 +4223,17 @@ OpenBSDUnveilPaths(const nsACString& uPath, const nsACString& pledgePath) {
continue;
}
+ if (linenum == 1 && line.EqualsLiteral("disable")) {
+ disabled = true;
+ break;
+ }
+
int32_t space = line.FindChar(' ');
if (space <= 0) {
errx(1, "%s: line %d: invalid format", PromiseFlatCString(uPath).get(),
linenum);
}
- if (linenum == 1 && line.EqualsLiteral("disable")) {
- disabled = true;
- break;
- }
-
nsAutoCString uPath(Substring(line, 0, space));
ExpandUnveilPath(uPath);
Comment 52•5 years ago
|
||
Fwiw, i think $XDG_CACHE_HOME/mozilla/
(or only the firefox subdir of that dir ?) is also needed as i got crashes on
console.error: services.settings:
Message: Unix error 2 during operation makeDir on file /home/landry/.cache/mozilla/firefox/qu6wv3d0.empty/settings (No such file or directory)
Comment 53•5 years ago
|
||
/usr/share/zoneinfo r
might also be needed for the main process, as icu seems to use it (and we link with it), ktrace shows that the dir is accessed, https://searchfox.org/mozilla-central/source/js/src/vm/DateTime.cpp points at it, and i dunno what we gain (or what we break) at preventing access to the system timezones - /etc/localtime
is also accessed, but its not needed to unveil() it as it's bypassed.
Apparently the macos sandbox gives access to it too: https://searchfox.org/mozilla-central/source/security/sandbox/mac/SandboxPolicyUtility.h#57
Comment 54•5 years ago
|
||
This can be tested by changing nsExternalHelperAppService::GetSingleton() to use nsOSHelperAppServiceChild for child processes like it does for Mac on other platforms.
I will do this in bug 1594679 to see if we can avoid the xdg-open hack here.
Comment 55•5 years ago
|
||
Comment 56•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/faf2b623b315
https://hg.mozilla.org/mozilla-central/rev/0e0f33fd72b8
Description
•