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

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.

> +} > + > +#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.