Layouts: re-mapping snap directories

@wnmmihha Unlike @ogra stated, this feature can actually help fixing the problem for you as you’ll be able to redefine what /usr/lib/x86_64-linux-gnu means.

As for the classic snaps improvements, I suggest keeping an eye on that topic and subscribing to it. The changes are already in progress.

There are a lot of PRs in flight that are contributing to the building blocks of layouts now.

From the top of my head I still need to implement symlinks (requires for content interface improvements but actually free to reuse for layouts) and that’s about it. I have a hell lot of tests to write (integration tests, unit tests are written alongside each function added or changed.

I don’t know how we’re looking like for 2.30 (AFAIK it will be branched early next week) but if not 2.30 then 2.31 looks very realistic.

With everyone in the team back from holidays it’s due time for some progress updates. A few things have landed and some proposed we are quite close to getting everything to click and work together. Let’s see what that is:

  • Parsing layout sections in snap.yaml [done]
  • Creating mount destination on demand (for directories) [done]
  • Creating mount destination on demand (for files) [under review: 4452]
  • Creating symlinks on demand [under review: 4452]
  • Planning and executing mimic plan [done]
  • Creating writable spaces using mimic [under review: 4452]
  • Using layout definitions [todo]

The last element needs some security discussion as it will likely be just copying stuff from the layout definition into the existing mount profiles. What is new (complex) is writing apparmor rules that allow it to operate without opening super wide mount permissions anywhere.

The under review PR will be expanded to cover symlinks and I’ll push the update for that early tomorrow. EDIT: The PR now covers everything needed for layouts, content interface generalization, themes etc.

I will now merge / rebase https://github.com/snapcore/snapd/pull/4068 because it brings in the new mechanic for “spooling” entries into one directory. After that we only need using layout definitions from the list above. This will simply take the layout yaml definition and add it to the mount profile of a snap, taking advantage of all the mechanism added here.

1 Like

There is definitely a policy discussion around layouts to be had, @niemeyer @jdstrand @ratliff and I can take this up next week.

The tip of my review is https://github.com/snapcore/snapd/pull/4452 which is small but contains useful new logic and tests for effectively all of layouts. The rest is as Jamie said above, a policy question as to what is allowed where.

Following that is https://github.com/snapcore/snapd/pull/4068 but it will need a merge with master after 4452 lands to actually be useful.

4452 can be merged this week. With some luck so can 4068 as it had some reviews already. This leaves next week for working on layout plumbing/integration tests, with the assumption that the policy is decided separately (policy as to what is allowed). I think we can realistically look at merging layouts (even if restricted by policy) just after the sprint next week when key participants to discuss the policy question are back.

1 Like

Another small update. I chopped the PR that has grown since I opened it (I added lots of unit tests that made it “scary” to review). Four small prerequisite branches have landed (mainly simple fixes and new test helpers). There are two PRs pending review now but both are blocked on Fedora infrastructure issue that should be resolved shortly, please have a look:

After those I have two more branches: one that adds a spread test and another that adds a whole swarm of carefully documented unit tests for all kinds of success and failure scenarios that may arise.

After this lands I’d like to return to https://github.com/snapcore/snapd/pull/4068 which will directly benefit from this work (it will, among other things enable plug-in snaps to work correctly and will open the work for themes for @jamesh). In parallel I’m preparing a small RFC branch that populates mount profiles with everything from snap layout data.

3 Likes

The update this week i smaller. I was plagued by some issues and this week seems to start with similar vein but I hope to land the outstanding PRs and open a PR that will bridge the layout definition with the mount profile mechanism. This will allow us to write first test snaps that use layouts for real to explore what’s missing.

1 Like

This PR contains the initial glue that enables layouts. https://github.com/snapcore/snapd/pull/4505

Note that this doesn’t mean things work just yet, the apparmor policy won’t allow any layout to be constructed in practice. My plan is to work on this next week when @jdstrand returns from a sprint. Meanwhile I’m still blocked by https://github.com/snapcore/snapd/pull/4471 - please review it if you can.

1 Like

Oh, and on the same vein I realized that the logic that can poke writable holes dones’t work for the root filesystem (and probably wont work immediately). I’m thinking about possible ways to support that.

@zyga:

  /mytmp:
    type: tmpfs
    user: nobody
    group: nobody

We should not allow specifying ‘nobody’ or ‘nogroup’. This user (and group) is only meant to be used by NFS for assigning ownership to unmapped users. It is a common security mistake to misappropriate this user and group for other things. If this is only meant for demo purposes, please use ‘someuser’ or ‘somegroup’.

Besides, the user and group feature in layouts isn’t going to be useful until we have proper uid/gid support in snapd since snaps will find themselves in the awkward position of having files not owned by root or the calling user, and won’t be able to chown/setuid/setgid to these users, may have file access issues (apparmor owner match) or capability denials (dac_read_search, dac_override).

In that case it seems like you are saying we should just drop user/group and perhaps mode altogether. That’s easy, am I reading this right?

For now, yes. When we have uid/gid support in snapd, we can happily expand this to the users that the snap is allowed to use.

That doesn’t mean you have to rip out all the uid/gid code for layouts-- continue to use it for root:root. You could even leave the yaml exposed (but only allow root:root for now) if you don’t advertise it. I’ll leave that up to you (though, it seems possible the yaml might change when uid/gid support lands, so maybe it is best to not expose the yaml at this time).

1 Like

This is now done in https://github.com/snapcore/snapd/pull/4530

1 Like

So today was exceptionally fruitful for layouts. I discussed the feature extensively with @jdstrand and we agreed on how to implement the security policy.

I’m a bit tired but this branch makes layouts functional and improves validation https://github.com/snapcore/snapd/pull/4590

I will iterate with spread test that shows what I implemented. I’m also consider splitting this so that the improved validation is cherry-picked into 2.31 (CC @mvo).

2 Likes

So, some good news and some updates on where we are and what’s ahead:

  • The good news, layouts are now in master / edge.
  • The bad news, a sanity check spread test doesn’t work on core devices yet, I’m investigating this (https://github.com/snapcore/snapd/pull/4644)
  • The security news is that layout mechanics needs to go through a hardening phase where snap-update-ns will come with an apparmor profile tailored to a given snap. This is my 2nd priority item after the spread test above
  • I have one more branch that ensures layouts don’t construct partially (https://github.com/snapcore/snapd/pull/4658).

So the big picture is that things are mostly there for layouts. If you are keen to try them out you can grab master, look at the spread test for inspiration and start experimenting.

I’m still due to write a documentation chapter and that will be my focus after the code is ready. I will be taking a small detour to help land the per-user mount namespaces to help the desktop team with XDG portal support so I cannot say exactly how timing looks like.

3 Likes

Two PRs that fixed a bug affecting symlinks and read-only directories and layouts on core were merged both into master and into the 2.32 release branch. With those fixes layouts should now operate on core and classic without any major issues.

I will look into updating the documentation and I’m looking forward to 2.32 :slight_smile:

2 Likes

Last week I identified two bugs that affect layouts:

  • layouts that use symlinks cannot cope with the symlink already being present and correct
  • layouts can leak some empty files, empty directories or symlinks into the non-snap space

Both of the issues are in progress, the first is under review at https://github.com/snapcore/snapd/pull/4851 and the second one is under development.

I was also made aware of an issue when refreshing try-mode snaps that are using layouts and I am investigating the issue. It looks like a bug in the update algorithm. I will post more updates as I get to the bottom of the issue.

Given that layouts are still a bit experimental we have decided to put them under feature flag.

To use layouts in 2.32 you will need to enable it with snap set core experimental.layouts=true.

1 Like

One small issue I hit during my experiment with (still experimental) layouts (discussed this on IRC, mentioning here so we don’t forget):

I was trying to define a layout such as this:

/usr/lib/tcltk/x86_64-linux-gnu/tk8.5:
  bind: $SNAP/usr/lib/tcltk/x86_64-linux-gnu/tk8.5

but got the following error when running my app:
cannot update snap namespace: cannot create writable mimic over "/usr/lib": permission denied;

and the following denial:
[68891.838823] audit: type=1400 audit(1524071502.757:492): apparmor="DENIED" operation="mount" info="failed mntpnt match" error=-13 profile="snap-update-ns.scid" name="/tmp/.snap/usr/lib/" pid=15755 comm="3" srcname="/usr/lib/" flags="rw, rbind"

It seems that the directories prior to “tk8.5” must exist for layout to work - I got it working by bind-mounting everything at the /usr/lib/tcltk level.

Thanks for reporting. That sounds like something we need to fix indeed.

@zyga Do you know what went wrong there?

Yes, when we grant permissions for snap-update-ns to construct a writable mimic we do so for the directory that was specified in the layout. The problem is that here we, at runtime, notice that /usr/lib/tcltk/ doesn’t exist and so decide to make a mimic at /usr/lib.

One way of fixing it would be to use the container work from Chipaca to inspect the base snap and know what we are dealing with. I need to think about other approaches.