Closed Bug 1170092 Opened 10 years ago Closed 2 years ago

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

Categories

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

Product:
Core
Component:
Preferences: Backend
Platform:
Unspecified
Linux
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
112 Branch
Tracking Flags:
Tracking Status
firefox41 --- wontfix
firefox112 --- fixed

People

(Reporter: jhorak, Assigned: jhorak)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files, 1 obsolete file)

etc-config-dir v2
(deleted), patch
benjamin
: review-
Details | Diff | Splinter Review
Bug 1170092 Load preferences from /etc/firefox/defaults/pref on Linux; r=mkaply
(deleted), text/x-phabricator-request
Details
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.
Attached patch etc-config-dir v1 (obsolete) (deleted) — — Splinter Review
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
Assignee: nobody → jhorak
Attachment #8613410 - Flags: review?(benjamin)
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 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 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.
Attachment #8613410 - Flags: review?(benjamin)
Attached patch etc-config-dir v2 (deleted) — — Splinter Review
Attaching patch which address issues mentioned by Mike. Corresponding try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=162055a3b6da
Attachment #8613410 - Attachment is obsolete: true
Attachment #8614662 - Flags: review?(benjamin)
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?
Attachment #8614662 - Flags: review?(benjamin) → review-
So I believe this should be WONTFIX.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
(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.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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?
Flags: needinfo?(benjamin)
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.
Flags: needinfo?(benjamin)
(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.
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?
Flags: needinfo?(mconnor)
Priority: -- → P5
This would be also helpful for NixOS which does not allow writing to /usr, preventing the use of global configuration without re-compiling Firefox.

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.

Type: defect → enhancement
Flags: needinfo?(mozilla)

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

Flags: needinfo?(mozilla)

(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

Flags: needinfo?(mozilla)

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.

Flags: needinfo?(mozilla)
Severity: normal → S3

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.

Flags: needinfo?(smujahid)
Flags: needinfo?(smujahid) → needinfo?(jhorak)

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

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

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.

Pushed by stransky@redhat.com: https://hg.mozilla.org/integration/autoland/rev/d0d2fe421be4 Load preferences from /etc/firefox/defaults/pref on Linux; r=mkaply

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

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

Flags: needinfo?(mconnor)
Flags: needinfo?(jhorak)

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.

Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/d25171536bbb Load preferences from /etc/firefox/defaults/pref on Linux; r=mkaply

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

Status: REOPENED → RESOLVED
Closed: 10 years ago → 2 years ago
status-firefox112: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
Regressions: 1826234

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

Summary: Search for default preferences in /etc/mozilla/firefox/default/prefs → Search for default preferences in /etc/firefox/defaults/pref/
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: