From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51160) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RILJS-0006b4-Kw for qemu-devel@nongnu.org; Mon, 24 Oct 2011 10:15:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RILJQ-0005ew-PF for qemu-devel@nongnu.org; Mon, 24 Oct 2011 10:15:38 -0400 Received: from e1.ny.us.ibm.com ([32.97.182.141]:55892) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RILJQ-0005ep-J1 for qemu-devel@nongnu.org; Mon, 24 Oct 2011 10:15:36 -0400 Received: from /spool/local by e1.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 24 Oct 2011 10:14:51 -0400 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9OEEFHI202352 for ; Mon, 24 Oct 2011 10:14:15 -0400 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9OEDqX2012222 for ; Mon, 24 Oct 2011 08:13:53 -0600 Message-ID: <4EA5729C.60509@linux.vnet.ibm.com> Date: Mon, 24 Oct 2011 10:13:48 -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> 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/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. >> + 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 >> >> >> > -- Regards, Corey