RFC: introduce alternate snap-confine for classic exec transitions (LP: #1849753) #10029
Conversation
…849753) LP: #1849753 describes how classic snap policy doesn't account for file-inherit denials when a classic snap execs another snap. Many see this as noise in their logs but it also breaks real-world scenarios such as using the go snap inside of an LXD container (or a classic snap calling another snap). At this time, the identified additional rules are: ptrace readby peer=unconfined, unix, owner /** rw, The ptrace rule simply allows an unconfined process (eg, systemd) to read some info from /proc associated with this process. The 'unix' rule was observed as needed with the go snap, et al. The broad file rule is unfortunate, but needed by the go snap for /tmp files, files in HOME, etc and by chromium content API classic snaps access to shared memory files. While unfortunate, the file-inherit denials cause noisy logs and in the case of (at least) compiler snaps, broken functionality (see bugs in the References, below). A workaround is to add a file to /var/lib/snapd/apparmor/snap-confine with these rules and reloading all the snap-confine profiles. However, snap refreshes will remove the file so this doesn't work very well. There is currently no proper solution as the proper solution would be fuller delegation support in AppArmor. Several workarounds are possible: 1. change the snap-confine policy to allow more access 2. introduce separate snap-confine policy that is wider then the normal and let snap-confine decide what to do 3. introduce separate snap-confine policy and adjust the classic template to transition to this policy if it is executing snap-confine All cases weaken the security protections afforded by AppArmor for snap-confine to varying degrees, though snap-confine's design does not rely on AppArmor being present (ie, it is meant to be securely coded). Option '1' weakens the policy for everyone and is not desirable since only classic snaps need snap-confine to have additional access (since a strict mode snap isn't allowed to execute other snaps, by design). '2' is better but means that if there was a vulnerability in snap-confine, snap-confine could potentially change to the weaker profile. '3' is better still since snap-confine keeps its normal policy except in the case where the classic snap's profile chooses to transition snap-confine to a more lenient policy. A problem with '2' and '3' is that there is an additional profile on the system that users may use with 'aa-exec -p' to run snap-confine under (eg, 'SNAP_INSTANCE_NAME=app aa-exec -p <some other policy> /usr/lib/snapd/snap-confine snap.app.thing <blah>') to avoid the binary attachment and try to exploit a bug in snap-confine to escalate, but '2' and '3' isn't appreciably worse on a system with a classic snap installed since today that classic's snap's profile can be specified during the attack attempt. This commit is a PoC and is barely tested. I suggest the following be added: * spread tests * fixing addContent() to use snapConfineFromSnapProfile() so it will work in the re-exec case (see TODO in code) * only loading the alternate profile if there is a classic snap installed on the system (addresses the "main problem with '2' and '3'" since a system without a classic snap doesn't have the extra profile to leverage in a local privilege escalation attack attempt and a system with a classic snap installed already can use the nearly wide open classic policy (eg, aa-exec -p snap.classic.thing). Ie, if conditionally loading the classic profile, there is no difference with today) * deb/etc packaging: don't install snap-confine-classic.apparmor in /etc/apparmor.d * core snap: do install snap-confine-classic.core.<REVISION> in /var/lib/snapd/apparmor/profiles * snapd snap: do install snap-confine-classic.snapd.<REVISION> in /var/lib/snapd/apparmor/profiles When full delegation support is available in AppArmor, this workaround alternate-classic-profile approach can be removed. References: - https://launchpad.net/bugs/1849753 - https://launchpad.net/bugs/1850552
|
FYI, I'm not going to be able to work on this any time soon, so please feel free to pick this up and run with it. I suggest either @alexmurray or @anonymouse64 but obviously choose who you'd like. :) |
|
As for the CLA check, I'm not sure what that is about. I signed the CLA years ago. |
I should mention, please request me as a reviewer if picking this up (I will find time for reviews). |
|
@jdstrand Thank you so much for this PR! About the CLA, I can chase that internally. But it might be easier/quicker if you just sign it again. Maybe your signature predates all the automatic processes we have around that now? |
Ok, I signed it again just now. |
|
I'll be working on this when I get some more time |
|
This PR seems likely to fix some nasty issues when using the Node snap: nodejs/node#37982 |
|
FYI, I've been using this (rather terrible) workaround to get things done: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1849753/comments/22 |
|
@mvo5 - hi! Curious if this has bubbled up on a roadmap or scheduled for some release. I've seen people in the node community suggesting to skip the snap. While the .bashrc workaround I have 'works' (I use it a lot with the go snap), it has to be reapplied over and over and isn't discoverable. |
|
@jdstrand I always used the snap, but it caused me so much headache trying to circumvent that bug that I moved away from it |
|
Is this workaround on the roadmap in the not-too-distant future? Or is the idea to wait for AppArmor to support everything needed to "do it properly"? I tried parsing https://gitlab.com/apparmor/apparmor/-/wikis/AppArmorDelegation, but couldn't quickly judge where we stand here. |
|
Unfortunately this is not on the official roadmap, and the past few months have been quite busy, I still hope to get around to this though it may be a while before that happens. |
pedronis
added
the
Precious but later ❤️
|
something to go back at later |
|
Hey, will this be fixed anytime soon ? |
LP: #1849753 describes how classic snap policy doesn't account for
file-inherit denials when a classic snap execs another snap. Many see
this as noise in their logs but it also breaks real-world scenarios such
as using the go snap inside of an LXD container (or a classic snap
calling another snap).
At this time, the identified additional rules are:
ptrace readby peer=unconfined,
unix,
owner /** rw,
The ptrace rule simply allows an unconfined process (eg, systemd) to
read some info from /proc associated with this process. The 'unix' rule
was observed as needed with the go snap, et al. The broad file rule is
unfortunate, but needed by the go snap for /tmp files, files in HOME,
etc and by chromium content API classic snaps access to shared memory
files. While unfortunate, the file-inherit denials cause noisy logs and
in the case of (at least) compiler snaps, broken functionality (see bugs
in the References, below).
A workaround is to add a file to /var/lib/snapd/apparmor/snap-confine
with these rules and reloading all the snap-confine profiles. However,
snap refreshes will remove the file so this doesn't work very well.
There is currently no proper solution as the proper solution would be
fuller delegation support in AppArmor. Several workarounds are possible:
and let snap-confine decide what to do
template to transition to this policy if it is executing snap-confine
All cases weaken the security protections afforded by AppArmor for
snap-confine to varying degrees, though snap-confine's design does not
rely on AppArmor being present (ie, it is meant to be securely coded).
Option '1' weakens the policy for everyone and is not desirable since
only classic snaps need snap-confine to have additional access
(since a strict mode snap isn't allowed to execute other snaps, by
design).
'2' is better but means that if there was a vulnerability in
snap-confine, snap-confine could potentially change to the weaker
profile.
'3' is better still since snap-confine keeps its normal policy
except in the case where the classic snap's profile chooses to
transition snap-confine to a more lenient policy.
A problem with '2' and '3' is that there is an additional profile on the
system that users may use with 'aa-exec -p' to run snap-confine under
(eg, 'SNAP_INSTANCE_NAME=app aa-exec -p <some other policy>
/usr/lib/snapd/snap-confine snap.app.thing <blah>') to avoid the binary
attachment and try to exploit a bug in snap-confine to escalate, but '2'
and '3' isn't appreciably worse on a system with a classic snap
installed since today that classic's snap's profile can be specified
during the attack attempt.
This commit is a PoC and is barely tested. I suggest the following be
added:
work in the re-exec case (see TODO in code)
installed on the system (addresses the "main problem with '2' and '3'"
since a system without a classic snap doesn't have the extra profile
to leverage in a local privilege escalation attack attempt and a
system with a classic snap installed already can use the nearly wide
open classic policy (eg, aa-exec -p snap.classic.thing). Ie, if
conditionally loading the classic profile, there is no difference with
today)
/etc/apparmor.d
/var/lib/snapd/apparmor/profiles
/var/lib/snapd/apparmor/profiles
When full delegation support is available in AppArmor, this workaround
alternate-classic-profile approach can be removed.
References: