Re: [libvirt] [RFC][PATCH] lxc: drop CAP_SYS_BOOT capability to preventrebooting from inside containers
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 wrote: > > Quoting Ryota Ozaki (ozaki.ry...@gmail.com): > >> Hi Serge, > >> > >> On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn wrote: > >> > Quoting Ryota Ozaki (ozaki.ry...@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 :| -- 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 preventrebooting from inside containers
Quoting Ryota Ozaki (ozaki.ry...@gmail.com): > Hi Serge, > > On Fri, May 8, 2009 at 11:04 AM, Serge E. Hallyn wrote: > > Quoting Ryota Ozaki (ozaki.ry...@gmail.com): > >> Hi Serge, > >> > >> On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn wrote: > >> > Quoting Ryota Ozaki (ozaki.ry...@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 -- 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 preventrebooting from inside containers
Hi Serge, On Fri, May 8, 2009 at 11:04 AM, Serge E. Hallyn wrote: > Quoting Ryota Ozaki (ozaki.ry...@gmail.com): >> Hi Serge, >> >> On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn wrote: >> > Quoting Ryota Ozaki (ozaki.ry...@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 > -- 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 preventrebooting from inside containers
Quoting Ryota Ozaki (ozaki.ry...@gmail.com): > Hi Serge, > > On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn wrote: > > Quoting Ryota Ozaki (ozaki.ry...@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 -- 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 preventrebooting from inside containers
Hi Serge, On Fri, May 8, 2009 at 9:12 AM, Serge E. Hallyn wrote: > Quoting Ryota Ozaki (ozaki.ry...@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 >> >> >From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 >> From: Ryota Ozaki >> 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 >> #include >> #include >> +#include >> +#include >> #include >> #include >> >> @@ -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 > -- 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 preventrebooting from inside containers
Quoting Ryota Ozaki (ozaki.ry...@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 > > >From 0e7a7622bc6411bbe76c05c63c6e6e61d379d97b Mon Sep 17 00:00:00 2001 > From: Ryota Ozaki > 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 > #include > #include > +#include > +#include > #include > #include > > @@ -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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list