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

2009-05-11 Thread Daniel Veillard
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

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-11 Thread Matthias Bolte
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 {

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-11 Thread 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


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

2009-05-11 Thread Daniel P. Berrange
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 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-11 Thread Matthias Bolte
2009/5/11 Daniel P. Berrange berra...@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.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-11 Thread Dave Allan

Matthias Bolte wrote:

2009/5/11 Daniel P. Berrange berra...@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

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-11 Thread Daniel P. Berrange
On Mon, May 11, 2009 at 12:37:25PM -0400, Dave Allan wrote:
 Matthias Bolte wrote:
 2009/5/11 Daniel P. Berrange berra...@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 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-11 Thread Daniel P. Berrange
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 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2009-05-08 Thread Daniel P. Berrange
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.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
 
 @@ -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 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list