From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36185) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIQ4A-0008Jw-3P for qemu-devel@nongnu.org; Mon, 24 Oct 2011 15:20:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RIQ48-0006jZ-JP for qemu-devel@nongnu.org; Mon, 24 Oct 2011 15:20:10 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:53661) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIQ48-0006hc-7k for qemu-devel@nongnu.org; Mon, 24 Oct 2011 15:20:08 -0400 Received: from /spool/local by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 24 Oct 2011 13:20:05 -0600 Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p9OJJVLx178092 for ; Mon, 24 Oct 2011 13:19:34 -0600 Received: from d03av05.boulder.ibm.com (loopback [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p9OJJURI008070 for ; Mon, 24 Oct 2011 13:19:31 -0600 Message-ID: <4EA5BA40.1020409@us.ibm.com> Date: Mon, 24 Oct 2011 14:19:28 -0500 From: Anthony Liguori 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: <4EA5729C.60509@linux.vnet.ibm.com> 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: Corey Bryant Cc: Blue Swirl , rmarwah@linux.vnet.ibm.com, qemu-devel@nongnu.org On 10/24/2011 09:13 AM, 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. But the whole pointer of the helper is to run as root. It's a small trusted piece of code. Obviously, it's better to drop unneeded privileges when that's possible but in the event that is isn't, we shouldn't bail out completely. Regards, Anthony Liguori