-
Notifications
You must be signed in to change notification settings - Fork 18.9k
apparmor: allow receiving of signals from 'docker kill' #36822
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
Conversation
7516095 to
7e98b44
Compare
Codecov Report
@@ Coverage Diff @@
## master #36822 +/- ##
=========================================
Coverage ? 35.22%
=========================================
Files ? 614
Lines ? 45638
Branches ? 0
=========================================
Hits ? 16076
Misses ? 27430
Partials ? 2132 |
|
@cyphar looks like this PR only contains the fix now, correct? If so, is this still WIP, or is the fix ready for review/merge? |
|
I'm going to test this one. |
|
@thaJeztah Yes, this just includes the fix. I haven't had time to work on the more general issue (mainly due to a variety of UX problems). I will change the title and remove the |
In newer kernels, AppArmor will reject attempts to send signals to a container because the signal originated from outside of that AppArmor profile. Correct this by allowing all unconfined signals to be received. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> Signed-off-by: Aleksa Sarai <asarai@suse.de>
7e98b44 to
05db2ee
Compare
|
Thanks, that's what I expected; I asked some people to test this so that we can move forward 👍 |
|
@cyphar I see the title of this PR is updated; can you please also update the description (as it still says about improving profile transparency and links to the relevant issue)? |
| capability, | ||
| file, | ||
| umount, | ||
| {{if ge .Version 208096}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like signal mediation functionality appears in apparmor 2.9.0,
as per https://gitlab.com/apparmor/apparmor/wikis/Release_Notes_2.9.0.
Oh, OK, I see that there was a bug in Ubuntu shipping 2.9.0 as 2.8.95. Maybe makes sense to add a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still unclear why you have it set to 2.8.96 @cyphar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because the Ubuntu AppArmor versions are slightly different -- from memory 2.8.95 does not have the change but anything after it does. I can double check this though (finding the versions requires you to trawl through the different branches on Launchpad -- the official release notes don't tell the full story for some reason).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I guess it's enough to share the above info in the patch commit message.
|
It looks like signals mediation appeared in the kernel 4.14 (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd1dbf76b23), still I can't reproduce the bug locally with (vanilla) kernel 4.14.34, going to try 4.16. |
|
Can't repro on 4.16 either...
@cyphar can you pinpoint to a specific kernel change? |
|
I have since discovered that the corresponding kernel patch that required this change was actually reverted in the upstream kernel. We still have it in the SUSE kernels, so this is still a problem for us, but we can drop this from being merged in upstream Docker (unless the kernel change gets re-merged upstream). However the reason it was reverted is because it broke userspace -- so in theory if it ever gets merged we still won't need to modify the default profile. |
|
Thanks for the update @cyphar! |
In newer kernels, AppArmor will reject attempts to send signals to a
container because the signal originated from outside of that AppArmor
profile. Correct this by allowing all unconfined signals to be received.
Fixes #36809
Signed-off-by: Aleksa Sarai asarai@suse.de