Option to disable PR_SET_NO_NEW_PRIVS to be able to apply new AppArmor LSM profile/restrictions

[mk-fg]

Hello,

Similar issues were raised earlier, in #378, #679 and #681, but for a different use-cases, where I think common suggestion was "use something else, your use-case isn't a good fit for bwrap".
As this is a different use-case where I bumped into same limitation, wanted to open a separate issue to suggest adding option for it instead.

In my case, what happens is, in my understanding:

• Chromium-based browser runs bwrap to run glycin image-parser in it.
• bwrap sets up unprivileged container with no-new-privs, and then exec's e.g. glycin-svg image parser binary.
• glycin-svg does pretty much nothing but parsing an image from file descriptor or shm and returning some kind of bitmap.

For many good reasons, it's nice to run app such as browser with as much sandboxing as possible, for example with AppArmor LSM profile.

In which case, AppArmor assigns profile to a chromium binary, with many rules for it.
Then when it runs bwrap ... /usr/lib/glycin-loaders/2+/glycin-svg, bwrap has its own distinct profile, transitioned to via e.g. /usr/bin/bwrap px -> chromium-wrapper//chromium//bwrap rule. That profile has a ton of complicated permissive rules allowing bwrap to setup its sandbox.
And then when bwrap runs glycin-svg image parser (still quite a vulnerable thing - see long history of SVG parsing bugs), it needs a similar transition rule, with its own profile, which is almost empty - all image-parsing is done in userspace code, no extra access is needed or should be allowed there.

Problem is, as far as I understand it, bwrap uses prctl (PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), which among other things prevents any AppArmor profile transitions from then on, as those potentially can be made less restrictive, and kernel has no good way to compare profiles for "this is a subset of rules" or layering those at the moment.

That unfortunate restriction logs auditing error like this (long line split for readability):

audit: type=1400 audit(1764715027.910:552): apparmor="DENIED" operation="exec" 
  class="file" info="no new privs" error=-1 profile="chromium-wrapper//chromium//bwrap" 
  name="/usr/lib/glycin-loaders/2+/glycin-svg" pid=60431 comm="bwrap" 
  requested_mask="x" denied_mask="x" fsuid=1000 ouid=0 target="chromium-wrapper//chromium//glycin"

And prevents enforcing extra LSM restrictions on that binary, leaving it running with same DAC permissions as bwrap, only limited by various namespaces and mechanisms that bwrap setup, as well as bwrap's relatively lax LSM profile that has to allow things like netlink access, capabilities, userns, mount, pivot_root, etc etc - instead of pretty much literally nothing ("you parse image you're given without any fs access or fancy syscalls").

It would be nice if separate AppArmor profile could be applied to that final glycin-svg image parser, on top of any bwrap sandboxing/restrictions, which would require bwrap to loose that PR_SET_NO_NEW_PRIVS bit.

I think it's a legitimate use-case where trade-off of loosing that prctl bit is allowing to apply more restrictions overall via LSM profile, not less, and perfectly fine and appropriate usage of bwrap in addition to an LSM profile.
General approach here is "defence in depth" where layering more security mechanisms has afaik proven to be pretty much only good way to mitigate "attacker has to find ANY bug" scaling problem, hence it'd be nice to have "bwrap AND LSM", instead of "bwrap OR LSM", as it currently is due to PR_SET_NO_NEW_PRIVS.

Sorry if a bit long, thought to make a full case for why option might be useful, in light of previous rejections, to maybe give it more chance to not be (understandably) dismissed as "duplicate".
Hopefully it might make this limitation more easy to find/understand too, if nothing else.

Thanks for your consideration.

P.S. If added as e.g. --allow-new-privs option, patching that argument into a browser is probably not an issue for my use-case, as it's a custom chromium build anyway.

P.P.S. Example of an AppArmor profile limited by this issue is "chrome" in mk-fg/apparmor-profiles github repo.

[mk-fg]

Also thinking about SELinux-specific --exec-label, maybe another and somewhat better fix for this might be to add similar options(s) like --apparmor-profile (or also --smack-label and such if needed for other LSMs).
Though I think I'd prefer to have transition configured/managed in the same profile file, not bwrap command-line for this specific use-case.

[smcv]

I would not be comfortable with not setting PR_SET_NO_NEW_PRIVS in a tool that is sometimes setuid root. Other maintainers are welcome to overrule me on this, but only if they are willing to take responsibility for handling any CVEs that result from it.

bwrap uses prctl (PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), which among other things prevents any AppArmor profile transitions from then on, as those potentially can be made less restrictive, and kernel has no good way to compare profiles for "this is a subset of rules" or layering those at the moment

This suggests a potential solution: teaching AppArmor to have some sort of stacking/layering that can only apply new restrictions, and cannot relax existing restrictions. (In fact I thought this already existed, used by lxd/Incus when running an AppArmor-aware OS inside a container on an AppArmor-aware host?)

[smcv]

maybe another and somewhat better fix for this might be to add similar options(s) like --apparmor-profile (or also --smack-label and such if needed for other LSMs)

I'd consider merge requests, but the amount of time I have available for reviewing is very limited at the moment.

I am unlikely to have time to implement this feature any time within the next few years.

[mk-fg]

In fact I thought this already existed, used by lxd/Incus when running an AppArmor-aware OS inside a container on an AppArmor-aware host?

Yeah, I think you can load and manage namespace profiles fully within that namespace, iirc there're sysctl knobs to disable it.
And there're rules to say "run this in profile from that namespace" (haven't used those myself), but don't think any of this is nested, so still affected by prctl flag.

maybe another and somewhat better fix for this might be to add similar options(s) like --apparmor-profile

I'd consider merge requests, but the amount of time I have available for reviewing is very limited at the moment.

On reflection, I think this is indeed a much better approach, as it seems like a bad idea to choose between these security mechanisms when you can have both, even if those have to be configured in separate places (which would be the case for adding a flag to disable prctl restriction anyway).

Understood with regards to PR, will put it on my todo-list as well. Thanks.

[smcv]

Some bubblewrap features also hard-require PR_SET_NO_NEW_PRIVS: I believe seccomp already does, and if I'm reading documentation correctly, #704 would too.