bwrap: Implement ability to switch AppArmor profiles#425
Open
pks-t wants to merge 1 commit intocontainers:mainfrom
Open
bwrap: Implement ability to switch AppArmor profiles#425pks-t wants to merge 1 commit intocontainers:mainfrom
pks-t wants to merge 1 commit intocontainers:mainfrom
Conversation
Bubblewrap is currently hard to use in combination with AppArmor
profiles. The root cause of this is that it sets the NO_NEW_PRIVS flag
quite early in the process, and if that flag is set then most AppArmor
profile transitions are disallowed (except for unconfined -> confined
and profile stacking). This makes it rather hard to have a central
profile for bwrap acting as a portal with "normal" profiles. While this
could be solved by granting the bwrap profile itself full permissions to
everything on the system and then only use stacked transitions, this
feels overly dangerous especially considering that bwrap is typically
installed setuid.
To fix this issue, this commit instead introduces the ability to
explicitly transition to a specific target AppArmor profile. This allows
us to perform the transition before we set the NO_NEW_PRIVS flag and
thus make them both work together. There are two downsides to this:
- NO_NEW_PRIVS must be set at a much later point in time, namely
after the sandbox has been constructed and the new profile has
been changed to. While we could switch to the profile at a much
earlier point in time, this would require the target profile to
grant permissions required to construct the sandbox. We cannot use
AppArmor's "change_onexec" transition either: even if we set the
transition up while NO_NEW_PRIVS is not yet effective, the profile
switch on exec is going to be denied anyway.
- In order to allow switching the profile, we need to have a
writable "/proc". This is required such that we can effect the
transition via a write to "/proc/self/attr/apparmor/current".
Neither of these downsides should be a problem though: NO_NEW_PRIVS'
main intent is to avoid granting new privileges on execve(2), which it
still does given that execve(2) is the last step. And "/proc" being
writable shouldn't matter much when a pid namespace is in use.
Implement AppArmor profile switching via a new "--apparmor-profile"
switch and document it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Do you mean using bwrap within permissive apparmor profile is dangerous and with no profile at all it isn't?
Typically where? Debian stable has it as setuid but current testing has switched to non-setuid bwrap. Other most popular distro used non-setuid bwrap already. |
Author
|
On Tue, May 25, 2021 at 07:27:07AM -0700, Maryse47 wrote:
> this feels overly dangerous especially considering that bwrap is typically installed setuid.
Like where? Debian stable has it as setuid but current testing has
switched to non-setuid bwrap. Other most popular distro used
non-setuid bwrap already.
Fair enough, that statement may have been overly general and was based
on a knowledge gap on my side. I still think it's useful to manually
specify an AppArmor profile transition such that one can easily specify
the profile under which the contained process should be running in the
end.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bubblewrap is currently hard to use in combination with AppArmor
profiles. The root cause of this is that it sets the NO_NEW_PRIVS flag
quite early in the process, and if that flag is set then most AppArmor
profile transitions are disallowed (except for unconfined -> confined
and profile stacking). This makes it rather hard to have a central
profile for bwrap acting as a portal with "normal" profiles. While this
could be solved by granting the bwrap profile itself full permissions to
everything on the system and then only use stacked transitions, this
feels overly dangerous especially considering that bwrap is typically
installed setuid.
To fix this issue, this commit instead introduces the ability to
explicitly transition to a specific target AppArmor profile. This allows
us to perform the transition before we set the NO_NEW_PRIVS flag and
thus make them both work together. There are two downsides to this:
Neither of these downsides should be a problem though: NO_NEW_PRIVS'
main intent is to avoid granting new privileges on execve(2), which it
still does given that execve(2) is the last step. And "/proc" being
writable shouldn't matter much when a pid namespace is in use.
Implement AppArmor profile switching via a new "--apparmor-profile"
switch and document it.
Signed-off-by: Patrick Steinhardt ps@pks.im