[libvirt] [PATCH 13/15] util: virSetUIDGIDWithCaps - change uid while keeping caps

[Eric Blake]

On 02/07/2013 02:37 PM, Laine Stump wrote:
> Normally when a process' uid is changed to non-0, all the capabilities
> bits are cleared, even those explicitly set with calls to
> capng_update()/capng_apply() made immediately before setuid. And
> *after* the process' uid has been changed, it no longer has the
> necessary privileges to add capabilities back to the process.
> 
> In order to set a non-0 uid while still maintaining any capabilities
> bits, it is necessary to either call capng_change_id() (which
> unfortunately doesn't not currently call initgroups to setup auxiliary

s/doesn't not/doesn't/

> group membership), or to perform the small amount of calisthenics
> contained in the new utility function virSetUIDGIDWithCaps().
> 

> 
> Note that, due to the way that effective capabilities are computed (at
> time of execve) for a process that has uid != 0, the *file*
> capabilities of the binary being executed must also have the desired
> capabilities bit(s) set (see "man 7 capabilities"). This can be done
> with the "filecap" command
> (e.g. "filecap /usr/bin/qemu-kvm sys_rawio").

Sheesh - does that mean that we have to fix up qemu packaging for Fedora
to include that capability by default?

>  
> +#if WITH_CAPNG
> +/* Set the real and effective uid and gid to the given values, while
> + * maintaining the capabilities indicated by bits in @capBits. return
> + * 0 on success, -1 on failure (the original system error remains in
> + * errno).
> + */
> +int
> +virSetUIDGIDWithCaps(uid_t uid, gid_t gid, unsigned long long capBits)
> +{
> +    int ii, capng_ret, ret = -1;
> +    bool need_setgid = false, need_setuid = false;
> +    bool need_prctl = false, need_setpcap = false;
> +
> +    /* First drop all caps except those in capBits + the extra ones we
> +     * need to change uid/gid and change the capabilities bounding
> +     * set.
> +     */
> +
> +    capng_clear(CAPNG_SELECT_BOTH);
> +
> +    for (ii = 0; ii <= CAP_LAST_CAP; ii++) {
> +        if (capBits & (1ULL << ii)) {
> +            capng_update(CAPNG_ADD,
> +                         CAPNG_EFFECTIVE|CAPNG_INHERITABLE|
> +                         CAPNG_PERMITTED|CAPNG_BOUNDING_SET,
> +                         ii);
> +        }
> +    }
> +
> +    if (gid != -1 && gid > 0 && !capng_have_capability(CAPNG_EFFECTIVE, CAP_SETGID)) {

Another site that needs casting (gid != (gid_t) -1); and do we really
want to short-circuit the gid>0 case?

> +        need_setgid = true;
> +        capng_update(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETGID);
> +    }
> +    if (uid != -1 && uid > 0 && !capng_have_capability(CAPNG_EFFECTIVE, CAP_SETUID)) {
> +        need_setuid = true;
> +        capng_update(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETUID);
> +    }
> +# ifdef PR_CAPBSET_DROP
> +    // If newer kernel, we need also need setpcap to change the bounding set

Prefer /**/ comment style, please.

> +    if ((capBits || need_setgid || need_setuid) &&
> +        !capng_have_capability(CAPNG_EFFECTIVE, CAP_SETPCAP)) {
> +        need_setpcap = true;
> +    }
> +    if (need_setpcap)
> +        capng_update(CAPNG_ADD,    CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETPCAP);

Odd spacing.

> +# endif
> +
> +    need_prctl = capBits || need_setgid || need_setuid || need_setpcap;
> +
> +    // Tell system we want to keep caps across uid change

More comment style.

> +    if (need_prctl && prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0)) {
> +        virReportSystemError(errno, "%s",
> +                             _("prctl failed to set KEEPCAPS"));
> +        goto cleanup;
> +    }
> +
> +    // Change to the temp capabilities

And again.

> +    if ((capng_ret = capng_apply(CAPNG_SELECT_BOTH)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("cannot apply process capabilities %d"), capng_ret);
> +        goto cleanup;
> +    }
> +
> +    /* Set UID/GID */
> +    if (virSetUIDGID(uid, gid) < 0)
> +        goto cleanup;
> +
> +    // Tell it we are done keeping capabilities

and again

> +}
> +
> +#else
> +/*
> + * On platforms without libcapng, the capabilities setting is treated
> + * as a NOP.
> + */
> +
> +int
> +virSetUIDGIDWithCaps(uid_t uid, gid_t gid, unsigned long long capBits)

Need ATTRIBUTE_UNUSED on capBits.

> +{
> +    return virSetUIDGID(uid, gid);
> +}
> +#endif
> +
>  
>  #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
>  /* search /proc/mounts for mount point of *type; return pointer to
> diff --git a/src/util/virutil.h b/src/util/virutil.h
> index 4201aa1..2dc3403 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -54,6 +54,7 @@ int virPipeReadUntilEOF(int outfd, int errfd,
>                          char **outbuf, char **errbuf);
>  
>  int virSetUIDGID(uid_t uid, gid_t gid);
> +int virSetUIDGIDWithCaps(uid_t uid, gid_t gid, unsigned long long capBits);
>  
>  int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
>  
> 

-- 
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/20130208/93693d6a/attachment-0001.sig>