libapparmor's aa_query_label() always returns allowed = 0 for file rules containing the "owner" conditional

I think that the problem here stems from the fact that the home interface's
rules use the "owner" prefix:

 # Allow read access to toplevel $HOME for the user
 owner @{HOME}/ r,

 # Allow read/write access to all non-hidden files that aren't in ~/snap/
 owner @{HOME}/[^s.]** rwk,
 owner @{HOME}/s[^n]** rwk,
 owner @{HOME}/sn[^a]** rwk,
 owner @{HOME}/sna[^p]** rwk,
 # allow creating a few files not caught above
 owner @{HOME}/{s,sn,sna}{,/} rwk,

The kernel tracks these owner permissions differently than permissions that are
not tied to the owner. You can see that the allow vector is not 0x00 with strace:

 $ strace -s1024 ./query_file snap.demo-wget.wget /home/tyhicks/testfile
 ...
 read(3, "allow 0x00000200\ndeny 0x00000000\naudit 0x00000000\nquiet 0x00000000\n", 67) = 67
 ...

The allow vector is non-zero (0x00000200) but it isn't AA_MAY_READ
(0x00000011). Instead, I think AA_MAY_READ is being left shifted by the kernel
to indicate that the "owner" prefix was present on the rule. I'll need to
verify that and then discuss among the upstream developers what to do about
this in the libapparmor query interface.

Additionally, the query_file.c test program has a bug. It redefines
AA_MAY_READ, AA_MAY_WRITE, and AA_CLASS_FILE. It gets the definitions of
AA_MAY_READ and AA_MAY_WRITE wrong. Please just use the definitions provided by
<sys/apparmor.h>.

After making that change, you can remove the "owner" prefix from the rules that
grant access to $HOME in the snap.demo-wget.wget profile, reload the profile,
and then test program will work as expected. This confirms that the "owner" prefix causes the unexpected test program results.

Marking the Snappy task as "Wont't Fix" for now. This theoretically could be fixed in snapd's home interface by dropping the "owner" prefix but I don't think that's the correct fix for this bug. Either libapparmor or the kernel need to handle the owner conditional better or the calling application should do another query for owned files.

After thinking this through some more and discussing with John Johansen, the current query interface is not sufficient to support querying of permissions granted by owner file rules. The reason is that, when dealing with owner file rules, the decision to allow or not depends on two objects. The first is the file itself and the second is the UID associated with the process accessing the file. The current query interface only knows about the file and the UID associated with the process doing the *query*. The process doing the query is almost never the same as the process attempting to access the file.

Important is high as we'll need a fix soon in order for thumbnailer-service to run as a snap.

Triaging this bug lead me to discover bug #1620791. This bug will need to be fixed before, or at the same time as, bug #1620791 is fixed.

this is the bug that causes the thumbnails to be blank for camera-app and gallery-app snaps

This also affects scopes, we get empty art for thumbnailers uri such as image://thumbnailer/file:///snap/unity8-session/...

Please ignore my last comment about this affecting scopes... It was a different root cause afterwards (missing mime database and gdk-pixbuf loaders/cache - https://code.launchpad.net/~stolowski/unity8-session-snap/thumbnailer-fixes/+merge/310756 should fix it).

I had a go writing a custom interface to allow thumbnailer to access the private files of another snap here:

https://github.com/snapcore/snapd/pull/2783

Unfortunately access to ~/snap/$name is also guarded by the "owner" modifier, so it suffers from the same problems as checking for access granted by the home interface. So this will be a problem on systems built on core as well as classic desktops.

James, I can give you access to a custom kernel and library that provides a fix for the apparmor end if you would like. The issue is that these are not in the distro yet, and have not been backported to earlier releases (yet).

John: that would be useful. Our code already tracks the peer's UID, so it will hopefully be quite easy to hook up what ever you've come up with.

FWIW, this bug was stopping the webbrowser from being able to export a PDF to the printing 'app'.

This was due to content-hub using libapparmor to check if the app was able to read the path, and the webbrowser-app using an apparmor manifest which generated a rule with owner in it.

It would be good to get this fixed to prevent other apps using content-hub failing to import/export.

Retriaging these down to Medium. People worked around this in different ways and High was obviously inflated since it isn't fixed yet (I just verified with 5.0.0-25.26-generic and apparmor 2.13.2-9ubuntu6.1).

We're hitting this bug in UBports Ubuntu Touch on the Sony Xperia X (4.4 kernel, xenial AppArmor) as in #6 and #12, apps using content-hub to request files from a confined application fail receiving the file due to aa_query_label returning a denial before trying transfering a file.

How is content hub looking up the confinement (label) of the task. Are you using pids, looking through /proc/<pid>/, using aa_gettaskcon?

This will help with creating an interface wrapper for query_label so we can pass the needed information to the kernel.

Alfred,

which version of apparmor userspace is Ubuntu touch using? You can use

apparmor_parser -V

to find out

Nothing special from what I could gather, just:

-) Calling an internal utility function for getting the allow status:
https://github.com/ubports/content-hub/blob/xenial/src/com/ubuntu/content/detail/transfer.cpp#L187
-) Which eventually lands here:
https://github.com/ubports/content-hub/blob/xenial/src/com/ubuntu/content/utils.cpp#L354

No special pid lookups or anything.

AppArmor userspace in use: AppArmor parser version 2.10.95

Seems as if label lookup is done using dbus: https://github.com/ubports/content-hub/blob/xenial/src/com/ubuntu/content/utils.cpp#L254

To answer the question posed on IRC.

I do not know at this time if any fix to this will be SRUed to Xenial.

A proper generic fix will require a new userspace api. The owner conditional can not be properly generically answered without subject context. This api can be fixed for the inquiring tasks subject querying against the the object, but the the generic case of querying where an external helper task H needs to query whether task A with profile P can access file F can not be fixed with the current api.

Fixing the query using the subjects task is possible to SRU Xenial. The generic fix of a new API will not be SRUed.