The need for snap-update-device-cgroup

snapd
1

Hey everyone

As a part of fixing https://bugs.launchpad.net/snapd/+bug/1838937 I realized the device cgroup needs some non-trivial changes. Let me explain my thought process quickly.

The issue is that, effectively, we don’t create the device cgroup when there are no devices tagged to a specific snap application. When such devices are then added to the system (connecting a device) or when the set of tagging rules changes (connecting a snap interface) then the changes are applied to a cgroup other than the one used by running application.

To fix that we can create a device cgroup and always put processes in it, one for each snap application or hook. The cgroup must have one of two configurations:

  • Unconfined: allowing access to all devices
  • Confined: allowing access to more-less fixed number of devices as well as all the tagged devices

The unconfined configuration is important because some snaps have broad permissions and really does require this mode.

The problem is transition between the two configurations. While technically easy to perform, it must be done due to one of two different reasons:

  • Device added or removed to the system This is currently handled by udev and snap-device-helper shell script which adds or removes entries to an existing confined device cgroup.
  • Interface connected or disconnected. This is handled by snapd and indirectly, by udev that we “trigger” and “settle”.

Note that neither of those can change a device cgroup from confined to unconfined.

In my eyes, we need a pair of new components. The first is a per snap app/hook file that list devices to be tagged. This is something we can ask udev to generate for us based on udev device database and our tagging rules. We can effectively re-create this each time something changes (either the tagging rules or the actual devices). The second is a tool that reads that file and re-configures a cgroup to either confined or unconfined mode. This is relatively straightforward.

Each time interface connection relating to udev occurs we must invoke that tool.
Each time udev device is added or removed we must invoke that tool.

In other words, we need snap-update-ns equivalent for device cgroups.

2

I would probably rephrase this in cgroups terms: perhaps ‘all devices’ and ‘allowed devices’ or something.

1 Like
3

It was always considered a limitation of the implementation that, like with seccomp, the application would have to restart to have the device cgroup take effect when going from ‘unconfined’ to ‘confined’ and vice versa. I like how you are trying to remove this limitation.

I thought the new idea was to always, unconditionally create the device cgroup and always, unconditionally put the snap command in it at which point we have only one thing to operate on. snap-device-helper can always update it and so can snap-confine. Interface connect/disconnnect would also need to operate on it. All would need to understand that ‘nothing is tagged’ should switch to ‘unconfined’ and ‘stuff is now tagged’ should switch to ‘confined’. With that in mind, I guess that is the ‘snap-update-ns’ equivalent you speak of?

4

Your understanding is in line with my thinking, I’m just expressing myself poorly. I will share the patches soon, we can discuss in concrete terms.

5

Is there a reason (other than the fact this isn’t updating namespaces) why we couldn’t just add this to snap-update-ns and perform this update at the same time as we update the namespace?

6

There are several:

  • the motivation for updating one is separate from updating the other
  • mount namespace only changes when snapd says so, device cgroup must change when a device is changed in the system
  • we use libudev and I would rather not re-implemement it in go for no specific reason
  • cgroupv2 will require us to use more low-level kernel interfaces that would need a go bridge first
1 Like