Re: [libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers

[Dave Allan]

Daniel Veillard wrote:
On Fri, May 08, 2009 at 09:04:35AM +0900, Ryota Ozaki wrote:
Hi,
Current lxc driver unexpectedly allows users inside containers to reboot
host physical machine. This patch prevents this by dropping CAP_SYS_BOOT
capability in the bounding set of the init processes in every containers.

Note that the patch intends to make it easy to add further capabilities
to drop if needed, although I'm not sure which capabilities should be
dropped. (We might need to drop CAP_SETFCAP as well to be strict...)

Thanks,
  ozaki-r

Signed-off-by: Ryota Ozaki <ozaki.ry...@gmail.com>

>From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001
From: Ryota Ozaki <ozaki.ry...@gmail.com>
Date: Fri, 8 May 2009 04:29:24 +0900
Subject: [PATCH] lxc: drop CAP_SYS_BOOT capability to prevent
rebooting from inside containers

Current lxc driver unexpectedly allows users inside containers to reboot
host physical machine. This patch prevents this by dropping CAP_SYS_BOOT
capability in the bounding set of the init processes in every containers.
---
 src/lxc_container.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/src/lxc_container.c b/src/lxc_container.c
index 3946b84..37ab216 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -32,6 +32,8 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include <sys/wait.h>
+#include <sys/prctl.h>
+#include <sys/capability.h>
 #include <unistd.h>
 #include <mntent.h>

  I had to move those 2 includes after #include <linux/fs.h>
otherwise MS_MOVE which is defined in the later would not be found
anymore. Weird but true !

@@ -639,6 +641,30 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
         return lxcContainerSetupExtraMounts(vmDef);
 }

+
+static int lxcContainerDropCapabilities( virDomainDefPtr vmDef )
+{
+    int i;
+    const struct {
+        int id;
+        const char *name;
+    } caps[] = {
+#define ID_STRING(name) name, #name
+        { ID_STRING(CAP_SYS_BOOT) },
+    };
+
+    for (i = 0 ; i < ARRAY_CARDINALITY(caps) ; i++) {
+        if (prctl(PR_CAPBSET_DROP, caps[i].id, 0, 0, 0)) {
+            lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                     "%s", _("failed to drop %s"), caps[i].name);

   Here the compiler complained about the args it really should be 

               lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                        _("failed to drop %s"), caps[i].name);

+            return -1;
+        }
+    }
+
+    return 0;
+}
+

  That said with the two fixes this looks like a good patch,
so applied and commited, thanks !

Daniel


I had a build failure today because of an unused parameter to
lxcContainerDropCapabilities.  The attached oneliner fixes it.  I don't 
know the code, though, so sanity check it.

Dave
diff --git a/src/lxc_container.c b/src/lxc_container.c
index 3687750..314f293 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -642,7 +642,7 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
         return lxcContainerSetupExtraMounts(vmDef);
 }
 
-static int lxcContainerDropCapabilities( virDomainDefPtr vmDef )
+static int lxcContainerDropCapabilities( virDomainDefPtr vmDef 
ATTRIBUTE_UNUSED )
 {
     int i;
     const struct {
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list