Skip to content

Conversation

@cyphar
Copy link
Contributor

@cyphar cyphar commented Apr 10, 2018

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

@codecov
Copy link

codecov bot commented Apr 19, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@9c2c887). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #36822   +/-   ##
=========================================
  Coverage          ?   35.22%           
=========================================
  Files             ?      614           
  Lines             ?    45638           
  Branches          ?        0           
=========================================
  Hits              ?    16076           
  Misses            ?    27430           
  Partials          ?     2132

@thaJeztah
Copy link
Member

@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?

@kolyshkin
Copy link
Contributor

I'm going to test this one.

@cyphar
Copy link
Contributor Author

cyphar commented May 25, 2018

@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 [wip]. The more general issue will need to be worked on separately.

@cyphar cyphar changed the title [wip] apparmor: improve profile transparency apparmor: allow receiving of signals from 'docker kill' May 25, 2018
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>
@cyphar cyphar force-pushed the apparmor-external-templates branch from 7e98b44 to 05db2ee Compare May 25, 2018 04:33
@thaJeztah
Copy link
Member

Thanks, that's what I expected; I asked some people to test this so that we can move forward 👍

@kolyshkin
Copy link
Contributor

kolyshkin commented May 25, 2018

@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}}
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor Author

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).

Copy link
Contributor

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.

@kolyshkin
Copy link
Contributor

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.

@kolyshkin
Copy link
Contributor

Can't repro on 4.16 either...

It's caused by an kernel-side apparmor change

@cyphar can you pinpoint to a specific kernel change?

@cyphar
Copy link
Contributor Author

cyphar commented Jun 5, 2018

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.

@cyphar cyphar closed this Jun 5, 2018
@cyphar cyphar deleted the apparmor-external-templates branch June 5, 2018 09:43
@thaJeztah
Copy link
Member

Thanks for the update @cyphar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants