[libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to prevent rebooting from inside containers
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.ryota@gmail.com>
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota@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> @@ -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); + return -1; + } + } + + return 0; +} + + /** * lxcChild: * @argv: Pointer to container arguments @@ -705,6 +731,10 @@ static int lxcContainerChild( void *data ) if (lxcContainerEnableInterfaces(argv->nveths, argv->veths) < 0) return -1; + /* drop a set of root capabilities */ + if (lxcContainerDropCapabilities(vmDef) < 0) + return -1; + /* this function will only return if an error occured */ return lxcContainerExecInit(vmDef); } -- 1.6.0.6
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
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.ryota@gmail.com>
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota@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>
@@ -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); + return -1;
Ideally you should also drop it from pI.
+ } + } + + return 0; +} + + /** * lxcChild: * @argv: Pointer to container arguments @@ -705,6 +731,10 @@ static int lxcContainerChild( void *data ) if (lxcContainerEnableInterfaces(argv->nveths, argv->veths) < 0) return -1;
+ /* drop a set of root capabilities */ + if (lxcContainerDropCapabilities(vmDef) < 0) + return -1; + /* this function will only return if an error occured */ return lxcContainerExecInit(vmDef); } -- 1.6.0.6
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi Serge, On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
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.ryota@gmail.com>
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota@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>
@@ -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); + return -1;
Ideally you should also drop it from pI.
If not drop it, a user in a container could set CAP_SYS_BOOT fI bit of /bin/reboot on and then the user could gain CAP_SYS_BOOT back through the fI. Is this understanding right? Thanks, ozaki-r
+ } + } + + return 0; +} + + /** * lxcChild: * @argv: Pointer to container arguments @@ -705,6 +731,10 @@ static int lxcContainerChild( void *data ) if (lxcContainerEnableInterfaces(argv->nveths, argv->veths) < 0) return -1;
+ /* drop a set of root capabilities */ + if (lxcContainerDropCapabilities(vmDef) < 0) + return -1; + /* this function will only return if an error occured */ return lxcContainerExecInit(vmDef); } -- 1.6.0.6
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi Serge,
On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi,
...
+ 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); + return -1;
Ideally you should also drop it from pI.
If not drop it, a user in a container could set CAP_SYS_BOOT fI bit of /bin/reboot on and then the user could gain CAP_SYS_BOOT back through the fI. Is this understanding right?
Yup. Of course most tasks run with pI empty, so it seems unlikely that it would be a problem, but unless the libcap dependecy becomes a problem, it seems worth making sure that doesn't happen. thanks, -serge
Hi Serge, On Fri, May 8, 2009 at 11:04 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi Serge,
On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi,
...
+ 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); + return -1;
Ideally you should also drop it from pI.
If not drop it, a user in a container could set CAP_SYS_BOOT fI bit of /bin/reboot on and then the user could gain CAP_SYS_BOOT back through the fI. Is this understanding right?
Yup.
Of course most tasks run with pI empty, so it seems unlikely that it would be a problem, but unless the libcap dependecy becomes a problem, it seems worth making sure that doesn't happen.
Oh, I slightly misread your suggestions, sorry. You are suggesting making sure requires dropping a capability in both bounding set AND pI of a process and to do so we need an additional package (libcap2 or somewhat) because prctl(2) doesn't have the function to drop pI, aren't you? um, I hope my patch is sufficient as a first step, but ok, I'll try to implement the function to drop pI as well and confirm whether it is feasible for libvirt. Thanks, ozaki-r
thanks, -serge
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi Serge,
On Fri, May 8, 2009 at 11:04 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi Serge,
On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi,
...
+ 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); + return -1;
Ideally you should also drop it from pI.
If not drop it, a user in a container could set CAP_SYS_BOOT fI bit of /bin/reboot on and then the user could gain CAP_SYS_BOOT back through the fI. Is this understanding right?
Yup.
Of course most tasks run with pI empty, so it seems unlikely that it would be a problem, but unless the libcap dependecy becomes a problem, it seems worth making sure that doesn't happen.
Oh, I slightly misread your suggestions, sorry. You are suggesting making sure requires dropping a capability in both bounding set AND pI of a process and to do so we need an additional package (libcap2 or somewhat) because prctl(2) doesn't have the function to drop pI, aren't you?
Yes.
um, I hope my patch is sufficient as a first step, but ok, I'll try to implement the function to drop pI as well and confirm whether it is feasible for libvirt.
Yes, there is nothing wrong with applying your patch as is for now. thanks, -serge
On Fri, May 08, 2009 at 12:43:19PM +0900, Ryota Ozaki wrote:
Hi Serge,
On Fri, May 8, 2009 at 11:04 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi Serge,
On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
Quoting Ryota Ozaki (ozaki.ryota@gmail.com):
Hi,
...
+ 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); + return -1;
Ideally you should also drop it from pI.
If not drop it, a user in a container could set CAP_SYS_BOOT fI bit of /bin/reboot on and then the user could gain CAP_SYS_BOOT back through the fI. Is this understanding right?
Yup.
Of course most tasks run with pI empty, so it seems unlikely that it would be a problem, but unless the libcap dependecy becomes a problem, it seems worth making sure that doesn't happen.
Oh, I slightly misread your suggestions, sorry. You are suggesting making sure requires dropping a capability in both bounding set AND pI of a process and to do so we need an additional package (libcap2 or somewhat) because prctl(2) doesn't have the function to drop pI, aren't you?
um, I hope my patch is sufficient as a first step, but ok, I'll try to implement the function to drop pI as well and confirm whether it is feasible for libvirt.
The patch I have just posted should take care of this issue with pI http://www.redhat.com/archives/libvir-list/2009-June/msg00413.html Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
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...)
Great, the dropping of capabilities has been one of our major todo items for LXC. ACK to this patch Daniel
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota@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>
@@ -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); + return -1; + } + } + + return 0; +} + + /** * lxcChild: * @argv: Pointer to container arguments @@ -705,6 +731,10 @@ static int lxcContainerChild( void *data ) if (lxcContainerEnableInterfaces(argv->nveths, argv->veths) < 0) return -1;
+ /* drop a set of root capabilities */ + if (lxcContainerDropCapabilities(vmDef) < 0) + return -1; + /* this function will only return if an error occured */ return lxcContainerExecInit(vmDef); } -- 1.6.0.6
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
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.ryota@gmail.com>
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota@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 -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi, I needed to apply the following two small changes to get it compile. On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. The second change makes the code compile with -Werror, because vmDef is not used in the lxcContainerDropCapabilities function. diff --git a/src/lxc_container.c b/src/lxc_container.c index 3687750..a2b3051 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -42,7 +42,7 @@ #include <linux/fs.h> #include <sys/prctl.h> -#include <sys/capability.h> +#include <linux/capability.h> #include "virterror_internal.h" #include "logging.h" @@ -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 {
On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote:
Hi,
I needed to apply the following two small changes to get it compile.
On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package.
That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed. $ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386
diff --git a/src/lxc_container.c b/src/lxc_container.c index 3687750..a2b3051 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -42,7 +42,7 @@ #include <linux/fs.h>
#include <sys/prctl.h> -#include <sys/capability.h> +#include <linux/capability.h>
#include "virterror_internal.h" #include "logging.h"
NACK to this change.
@@ -642,7 +642,7 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef, return lxcContainerSetupExtraMounts(vmDef); }
-static int lxcContainerDropCapabilities( virDomainDefPtr vmDef ) +static int lxcContainerDropCapabilities( virDomainDefPtr vmDef ATTRIBUTE_UNUSED )
I committed this fix a little while ago... Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2009/5/11 Daniel P. Berrange <berrange@redhat.com>:
On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote:
Hi,
I needed to apply the following two small changes to get it compile.
On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package.
That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed.
$ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386
You guess was correct. With libcap-dev installed it compiles without problems.
Matthias Bolte wrote:
2009/5/11 Daniel P. Berrange <berrange@redhat.com>:
On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote:
Hi,
I needed to apply the following two small changes to get it compile.
On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed.
$ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386
You guess was correct. With libcap-dev installed it compiles without problems.
We should check for the presence of libcap-dev in the configure script. Dave
On Mon, May 11, 2009 at 12:37:25PM -0400, Dave Allan wrote:
Matthias Bolte wrote:
2009/5/11 Daniel P. Berrange <berrange@redhat.com>:
On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote:
Hi,
I needed to apply the following two small changes to get it compile.
On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package. That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed.
$ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386
You guess was correct. With libcap-dev installed it compiles without problems.
We should check for the presence of libcap-dev in the configure script.
And also add a BuildRequires to the RPM specfile Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, May 11, 2009 at 05:22:15PM +0100, Daniel P. Berrange wrote:
On Mon, May 11, 2009 at 05:59:45PM +0200, Matthias Bolte wrote:
Hi,
I needed to apply the following two small changes to get it compile.
On my system (Ubuntu 9.04) I don't have a sys/capability.h header, but a linux/capability.h header as part of the linux-libc-dev package.
That is because sys/capability.h is provided by libcap, not libc. I guess you don't have libcap-dev installed.
$ rpm -qf /usr/include/sys/capability.h libcap-devel-2.06-4.fc9.i386
diff --git a/src/lxc_container.c b/src/lxc_container.c index 3687750..a2b3051 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -42,7 +42,7 @@ #include <linux/fs.h>
#include <sys/prctl.h> -#include <sys/capability.h> +#include <linux/capability.h>
#include "virterror_internal.h" #include "logging.h"
NACK to this change.
Actually I take that back. We don't need anything in sys/capability.h, so its pointless adding a dep on libcap. Lets just use the linux/capability.h header to get the prctl() definition & constants. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
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.ryota@gmail.com>
From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota@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 {
participants (6)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
Matthias Bolte -
Ryota Ozaki -
Serge E. Hallyn