From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38601) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIPz3-0007Er-AQ for qemu-devel@nongnu.org; Mon, 24 Oct 2011 15:14:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RIPz1-0005kh-IV for qemu-devel@nongnu.org; Mon, 24 Oct 2011 15:14:53 -0400 Received: from e3.ny.us.ibm.com ([32.97.182.143]:48378) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIPz1-0005k5-CL for qemu-devel@nongnu.org; Mon, 24 Oct 2011 15:14:51 -0400 Received: from /spool/local by e3.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 24 Oct 2011 15:14:46 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9OJDklJ281716 for ; Mon, 24 Oct 2011 15:13:46 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9OJDiDf030600 for ; Mon, 24 Oct 2011 17:13:46 -0200 Message-ID: <4EA5B8E5.6040306@linux.vnet.ibm.com> Date: Mon, 24 Oct 2011 15:13:41 -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> <4EA5B0BC.10203@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 02:58 PM, Blue Swirl wrote: > On Mon, Oct 24, 2011 at 18:38, Corey Bryant wrote: >> >> >> 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. > > Right, it's not desirable, but isn't that the best we can do without > libcap or FS capabilities? > I think the best we can do is not let it run in those cases. :) I'd like see if others in the community have an opinion on this though. -- Regards, Corey >> -- >> 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 >>>>>> >>>>>> >>>>>> >>>>> >> >> >