[libvirt] [PATCH 2/5] util: allow using virCommandAllowCap with setuid helpers

[Eric Blake]

On 03/25/2013 08:25 AM, Paolo Bonzini wrote:
> When running unprivileged, virSetUIDGIDWithCaps will fail because it
> tries to add the requested capabilities to the permitted and effective
> sets.
> 
> Detect this case, and invoke the child with cleared permitted and
> effective sets.  If it is a setuid program, it will get them.
> 
> Some care is needed also because you cannot drop capabilities from the
> bounding set without CAP_SETPCAP.  Because of that, ignore errors from
> setting the bounding set.

That seems like a kernel flaw - it makes sense that you can't _add_
capabilities without CAP_SETPCAP, but being unable to _drop_
capabilities without first acquiring a capability seems backwards.  I
wonder if lkml would accept a patch that makes CAP_SETPCAP unnecessary
for the restriction case, and only require it for the case of gaining
capabilities.  But regardless of that thought experiment, I agree that
the behavior is what it is in released kernels, so we must deal with it.

> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  src/util/virutil.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 36e6cee..8104e89 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -3052,9 +3052,21 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, unsigned long long capBits,
>  
>      /* Change to the temp capabilities */
>      if ((capng_ret = capng_apply(CAPNG_SELECT_CAPS)) < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("cannot apply process capabilities %d"), capng_ret);
> -        goto cleanup;
> +        /* Failed.  If we are running unprivileged, and the arguments make sense
> +         * for this scenario, assume we're starting some kind of setuid helper:
> +         * do not set any of capBits in the permitted or effective sets, and let
> +         * the program get them on its own.
> +         *
> +         * (Too bad we cannot restrict the bounding set to the capabilities we
> +         * would like the helper to have!).
> +         */
> +        if (getuid() > 0 && clearExistingCaps && !need_setuid && !need_setgid) {
> +            capng_clear(CAPNG_SELECT_CAPS);
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("cannot apply process capabilities %d"), capng_ret);
> +            goto cleanup;
> +        }

Hmm, this seems like we may want it for 1.0.4, but I still had questions
on 1/5 which needs to be applied first.

As written, the patch makes sense.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130327/7f9c4b9d/attachment-0001.sig>