Allowing interfaces to disable device cgroup/udev

#1

We would like to be able to disable enforcing the udev tags for a particular snap when it plugs the greengrass-support interface, because this snap launches containers and sets up control groups for the containers it launches. This is problematic, because we want this snap to also plug interfaces that allow for device access, but trigger generating udev rules and thus enable a device cgroup for the snap. Enabling a device cgroup for the snap is incompatible with the daemon inside the snap also enabling device cgroups for the containers.

Discussing with @jdstrand over IRC a while ago, one way to handle this is to have the greengrass-support interface mark any snap that plugs it as “controlling device cgroups”, such that when the udev backend goes to generate rules, it doesn’t generate any. This is similar to what was done for ptrace (trace) with AppArmor. My basic implementation (minus tests, etc.) can be found here: https://github.com/anonymouse64/snapd/commit/09859a617683ab68e1d352d4bd85b2e15ae3e106

Thoughts on this? Specifically @pedronis and @jdstrand

#2

I think the idea is fine in general, the only thing is that some interfaces use apparmor glob rules for device access since it is expected the device cgroup will be in use. We’d want to conditionally remove those glob rules for interfaces participating in this. It might be best to have interfaces declare they are cgroup-disablable (if you will) then have the code to conditionally remove the apparmor globs in those interfaces rather than trying to do it for all interfaces.

#3

I assume you are referring to snippets such as this for the joystick interface: https://github.com/snapcore/snapd/blob/master/interfaces/builtin/joystick.go#L57-L63

To be clear, what you are suggesting is that AppArmor snippet would be removed entirely if used with an interface that disabled the device cgroup enablement? Do you think most applications would still work if that snippet was removed? Unfortunately I don’t have an explicit example I can run to see if a joystick container (for example) would still work properly if it didn’t have access to /dev/input/event[0-9]*. I guess if the application is unlikely to work due to this denial then it doesn’t make much sense to do this kind of removal at all and we either would need another solution or just make the joystick interface entirely incompatible with this type of “device cgroup disabling” interface…

For reference, the initial list of interfaces for this snap that will be pluggable that currently enable udev tagging are:

  • network-control
  • bluetooth-control
  • camera
  • dvb
  • gpio-memory-control
  • gpio
  • hardware-random-control
  • i2c
  • iio
  • io-port-control
  • joystick
  • opengl
  • optical-drive
  • raw-usb
  • serial-port
  • spi

The interfaces I see that generic AppArmor /dev/... snippets which also correspond to a stricter udev tagging rule seem to be:

  • joystick (for /dev/input/event[0-9]*)
  • serial-port
  • raw-usb (for /dev/tty{USB,ACM}[0-9]*)

I don’t know how often these containers will read data from a joystick, so I’m okay with not connecting that interface for now if it’s not useful without the event access, however the serial-port interface is much more problematic as I don’t think you could use the interfaces at all without the access for /dev/tty... patterns. What do you propose for handling those interfaces with this device cgroup “incompatibility” or “disabling” flag?

It seems that if the serial-port AppArmor snippet is only generic if the slot doesn’t specify usb attributes then a more specific path rule is used instead, which may be okay and in this case we could just require that:

if snap has device cgroup disabled
    if slot doesn't have raw-usb attrs:
        use specific device path in AppArmor
    else:
        fail
else:
    do current logic

This would mean that the snap could only plug into certain serial-port slots rather than any serial-port slot which is undesirable. I think that ideally we would use hotplug here but my understanding is that work isn’t done yet.

As for the raw-usb, I don’t see a similar solution to reducing the coverage of /dev/tty{USB,ACM}[0-9]* (because currently the udev rule restricts to only devices with ENV{ID_BUS}=usb.

#4

raw-usb isn’t an issue since it doesn’t try to do more finegrained mediation with the device cgroup.

joystick is definitely one that should not use the glob rules since without the cgroup but with the glob rules it gives access to all input devices.

serial-port isn’t going to work for you anyway, today, since the gadget snap has to define what is available since hotplug isn’t in place yet. However, serial-port is one of the first interfaces that will start to use hotplug, aiui.

There is a new yubikey interface coming that operates like joystick, but for hidraw devices, so you’d want to avoid that too.

For the others:

  • camera, dvb, optical-drive: currently grants access to all devices of this type, but will (eventually) change with hotplug
  • gpio, i2c, iio, spi: specific access where apparmor and cgroup are the same and gadget defines the specific devices
  • network-control, bluetooth-control, gpio-memory-control, hardware-random-control, io-port-control: specific access where cgroup and apparmor are same
  • opengl: like raw-usb with glob access where cgroup and apparmor are essentially the same

But there were others not listed in the above:

  • alsa, broadcom-asic-control, framebuffer, mir, modem-manager, ofono, ppp, pulseaudio, tpm, udisks2, wayland, x11: glob access where cgroup and apparmor are essentially the same
  • adb-support, device-buttons: like joystick where cgroup is relied on for fine-grained mediation
  • bluez, fuse-support, hardware-random-observe, kernel-module-control, kvm, kubernetes-support, network-manager, physical-memory-control, physical-memory-observe, time-control: specific access where apparmor and cgroup is the same
  • hidraw: like serial-port where glob is used when usb attributes specified
  • uhid: specific access for which udev tagging can’t be used

So, with the above, the only ones to worry about are joystick, adb-support, device-buttons, serial-port with usb attributes, hidraw with usb attributes and the upcoming yubikey interface.

#5

But isn’t this just for the slot end? Or do you have to specify in the plug end as well?

#6

I guess I had missed device-buttons but I don’t think adb-support is likely to be needed… I think device-buttons is unlikely to be used now and I’m fine with figuring out how to handle that properly (along with joystick and the others that seem to be incompatible) when we have an actual use case we can look at in depth.

Yeah the only one I can see being especially problematic for us to not have right now is the serial-port interface which I’m okay with accepting that it will only work if the serial port is defined without usb attributes in the gadget snap (with the logic I had above) as the main consumer of this snap that will also use device containers will also be likely be brand store customers and can control their gadget snap. And eventually hotplug should solve this anyways.

#7

Without the slot end, there is nothing to plug into (this is why serial ports don’t work on classic distro).

#8

Why would the slot end be a problem for the snap? This snap is just going to plug the interface, and the slot will come from the gadget snap. What I’m proposing is that the connection only works if the slot doesn’t have any usb attributes (and thus doesn’t trigger the generic AppArmor rule). If the slot from the gadget snap has usb attributes then the connection is denied. This at least gives an option to customers that are in control of their gadget snap (which will be most of the users of this snap I imagine) to use the serial-port interface with this snap.

#9

I think there is some miscommunication. Obviously there needs to be a slot side so that a snap can plug into it. If you have a gadget that defines these slots, then sure, only have to worry about when usb attributes are specified (thus triggering the apparmor glob). We don’t currently have a concept of one interface causing another to be denied a connection. This would need to be discussed with @pedronis and I suspect @pstolowski (due to all the hotplug work he has implemeted). It might be sufficient to instead of denying the connection, omit the glob rule. Of course, that would be a usability issue since the plug would be connected but the access denied.

Where our disconnect happened is with a greengrass snap on classic distro, which I thought I remembered you saying was supported but where a gadget snap hasn’t defined any serial-port slots. Sorry for the confusion.

#10

Ah I didn’t realize this was a new concept…

In light of this I think a reasonable compromise would be to do as you say and conditionally remove that generic snippet rather than deny the connection.

This use case is less important and mainly for development. It’s reasonable to say that for development, if you need to test the serial-port interface, you need to craft a UC image with an appropriate gadget snap slot for the serial-port. The overwhelming majority of use cases we have gotten is for using the snap on UC.

#11

The code in serial-port is slightly confusing, some of it seems to be ok with mixing a path with usb attributes, but then we generate a glob just based on usb attributes even if possibly there was a path set as well.

@ijohnson it would be good if you could double check whether relevant gadgets tend to use paths or usb attributes for serial-ports

#12

Sure, so for the common developer use-case of a Raspberry Pi, the only serial port in the gadget snap are declared via path: https://github.com/snapcore/pi3-gadget/blob/master/snapcraft.yaml#L173-L175
For the dragonboard, all 32 serial ports are declared with the path: https://github.com/snapcore/dragonboard-gadget/blob/master/snapcraft.yaml#L157-L252

And for the most relevant large customer deployment that will use this snap, all of the serial ports declared (of which there are 8) are all declared via path. For all the other customer deployments I have access to, if there are serial ports declared in the gadget snap, they all use the path to declare the slot.

#13

Typically the serial ports defined in our gadgets are actual native serial ports, not attached via USB (so no USB attributes available) …

IIRC the initial idea for adding the USB attribute bits were additional serial ports via FTDI so that you could have a device with pre-attched FTDI serial dongle to have additional serial ports (IIRC thats used a lot in roboticts, where you most likely build a custom local gadget for your project when using Core) … @kyrofa might have some deeper insight here

#14

Rereading that I’m reminded that the path is always required. When specified without usb attributes, it is meant to specify an actual path on the system, like /dev/ttyS1. When specified with usb attributes, it signifies the name of the symlink that will be created to point at the dynamically assigned device as the vendor and product matches. In this manner, the glob is required when usb attributes are specified (since we don’t know what udev will assign and apparmor resolves symlinks) and it is not used otherwise, since the device is predictable.

Also see The serial-port interface

#15

Heh, if I were making a robot using Ubuntu Core that’s the last thing I’d want to do. If I could get away with using Canonical’s gadgets I totally would. Who wants to own uboot just to expose serial ports?

#16

I see, the code in AppArmorConnectedPlug probably merits a comment about this (is it not obvious just reading it alone)

#17

I’ve added this as a todo for the next batch of policy updates.

#18

This seems to ask for very confusing behavior, I think we need a discussion to explore our options and our goals here. Hand-waving: couldn’t we require the snap creating device cgroups to make them children of the snapd created/managed one? is the worry that is not implementable or that it cannot be enforced, I imagine the relevant interface for the snap here gives a lot of permissions to start with anyway?

#19

As far as I can tell this doesn’t seem to be implementable (and is not at all considered by the snap application itself). Whenever this app tries to run and is confined by a device cgroup it isn’t able to create cgroups for the containers and fails to run at all. And also, the app creates more than just a device cgroup, it creates memory, cpu, etc. cgroups as well. (Note for compatibility, the app is able to detect what cgroups it can use so if one isn’t available it won’t use that I’m pretty sure) The interface is already super-privileged so I don’t see an issue with allowing it the ability to also manage it’s own cgroups from a security perspective, just that how this interface interacts with these other device oriented interfaces is the question at hand.

#20

I wonder if it makes sense to go back to the idea that the snap only plugs greengrass-support, which itself would not use tagging, and put all the security policy in it. Where it makes sense, we can use the apparmor policy variables from other interfaces and concatenate the policy together (rather than hard-coding duplicate policy in it).