utils: fix apparmor profile not applied in user namespaces#2065
utils: fix apparmor profile not applied in user namespaces#2065giuseppe merged 1 commit intocontainers:mainfrom
Conversation
|
@jnovy PTAL |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors libcrun_initialize_apparmor to improve AppArmor detection within user namespaces by directly checking /sys/module/apparmor/parameters/enabled instead of relying on securityfs. It also adds a regression test, test_process_apparmor_profile_userns, to verify profile application when UID/GID mappings are present. Feedback was provided to remove an unnecessary --raw flag from the apparmor_parser command used for test cleanup to ensure correct behavior across different versions of the tool.
tests/test_linux_features.py
Outdated
| subprocess.run(["apparmor_parser", "--remove", "--raw"], | ||
| input=profile_text.encode(), capture_output=True) |
There was a problem hiding this comment.
The --raw flag in apparmor_parser is used to dump the binary representation of a profile to stdout. It is not intended for use with the --remove (or -R) action. While it might be ignored by some versions of the parser, it's better to remove it to avoid confusion and ensure the command behaves as expected.
| subprocess.run(["apparmor_parser", "--remove", "--raw"], | |
| input=profile_text.encode(), capture_output=True) | |
| subprocess.run(["apparmor_parser", "--remove"], | |
| input=profile_text.encode(), capture_output=True) |
The refactoring in commit 0a3e929 ("utils: fix memory leak and missing cache in libcrun_initialize_apparmor()") introduced a regression where AppArmor profiles were silently not applied when user namespaces with UID/GID mappings were used. The issue was in libcrun_initialize_apparmor(): when crun_dir_p_at() fails to stat /sys/kernel/security/apparmor (which is expected inside a user namespace where securityfs may not be accessible), the refactored code immediately concluded AppArmor was disabled and returned 0. Fix it by dropping the /sys/kernel/security/apparmor directory check and only using /sys/module/apparmor/parameters/enabled. Only ENOENT on the open indicates AppArmor is not available; any other error is reported to the caller. Add a test that verifies AppArmor profiles are correctly applied when running with a user namespace and UID/GID mappings. Closes: containers#2063 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe
force-pushed
the
fix-apparmour-with-userns
branch
from
abf105b to
94d7f64
Compare
|
Thanks, LGTM |
2 participants
The refactoring in commit 0a3e929 ("utils: fix memory leak and missing cache in libcrun_initialize_apparmor()") introduced a regression where AppArmor profiles were silently not applied when user namespaces with UID/GID mappings were used.
The issue was in libcrun_initialize_apparmor(): when crun_dir_p_at() fails to stat /sys/kernel/security/apparmor (which is expected inside a user namespace where securityfs may not be accessible), the refactored code immediately concluded AppArmor was disabled and returned 0.
Fix it by dropping the /sys/kernel/security/apparmor directory check and only using /sys/module/apparmor/parameters/enabled. Only ENOENT on the open indicates AppArmor is not available; any other error is reported to the caller.
Add a test that verifies AppArmor profiles are correctly applied when running with a user namespace and UID/GID mappings.
Closes: #2063