Move AppArmor policy to contrib & deb packaging #14609
Conversation
|
Re-opening of #13144 |
| @@ -6,7 +6,7 @@ Vcs-Git: git://github.com/docker/docker.git | |||
|
|
|||
| Package: docker-engine | |||
| Architecture: linux-any | |||
| Depends: iptables, ${misc:Depends}, ${perl:Depends}, ${shlibs:Depends} | |||
| Depends: iptables, debhelper, dh-apparmor, ${misc:Depends}, ${perl:Depends}, ${shlibs:Depends} | |||
There was a problem hiding this comment.
If you use dh-apparmor, you need to Depends the package. It's not a requirement for apparmor itself. A bug in trusty (and possible other distros) has the debhelper requirement missing in the Depends for dh-apparmor.
There was a problem hiding this comment.
er... I see what you mean. It should be build-depends.
There was a problem hiding this comment.
|
Other than the change to |
ewindisch
force-pushed
the
apparmor-policy
branch
from
a195eef
to
83c196f
Compare
|
Updated moving dh_apparmor to build-depends |
|
@tianon is this better? |
ewindisch
force-pushed
the
apparmor-policy
branch
from
83c196f
to
ad48480
Compare
|
Rebased... @tianon @crosbymichael @icecrime - libcontainer has been updated with the apparmor profiles now disabled. We need this patch to restore (out of box) AppArmor support. |
| @@ -7,6 +7,7 @@ Vcs-Git: git://github.com/docker/docker.git | |||
| Package: docker-engine | |||
| Architecture: linux-any | |||
| Depends: iptables, ${misc:Depends}, ${perl:Depends}, ${shlibs:Depends} | |||
| Build-Depends: debhelper, dh-apparmor | |||
There was a problem hiding this comment.
This line still isn't necessary -- #14609 (comment)
our build-depends lives in the Dockerfiles directly, not in the package metadata
There was a problem hiding this comment.
except if we distributed source debs, this would be necessary for someone to build from them, yes? (or rather, more convenient... they wouldn't need to track down the build depends)
There was a problem hiding this comment.
ewindisch
force-pushed
the
apparmor-policy
branch
from
ad48480
to
54f1dae
Compare
|
Removed the build-depends per @tianon's request |
|
Thanks ❤️
LGTM
|
| @@ -50,10 +50,6 @@ func NewDriver(root, initPath string, options []string) (*driver, error) { | |||
| if err := sysinfo.MkdirAll(root, 0700); err != nil { | |||
| return nil, err | |||
| } | |||
| // native driver root is at docker_root/execdriver/native. Put apparmor at docker_root | |||
There was a problem hiding this comment.
Would you mind to remove daemon/execdriver/native/apparmor.go then?
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. - Add deb package suggests for apparmor. - Ubuntu postinst use aa-status & fix policy path - Add the policies to the debian packages. - Add apparmor tests for writing proc files 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. - Remove /proc/k?mem from AA policy 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. - Load contrib/apparmor during integration tests Note that this is somewhat dirty because we cannot restore the host to its original configuration. However, it should be noted that prior to this patch series, the Docker daemon itself was loading apparmor policy from within the tests, so this is no dirtier or uglier than the status-quo. Signed-off-by: Eric Windisch <eric@windisch.us>
ewindisch
force-pushed
the
apparmor-policy
branch
from
54f1dae
to
80d9923
Compare
|
LGTM |
Move AppArmor policy to contrib & deb packaging
|
+impact/changelog |
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.
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.
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.
Note that this is somewhat dirty because we
cannot restore the host to its original configuration.
However, it should be noted that prior to this patch
series, the Docker daemon itself was loading apparmor
policy from within the tests, so this is no dirtier or
uglier than the status-quo.
Re-opening of #13144
Signed-off-by: Eric Windisch eric@windisch.us