From e0976913a0de3f39c3617d972bebb0a581fb7686 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 6 Dec 2016 00:10:08 +1100 Subject: [PATCH 1/2] apparmor: switch IsLoaded to return bool Signed-off-by: Aleksa Sarai Upstream-commit: e440a57a793feb15c0f06d27178ee8241a2a9081 Component: engine --- components/engine/daemon/apparmor_default.go | 2 +- components/engine/profiles/apparmor/apparmor.go | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/components/engine/daemon/apparmor_default.go b/components/engine/daemon/apparmor_default.go index e4065b4ad9..790e14b656 100644 --- a/components/engine/daemon/apparmor_default.go +++ b/components/engine/daemon/apparmor_default.go @@ -21,7 +21,7 @@ func installDefaultAppArmorProfile() { // Allow daemon to run if loading failed, but are active // (possibly through another run, manually, or via system startup) for _, policy := range apparmorProfiles { - if err := aaprofile.IsLoaded(policy); err != nil { + if loaded, err := aaprofile.IsLoaded(policy); err != nil || !loaded { logrus.Errorf("AppArmor enabled on system but the %s profile could not be loaded.", policy) } } diff --git a/components/engine/profiles/apparmor/apparmor.go b/components/engine/profiles/apparmor/apparmor.go index 36eb10cdf9..449a8f159b 100644 --- a/components/engine/profiles/apparmor/apparmor.go +++ b/components/engine/profiles/apparmor/apparmor.go @@ -94,22 +94,28 @@ func InstallDefault(name string) error { return nil } -// IsLoaded checks if a passed profile has been loaded into the kernel. -func IsLoaded(name string) error { +// IsLoaded checks if a profile with the given name has been loaded into the +// kernel. +func IsLoaded(name string) (bool, error) { file, err := os.Open("/sys/kernel/security/apparmor/profiles") if err != nil { - return err + return false, err } defer file.Close() r := bufio.NewReader(file) for { p, err := r.ReadString('\n') + if err == io.EOF { + break + } if err != nil { - return err + return false, err } if strings.HasPrefix(p, name+" ") { - return nil + return true, nil } } + + return false, nil } From 2613b94bd41b34dc7159db62d8804ab65b78129d Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 6 Dec 2016 00:12:17 +1100 Subject: [PATCH 2/2] daemon: switch to 'ensure' workflow for AppArmor profiles In certain cases (unattended upgrades), system services can disable loaded AppArmor profiles. However, since /etc being read-only is a supported setup we cannot just write a copy of the profile to /etc/apparmor.d. Instead, dynamically load the docker-default AppArmor profile if a container is started with that profile set. This code will short-cut if the profile is already loaded. Fixes: 2f7596aaef3a ("apparmor: do not save profile to /etc/apparmor.d") Signed-off-by: Aleksa Sarai Upstream-commit: 567ef8e7858ca4f282f598ba1f5a951cbad39e83 Component: engine --- components/engine/daemon/apparmor_default.go | 28 +++++++++++-------- .../daemon/apparmor_default_unsupported.go | 3 +- components/engine/daemon/daemon.go | 5 +++- components/engine/daemon/oci_linux.go | 19 +++++++++++-- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/components/engine/daemon/apparmor_default.go b/components/engine/daemon/apparmor_default.go index 790e14b656..09dd0541b8 100644 --- a/components/engine/daemon/apparmor_default.go +++ b/components/engine/daemon/apparmor_default.go @@ -3,7 +3,8 @@ package daemon import ( - "github.com/Sirupsen/logrus" + "fmt" + aaprofile "github.com/docker/docker/profiles/apparmor" "github.com/opencontainers/runc/libcontainer/apparmor" ) @@ -13,18 +14,23 @@ const ( defaultApparmorProfile = "docker-default" ) -func installDefaultAppArmorProfile() { +func ensureDefaultAppArmorProfile() error { if apparmor.IsEnabled() { - if err := aaprofile.InstallDefault(defaultApparmorProfile); err != nil { - apparmorProfiles := []string{defaultApparmorProfile} + loaded, err := aaprofile.IsLoaded(defaultApparmorProfile) + if err != nil { + return fmt.Errorf("Could not check if %s AppArmor profile was loaded: %s", defaultApparmorProfile, err) + } - // Allow daemon to run if loading failed, but are active - // (possibly through another run, manually, or via system startup) - for _, policy := range apparmorProfiles { - if loaded, err := aaprofile.IsLoaded(policy); err != nil || !loaded { - logrus.Errorf("AppArmor enabled on system but the %s profile could not be loaded.", policy) - } - } + // Nothing to do. + if loaded { + return nil + } + + // Load the profile. + if err := aaprofile.InstallDefault(defaultApparmorProfile); err != nil { + return fmt.Errorf("AppArmor enabled on system but the %s profile could not be loaded.", defaultApparmorProfile) } } + + return nil } diff --git a/components/engine/daemon/apparmor_default_unsupported.go b/components/engine/daemon/apparmor_default_unsupported.go index f186a68af9..cd2dd9702e 100644 --- a/components/engine/daemon/apparmor_default_unsupported.go +++ b/components/engine/daemon/apparmor_default_unsupported.go @@ -2,5 +2,6 @@ package daemon -func installDefaultAppArmorProfile() { +func ensureDefaultAppArmorProfile() error { + return nil } diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index 7c2ce50559..6a339a2870 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/daemon/daemon.go @@ -524,7 +524,10 @@ func NewDaemon(config *Config, registryService registry.Service, containerdRemot logrus.Warnf("Failed to configure golang's threads limit: %v", err) } - installDefaultAppArmorProfile() + if err := ensureDefaultAppArmorProfile(); err != nil { + logrus.Errorf(err.Error()) + } + daemonRepo := filepath.Join(config.Root, "containers") if err := idtools.MkdirAllAs(daemonRepo, 0700, rootUID, rootGID); err != nil && !os.IsExist(err) { return nil, err diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index bf21df86c4..1daefc587b 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -733,12 +733,27 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { } if apparmor.IsEnabled() { - appArmorProfile := "docker-default" - if len(c.AppArmorProfile) > 0 { + var appArmorProfile string + if c.AppArmorProfile != "" { appArmorProfile = c.AppArmorProfile } else if c.HostConfig.Privileged { appArmorProfile = "unconfined" + } else { + appArmorProfile = "docker-default" } + + if appArmorProfile == "docker-default" { + // Unattended upgrades and other fun services can unload AppArmor + // profiles inadvertently. Since we cannot store our profile in + // /etc/apparmor.d, nor can we practically add other ways of + // telling the system to keep our profile loaded, in order to make + // sure that we keep the default profile enabled we dynamically + // reload it if necessary. + if err := ensureDefaultAppArmorProfile(); err != nil { + return nil, err + } + } + s.Process.ApparmorProfile = appArmorProfile } s.Process.SelinuxLabel = c.GetProcessLabel()