[libvirt] [PATCH] lxc: drop sys_admin caps by default
To make sure the container user doesn't play with mounts, like changing them from ro to rw, drop the sys_admin capability by default. If user really needs to play with those, it can be enabled in the configuration. --- Note: it seems that patch 3/3 or my last series never reached the list. Here it is. src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index c5a70a1..d6d6fba 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2083,6 +2083,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes */ case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with auditing status */ case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM config */ + case VIR_DOMAIN_CAPS_FEATURE_SYS_ADMIN: /* No messing with mounts */ toDrop = (state != VIR_TRISTATE_SWITCH_ON); break; default: /* User specified capabilities to drop */ -- 2.1.4
On Wed, Nov 25, 2015 at 03:40:36PM +0100, Cédric Bosdonnat wrote:
To make sure the container user doesn't play with mounts, like changing them from ro to rw, drop the sys_admin capability by default. If user really needs to play with those, it can be enabled in the configuration. --- Note: it seems that patch 3/3 or my last series never reached the list. Here it is.
src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index c5a70a1..d6d6fba 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2083,6 +2083,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes */ case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with auditing status */ case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM config */ + case VIR_DOMAIN_CAPS_FEATURE_SYS_ADMIN: /* No messing with mounts */ toDrop = (state != VIR_TRISTATE_SWITCH_ON); break; default: /* User specified capabilities to drop */
I don't think we really need/want this. If usernamespace is enabled, it is perfectly safe to have CAP_SYS_ADMIN. If usernamespace is disabled, then whether or not you have CAP_SYS_ADMIN is not significant - you need to use SELinux/AppArmour to provide any kind of protection. For those existing feature flags we just disable them by default for historical reasons, and I don't think we should add more to them. If it weren't for historical practice, we'd just leave all capabilities enabled all the time. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, 2015-11-25 at 15:22 +0000, Daniel P. Berrange wrote:
On Wed, Nov 25, 2015 at 03:40:36PM +0100, Cédric Bosdonnat wrote:
To make sure the container user doesn't play with mounts, like changing them from ro to rw, drop the sys_admin capability by default. If user really needs to play with those, it can be enabled in the configuration. --- Note: it seems that patch 3/3 or my last series never reached the list. Here it is.
src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index c5a70a1..d6d6fba 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2083,6 +2083,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes */ case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with auditing status */ case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM config */ + case VIR_DOMAIN_CAPS_FEATURE_SYS_ADMIN: /* No messing with mounts */ toDrop = (state != VIR_TRISTATE_SWITCH_ON); break; default: /* User specified capabilities to drop */
I don't think we really need/want this.
If usernamespace is enabled, it is perfectly safe to have CAP_SYS_ADMIN.
If usernamespace is disabled, then whether or not you have CAP_SYS_ADMIN is not significant - you need to use SELinux/AppArmour to provide any kind of protection.
For those existing feature flags we just disable them by default for historical reasons, and I don't think we should add more to them. If it weren't for historical practice, we'd just leave all capabilities enabled all the time.
I really wasn't sure what to do about this patch... will drop it then. -- Cedric
participants (3)
-
Cedric Bosdonnat -
Cédric Bosdonnat
-
Daniel P. Berrange