1812339 - Policy on Linux can be bypassed by setting MOZ_SYSTEM_CONFIG directory

Status: RESOLVED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[Mike Kaply [:mkaply]]

In bug 1785278 we added basic autoconfig support for the snap.

A change was added to policy to read policies from the system config directory, but that could be overridden by an environment variable for testing purposes.

That should not have been put in policy.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 1812339 - Always use /etc directory on Linux for policy. r?gerard-majax

Comment 2

[Tom Chiverton]

Not sure on the threat model here. Users can set LD_PRELOAD or other environmental variables already, and achieve the same effect (with more effort)

Comment 3

[Robert McQueen [:ramcq]]

(In reply to Mike Kaply [:mkaply] from comment #0)

A change was added to policy to read policies from the system config directory, but that could be overridden by an environment variable for testing purposes.

That should not have been put in policy.

This patch will regress the approach proposed in #1795998 which is using this environment variable to provide an alternative policy path which is accessible from within the Flatpak sandbox.

Comment 4

[Mike Kaply [:mkaply]]

Did we decide that there was no way to have the Flatpak sandbox access the standard /etc/firefox location?

I'm not sure why we need to add a new location for policies for Flatpak, I thought it would read policies from the same central location.

Comment 5

[Emerson Bernier]

/etc/firefox is problematic for flatpak sandboxing because it's not possible to grant access only to /etc/firefox but to whole /etc directory which may contain stuff which sanboxed apps has no business to touch. Relying on /etc/firefox would mean ff flatpak would need permission for /etc forever which is at odds with the stricter permissions goal: https://bugzilla.mozilla.org/show_bug.cgi?id=1726217

Similar problem was fixed for flatpak chromium without providing access to /etc so same thing for ff should be doable.

Comment 6

[Mike Kaply [:mkaply]]

What I can do is make this check only happen if we are a packaged app:

if (Services.sysinfo.getProperty("isPackagedApp"))

which is similar to the CPP changes.

Comment 7

[Robert McQueen [:ramcq]]

(In reply to Mike Kaply [:mkaply] from comment #6)

What I can do is make this check only happen if we are a packaged app:

if (Services.sysinfo.getProperty("isPackagedApp"))

which is similar to the CPP changes.

We could also hardcode the Flatpak path (I suggested /app/etc/firefox) or prefix /app in the case Flatpak is detected?

Comment 8

[Mike Kaply [:mkaply]]

We could also hardcode the Flatpak path (I suggested /app/etc/firefox) or prefix /app in the case Flatpak is detected?

I think the quickest fix is the isPackagedApp. I'll do that today.

We might rethink based on your other patch, but I'd like to get something in.

Comment 9

[Jan Horak [:jhorak]]

I'm also for the hardcoded path as Robert suggested. We could then use the unmaintained extension to mount the system config files to the /app/etc/firefox location.

Comment 10

[Mike Kaply [:mkaply]]

I don't currently have a way to detect Flatpak versus Snap in JS, but I'll see what I can figure out.

Comment 11

[Mike Kaply [:mkaply]]

We're not going to fix this before 110.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1785278

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The following patch is waiting for review from an inactive reviewer:

ID	Title	Author	Reviewer Status
D167860	Bug 1812339 - Always use /etc directory on Linux for policy. r?gerard-majax	mkaply	gerard-majax: Back Feb 28, 2023

:mkaply, could you please find another reviewer?

For more information, please visit auto_nag documentation.

Comment 14

[Donal Meehan [:dmeehan]]

:mkaply do you think you'll have a patch in time for 111, wondering if we still need to track this?
Or, will it land later and ride the trains

Comment 15

[Mike Kaply [:mkaply]]

Yes, patch is done and going for review today.

Comment 16

[Mike Kaply [:mkaply]]

Attachment: Bug 1812339 - Make MOZ_SYSTEM_CONFIG test only. r?stransky

Comment 17

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Hey :mkaply, Do we still want to land this for the 111 cycle, or can we push this to 112?

Comment 18

[Mike Kaply [:mkaply]]

I'm having trouble getting the right fix for this, but I want to do something. I'm going to go back to the temporary simple patch for now. Thanks for pinging.

Comment 19

[Mike Kaply [:mkaply]]

I've landed the old patch while I figure things. I'll request uplift once it clears.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee139bb6ad78
Always use /etc directory on Linux for policy. r=stransky

Comment 21

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ee139bb6ad78

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:mkaply, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox111 to wontfix.

For more information, please visit auto_nag documentation.

Comment 23

[Mike Kaply [:mkaply]]

Comment on attachment 9314107 [details]
Bug 1812339 - Always use /etc directory on Linux for policy. r?gerard-majax

Beta/Release Uplift Approval Request

• User impact if declined: Users can bypass policy on Linux
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: Doesn't need verification because it's just backing out a patch
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just reverts code to what it was before.
• String changes made/needed:
• Is Android affected?: No

Comment 24

[Donal Meehan [:dmeehan]]

Comment on attachment 9314107 [details]
Bug 1812339 - Always use /etc directory on Linux for policy. r?gerard-majax

Approved for 111.0b8

Comment 25

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0e43c2671883