Bug 1899701 - Add home-read-all plug

[bandali]

Add a home-read-all plug that sets the read attribute for the home interface to the special value all, to allow the Firefox snap to read files in the user's home directory that are owned by another user but that the current user should still be able to read via group membership.

Note: read: all technically allows reading the home directory of any user, but per snapd folks that's as granular as we can get because AppArmor does not have per-user profiles.

@seb128 I tried your suggestion for a potential alternative workaround of opening the file using Firefox's open file dialog, and it failed the same way (Firefox doesn't seem to try to use the document portal).

@alexmurray would you please review this? I imagine I'd then need to submit a store request for a formal review/approval once we've merged this and uploaded a build with it to the store.

/cc @lissyx @zyga

https://bugzilla.mozilla.org/1899701
https://snapcraft.io/docs/home-interface

[zyga]

I think this is correct and in line with my understanding of what the issue is and how to avoid it. A non-root user won't be gaining any extra permissions through this, as DAC is still in effect.

[zyga]

To be precise: the home interface uses @{HOME} apparmor parser variable, which expands to /home/*, which is true for both variants (read and read:all). What all does is it drops the owner constraint.

[seb128]

I've been tagged so I'm going to comment but restrain from voting since I don't feel like I understand enough the situation to give an informed opinion.

The change seems fine to me so if snapd/security team are +1 that works for me.

It seems a bit suboptimal/hackish to have to do that though. If there is no downside to the change shouldn't it just be the default? It feels like that issue of 'can't open a file it should be able to` is just going to impact any snap trying to access content no?

[seb128]

Hum, also reading the documentation it states

When set to ‘all’ this plug becomes non-autoconnect.

isn't that going to be an usability regression?

[zyga]

Hum, also reading the documentation it states

When set to ‘all’ this plug becomes non-autoconnect.

isn't that going to be an usability regression?

We need to restore the auto-connection through the store.

[zyga]

It seems a bit suboptimal/hackish to have to do that though. If there is no downside to the change shouldn't it just be the default? It feels like that issue of 'can't open a file it should be able to` is just going to impact any snap trying to access content no?

I wonder if I'm missing something finer in the security model but I agree with this opinion. CC @alexmurray

[alexmurray]

I tend to agree that the use of owner on the home interface in snapd is problematic since the semantics of owner essential mean checking that the user id of the process opening the file is the same as the user id of the owner of file. It doesn't support extended attributes / ACLs and it doesn't support checking the group either. So this means that there will be cases that files that should legitimately be able to be read by the process end up getting blocked by AppArmor.

I suspect this restriction was added due to the fact that AppArmor doesn't actually know the users real home directory, ie it defines HOME=/home/* (ie. any rule which references the @{HOME} variable grants access to those files in any user's home directory - and so then the use of owner means that owner @{HOME} in AppArmor is then restricted to only files owned by the user - which more closely approximates their actual HOME directory (and not everyone elses).

However as @zyga said, we still have DAC which would then block such access (unless other users have explicitly opted in to making their files readable OR they installed Ubuntu before we introduced private home dirs in 21.04). Which means that if we remove the owner attribute from the home interface in snapd (or say grant the use of read-all to a particular snap) then this will now open up the possibility that a snap is able to read files owned by other users.

But this is no different than that user directly reading those same files and providing them to the snap etc. So I do not think there is a huge risk here. Personally, I feel we lose more in terms of usability by the use of owner for home in snapd than we gain for security/privacy - however I don't have enough knowledge of the history here. Perhaps @jrjohansen or maybe even @jdstrand could weigh in with some historical context around the use of owner for the home interface?

So for the purpose of firefox, +1 from me for this change. @bandali0 please can you open a corresponding topic on the snapcraft forum so we can start the ball rolling on discussing granting this from the store side to ensure the home interface still auto-connects?

[bandali]

Many thanks all for your comments and explanations.

@alexmurray Ack, done: Request for auto-connection of 'home' with 'read: all' for Firefox snap

Thanks again.

[jdstrand]

I don't object to firefox having the home-read-all access as it has its own sandbox and is defensively programmed and this seems to solve a real issue for users.

As for historical context, when the home interface was defined, we were thinking about a few things: that snaps can run on all kinds of systems (not just Ubuntu and not just systems with restricted HOME of 0700) and that AppArmor had limited options for restricting access (eg, only had glob syntax, owner and tunables to work with). Portals wasn't yet a thing nor was apparmor prompting, a group match, etc. The use of owner with $HOME was known to be problematic for the reasons you listed (file ACLs, group membership, etc). I've never cared for the home interface, but I'll admit it historically and continues to serve a purpose.

All considered, owner @{HOME}... was used despite its limitations since we could NOT rely on DAC (ie, plenty of systems don't have $HOME with 0700) but the hope was always to improve the interface. Portals and apparmor prompting were always supposed to be the path forward with the former needing the application to use specific versions of libraries and the latter needing upstream kernel support to really use in earnest. If I were to implement this again without portals or prompting, I might think through:

• adding group support to AppArmor alongside owner
• adding facl (or similar) to AppArmor alongside owner (or something along those lines; ie, make AppArmor aware of file ACLs)
• most importantly, have AppArmor OR snapd better understand the user's home directory. Eg, perhaps AppArmor could evaluate the passwd database when compiling the profile or have snapd create per-user profiles such that instead of having owner @{HOME}...blah the rules are evaluated as /home/foo...blah

Altogether, the home interface might spit out rules like (eg, in /var/lib/snapd/apparmor/profiles/snap.chromium.chromium.foo (ie, per-user policy for user foo):

# Can access non-hidden files in the foo user's home (as calculated by apparmor or snapd)
/home/foo/ r,
/home/foo/[^s.]**             rwklix,
...

# Can access non-hidden files in other users' homes where we are the owner, in the group or file ACLs allow access
owner @{HOME}/ r,
owner @{HOME}/[^s.]**             rwklix,
...
group @{HOME}/ r,
group @{HOME}/[^s.]**             rwklix,
...
facl @{HOME}/ r,
facl @{HOME}/[^s.]**             rwklix,
...

This is still not perfect, but somewhat better in terms of usability (without portals and prompting). It certainly would require more thought than I just gave it (most importantly, ensuring that when users are added/deleted/modified that the corresponding per-user profiles are created). Better would be for the kernel to maintain the user/home mapping so we don't need per-user profiles (eg, add the @{USERHOME} kernel variable (or similar) so that /home/foo/ r, can be written in a profile as @{USERHOME}/ r, (or similar) and the kernel can take the fsuid of the running process to lookup the home directory in the in-kernel user-home mapping when doing the permissions check). I don't know how doable that is.

Hope this helps!

[alexmurray]

Thanks for the context @jdstrand - I really appreciate it. The is work ongoing in AppArmor to support per-user rules in AppArmor profiles (this actually comes out of the prompting work) which the parser translates into appropropriate UID checks on the subject IIRC (@jrjohansen will correct me if I am wrong on the details here ;)) so once that lands we may be able to make use of it in snapd to make the home interface less restrictive (ie. replace use of owner with per-user rules in a single profile - so similar to what you are suggesting with per-user profiles but still having one profile with per-user scoped blocks of rules).

[jrjohansen]

So @alexmurray remembers correctly, you will be able to do several different things. You won't even have to have the whole thing to do what we are discussing. The user work has several parts to it.

1. user conditional rules, and rule blocks. So a portion of a profile can be conditional on a user or users. Combined with variables it will allow making a form of groups. In the future we may look at extending it to automatically sync to system groups but atm, that will be manual.
2. user profile attachment, similar to the user conditional rules profiles attachment will be extended to support attachment based on user(s). This will not just apply to exec rules but also change_profile and change_hat
3. owner conditional will get extended to allow specifying specific user, ie. owner=root, and owner!=root
4. the HOME dir will become a kernel variable, its value will get loaded via pam at session creation. This will allow home to be locked down to the specific user correctly.

That is the ordering that the extensions should land with 1 coming first, followed by 2, ...

[lissyx]

@seb128 Can we land this ?