hw-assign and oem assign are inconsistent

[Jamie Strandboge]

snappy hw-assign currently:
* adds a udev rule to /etc/udev/rules.d
* adds a specific apparmor write-path for the device (eg, /dev/foo)
* adds a read-path for /run/udev/data/*

oem assign currently:
* adds a udev rule to /etc/udev/rules.d
* adds a generic apparmor write-path for all devices (ie, /dev/**)
* adds a read-path for /run/udev/data/*

Step '2' in the above is inconsistent and when combined with the added udev rule, it causes confusion and a different security mechanism to be used. With hw-assign, the 'needle check' will fail in the launcher and a cgroup will not be used and instead rely on apparmor. With oem assign, the 'needle check' passes in the launcher and a cgroup is used. In both cases, udev tags are used and added to the system, but udev tags are only needed with cgroups.

This inconsistency was introduced in r415 (https://code.launchpad.net/~mvo/snappy/snappy-fix-static-hwassign)

Unless I am missing something, either:
 * hw-assign should *not* add a udev rule to /etc/udev/rules.d or the read-path for /run/udev/data/* (ie, revert r415)
 * hw-assign should instead of adding a specific apparmor write-path, add the generic apparmor write-path for all devices (ie, /dev/**)

For consistency, the 2nd approach seems most correct, but the former would be ok too.

This does not constitute a security vulnerability because in both cases hardware access is mediated.

[Michael Vogt (mvo)]

I think we need to go with option (2), i.e. have hw-assign use the /dev/** and generate a matching udev-rule. However I think there is another bug here where snappy will just override the existing udev rule if you run hw assign two times for the same app (e.g. /dev/video and /dev/dsp). That should probably also get fixed in the process.

[Jamie Strandboge (jdstrand)]

Ok, cool, I thought about this last night and couldn't think of an issue (doesn't mean there might not be one, but we'll fix it). The bug you are referring to is bug #1497299 but you probably also want to keep bug #1499095 in mind when fixing this.

[Mark Shuttleworth (sabdfl)]

Guys, we should not have an end-user typing any command involving "/dev/".

We should be typing commands associated with capabilities, and those
should discover the /dev/X they need.

Mark

[John Lenton (chipaca)]

Mark, I'm imagining there's a spec for what capabilities are in this context, but I haven't seen it. Nevertheless it's probably safe to say it'll take a while to implement that, and we should probably fix this while we build that.

Michael, #1497299 is already being fixed by a community person in lp:~c-lobrano/snappy/hw-assign-fix-overwrite-udev-rule

[John Lenton (chipaca)]

OK, a few questions about this bug.

Jamie, right now devices can start with "/dev/", "/sys/devices/", or "/sys/class/gpio/". AIUI the proposal is that in all cases the apparmor write path to add is "**" added to that prefix, correct?

For "hw-info" I need the full path to a device. Right now the code uses the apparmor rule to retrieve the full path to devices; is there another place this can be retrieved from?

Additionally the full device path is used to avoid duplicate entries. I can change the code to use the udev rule, but that only uses the device's basename. Is this enough? Or should we eschew the idea of detecting duplicates at this level?