From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55056) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIPQV-0005gC-Ot for qemu-devel@nongnu.org; Mon, 24 Oct 2011 14:39:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RIPQU-0005vH-Ey for qemu-devel@nongnu.org; Mon, 24 Oct 2011 14:39:11 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:40973) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIPQU-0005u0-6z for qemu-devel@nongnu.org; Mon, 24 Oct 2011 14:39:10 -0400 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 24 Oct 2011 12:39:04 -0600 Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9OIcvFZ184616 for ; Mon, 24 Oct 2011 12:38:59 -0600 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9OIctme004053 for ; Mon, 24 Oct 2011 12:38:57 -0600 Message-ID: <4EA5B0BC.10203@linux.vnet.ibm.com> Date: Mon, 24 Oct 2011 14:38:52 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1319209643-3866-1-git-send-email-coreyb@linux.vnet.ibm.com> <1319209643-3866-4-git-send-email-coreyb@linux.vnet.ibm.com> <4EA5729C.60509@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: rmarwah@linux.vnet.ibm.com, aliguori@us.ibm.com, qemu-devel@nongnu.org On 10/24/2011 01:10 PM, Blue Swirl wrote: > On Mon, Oct 24, 2011 at 14:13, Corey Bryant wrote: >> >> >> On 10/23/2011 09:22 AM, Blue Swirl wrote: >>> >>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant >>> 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 >>>> Signed-off-by: Richa Marwaha >>>> Signed-off-by: Corey Bryant >>>> --- >>>> 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 >>>> +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-" >>>> @@ -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 >>>> +#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. -- 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 >>>> >>>> >>>> >>>