On 02/08/2013 02:01 PM, Eric Blake wrote:
> 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?
Well, the kernel guys actually have a patch I'm about to try out that
will hopefully eliminate the need for CAP_COMPROMISE_KERNEL, so we won't
need it for that. And the demand to have generic SCSI command
passthrough is apparently so small that nobody has even tried it (or
possibly they just all gave up and set qemu user to root). But at any
rate, for now I think we can just leave qemu as it is and let people set
the filecap themselves if they need this (unusual) capability; if there
is enough whining, then maybe we can pass it along to qemu.
>
>>
>> +#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?
Yeah, I've fixed all of those.
>
>> + 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.
Oops. That's due to pasting the source of capng_change_id(). I'll fix
all of those.
>
>
>> +}
>> +
>> +#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.
Right.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list