From patchwork Sun Jun 17 00:56:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keno Fischer X-Patchwork-Id: 930437 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=juliacomputing.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=juliacomputing-com.20150623.gappssmtp.com header.i=@juliacomputing-com.20150623.gappssmtp.com header.b="RqWqlObr"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 417bgh3txmz9s19 for ; Sun, 17 Jun 2018 11:08:00 +1000 (AEST) Received: from localhost ([::1]:53405 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fUMAg-0004uT-5z for incoming@patchwork.ozlabs.org; Sat, 16 Jun 2018 21:07:58 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56899) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fUM0b-0005nl-RX for qemu-devel@nongnu.org; Sat, 16 Jun 2018 20:57:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fUM0Y-0003I4-2y for qemu-devel@nongnu.org; Sat, 16 Jun 2018 20:57:33 -0400 Received: from mail-qk0-x243.google.com ([2607:f8b0:400d:c09::243]:34526) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fUM0X-0003Hv-TJ for qemu-devel@nongnu.org; Sat, 16 Jun 2018 20:57:30 -0400 Received: by mail-qk0-x243.google.com with SMTP id q70-v6so7676318qke.1 for ; Sat, 16 Jun 2018 17:57:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=juliacomputing-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=m1kYcz4sX7cJoOtZ1tb9MxDZscufeKa7l2chgfbmT8E=; b=RqWqlObrWM3GfaQcGIlSU/TtYy3+zLAwjID0KcAF5yjsH3DudPFcKvsmoodYHvWklw BSRf0cDqKDF9hasBgFr3sg9M85MJgnrfbM0rywRRkZ4GQNNRHzBnxvZ+USuFB0JTAXwQ /rfe4zNp6dyK0/C+jdpqxSlfnuSJpHOHx+aIOrnV+ngOVlTlIJMfRa8dKS7WdP93sv4g 8afeoX7Cll38DddUcIf78NX4F8h3wMVoGyo41um2WphFHyuKYR4365WvB7Q+IDqw4ByO H8i+51aXE3cepzpNXLrA5jD7sd2qBTfZrogHlsLKGBWULGfUUpTdBXaLuuw2uXA3VtMy lOKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=m1kYcz4sX7cJoOtZ1tb9MxDZscufeKa7l2chgfbmT8E=; b=HlNZ49ll1hDo7bmbwD7HSNCOZg2xr611xLDXi8rKR54hm776jRfXzjH1ppC7KN1++Q tDFJphuayfJieeqTXmnEM2gQb8DvQZubunVK0Zgsoh+mTVEBQEjSkBD0ZjvOeIUauHo1 zhPkGaVqWuXgexzEFnWftQ1NuFmrYrYDWkVeVpOSK81cYPqMQU3Ey7BoCQ5lBeCoToFb 3aIt85ihGnWa81ASW6CMSDWUZOSnkLMVxzIUe6CaOD+NhTR4nd1fUAMK8Th6S/5OJqo8 LjIzJXQAcK5TXoYpQ+ShnWrgDTkEOMP4Y6TCk1m3nllc58y5eaG3a1vyWfkAGmLHQYwb dLMw== X-Gm-Message-State: APt69E0L+0cqRA7pvIgg3kPVu1Wovu1092QHwyjBBy30Brdp/EdrFsRO krpQ2uzkuLl3eL8Z8QbddB9pllFOPrg= X-Google-Smtp-Source: ADUXVKKFdUN22ySwY+HqWvPWCWk+V4GmRjmSGLvpPOjvZIRfef2hNbwzdZ9G3RmFB5kQjv8MTlyryg== X-Received: by 2002:a37:204c:: with SMTP id g73-v6mr6152195qkg.164.1529197048876; Sat, 16 Jun 2018 17:57:28 -0700 (PDT) Received: from localhost.localdomain (96-86-104-61-static.hfc.comcastbusiness.net. [96.86.104.61]) by smtp.gmail.com with ESMTPSA id x21-v6sm7302186qto.2.2018.06.16.17.57.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 16 Jun 2018 17:57:28 -0700 (PDT) From: Keno Fischer To: qemu-devel@nongnu.org Date: Sat, 16 Jun 2018 20:56:56 -0400 Message-Id: X-Mailer: git-send-email 2.8.1 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400d:c09::243 Subject: [Qemu-devel] [PATCH v3 12/13] 9p: darwin: virtfs-proxy: Implement setuid code for darwin X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Keno Fischer , groug@kaod.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Darwin does not have linux capabilities, so make that code linux-only. Darwin also does not have setresuid/gid. The correct way to temporarily drop capabilities is to call seteuid/gid. Also factor out the code that acquires acquire_dac_override into a separate function in the linux implementation. I had originally done this when I thought it made sense to have only one `setugid` function, but I retained this because it seems clearer this way. Signed-off-by: Keno Fischer --- fsdev/virtfs-proxy-helper.c | 200 +++++++++++++++++++++++++++----------------- 1 file changed, 125 insertions(+), 75 deletions(-) diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index d8dd3f5..6baf2a6 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -82,6 +82,7 @@ static void do_perror(const char *string) } } +#ifdef CONFIG_LINUX static int do_cap_set(cap_value_t *cap_value, int size, int reset) { cap_t caps; @@ -121,6 +122,85 @@ error: return -1; } +static int acquire_dac_override(void) +{ + cap_value_t cap_list[] = { + CAP_DAC_OVERRIDE, + }; + return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0); +} + +/* + * from man 7 capabilities, section + * Effect of User ID Changes on Capabilities: + * If the effective user ID is changed from nonzero to 0, then the permitted + * set is copied to the effective set. If the effective user ID is changed + * from 0 to nonzero, then all capabilities are are cleared from the effective + * set. + * + * The setfsuid/setfsgid man pages warn that changing the effective user ID may + * expose the program to unwanted signals, but this is not true anymore: for an + * unprivileged (without CAP_KILL) program to send a signal, the real or + * effective user ID of the sending process must equal the real or saved user + * ID of the target process. Even when dropping privileges, it is enough to + * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't + * be exposed to signals. So just use setresuid/setresgid. + */ +static int setugid(int uid, int gid, int *suid, int *sgid) +{ + int retval; + + *suid = geteuid(); + *sgid = getegid(); + + if (setresgid(-1, gid, *sgid) == -1) { + retval = -errno; + goto err_out; + } + + if (setresuid(-1, uid, *suid) == -1) { + retval = -errno; + goto err_sgid; + } + + if (uid != 0 || gid != 0) { + /* + * We still need DAC_OVERRIDE because we don't change + * supplementary group ids, and hence may be subjected DAC rules + */ + if (acquire_dac_override() < 0) { + retval = -errno; + goto err_suid; + } + } + return 0; + +err_suid: + if (setresuid(-1, *suid, *suid) == -1) { + abort(); + } +err_sgid: + if (setresgid(-1, *sgid, *sgid) == -1) { + abort(); + } +err_out: + return retval; +} + +/* + * This is used to reset the ugid back with the saved values + * There is nothing much we can do checking error values here. + */ +static void resetugid(int suid, int sgid) +{ + if (setresgid(-1, sgid, sgid) == -1) { + abort(); + } + if (setresuid(-1, suid, suid) == -1) { + abort(); + } +} + static int init_capabilities(void) { /* helper needs following capabilities only */ @@ -135,6 +215,51 @@ static int init_capabilities(void) }; return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 1); } +#else +static int setugid(int uid, int gid, int *suid, int *sgid) +{ + int retval; + + *suid = geteuid(); + *sgid = getegid(); + + if (setegid(gid) == -1) { + retval = -errno; + goto err_out; + } + + if (seteuid(uid) == -1) { + retval = -errno; + goto err_sgid; + } + +err_sgid: + if (setgid(*sgid) == -1) { + abort(); + } +err_out: + return retval; +} + +/* + * This is used to reset the ugid back with the saved values + * There is nothing much we can do checking error values here. + */ +static void resetugid(int suid, int sgid) +{ + if (setegid(sgid) == -1) { + abort(); + } + if (seteuid(suid) == -1) { + abort(); + } +} + +static int init_capabilities(void) +{ + return 0; +} +#endif static int socket_read(int sockfd, void *buff, ssize_t size) { @@ -279,81 +404,6 @@ static int send_status(int sockfd, struct iovec *iovec, int status) } /* - * from man 7 capabilities, section - * Effect of User ID Changes on Capabilities: - * If the effective user ID is changed from nonzero to 0, then the permitted - * set is copied to the effective set. If the effective user ID is changed - * from 0 to nonzero, then all capabilities are are cleared from the effective - * set. - * - * The setfsuid/setfsgid man pages warn that changing the effective user ID may - * expose the program to unwanted signals, but this is not true anymore: for an - * unprivileged (without CAP_KILL) program to send a signal, the real or - * effective user ID of the sending process must equal the real or saved user - * ID of the target process. Even when dropping privileges, it is enough to - * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't - * be exposed to signals. So just use setresuid/setresgid. - */ -static int setugid(int uid, int gid, int *suid, int *sgid) -{ - int retval; - - /* - * We still need DAC_OVERRIDE because we don't change - * supplementary group ids, and hence may be subjected DAC rules - */ - cap_value_t cap_list[] = { - CAP_DAC_OVERRIDE, - }; - - *suid = geteuid(); - *sgid = getegid(); - - if (setresgid(-1, gid, *sgid) == -1) { - retval = -errno; - goto err_out; - } - - if (setresuid(-1, uid, *suid) == -1) { - retval = -errno; - goto err_sgid; - } - - if (uid != 0 || gid != 0) { - if (do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0) < 0) { - retval = -errno; - goto err_suid; - } - } - return 0; - -err_suid: - if (setresuid(-1, *suid, *suid) == -1) { - abort(); - } -err_sgid: - if (setresgid(-1, *sgid, *sgid) == -1) { - abort(); - } -err_out: - return retval; -} - -/* - * This is used to reset the ugid back with the saved values - * There is nothing much we can do checking error values here. - */ -static void resetugid(int suid, int sgid) -{ - if (setresgid(-1, sgid, sgid) == -1) { - abort(); - } - if (setresuid(-1, suid, suid) == -1) { - abort(); - } -} - -/* * send response in two parts * 1) ProxyHeader * 2) Response or error status