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

New apparmor rules don't trigger a new profile unless upstream version is changed #2054

Open
panlinux opened this issue Jun 17, 2024 · 2 comments

Comments

@panlinux
Copy link

When the apparmor template is changed, like was done recently to fix issue #1898 via PR #2004, the upstream version was not chagned, which means the default apparmor profile name remained the same.

This means that even with a new build, users launching new containers will still be affected by the bug as long as the previous apparmor profile is still loaded. That's because when a new container is launched, and no apparmor profile specified, it will check if the default profile is already loaded into the kernel before loading it again. But in this case the profile content is different, as it has fixes, but the version didn't change, so it's not loaded. In other words, the name of the default profile should have been changed, or another logic needs to be in place to accommodate for cases when the embedded apparmor profile needs to be reloaded.

For downstream distributions, they could patch apparmor.go and include the full package version in there, so instead of being something like containers-default-0.57.4, it would become something like 0.57.4+ds1-2ubuntu1, which is the full package version. This would make the new, fixed, apparmor profile be loaded, instead of reusing the old one which does not have the fix.

This is not needed for every single version bump, but only when the profile has changed. So maybe another mechanism should be considered. Was it ever considered to always load the profile, and that had some negative impact, like performance? Or fear that it could negatively impact already existing containers?

@Luap99
Copy link
Member

Luap99 commented Jun 17, 2024

Keep in mind that there are no apparmor exports on the team at all.

Well note that logicically the profile will never change on a user system without us doing a new version so purely upstream speaking this bug does not effect us really. If downstream patches the profile they likely should just path the version as well.

My understanding is that loading the profile new every single time is slow so that is why it should be reused. In general I don't think this approach of hot loading the profile is sensible at all. The proper way would be to ship the profile as a file load it on boot/via packing scripts like most other profiles are. That would avoid this whole mess of embedding/patching these profiles in the podman binary and even rootless users could then use it, compare #958.

@gegarcia
Copy link

gegarcia commented Sep 3, 2024

Loading (or replacing) the profile is not slow if using AppArmor policy caches (loading the profile with apparmor_parser -W). The cached profile is stored in its binary form under /var/cache/apparmor/ (could be a different path, can be checked with apparmor_parser --print-cache-dir). The kernel also checks if the profile is unchanged, so there's minimal overhead there too.

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

No branches or pull requests

3 participants