On Thu, Oct 06, 2011 at 11:38:27AM -0400, Richa Marwaha wrote:
> The ideal way to use qemu-bridge-helper is to give it an fscap of using:
>
> setcap cap_net_admin=ep qemu-bridge-helper
>
> Unfortunately, most distros still do not have a mechanism to package files
> with fscaps applied. This means they'll have to SUID the qemu-bridge-helper
> binary.
>
> To improve security, use libcap to reduce our capability set to just
> cap_net_admin, then reduce privileges down to the calling user. This is
> hopefully close to equivalent to fscap support from a security perspective.
> +#ifdef CONFIG_LIBCAP
> +static int drop_privileges(void)
> +{
> + cap_t cap;
> + cap_value_t new_caps[] = {CAP_NET_ADMIN};
> +
> + cap = cap_init();
Check for NULL ?
> +
> + /* set capabilities to be permitted and inheritable. we don't need the
> + * caps to be effective right now as they'll get reset when we seteuid
> + * anyway */
> + cap_set_flag(cap, CAP_PERMITTED, 1, new_caps, CAP_SET);
> + cap_set_flag(cap, CAP_INHERITABLE, 1, new_caps, CAP_SET);
Check for failure ?
> +
> + if (cap_set_proc(cap) == -1) {
> + return -1;
> + }
> +
> + cap_free(cap);
Check for failure ?
> +
> + /* reduce our privileges to a normal user */
> + setegid(getgid());
> + seteuid(getuid());
Check for failure ?
> + cap = cap_init();
Check for NULL ?
> +
> + /* enable the our capabilities. we marked them as inheritable earlier
> + * which is what allows this to work. */
> + cap_set_flag(cap, CAP_EFFECTIVE, 1, new_caps, CAP_SET);
> + cap_set_flag(cap, CAP_PERMITTED, 1, new_caps, CAP_SET);
Check for failure ?
> +
> + if (cap_set_proc(cap) == -1) {
> + return -1;
> + }
> +
> + cap_free(cap);
Check for failure ?
> +
> + return 0;
> +}
> +#endif
It may seem like checking for failure on cap_free/cap_set_flag is
not required because they can only return EINVAL for invalid
args, but since this is missing the check for NULL on cap_init
you can actually see errors from those latter functions in an
OOM cenario.
I think I'd suggest not using libcap, instead try libcap-ng [1] whose
APIs are designed with safety in mind & result in much simpler and
clearer code:
eg, that entire function above can be expressed using capng with
something approximating:
capng_clear(CAPNG_SELECT_BOTH);
if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_PERMITTED,
CAP_NET_ADMIN) < 0)
error(...);
if (capng_change_id(getuid(), getgid(), CAPNG_DROP_SUPP_GRP |
CAPNG_CLEAR_BOUNDING))
error(...);
Regards,
Daniel
[1] http://people.redhat.com/sgrubb/libcap-ng/
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
