On Mon, Oct 24, 2011 at 18:38, Corey Bryant <cor...@linux.vnet.ibm.com> wrote:
>
>
> On 10/24/2011 01:10 PM, Blue Swirl wrote:
>>
>> On Mon, Oct 24, 2011 at 14:13, Corey Bryant<cor...@linux.vnet.ibm.com>
>> wrote:
>>>
>>>
>>> On 10/23/2011 09:22 AM, Blue Swirl wrote:
>>>>
>>>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<cor...@linux.vnet.ibm.com>
>>>> 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.
>>>>>
>>>>> Signed-off-by: Anthony Liguori<aligu...@us.ibm.com>
>>>>> Signed-off-by: Richa Marwaha<rmar...@linux.vnet.ibm.com>
>>>>> Signed-off-by: Corey Bryant<cor...@linux.vnet.ibm.com>
>>>>> ---
>>>>> configure | 34 ++++++++++++++++++++++++++++++++++
>>>>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index 6c8b659..fed66b0 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -128,6 +128,7 @@ vnc_thread="no"
>>>>> xen=""
>>>>> xen_ctrl_version=""
>>>>> linux_aio=""
>>>>> +cap=""
>>>>> attr=""
>>>>> xfs=""
>>>>>
>>>>> @@ -653,6 +654,10 @@ for opt do
>>>>> ;;
>>>>> --enable-kvm) kvm="yes"
>>>>> ;;
>>>>> + --disable-cap) cap="no"
>>>>> + ;;
>>>>> + --enable-cap) cap="yes"
>>>>> + ;;
>>>>> --disable-spice) spice="no"
>>>>> ;;
>>>>> --enable-spice) spice="yes"
>>>>> @@ -1032,6 +1037,8 @@ echo " --disable-vde disable support
>>>>> for vde network"
>>>>> echo " --enable-vde enable support for vde network"
>>>>> echo " --disable-linux-aio disable Linux AIO support"
>>>>> echo " --enable-linux-aio enable Linux AIO support"
>>>>> +echo " --disable-cap disable libcap-ng support"
>>>>> +echo " --enable-cap enable libcap-ng support"
>>>>> echo " --disable-attr disables attr and xattr support"
>>>>> echo " --enable-attr enable attr and xattr support"
>>>>> echo " --disable-blobs disable installing provided firmware
>>>>> blobs"
>>>>> @@ -1638,6 +1645,29 @@ EOF
>>>>> fi
>>>>>
>>>>> ##########################################
>>>>> +# libcap-ng library probe
>>>>> +if test "$cap" != "no" ; then
>>>>> + cap_libs="-lcap-ng"
>>>>> + cat> $TMPC<< EOF
>>>>> +#include<cap-ng.h>
>>>>> +int main(void)
>>>>> +{
>>>>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>>>>> + return 0;
>>>>> +}
>>>>> +EOF
>>>>> + if compile_prog "" "$cap_libs" ; then
>>>>> + cap=yes
>>>>> + libs_tools="$cap_libs $libs_tools"
>>>>> + else
>>>>> + if test "$cap" = "yes" ; then
>>>>> + feature_not_found "cap"
>>>>> + fi
>>>>> + cap=no
>>>>> + fi
>>>>> +fi
>>>>> +
>>>>> +##########################################
>>>>> # Sound support libraries probe
>>>>>
>>>>> audio_drv_probe()
>>>>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>>>>> echo "madvise $madvise"
>>>>> echo "posix_madvise $posix_madvise"
>>>>> echo "uuid support $uuid"
>>>>> +echo "libcap-ng support $cap"
>>>>> echo "vhost-net support $vhost_net"
>>>>> echo "Trace backend $trace_backend"
>>>>> echo "Trace output file $trace_file-<pid>"
>>>>> @@ -2846,6 +2877,9 @@ fi
>>>>> if test "$vde" = "yes" ; then
>>>>> echo "CONFIG_VDE=y">> $config_host_mak
>>>>> fi
>>>>> +if test "$cap" = "yes" ; then
>>>>> + echo "CONFIG_LIBCAP=y">> $config_host_mak
>>>>> +fi
>>>>> for card in $audio_card_list; do
>>>>> def=CONFIG_`echo $card | tr '[:lower:]' '[:upper:]'`
>>>>> echo "$def=y">> $config_host_mak
>>>>> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
>>>>> index db257d5..b1562eb 100644
>>>>> --- a/qemu-bridge-helper.c
>>>>> +++ b/qemu-bridge-helper.c
>>>>> @@ -33,6 +33,10 @@
>>>>>
>>>>> #include "net/tap-linux.h"
>>>>>
>>>>> +#ifdef CONFIG_LIBCAP
>>>>> +#include<cap-ng.h>
>>>>> +#endif
>>>>> +
>>>>> #define MAX_ACLS (128)
>>>>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>>>>
>>>>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>>>>> return sendmsg(c,&msg, 0);
>>>>> }
>>>>>
>>>>> +#ifdef CONFIG_LIBCAP
>>>>> +static int drop_privileges(void)
>>>>> +{
>>>>> + /* clear all capabilities */
>>>>> + capng_clear(CAPNG_SELECT_BOTH);
>>>>> +
>>>>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>>>>> + CAP_NET_ADMIN)< 0) {
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + /* change to calling user's real uid and gid, retaining
>>>>> supplemental
>>>>> + * groups and CAP_NET_ADMIN */
>>>>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> int main(int argc, char **argv)
>>>>> {
>>>>> struct ifreq ifr;
>>>>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>>>>> int acl_count = 0;
>>>>> int i, access_allowed, access_denied;
>>>>>
>>>>> + /* if we're run from an suid binary, immediately drop privileges
>>>>> preserving
>>>>> + * cap_net_admin -- exit immediately if libcap not configured */
>>>>> + if (geteuid() == 0&& getuid() != geteuid()) {
>>>>> +#ifdef CONFIG_LIBCAP
>>>>> + if (drop_privileges() == -1) {
>>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>>> + return 1;
>>>>> + }
>>>>> +#else
>>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>>
>>>> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
>>>> possible to use setfsuid() instead for Linux?
>>>>
>>>> Some fork+setuid helper could be used for other Unix and for the lame
>>>> OSes without any file system DAC capabilities, a different syntax that
>>>> does not rely on underlying FS may need to be introduced. Again, I
>>>> don't know if the tool is even interesting for non-Linux.
>>>>
>>>
>>> I just want to make sure that there is no chance that the helper is run
>>> as
>>> root beyond this point. Are you saying to seteuid(getuid) and
>>> setfsuid(root)? I'm not sure that would drop the privileges enough.
>>
>> Without capabilities, we can't drop root privileges because bridge
>> setup would fail otherwise, but we could use setfsuid(getuid()) and
>> setfsgid(getgid()) during file access so permission checks work.
>> Perhaps non-Linux could use seteuid() etc. instead.
>>
>
> This would reduce file system access from effective UID/GID (root/root) to
> real UID/GID (non-root/non-root). Other than file system access, the helper
> would still run under root/root, right? I don't think we want that from a
> security aspect.
Right, it's not desirable, but isn't that the best we can do without
libcap or FS capabilities?
> --
> Regards,
> Corey
>
>>>>> + return 1;
>>>>> +#endif
>>>>> + }
>>>>> +
>>>>> /* parse arguments */
>>>>> if (argc< 3 || argc> 4) {
>>>>> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
>>>>> --
>>>>> 1.7.3.4
>>>>>
>>>>>
>>>>>
>>>>
>
>
