cmd/snap-confine: allow running snap-exec without confinement

[zyga]

This is a openSUSE specific patch that I think only makes sense in the
case when snap-confine is confined but snap applications are not.
Proposed to gather feedback.

Signed-off-by: Zygmunt Krynicki me@zygoon.pl

[jdstrand]

From my comments in zyga/snapd@cfc9841:

This won't do what you want. This rule uses binary attachment and therefore any time the path matches, snap-exec will go unconfined and therefore all strict mode snaps will go unconfined. In other words, it does more than 'Allow executing with confinement', it forces it unconditionally.

I'm thinking this came up as a result of the conversation here: https://forum.snapcraft.io/t/snaps-and-nfs-home/438/8

Importantly, the 'ux' rule for snap-exec should only be added conditionally-- and we must be extremely careful with managing it because, like I said, there is potential for all strict mode snaps going unconfined if the logic is wrong, which would be a beyond critical bug. This is how we should handle this IMO:

• add to snap-confine's profile '#include </var/lib/snapd/apparmor/snapd-confine.d>'
• on snapd start, detect if in forced devmode
• if in forced devmode:
  • create /var/lib/snapd/apparmor/snapd-confine.d/forced-devmode with the above snap-exec rule and reload all the snap-confine profiles (with apparmor_parser -r -T -W)
  • prominently log that snapd is starting in forced devmode and that all snaps are unconfined
• if not in forced devmode: remove /var/lib/snapd/apparmor/snapd-confine.d/forced-devmode and reload all the snap-confine profiles (with apparmor_parser -r -T -W)

Do we allow forced devmode on Core? It would be ideal if !OnClassic never created forced-devmode to avoid the possibility of accidentally putting the rule on a Core device.

We need to have spread tests for this to ensure strict mode snaps are actually strict. This can be done by leveraging /proc/*/attr/current and verifying strict mode snap have the correct security label and snaps launched under forced devmode use 'unconfined'.

[zyga]

Thank you for the detailed feedback. I'll do as suggested!

[codecov-io]

Codecov Report

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

@@            Coverage Diff             @@
##           master    #3792      +/-   ##
==========================================
- Coverage   75.81%   75.78%   -0.03%     
==========================================
  Files         402      402              
  Lines       34745    34765      +20     
==========================================
+ Hits        26341    26347       +6     
- Misses       6529     6543      +14     
  Partials     1875     1875

Impacted Files	Coverage Δ
cmd/snap/cmd_aliases.go	93.33% <0%> (-1.67%)	⬇️
overlord/hookstate/hooks.go	15.68% <0%> (-0.11%)	⬇️
overlord/snapstate/snapstate.go	80.66% <0%> (-0.06%)	⬇️
overlord/ifacestate/helpers.go	62.33% <0%> (ø)	⬆️
snap/hooktypes.go	100% <0%> (ø)	⬆️

---

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 2101bac...db69e99. Read the comment docs.

[zyga]

I'm closing this, a new PR with better approach will follow.