1170092 - Search for default preferences in /etc/firefox/defaults/pref/

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement, P5)

Description

[Jan Horak [:jhorak]]

To allow system administrators to easily set Firefox default preferences for all users and all installed versions of Firefox we'd like to have system configuration directory in /etc directory.

Why customers want this:
If multiple versions of firefox are installed or if firefox is not installed under /usr/lib64, Firefox will still get it the organizationally/security defined settings from /etc. 

Also, any settings under /usr/lib64/firefox do not run the risk of being modified or removed when Firefox is updated or uninstalled creating a security risk. 

This is also beneficial for creating SCAP OVAL content and having admins consistently make system/application configuration settings/locks under /etc.

Comment 1

[Jan Horak [:jhorak]]

Attachment: etc-config-dir v1

There's the patch, please have a look. Code also allows to store .cfg file for locking preferences in /etc/mozilla/firefox/defaults/pref, more info about locking prefs there: http://kb.mozillazine.org/Lock_Prefs

Comment 2

[Jan Horak [:jhorak]]

There's the try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8ec45ee5bfc

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8613410 [details] [diff] [review]
etc-config-dir v1

Review of attachment 8613410 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/pref/autoconfig/src/nsReadConfig.cpp
@@ +259,5 @@
> +          NS_ENSURE_SUCCESS(rv, rv);
> +
> +          rv = NS_NewLocalFileInputStream(getter_AddRefs(inStr), jsFile);
> +          NS_ENSURE_SUCCESS(rv, rv);
> +        }

I, personally, would rather go with bug 440908, which has possibly more usecases (including having firefox itself ship with locked prefs).

::: modules/libpref/Preferences.cpp
@@ +1233,5 @@
>    // - jar:$gre/omni.jar!/defaults/preferences/*.js
>    // Thus, in omni.jar case, we always load app-specific default preferences
>    // from omni.jar, whether or not $app == $gre.
> +  // At very end load configuration from system config location:
> +  // - /etc/mozilla/firefox/defaults/pref/*.js

This is overly deep. Why not /etc/firefox/pref?

::: toolkit/xre/nsXREDirProvider.cpp
@@ +444,5 @@
>      }
>    }
> +#if defined(XP_UNIX)
> +  if (!strcmp(aProperty, NS_APP_PREFS_SYSTEM_CONFIG_DIR)) {
> +    static const char *const sysConfigDir = "/etc/mozilla/firefox/";

You can't hardcode this here. This effectively makes any application (including e.g. Thunderbird) use /etc/mozilla/firefox for its system prefs. Check nsIXULAppInfo.

@@ +700,5 @@
>    }
>  }
>  #endif // DEBUG_bsmedberg
>  
> +/* Tady se zjistuji adresare, je nutne implementovat novy NS_APP_PREFS...DIR_LIST a jinde zavolat funkci GetFiles s odpovidajici aProperty */

I guess that's what czech looks like...

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8613410 [details] [diff] [review]
etc-config-dir v1

Review of attachment 8613410 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/Preferences.cpp
@@ +1358,5 @@
> +      if (NS_SUCCEEDED(rv)) {
> +        rv = pref_LoadPrefsInDir(systemPrefDir, nullptr, 0);
> +        if (NS_FAILED(rv))
> +          NS_WARNING("Error parsing default preferences at system config location");
> +      }

Note this seems too late (this is happening after the telemetry pref are being set). Anyways, I'd just add the directory to NS_APP_PREFS_DEFAULTS_DIR_LIST (which, in fact, is what I've been doing in iceweasel for 7 years)

Comment 5

[Jan Horak [:jhorak]]

Comment on attachment 8613410 [details] [diff] [review]
etc-config-dir v1

Thanks for feedback Mike, putting down the review request until issues you mentioned are addressed.

Comment 6

[Jan Horak [:jhorak]]

Attachment: etc-config-dir v2

Attaching patch which address issues mentioned by Mike.

Corresponding try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=162055a3b6da

Comment 7

[Benjamin Smedberg]

Comment on attachment 8614662 [details] [diff] [review]
etc-config-dir v2

I don't think I agree with the premise of this bug. We've been getting rid of system config locations on purpose to avoid setting hijacking, and this is going in the opposite direction. Admittedly Linux users typically don't have to deal with hijacking, but why are we tempting fate?

Comment 8

[Benjamin Smedberg]

So I believe this should be WONTFIX.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> Comment on attachment 8614662 [details] [diff] [review]
> etc-config-dir v2
> 
> I don't think I agree with the premise of this bug. We've been getting rid
> of system config locations on purpose to avoid setting hijacking, and this
> is going in the opposite direction. Admittedly Linux users typically don't
> have to deal with hijacking, but why are we tempting fate?

I don't understand how configuration in /etc can create browser more vulnerable. /etc has root access only and also stores yum configuration - if an attacker can write to /etc you may have much more problems that broken ff config. If you can write to /etc you can also write to /usr/lib(64)/firefox and can do anything with the browser. So there's no gain from security perspective here - it only make global config more difficult which breaks enterprise installations.

You even don't need the root access to attack firefox config - you can install malicious extension from browser to user home - and don't touch system files.

Please reconsider that - disabling config in /etc does not improve security but makes Firefox deployment in enterprise harder.

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Benjamin, we'd like to use this patch for our enterprise customers in RHEL/Fedora packages only. We don't need that in mozilla binary. Can we get r+ for it in that case?

Comment 11

[Benjamin Smedberg]

You mean as a local patch? I don't much care about that. What I don't want to happen is have arbitrary changes to Firefox config by other packages installed on the system.

But we really need to figure out the future of autoconfig in general.

Comment 12

[Martin Stránský [:stransky] (ni? me)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #11)
> You mean as a local patch? I don't much care about that. What I don't want
> to happen is have arbitrary changes to Firefox config by other packages
> installed on the system.

Yes, I mean a local patch for RHEL and Fedora packages only.

Comment 13

[Jan Horak [:jhorak]]

We'd like to put this into Firefox in Fedora and RHEL because our users want to have some standard way to configure Firefox for all users and allow to share these settings among more installations of Firefox. What do you think about it?

Comment 14

[Jan Tojnar]

This would be also helpful for NixOS which does not allow writing to /usr, preventing the use of global configuration without re-compiling Firefox.

Comment 15

[Martin Stránský [:stransky] (ni? me)]

Mike,
what's your statement to this bug? It's another enterprise class request and we have to backport it locally every Firefox update for Fedora/RHEL. Would be great to have it landed in some shape and standardize a place for Firefox system wide configuration.
Thanks.

Comment 16

[Mike Kaply [:mkaply]]

What types of preferences are read from this location? Would having the policies.json in etc/firefox be enough?

Comment 17

[Martin Stránský [:stransky] (ni? me)]

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

What types of preferences are read from this location? Would having the policies.json in etc/firefox be enough?

This location has the same config as /usr/lib64/firefox/defaults/pref - i.e. .js files with pref("...") settings.
If the pref variables can be set from policies.json then we don't need /etc/mozilla/firefox/default/prefs or it's variant.

For rationale of it see https://bugzilla.mozilla.org/show_bug.cgi?id=1170092#c0

Comment 18

[Mike Kaply [:mkaply]]

If the pref variables can be set from policies.json then we don't need /etc/mozilla/firefox/default/prefs or it's variant.

We're slowing adding prefs that folks want to be set. See:

https://github.com/mozilla/policy-templates/blob/master/README.md#preferences

We're not giving free access to preferences because lots of folks set completely unnecessary preferences (or don't understand the consequences of preferences they set)

So if there are specific preferences folks need, I'd love to hear about them and I'll move up in priority setting global preferences via etc/firefox.

Comment 19

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

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 20

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.

Comment 21

[:gerard-majax]

Suhaib are you still working on this ? I'm landing such a fix for Snap and Flatpak in bug 1785278, we could build on top of that for the rest of packages.

Comment 22

[Martin Stránský [:stransky] (ni? me)]

This patch has been shipped downstream in Fedora for many years. May be worth to upstream it now.

Comment 23

[Mike Kaply [:mkaply]]

Yeah, I think I'm finally ready to ship this on Linux only since we've moved search and other things away from preferences.

Comment 24

[Jan Horak [:jhorak]]

Attachment: Bug 1170092 Load preferences from /etc/firefox/defaults/pref on Linux; r=mkaply

Preferences from /etc/firefox/defaults/pref overwrites preferences set in omni.jar
and firefox/defaults/pref. The code was used for flatpak and snap only, but it could
be used also for regular Linux builds.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0d2fe421be4
Load preferences from /etc/firefox/defaults/pref on Linux; r=mkaply

Comment 26

[Marian-Vasile Laza]

Backed out changeset d0d2fe421be4 (Bug 1170092) for xpcshell failures on /test_autoconfig_custom_path.js.
Backout link
Push with failures <--> X2
Failure Log

Comment 27

[Mike Kaply [:mkaply]]

Weird. I don't know why that would have caused that. I'll do some checking.

Comment 28

[Mike Kaply [:mkaply]]

So just getting the NS_OS_SYSTEM_CONFIG_DIR in Preferences.cpp

rv = NS_GetSpecialDirectory(NS_OS_SYSTEM_CONFIG_DIR,
getter_AddRefs(defaultSnapPrefDir));

is causing the tests to fail because once we've grabbed the directory, we are caching it somehow so setting MOZ_SYSTEM_CONFIG_DIR in the tests isn't working.

I don't understand why because as far as I know, we don't cache special directories.

Still investigating.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d25171536bbb
Load preferences from /etc/firefox/defaults/pref on Linux; r=mkaply

Comment 30

[Sandor Molnar[:smolnar]]

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

Comment 31

[Daniel Holbert [:dholbert]]

[clarifying bug title to reflect the actual directory that we settled on using here]