Add AppArmor policy for the engine#13156
Conversation
hack/make/ubuntu
Outdated
There was a problem hiding this comment.
so I think the easiest way is to also add these lines in hack/make/build-deb or hack/make/.build-deb/{files}
There was a problem hiding this comment.
@jfrazelle http://manpages.debian.org/dh_apparmor would be the proper way to accomplish this for non-FPM
There was a problem hiding this comment.
In this package, this should really only be a Suggests relation (not quite as strong as Recommends).
https://www.debian.org/doc/debian-policy/ch-relationships.html#s-binarydeps
Recommends
This declares a strong, but not absolute, dependency.The Recommends field should list packages that would be found together with this one in all but unusual installations.
Suggests
This is used to declare that one package may be more useful with one or more others. Using this field tells the packaging system and the user that the listed packages are related to this one and can perhaps enhance its usefulness, but that installing this one without them is perfectly reasonable.
There was a problem hiding this comment.
This is based on the change from #13144, but there I argue we should keep this as Recommends because this is already what we set for the Debian packages and because I strongly believe we should make this a Recommend rather than the (weaker) Suggests. Users should not be using Docker without an LSM.
There was a problem hiding this comment.
There was a problem hiding this comment.
See https://github.com/docker/docker/blob/ac7a33b45ce5061079226a254e1fe6173d3fbd81/hack/make/.build-deb/rules#L7 where this happens in the new packages. It's only Recommends for Ubuntu, where Docker literally won't work without the AppArmor package since AppArmor is installed and configured by default. Adding this as Recommends in this hacky FPM package has way too much potential to break people's existing setups by partially or even fully enabling AppArmor for them.
fb51a17 to
dae18ee
Compare
|
This PR has quite a bit of very confusing overlap with #13144. |
|
(I'm not going crazy -- we really are having the same conversation on two separate PRs.) |
The automatic installation of AppArmor policies prevents the management of custom, site-specific apparmor policies for the default container profile. Furthermore, this change will allow a future policy for the engine itself to be written without demanding the engine be able to arbitrarily create and manage AppArmor policies. Signed-off-by: Eric Windisch <eric@windisch.us>
If a container cannot mount, it cannot umount. We should also restrict writing paths libcontainer masks as of 1.6.1: * /proc/fs * /proc/timer_stats * /proc/latency_stats * /proc/kcore Signed-off-by: Eric Windisch <eric@windisch.us>
Install AppArmor policy during post-inst. Add package recommendation for apparmor. Signed-off-by: Eric Windisch <eric@windisch.us>
Signed-off-by: Eric Windisch <eric@windisch.us>
Signed-off-by: Eric Windisch <eric@windisch.us>
Adds the policies to the debian packages. Signed-off-by: Eric Windisch <eric@windisch.us>
Additional restrictions against modifying files in proc are enforced by AppArmor. Ensure that AppArmor is preventing access to these files, not simply Docker's configuration of proc. Signed-off-by: Eric Windisch <eric@windisch.us>
The tests against reading timer_stats and latency_stats are intended to ensure these files cannot be successfully read. For that reason, it is okay if reading these files outright fails and results in a non-zero exit code. Currently, when confined by AppArmor, these tests fail as AppArmor ensures that these files cannot be opened for read. Signed-off-by: Eric Windisch <eric@windisch.us>
The path to mem and kmem are in /dev, not /proc and cannot be restricted successfully through AppArmor. The device cgroup will need to be sufficient here. Signed-off-by: Eric Windisch <eric@windisch.us>
dae18ee to
698f6cc
Compare
Wraps the engine itself with an AppArmor policy. This restricts what may be done via a reexec, or through applications we call out to, such as 'xz'. Significantly, this policy restricts the policies to which a container may be spawned into. By default, users will be able to transition to an unconfined policy or any policy prefaced with 'docker-'. Local operators may add new local policies prefaced with 'docker-' without needing to modify this policy. Operators choosing to disable privileged containers will need to modify this policy to remove access to change_policy to unconfined. Builds on top of PR moby#13144. Signed-off-by: Eric Windisch <eric@windisch.us>
|
Collective review with @crosbymichael @jfrazelle @calavera We're gonna do it all in one PR, so it seems we can close that one. |
|
@icecrime this is a separate issue from the container policies. This is policy for the engine. We shouldn't combine these PRs (this one just happens to be dependent on the other landing) |
Wraps the engine itself with an AppArmor policy.
This restricts what may be done via a reexec,
or through applications we call out to, such as 'xz'.
Significantly, this policy restricts the policies
to which a container may be spawned into. By default,
users will be able to transition to an unconfined
policy or any policy prefaced with 'docker-'.
Local operators may add new local policies prefaced
with 'docker-' without needing to modify this policy.
Operators choosing to disable privileged containers
will need to modify this policy to remove access
to change_policy to unconfined.
Builds on top of PR #13144.