Allowing interfaces to disable device cgroup/udev

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.

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

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.

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

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.

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.

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.

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

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.

1 Like

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

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

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?

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

1 Like

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

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?

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.

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).

I would prefer to not do that, as then the snap always has those accesses, when in some cases it won’t need those accesses if it’s not being used for device specific containers (for example it’s just doing some data computation from a network source).

That also doesn’t solve the issue with the generic globbing and would complicate accesses such as for serial ports, gpio, etc.

The generic globbing was what I was thinking of with ‘where it makes sense’; it wouldn’t in that case and we’d need to pull in duplicate rules. We also add glob rules where it does make sense, eg, serial ports. There are other options in this arena, such as interface attributes that turn on or off bits of the interface or having greengrass-support and greengrass-support-devices (or something), where the former has common accesses that are always needed and the latter has those various devices that are only sometimes needed.

I think we should discuss this in realtime with @pedronis, perhaps at the upcoming sprint?

We discussed this over hangouts and came to the conclusion that it is okay for a super-privileged interface such as greengrass-support to disable udev+device cgroup unconditionally for now, with the caveat that it will sometimes give extra permissions than was desired (for example with the serial port if someone does have a gadget out there that declares the serial port using USB attributes, then the snap will gain access to more serial ports than that specific one with the glob pattern).
Additionally, we will work with upstream snap developers to evaluate “stacking” cgroups as well as cgroups v2 to try and make this snap not be incompatible with the cgroup that snapd declares and could eventually transition away from this interface disabling the device cgroup.

1 Like