many: make per-snap mount namespace MS_SHARED

[zyga]

When snap-confine creates the initial layout of the per-snap mount
namespace it was using MS_SLAVE in order not to propagate mount events
from that mount namespace to the initial mount namespace of the system.

This was working fine, including for updates via snap-update-ns, which
was always changing the per-snap mount namespace. It didn't fully work
for per-snap, per-user mount namespaces. This was mostly affecting
desktop applications that inhabit said namespace, but to a lesser degree
some of the other mount constructs established by snap-update-ns.

This patch changes snap-confine, so that each MS_SLAVE, is followed up
by MS_SHARED. This seems counter-intuitive, is it shared or not? The
behavior of the kernel is well documented but a little bit misleading at
first. When we first change to MS_SLAVE we are closing the propagation
channel between a mount point in the per-snap mount namespace and the
same location in the initial mount namespace. When we are subsequently
using MS_SHARED we are opening the channel to mount namespaces unshared
from our own.

Once sealed, propagation between mount namespaces cannot be
re-established. This is also visible when we carefully set up /media and
/mnt so that bi-directional sharing between those places is preserved.
We do that by never using MS_SLAVE there.

Because propagation settings are inherited by descendant mount points,
this change in snap-confine propagates to various other constructs,
specifically to the changes done by snap-update-ns, for both initial
construction as well as on-the-fly updates. All the mount points
established under shared locations remain shared.

There's one downside: this actively harms us in the code path that
constructs the writable mimic. That code recursively bind-mounts a
directory to a stub directory in /tmp/.snap/, so that we can still see
the original content, and then proceeds to re-create the original
content on top of a freshly mounted tmpfs. With the propagation settings
changed that tmpfs would also show up in the stash directory in /tmp,
therefore breaking the whole setup. To counter that, the writable mimic
construction code in snap-update-ns switched the stash mount point to
recursively private, so that prior semantics is retained.

With this change we can now observe mount changes performed in the
per-snap mount namespace propagate to the derived per-snap, per-user
mount namespace. A new spread regression test measures that for the use
case of layouts and writable mimics.

Somewhat surprisingly, an existing spread test that used to show how
layout changes from revision to revision can fail no longer fails! It
was, after all, related to propagation changes between layout elements
and content interface connections. That test is adjusted to show how the
application works in all the revisions used in the test.

Another test that was affected was performing cleanup, in the restore
section, to unmount a FUSE filesystem mounted in the per-instance view
of $SNAP_DATA, followed by the non-instance mount in the $SNAP_DATA.
With the propagation changes fixed, the unmount in either of the two
locations propagates to the other and a single unmount is sufficient.

Apart from the aforementioned changes there are some adjustments to the
apparmor policy, to allow the new uses of mount with MS_SHARED and
MS_PRIVATE. The associated unit tests are adjusted to reflect that.

All of this is coupled with a new regression test, that is reproducing
the case of a application running in the per-snap, per-user mount
namespace correctly observing all changes performed to the per-snap
mount namespace. The test uses persistent per-snap, per-user mount
namespaces to observe that as otherwise, since per-user mount namespace
persistence is not enabled by default yet, each invocation of the test
application would see a new mount namespace, constructed out of the
per-snap mount namespace, which never was affected by this issue to
begin with.

https://bugs.launchpad.net/snapd/+bug/1828354
Signed-off-by: Zygmunt Krynicki me@zygoon.pl

[zyga]

I've opened a draft while some other things land to master. I will open this only after that is done.

This is a v2 version of #6891 - it is shorter than the original because some parts have been split off and merged independently. Mount namespace have been updated to show the impact of the change.

[zyga]

I've rebased this on master but expect it to fail. More changes to follow.

[zyga]

This will now pass but I believe it still is incorrect because of how the relationship between per-snap and per-user mount namespaces are.

[zyga]

Oh my.

So this branch needs work but is a real problem. Sharing settings of each mount point inside what snap-confine creates is wrong and this has consequences on per-user mount namespace event propagation.

My advice to myself: rebase this on master, reduce the scope, so that remaining changes are commented out, change propagation settings on a per-mountpoint basis so that everyone can follow along and understand what is going on and how the code change makes an impact to the mount namespace test. In the mount namespace test measure the propagation setting in both the per-snap and the per-user mount namespace.

I'm happy to push this forward but it was always always starved for review.

[anonymouse64]

@zyga It seems I have stumbled upon this branch and I thought that everything was broken forever, but no actually you knew about this all along!

I was debugging why when a content snap refreshes, the app snaps actively running and thus using that content snap did not have their mount namespaces updated with the new content, i.e. if you have firefox running and update the gnome-xyz snap that firefox is using, why does the running version of firefox not process the new content of the gnome snap? Well it turns out the reason for this is because firefox is running inside the per-user per-snap mount namespace, which is not inheriting any of the changes made to the per-snap mount namespace and when we run snap-update-ns from the auto-connect task for the refresh of the gnome snap, we only end up updating the per-snap mount namespace and not the per-user per-snap mount namespace.

One side effect of this change though may be that now desktop applications are even more likely to crash when things are updated because things will actually be updated, we may actually today be benefiting somewhat from the fact that desktop snaps never have their content interface mounts updated if they are part of the per-user per-snap mount namespace.