ContentSandboxPolicy should require a broker client, instead of allowing full FS access if there isn't

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
4 months ago

People

(Reporter: jld, Assigned: jld)

Tracking

unspecified
mozilla67
Unspecified
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments)

ContentSandboxPolicy, if it doesn't have a file broker client, will allow read/write access to the entire filesystem.  This might have made sense back in the B2G era, when this was an extra layer on top of the Android security model and it had to be carefully set up one device at a time, but on desktop it's a mandatory part of the sandbox and we should treat it as such.

The one exception here is at sandbox level 1, where we use seccomp-bpf but not the broker, and we do need the allow-all behavior in that case; however, that can be checked explicitly.

Updated

8 months ago
Priority: -- → P3
…except that level 1 was broken (made equivalent to level 2) accidentally in bug 1365257: https://searchfox.org/mozilla-central/diff/af821e1fe384adbbeb9427688505c69989f14316/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#406

That patch landed almost exactly one year ago, and as far as I know this is the first time anyone's noticed the bug.

gcp, do you think I should fix level 1, or just leave it as-is and update the documentation?
Flags: needinfo?(gpascutto)
On second thought, it's a one-line change to fix this (and another line to make the comment a little more explicit), which is less work than changing the documentation.
Flags: needinfo?(gpascutto)
Level 1 is meant to enable some seccomp-bpf filtering, but still allow
direct access to the filesystem, and level 2 is where brokering starts.
This was accidentally broken in 1365257 (making "level 1" act like level
2); this patch fixes that.

This feature obviously isn't used much given how long nobody noticed it was
broken, but it's useful to have around for troubleshooting, and it's
actually easier to fix it than edit it out of the documentation.
ContentSandboxPolicy currently allows direct filesystem access if it
isn't given a broker client; this is a legacy design from the B2G era,
before the current idea of "sandbox level".  With this patch, it allows
filesystem access at level 1, and above that it requires brokering.

This is both to reduce the opportunities for accidentally having a
too-permissive sandbox and to prepare for refactoring the broker glue in
bug 1511560.

Depends on D14519

Updated

4 months ago
Assignee: nobody → jld

Comment 5

4 months ago
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bacaa3d58281
Fix Linux content sandbox level 1. r=gcp
https://hg.mozilla.org/integration/autoland/rev/56f39977c72c
Require a broker client in ContentSandboxPolicy at level > 1. r=gcp

Comment 6

4 months ago
bugherder
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.