interfaces/many, cmd/snap-confine: miscellaneous policy updates

[jdstrand]

• interfaces/many: for interfaces using nameservice abstraction, allow resolved
• interfaces/alsa: allow rw access to /proc/asound
• interfaces/optical-drive: add comment in apparmor policy
• interfaces/camera: allow read access to video4linux sysfs entries
• cmd/snap-confine: silence harmless fsetid denial
• interfaces/network-control: allow reading rfkill sysfs entries and writing to state (LP: #1707612)

The fsetid denial is caused because the newly created /var/lib mimicked directory is created with non-root group (since snap-confine is setuid, not setgid). The chmod() in sc_quirk_create_writable_mimic() causes the denial. The denial is harmless since the capability isn't needed to perform the chmod(), so silence the denial to avoid user confusion since we don't want snap-confine to have the extra permissions.

[zyga]

+1

[zyga]

Thank you for tracing this down!

[jdstrand]

Note that finding what caused the denial was tricky because:

• it only happens on classic
• it only happens when the namespace is first being setup
• it only happens when the invoking user is non-root
• there is no EPERM or EACCES in the strace-- the chmod() call works fine
• the kernel doesn't reliably log the denial except after fresh boot

To determine the cause (and recording here for posterity), I:

• rebooted
• added a sleep() call to main() in snap-confine.c
• ran 'hello-world' with the updated snap-confine
• used gdb to attach to snap-confine during the sleep
• 's'tepped, 'c'ontinued (and 'finish'ed) my way through snap-confine until the denial popped out
• added debugging information to the affected function
• rebooted and ran hello-world, which made the kernel log the denial and snap-confine output:

sc_quirk_create_writable_mimic(): mimic_dir=/var/lib
sc_quirk_create_writable_mimic(): mimic_dir st_uid=0,st_gid=1000
sc_quirk_create_writable_mimic(): mimic_dir st_mode=41777
sc_quirk_create_writable_mimic(): ref_dir=/snap/core/current//var/lib
sc_quirk_create_writable_mimic(): ref_dir st_uid=0,st_gid=0
sc_quirk_create_writable_mimic(): ref_dir st_mode=40755
sc_quirk_create_writable_mimic() after chown: mimic_dir=/var/lib
sc_quirk_create_writable_mimic() after chown: mimic_dir st_uid=0,st_gid=0
sc_quirk_create_writable_mimic() after chown: mimic_dir st_mode=41777
sc_quirk_create_writable_mimic() after chmod: mimic_dir=/var/lib
sc_quirk_create_writable_mimic() after chmod: mimic_dir st_uid=0,st_gid=0
sc_quirk_create_writable_mimic() after chmod: mimic_dir st_mode=40755

Reading man capabilities I could tell from the above that we were hitting "set the set-group-ID bit for a file whose GID does not match the filesystem or any of the supplementary GIDs of the calling process." sc_quirk_create_writable_mimic() does the chown before the chmod and changes the group to 'root', but the calling process' gid is still non-root. I think this could maybe be changed to make the chmod call before the chown, but typically you want to chown first so did it this way.

[codecov-io]

Codecov Report

Merging #3634 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3634      +/-   ##
==========================================
+ Coverage   75.19%   75.21%   +0.02%     
==========================================
  Files         386      387       +1     
  Lines       33418    33453      +35     
==========================================
+ Hits        25127    25161      +34     
  Misses       6480     6480              
- Partials     1811     1812       +1

Impacted Files	Coverage Δ
interfaces/builtin/optical_drive.go	100% <ø> (ø)	⬆️
interfaces/builtin/network_control.go	100% <ø> (ø)	⬆️
interfaces/builtin/network_manager.go	78.04% <ø> (ø)	⬆️
interfaces/builtin/camera.go	100% <ø> (ø)	⬆️
interfaces/builtin/network_observe.go	100% <ø> (ø)	⬆️
interfaces/builtin/modem_manager.go	58.13% <ø> (ø)	⬆️
interfaces/builtin/firewall_control.go	100% <ø> (ø)	⬆️
interfaces/builtin/alsa.go	100% <ø> (ø)	⬆️
interfaces/builtin/network.go	100% <ø> (ø)	⬆️
interfaces/builtin/ofono.go	61.53% <ø> (ø)	⬆️
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bb7d6d...330dbbb. Read the comment docs.

[chipaca]

LGTM, and feel free to land as is despite my comments which are no more than nitpicking.

[chipaca]

giggle at having to clean up this comment

[chipaca]

what's the rationale for not having a newline before this comment block?

[jdstrand]

I like to group like rules together with different rule groups separated by spaces (I may not be 100% consistent with this, but it is what I like to do and did here). This rule is about silencing something related to quirks, so I put it there, without a space.

[chipaca]

can't these all be replaced by a suitable include?

[jdstrand]

Upstream decided that while this is suited for the nameservice abstraction, they felt that since non-Ubuntu (and Ubuntu derivatives) don't have dbus mediation, they didn't want to have it in there yet. I did commit it to the Ubuntu archive, but it isn't available anywhere except 17.10. In cases like this I try to push the change upstream (eg, apparmor, Ubuntu, etc) and snapd, then at some future point (series 18?) we can evaluate if we can just use the abstraction.

[jdstrand]

Have two +1s and the zesty autopkgtest failure is unrelated. Committing.

[jdstrand]

@mvo5 - can you consider this PR for 2.27 inclusion?

[jdstrand]

Thanks for the reviews! :)