hw-assign overwrites existing udev rules

[Carlo Lobrano]

Each call to hw-assign for a given snap overwrites the (possibly) existing udev rule in 70-snappy_hwassign_<snapname>.rules.

The list "write_path" in <snapname>.json.additional is correctly updated, instead.

Expected behavior:

The udev rule "70-snappy_hwassign_<snapname>.rules" contains all the device nodes also present in <snapname>.json.additional's write_path.

Following a copy of the content of both files after a sequence of calls to hw-assign:

(amd64)ubuntu@localhost:/var/lib/apparmor/clicks$ sudo cat minicom.saviq.json.additional
{
  "write_path": [
    "/dev/ttyUSB*",
    "/dev/simlink",
    "/dev/simlink2",
    "/dev/simlink3"
  ],
  "read_path": [
    "/run/udev/data/*"
  ]
}
(amd64)ubuntu@localhost:/var/lib/apparmor/clicks$ cat /etc/udev/rules.d/70-snappy_hwassign_minicom.saviq.rules

 KERNEL=="simlink3", TAG:="snappy-assign", ENV{SNAPPY_APP}:="minicom.saviq"

(amd64)ubuntu@localhost:/var/lib/apparmor/clicks$

[Carlo Lobrano (c-lobrano)]

This bug also affect the part of the code I am changing to fix Bug #1496319.

Since the fix for this bug should be simple (open the file in append mode, instead of writing a new file each time), I will propose a fix for this, before continuing with the fix for #1496319.

[Jamie Strandboge (jdstrand)]

I can confirm this.

[Jamie Strandboge (jdstrand)]

Marking critical because hw-assign only works for single devices right now, which is not how it is supposed to work.

Test case:
$ ls /etc/udev/rules.d
$

$ sudo snappy hw-assign hello-world.canonical /dev/foo
$ ls /etc/udev/rules.d
70-snappy_hwassign_hello-world.canonical.rules
$ cat /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules

KERNEL=="foo", TAG:="snappy-assign", ENV{SNAPPY_APP}:="hello-world.canonical"

$ sudo snappy hw-assign hello-world.canonical /dev/bar
$ cat /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules

KERNEL=="bar", TAG:="snappy-assign", ENV{SNAPPY_APP}:="hello-world.canonical"

Now, unassign /dev/bar:
$ sudo snappy hw-unassign hello-world.canonical /dev/bar

$ cat /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules
cat: /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules: No such file or directory

(foo should still be there).

[Jamie Strandboge (jdstrand)]

Whoops, meant to show this too where /dev/foo was added first, then /dev/bar, then /dev/bar removed:

$ sudo snappy hw-assign hello-world.canonical /dev/foo
'hello-world.canonical' previously allowed access to '/dev/foo'. Skipping

$ cat /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules
cat: /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules: No such file or directory

[Jamie Strandboge (jdstrand)]

Furthermore, adding access to a snap and also a service/binary in the snap doesn't work right:

$ sudo snappy hw-assign hello-world.canonical /dev/foo
'hello-world.canonical' is now allowed to access '/dev/foo'
$ sudo snappy hw-assign hello-world.canonical_sh /dev/bar
'hello-world.canonical_sh' is now allowed to access '/dev/bar'

$ cat /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules

KERNEL=="bar", TAG:="snappy-assign", ENV{SNAPPY_APP}:="hello-world.canonical"

Adding access to multiple snaps doesn't work right either:
$ sudo snappy hw-assign hello-world.canonical /dev/foo
'hello-world.canonical' is now allowed to access '/dev/foo'
$ sudo snappy hw-assign ufw.sideload /dev/baz
'ufw.sideload' is now allowed to access '/dev/baz'
$ cat /etc/udev/rules.d/70-snappy_hwassign_hello-world.canonical.rules

KERNEL=="foo", TAG:="snappy-assign", ENV{SNAPPY_APP}:="hello-world.canonical"

In this case the second command did not take at all.

[Carlo Lobrano (c-lobrano)]

Hello Jamie,

the second behavior you noticed (the missing of the udev rule file after unassignment of only 1 of the devices) is due to the fact that RemoveHWAssign method just removes the file, without looking at its content. In the branch where I propose the fix for this bug, I also fixed this particular effect.

My understanding was that only snaps can have devices assigned, this could be the explaination of the errors assigning devices to services/binaries in the snap

[Carlo Lobrano (c-lobrano)]

I double checked the case of assignment to multiple snaps, currently each snaps has its own udev rule file

$ sudo snappy hw-assign minicom.saviq /dev/test1
'minicom.saviq' is now allowed to access '/dev/test1'

$ sudo snappy hw-assign hello-world.canonical /dev/test1
'hello-world.canonical' is now allowed to access '/dev/test1'

$ ll /etc/udev/rules.d/
total 16
drwxr-xr-x 2 root root 4096 Sep 23 06:13 ./
drwxr-xr-x 4 root root 4096 Sep 18 13:31 ../
-rw-r--r-- 1 root root 81 Sep 23 06:13 70-snappy_hwassign_hello-world.canonical.rules
-rw-r--r-- 1 root root 73 Sep 23 06:13 70-snappy_hwassign_minicom.saviq.rules

if this is the expected behavior, it should be ok

[Jamie Strandboge (jdstrand)]

Yeah, I just rechecked that-- I was wrong on that point. It is expected behavior, thanks for checking.