On Fri, May 29, 2009 at 9:20 PM, Daniel Veillard <veillard(a)redhat.com&gt; wrote: >  The lxcContainerDropCapabilities() function requires PR_CAPBSET_DROP > to be defined in order to compile, but it may not be defined in older > kernels. So I made the compilation of the core of the function > conditional, raise an error but still return 0 to not make the > container initialization fail. But I'm unsure, should we just fail and > return -1 if we can't drop capabilities instead ? I think it depends on applications. AFAIK, libvirt intends to support two types of applications; application workload isolation and virtual private servers. In the latter case, we MUST drop the capability and if it fails we have to fail booting a container as well. OTOH, in the former case, we might not need to fail booting. Nonetheless, I agree with the patch because old kernels that don't support PR_CAPBSET_DROP (they would be 2.6.24 or earlier) don't have enough facilities to support VPSs (e.g., they lacks sysfs, devpts, etc.). Therefore, with the old kernels we don't need to care much about the dropping-failed-but-booting-success case.

On Fri, May 29, 2009 at 04:42:54PM -0500, Serge E. Hallyn wrote: > Quoting Ryota Ozaki (ozaki.ryota(a)gmail.com): > > On Fri, May 29, 2009 at 9:20 PM, Daniel Veillard <veillard(a)redhat.com&gt; wrote: > > >  The lxcContainerDropCapabilities() function requires PR_CAPBSET_DROP > > > to be defined in order to compile, but it may not be defined in older > > > kernels. So I made the compilation of the core of the function > > > conditional, raise an error but still return 0 to not make the > > > container initialization fail. But I'm unsure, should we just fail and > > > return -1 if we can't drop capabilities instead ? > > > > I think it depends on applications. AFAIK, libvirt intends to support > > two types of applications; application workload isolation and > > virtual private servers. In the latter case, we MUST drop the capability > > and if it fails we have to fail booting a container as well. OTOH, in > > the former case, we might not need to fail booting. > > > > Nonetheless, I agree with the patch because old kernels that don't > > support PR_CAPBSET_DROP (they would be 2.6.24 or earlier) don't > > have enough facilities to support VPSs (e.g., they lacks sysfs, devpts, etc.). > > Therefore, with the old kernels we don't need to care much about the > > dropping-failed-but-booting-success case. > > Hmm, yeah but note that often userspace is out of date with respect to > "recent" new kernel-related defines. I do a lot of testing on a rhel > 5.3 partition with spanking-new kernels, so rare is the time that I > don't have to do > > #ifndef PR_CAPBSET_DROP > #define PR_CAPBSET_DROP 24 > #endif > > and same for clone flags (CLONE_NEWIPC), securebits, capabilities, > etc. > > So if the prctl(PR_CAPBSET_DROP) returns -ENOSYS then absolutely I > agree, but the patch just does > > #ifdef PR_CAPBSET_DROP > > which seems wrong. Well, if you have a better patch to suggest, fire :-)

On Fri, May 29, 2009 at 04:42:54PM -0500, Serge E. Hallyn wrote: > Quoting Ryota Ozaki (ozaki.ryota(a)gmail.com): > > On Fri, May 29, 2009 at 9:20 PM, Daniel Veillard <veillard(a)redhat.com&gt; wrote: > > Hmm, yeah but note that often userspace is out of date with respect to > "recent" new kernel-related defines.  I do a lot of testing on a rhel > 5.3 partition with spanking-new kernels, so rare is the time that I > don't have to do > > #ifndef PR_CAPBSET_DROP > #define PR_CAPBSET_DROP 24 > #endif > > and same for clone flags (CLONE_NEWIPC), securebits, capabilities, > etc. > > So if the prctl(PR_CAPBSET_DROP) returns -ENOSYS then absolutely I > agree, This makes sense as a way to deal with this problem, since it matches what we already do with the various CLONE_XXX & MOUNT_XXX flags too.

NB, in the not too distant future I'm going to submit code for making the libvirtd daemon drop alot of its capabilities, including clearing the bounding set to prevent inheritance by any child processes except in required circumstances. For that I'll likely use libcap-ng so we will be able to stop callin prctl() directly in the LXC driver.

Regards, Daniel [1] libcap-ng isn't technically yet announced to the world, but it'll    appear real soon... -- |: 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 Tue, Jun 02, 2009 at 11:15:58AM +0900, Ryota Ozaki wrote: > On Mon, Jun 1, 2009 at 6:24 PM, Daniel P. Berrange <berrange(a)redhat.com&gt; wrote: > > On Fri, May 29, 2009 at 04:42:54PM -0500, Serge E. Hallyn wrote: > >> Quoting Ryota Ozaki (ozaki.ryota(a)gmail.com): > >> > On Fri, May 29, 2009 at 9:20 PM, Daniel Veillard <veillard(a)redhat.com&gt; wrote: > >> > >> Hmm, yeah but note that often userspace is out of date with respect to > >> "recent" new kernel-related defines.  I do a lot of testing on a rhel > >> 5.3 partition with spanking-new kernels, so rare is the time that I > >> don't have to do > >> > >> #ifndef PR_CAPBSET_DROP > >> #define PR_CAPBSET_DROP 24 > >> #endif > >> > >> and same for clone flags (CLONE_NEWIPC), securebits, capabilities, > >> etc. > >> > >> So if the prctl(PR_CAPBSET_DROP) returns -ENOSYS then absolutely I > >> agree, > > > > This makes sense as a way to deal with this problem, since it matches > > what we already do with the various CLONE_XXX & MOUNT_XXX flags too. > > That convinces me too. > > > NB, in the not too distant future I'm going to submit code for making > > the libvirtd daemon drop alot of its capabilities, including clearing > > the bounding set to prevent inheritance by any child processes except > > in required circumstances. For that I'll likely use libcap-ng so we > > will be able to stop callin prctl() directly in the LXC driver. > > Oh, good. Will the new facility allow us to specify which capabilities > to be dropped in a XML file or somewhere? Or the set of caps will be > hard-coded? That's TBD. My currently proof of concept code is to restrict the set of capabilities to just those required by the libvirtd daemon itself. LXC is an interesting problem, because inside the container, you could argue that it should just get all capabilities. This would assume the container prevented use of the caapbilities impacting things outside the container.