1795998 - Flatpak permissions for reading /etc/firefox/defaults/pref and /etc/firefox for AutoConfig

Status: RESOLVED FIXED

(Firefox Build System :: Third Party Packaging, enhancement)

Comment 1

[Robert McQueen [:ramcq]]

Hi Alexandre, I'm a Flatpak developer and helped Mozilla get the Flatpak set up and publishing to Flathub - thanks for working on this issue and #1785278. I've put some comments on the related ticket https://bugzilla.mozilla.org/show_bug.cgi?id=1682462#c31 but the tl;dr is that in the Flatpak world, you can't map /etc paths from the host system outside the sandbox - the whole of /etc is provided by the runtime environment inside the sandbox, apart from specific files which Flatpak itself maps from outside (stuff like /etc/resolv.conf).

At the moment I think we have two problems - the Flatpak itself provides a policies.json to disable the non-functional in-app updater (see https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/docker/firefox-flatpak/policies.json) which we need to handle a different way, and then we can provide an extension point that the system administrator can use to add a config folder which does show up inside the Flatpak - eg /app/etc/firefox and have Firefox check there. Does the patch let us have the "/etc/firefox" path be influenced at runtime with an env var? If so we can set that and I can send a patch to get the Flatpak path in place.

Outside the sandbox, the sysadmin or an OS vendor that relies on Flatpak for Firefox can provide a symlink from the extension path back to /etc, which is how Endless OS arranges Chromium extensions:
/var/lib/flatpak/extension/org.chromium.Chromium.Policy.system-policies/x86_64/1 -> /etc/chromium-browser

Comment 2

[Robert McQueen [:ramcq]]

Does the patch let us have the "/etc/firefox" path be influenced at runtime with an env var?

Ah, is this MOZ_SYSTEM_CONFIG_DIR ? 👍

Comment 3

[:gerard-majax]

(In reply to Robert McQueen from comment #2)

Does the patch let us have the "/etc/firefox" path be influenced at runtime with an env var?

Ah, is this MOZ_SYSTEM_CONFIG_DIR ? 👍

Yes. So for context, we fixed bug 1785278 for Snap and when finalizing the patch I thought it would be a good idea to makae it workable for Flatpak as well. That variable was mostly introduced at first to be able to write tests, but also in case people needed to override, which seems to be the case.

Comment 4

[:gerard-majax]

So it seems you could use MOZ_SYSTEM_CONFIG_DIR for that matter? Feel free to steal this bug if you can fix it faster than me :)

Comment 5

[Robert McQueen [:ramcq]]

I can throw a patch together for the Flatpak bits but and then we can try and work out how to test it! Which branch of Firefox do I need to be able to use this env var?

In parallel, can you tell me whether setting MOZ_SYSTEM_CONFIG_DIR will prevent either of these being read correctly which the Flatpak relies on? Are policies and preferences added together from multiple files? Preferences I think yes but policies possibly not? Or can we get rid of any of this in favour of code patches that better adapt to Firefox being used inside a Flatpak?

ramcq@xi:~$ flatpak run --command=/bin/bash org.mozilla.firefox
[📦 org.mozilla.firefox ~]$ cd /app/lib/firefox/
[📦 org.mozilla.firefox firefox]$ cat distribution/policies.json 
{
  "policies": {
    "DisableAppUpdate": true,
    "DontCheckDefaultBrowser": true
  }
}
[📦 org.mozilla.firefox firefox]$ cat browser/defaults/preferences/default-preferences.js 
/*global pref*/
/*eslint no-undef: "error"*/
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
pref("intl.locale.requested", "");
pref("app.update.auto", false);
pref("app.update.enabled", false);
pref("app.update.autoInstallEnabled", false);
pref("browser.shell.checkDefaultBrowser", false);
pref("spellchecker.dictionary_path", "/usr/share/hunspell");
[📦 org.mozilla.firefox firefox]$ 

Comment 6

[Robert McQueen [:ramcq]]

(Also these seem a bit repetitive/redundant anyway...?)

Comment 7

[:gerard-majax]

This landed yesterday, so it's only on mozilla/central for the moment. We use MOZ_SYSTEM_CONFIG_DIR to:

• source extra default preferences ($MOZ_SYSTEM_CONFIG_DIR/defaults/pref)
• source AutoConfig file

I dont know about policies.

Comment 8

[Robert McQueen [:ramcq]]

Attachment: flatpak-system-config.diff

I've attached a patch which creates a Flatpak extension point named org.mozilla.firefox.system-config which if provided (which can be done by the system administrator or OS vendor in the extensions path) is mapped into /app/etc/firefox inside the sandbox, and Mozilla is directed to look there with the new env var. This is I think all you need for the "Flatpak bit" for a config to be provided from outside the sandbox.

The other half of the task is to understand whether this has any consequences with how policies and preferences are already used in the Flatpak - it seems like preferences are merged but maybe Mike Kaply can help clarify the situation for the policies. Some of the preferences + policies set by the Flatpak are probably garbage and it would be better for Firefox to detect when run in a Flatpak environment and disable the updater by itself.

ramcq@xi:~/.local/share/flatpak$ ls extension/org.mozilla.firefox.system-config/x86_64/1/hello 
extension/org.mozilla.firefox.system-config/x86_64/1/hello
ramcq@xi:~/.local/share/flatpak$ flatpak run --user --command=/bin/bash org.mozilla.firefox
[📦 org.mozilla.firefox flatpak]$ echo $MOZ_SYSTEM_CONFIG_DIR
/app/etc/firefox
[📦 org.mozilla.firefox flatpak]$ ls /app/etc/firefox
hello

Comment 9

[Mike Kaply [:mkaply]]

I would really like to get to a point where Flatpak doesn't need to use policies to disable updating. I opened this bug a while ago:

https://bugzilla.mozilla.org/show_bug.cgi?id=1769265

but it hasn't got any traction.

I'm starting to think I should just detect Flatpak and disable the updater internally, then we won't have the policy merging problem.

Comment 10

[behrmann]

If I understand the above patch (from comment #8) correctly, it would allow to pass set policy for sysadmins using the Firefox Flatpak. This would be tremendously valuable alone.

Comment 11

[Robert McQueen [:ramcq]]

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

I'm starting to think I should just detect Flatpak and disable the updater internally, then we won't have the policy merging problem.

👍 That's probably a lot more simple than what's being discussed in #1769265 and is sufficient for the Flatpak and Snap cases so that we can step out of needing to touch policy at all, which would be much cleaner.

(In reply to behrmann from comment #10)

If I understand the above patch (from comment #8) correctly, it would allow to pass set policy for sysadmins using the Firefox Flatpak. This would be tremendously valuable alone.

It should do, if my understanding of #1785278 is correct that it also changes system policy to be read from the path we can set with MOZ_SYSTEM_CONFIG_DIR.

Comment 12

[:gerard-majax]

What else needs to be done now ? Do you want to submit this change on phabricator?

Comment 13

[Robert McQueen [:ramcq]]

I've never done that before, I can have a dig and try to figure out. I'd be equally happy if someone else wanted to submit it. It's already useful and I believe what will happen is that if a local admin provides a policies.json it will be used in preference to the one built in to the Flatpak.

However in light of bug 1769265 we can strip out one half of our Flatpak policies.json - the other is disabling the default browser check because at present Flatpak doesn't offer a "blessed" API to modify the user's default app settings. Is there a light touch way for us to simply turn this check off in Flatpak mode?

(I'm keen to get rid of the baked in policies.json not least because it opens the way for site-specific policies with no conflicts/regressions, but also very recently an end-user was seeking help because they thought computer had been hacked because Firefox was showing "controlled by your organisation"... 😬)

Comment 14

[Mike Kaply [:mkaply]]

However in light of bug 1769265 we can strip out one half of our Flatpak policies.json - the other is disabling the default browser check because at present Flatpak doesn't offer a "blessed" API to modify the user's default app settings. Is there a light touch way for us to simply turn this check off in Flatpak mode?

Let's get a bug opened and I'll add a Flatpak check for the that.

Comment 15

[Robert McQueen [:ramcq]]

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

However in light of bug 1769265 we can strip out one half of our Flatpak policies.json - the other is disabling the default browser check because at present Flatpak doesn't offer a "blessed" API to modify the user's default app settings. Is there a light touch way for us to simply turn this check off in Flatpak mode?

Let's get a bug opened and I'll add a Flatpak check for the that.

Looking a little harder at this; maybe it's not necessary to do in code? Looking at making an additional patch which removes the policies.json and the update stuff from default-preferences.js - we seem to disable the browser check in two other places, once in default-preferences.js (https://github.com/mozilla/gecko-dev/blob/master/taskcluster/docker/firefox-flatpak/default-preferences.js#L10) and once in distribution.ini (https://github.com/mozilla-partners/flatpak/blob/master/desktop/flatpak/distribution/distribution.ini#L8). What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

Looking at the other preferences, as far as I can tell the spellchecker.dictionary_path is necessary, ie we can't define it anywhere else very easily (unless we set DICTDIR but this seems "worse") but I can't figure out what intl.locale.requested is doing - is that necessary?

Comment 16

[Robert McQueen [:ramcq]]

Attachment: WIP: Bug 1795998 - add flatpak extension point to define policies on host

This defines a Flatpak extension point which allows the system config
dir (ie /etc/flatpak) to be defined from outside the Flatpak sandbox.
A Linux system administrator can use what Flatpak calls an unmanaged
extension - ie a folder in the right place - to provide their own
policies.json, autoconf, etc.

For a system-wide Flatpak install in /var/lib/flatpak the path would
be /var/lib/flatpak/extension/org.mozilla.firefox.system-config/x86_64/1
and can even be a symlink to the host /etc/firefox if desired for
consistency with unsandboxed Firefox instances.

Comment 17

[Robert McQueen [:ramcq]]

Attachment: Bug 1795998 - remove policies.json as app updates are disabled elsewhere. r?jcristau

The patch in bug 1769265 means that Firefox disables its own updater when
running under a Linux package such as Flatpak. We can stop shipping a
policies.json now so that the system administrator is free to provide
their own site policies as they wish.

Depends on D161021

Comment 18

[Robert McQueen [:ramcq]]

Submitted WIP patches; not sure who should be marked to review - let me know.

Comment 19

[Mike Kaply [:mkaply]]

Did you verify that my Snap/Flatpak update check definitely disables the updater?

Comment 20

[Mike Kaply [:mkaply]]

You can mark me as the reviewer. You should be able to do a "Request review" in the phabricator UI and set it to me and it will move out of WIP.

Comment 21

[Mike Kaply [:mkaply]]

Looking at the other preferences, as far as I can tell the spellchecker.dictionary_path is necessary, ie we can't define it anywhere else very easily (unless we set DICTDIR but this seems "worse") but I can't figure out what intl.locale.requested is doing - is that necessary?

intl.locale.requested set to blank tells Firefox to use the language pack that corresponds to the operating system. So for Flatpak which I think bundles all languages, we make sure we use the right one.

Speaking of bundling all languages, assuming we do that, there's a better way to do it that is more performant.

Comment 22

[Emerson Bernier]

(In reply to Robert McQueen [:ramcq] from comment #15)

we seem to disable the browser check in two other places, once in default-preferences.js (https://github.com/mozilla/gecko-dev/blob/master/taskcluster/docker/firefox-flatpak/default-preferences.js#L10) and once in distribution.ini (https://github.com/mozilla-partners/flatpak/blob/master/desktop/flatpak/distribution/distribution.ini#L8). What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

Was this question answered anywhere?

Comment 23

[Robert McQueen [:ramcq]]

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

Did you verify that my Snap/Flatpak update check definitely disables the updater?

No, I suppose it would be more responsible to test this end to end before moving out of WIP. :) I'll have to work out how to fake out enough of the Taskcluster env to make this work. Are there binary tarballs that contain that patch now, which I can just test the repackaging with?

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

Looking at the other preferences, as far as I can tell the spellchecker.dictionary_path is necessary, ie we can't define it anywhere else very easily (unless we set DICTDIR but this seems "worse") but I can't figure out what intl.locale.requested is doing - is that necessary?

intl.locale.requested set to blank tells Firefox to use the language pack that corresponds to the operating system. So for Flatpak which I think bundles all languages, we make sure we use the right one.

OK, cool. Sounds like a keeper then. :)

Speaking of bundling all languages, assuming we do that, there's a better way to do it that is more performant.

Flatpak has special handling for Locales extensions which are structured correctly, so we move the langpacks into the paths Flatpak understands, and then symlink them back to where Firefox can find them. So it appears like all languages are bundled, but in practice the Flatpak client will only download the subdirectory matching the user or system locale. https://bugzilla.mozilla.org/show_bug.cgi?id=1621074#c21 has a bit more detail.

(In reply to Emerson Bernier from comment #22)

(In reply to Robert McQueen [:ramcq] from comment #15)

What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

Was this question answered anywhere?

Seconded! :)

Comment 24

[Mike Kaply [:mkaply]]

Any nightly should have that fix in it now.

we seem to disable the browser check in two other places, once in default-preferences.js (https://github.com/mozilla/gecko-dev/blob/master/taskcluster/docker/firefox-flatpak/default-preferences.js#L10) and once in distribution.ini (https://github.com/mozilla-partners/flatpak/blob/master/desktop/flatpak/distribution/distribution.ini#L8). What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

There's no difference. Since we're going to have a distribution.ini anyway (for the distribution ID), we can move everything from default-preferences to distribution.ini and get rid of it.

Comment 25

[Emerson Bernier]

The system config extension was added in https://phabricator.services.mozilla.com/D168803

I don't know if there is anything left to do here?

Comment 26

[Mike Kaply [:mkaply]]

Has policies.json been removed for the flatpak now?

Comment 27

[Emerson Bernier]

policies.json are still shipped [1] within flatpak even though they shouldn't be needed anymore[2]. TB dropped them form flatpak some time ago[3] and there were no regressions reported AFAIK. The proposed patch[4] for dropping policies.json from ff has stalled.

[1] https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/docker/firefox-flatpak/runme.sh#l75

[2] https://hg.mozilla.org/comm-central/rev/d4bf00e0702c86bb78faa7178fb33c16861d60a2

[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1843110

[4] https://phabricator.services.mozilla.com/D161022

Comment 28

[Mike Kaply [:mkaply]]

The reason it stalled is because I had no way to test it locally.
That patch doesn't seem to be where it was explicitly removed from TB. Is it definitely no longer packaged?

Comment 29

[Emerson Bernier]

Does mozilla have any testers for flatpak?

That patch doesn't seem to be where it was explicitly removed from TB. Is it definitely no longer packaged?

What do you mean? TB patch removed policies.json for sure[1]

The FF patch on phabricator removes few extra lines from taskcluster/docker/firefox-flatpak/default-preferences.js . TB doesn't have default-preferences.js at and uses only distribution.ini[2] with similar options active. In earlier comment you recommended[3] putting default-preferences.js content to distributon.ini and get rid of the former and I think this should be done (TB did exactly this).

[1] https://phabricator.services.mozilla.com/D184544

[2] https://hg.mozilla.org/comm-central/file/tip/taskcluster/docker/tb-flatpak/distribution.ini

[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1795998#c24

Comment 30

[Mike Kaply [:mkaply]]

Sorry, I was looking at the wrong TB patch.

I'll look into getting a new patch together for this.

Comment 31

[Mike Kaply [:mkaply]]

Pull request for updating distribution.ini:

https://github.com/mozilla-partners/flatpak/pull/1

Comment 32

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

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

ID	Title	Author	Reviewer Status
D161022	Bug 1795998 - remove policies.json as app updates are disabled elsewhere. r?jcristau	mkaply	ramcq: Inactive

:mkaply, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit BugBot documentation.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fc123de30da
remove policies.json as app updates are disabled elsewhere. r=jcristau

Comment 34

[Julien Cristau [:jcristau]]

Seems worth verifying when 123.0b1 is on flathub.

Comment 35

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/0fc123de30da