1718084 - Provide a way to blocklist certain subdirectories from an allowed dir.

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P5)

Description

[Gian-Carlo Pascutto [:gcp]]

See bug 259356.

We'd like to allow .config and everything below, but block access to .config/mozilla, which would contain the profile data.

For added fun, we need to figure out whether we need to support
https://bugzilla.mozilla.org/show_bug.cgi?id=1393805
which is ~/.mozilla/systemextensionsdev

But, but, for extra extra fun, note that this is XRE_USER_SYS_EXTENSION_DEV_DIR, and we also have XRE_USER_SYS_EXTENSION_DIR, which is whitelisted on Windows (only) https://searchfox.org/mozilla-central/rev/b355f4e36eb45ef84ce9d3dd6af7f1a417ca8bfe/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp#165 if ENABLE_SYSTEM_EXTENSION_DIRS is set.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

From discussion, apparently bug 1524327 means the last 2 parts can go.

Comment 2

[:gerard-majax]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #0)

We'd like to allow .config and everything below, but block access to .config/mozilla, which would contain the profile data.

From https://searchfox.org/mozilla-central/rev/5b3444ad300e244b5af4214212e22bd9e4b7088a/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#391-418 it seems like we already allow ~/.config/

Comment 3

[:gerard-majax]

Attachment: Bug 1718084 - Test allow $HOME/.config and block $HOME/.config/mozilla/ r?gcp

Depends on D119178

Comment 4

[:gerard-majax]

Attachment: Bug 1718084 - Block access to $HOME/.config/mozilla/ r?gcp

Depends on D119179

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

I think you're going to run into some problems with how recursive permissions work: any rules for paths underneath them have the inherited permissions automatically OR'ed into them. It would be possible to add a new flag (OVERRIDE?) for rules like ~./config/mozilla where that shouldn't happen, but we also assume elsewhere that all rules will have MAY_ACCESS set, which we don't want here. So… maybe a DENY_ALL flag which overrides the other bits, similar to CRASH_INSTEAD, would be the simplest fix.

It would be nicer if we could allowlist the .config subdirectories we actually need; is there a reason that won't work?

Comment 6

[:gerard-majax]

(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #5)

I think you're going to run into some problems with how recursive permissions work: any rules for paths underneath them have the inherited permissions automatically OR'ed into them. It would be possible to add a new flag (OVERRIDE?) for rules like ~./config/mozilla where that shouldn't happen, but we also assume elsewhere that all rules will have MAY_ACCESS set, which we don't want here. So… maybe a DENY_ALL flag which overrides the other bits, similar to CRASH_INSTEAD, would be the simplest fix.

Yeah, I just shared early versions but it's far from being finished, and this is exactly what I was running into and what I was expecting. So far I'm going with a MAY_DENY similar to what you suggest, and I'm poking where we need to change for the inheritance, whilst not breaking anything.

It would be nicer if we could allowlist the .config subdirectories we actually need; is there a reason that won't work?

Comment 7

[:gerard-majax]

Attachment: Bug 1718084 - Reorganize test for lower complexity r?gcp

Depends on D119180

Comment 8

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=04456fda98e99475a5e00e68d07a9b11be9d9200

Comment 9

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=9907167cf688479cd858bac1eac3416b91650650

Comment 10

[:gerard-majax]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #0)

For added fun, we need to figure out whether we need to support
https://bugzilla.mozilla.org/show_bug.cgi?id=1393805
which is ~/.mozilla/systemextensionsdev

There is test code that ensures we keep this readable, so this looks fine: https://searchfox.org/mozilla-central/rev/79a3ae0b0fa6abeca2cb7cf220df293c8dec9207/security/sandbox/test/browser_content_sandbox_fs.js#350-358

Comment 11

[:gerard-majax]

Attachment: Bug 1718084 - Rework XDG-specific policy and tests r?gcp

Depends on D119179

Comment 12

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=3566d93d111248145d59607274fb2cf69b371902

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e17f3c2886ed
Reorganize test for lower complexity r=gcp
https://hg.mozilla.org/integration/autoland/rev/75c7384df123
Block access to $HOME/.config/mozilla/ r=gcp
https://hg.mozilla.org/integration/autoland/rev/28f0d0747745
Test allow $HOME/.config and block $HOME/.config/mozilla/ r=gcp

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e17f3c2886ed
https://hg.mozilla.org/mozilla-central/rev/75c7384df123
https://hg.mozilla.org/mozilla-central/rev/28f0d0747745

Comment 15

[:gerard-majax]

For added fun, we need to figure out whether we need to support
https://bugzilla.mozilla.org/show_bug.cgi?id=1393805
which is ~/.mozilla/systemextensionsdev

https://treeherder.mozilla.org/logviewer?job_id=346741721&repo=try&lineNumber=14197

INFO - TEST-UNEXPECTED-FAIL | security/sandbox/test/browser_content_sandbox_fs.js | reading system extensions dev dir from a web process is allowed (/builds/worker/.config/mozilla/systemextensionsdev) -

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Bug 1732580 depends on this work, which we're trying to land on ESR91. Please nominate for approval.

Comment 17

[:gerard-majax]

Comment on attachment 9229867 [details]
Bug 1718084 - Block access to $HOME/.config/mozilla/ r?gcp

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: easier to uplift bug 1732580
• User impact if declined: broken webgl on snap
• Fix Landed on Version: 92
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): has tests, no known regression, landed three months ago
• String or UUID changes made by this patch:

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9229866 [details]
Bug 1718084 - Test allow $HOME/.config and block $HOME/.config/mozilla/ r?gcp

Approved for 91.3esr, thanks.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/cff20b995d56
https://hg.mozilla.org/releases/mozilla-esr91/rev/5f420e89059c
https://hg.mozilla.org/releases/mozilla-esr91/rev/27c27a50b68f