Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bwrap: Implement ability to switch AppArmor profiles #425

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pks-t
Copy link

@pks-t pks-t commented May 24, 2021

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

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>
@pks-t pks-t changed the title rbwrap: Implement ability to switch AppArmor profiles bwrap: Implement ability to switch AppArmor profiles May 24, 2021
@Maryse47
Copy link

Maryse47 commented May 25, 2021

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

Do you mean using bwrap within permissive apparmor profile is dangerous and with no profile at all it isn't?

especially considering that bwrap is typically installed setuid.

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.

@pks-t
Copy link
Author

pks-t commented Jun 1, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants