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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list