Skip to content

daemon: switch to 'ensure' workflow for AppArmor profiles#29130

Merged
vieux merged 2 commits intomoby:masterfrom
cyphar:29097-dynamically-reload-apparmor
Dec 12, 2016
Merged

daemon: switch to 'ensure' workflow for AppArmor profiles#29130
vieux merged 2 commits intomoby:masterfrom
cyphar:29097-dynamically-reload-apparmor

Conversation

@cyphar
Copy link
Contributor

@cyphar cyphar commented Dec 5, 2016

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.

cute bunneh

Fixes: #29097
Fixes: 2f7596a ("apparmor: do not save profile to /etc/apparmor.d")
Signed-off-by: Aleksa Sarai [email protected]

@cyphar
Copy link
Contributor Author

cyphar commented Dec 5, 2016

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐮

@cyphar
Copy link
Contributor Author

cyphar commented Dec 5, 2016

Note that my test case for this was:

% docker run --rm -it alpine echo "hello"
hello
% apparmor_parser -R /etc/apparmor.d/docker
Warning failed to create cache: (null)
% docker run --rm -it alpine echo "hello"
hello

Which I think is sufficient to check that the reloading works as expected.

Copy link
Member

@tonistiigi tonistiigi Dec 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do this later with if appArmorProfile == defaultApparmorProfile condition to load it also if c.AppArmorProfile equals docker-default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks.

@cyphar cyphar force-pushed the 29097-dynamically-reload-apparmor branch from 6eaf934 to 9f22d1d Compare December 6, 2016 21:45
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: 2f7596a ("apparmor: do not save profile to /etc/apparmor.d")
Signed-off-by: Aleksa Sarai <[email protected]>
@cyphar cyphar force-pushed the 29097-dynamically-reload-apparmor branch from 9f22d1d to 567ef8e Compare December 6, 2016 21:47
@tonistiigi
Copy link
Member

LGTM

cc @justincormack

@cyphar
Copy link
Contributor Author

cyphar commented Dec 8, 2016

/ping @justincormack

@justincormack
Copy link
Contributor

I guess there is a very short race still, but that seems to be the fault of other code removing our profile, and this is much better.

@vieux vieux merged commit 96a84ed into moby:master Dec 12, 2016
@cyphar cyphar deleted the 29097-dynamically-reload-apparmor branch December 12, 2016 21:32
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.

6 participants