From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57450C433E0 for ; Wed, 20 Jan 2021 14:52:30 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ECB662336D for ; Wed, 20 Jan 2021 14:52:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ECB662336D Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=xenproject.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.71473.128013 (Exim 4.92) (envelope-from ) id 1l2EqF-0000d0-Lc; Wed, 20 Jan 2021 14:52:15 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 71473.128013; Wed, 20 Jan 2021 14:52:15 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l2EqF-0000ct-IW; Wed, 20 Jan 2021 14:52:15 +0000 Received: by outflank-mailman (input) for mailman id 71473; Wed, 20 Jan 2021 14:52:14 +0000 Received: from mail.xenproject.org ([104.130.215.37]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l2EqE-0000co-P7 for xen-devel@lists.xenproject.org; Wed, 20 Jan 2021 14:52:14 +0000 Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l2EqE-0002dl-Ku for xen-devel@lists.xenproject.org; Wed, 20 Jan 2021 14:52:14 +0000 Received: from iwj (helo=mariner.uk.xensource.com) by xenbits.xenproject.org with local-bsmtp (Exim 4.92) (envelope-from ) id 1l2EqE-0001W8-JV for xen-devel@lists.xenproject.org; Wed, 20 Jan 2021 14:52:14 +0000 Received: from iwj by mariner.uk.xensource.com with local (Exim 4.89) (envelope-from ) id 1l2Eq7-0006Aa-8q; Wed, 20 Jan 2021 14:52:07 +0000 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xenproject.org; s=20200302mail; h=References:In-Reply-To:Subject:Cc:To:Date :Message-ID:Content-Transfer-Encoding:Content-Type:MIME-Version:From; bh=afOWdXx+VGz1pNkA6HFUcEbPiS9lbnsmIGVr033YdRk=; b=Tsgmg8PyvOcXQuO9c7ySklcZ7t V3+Nsdo2uy3a6IODYa+fPw4ri86Cca56UZ1jH9JRLkuHQ9lY6yN6+i8F8UoGtquwkbwC4LvIu6vtE /uTkSoJgtaiGO2kLYOe1DxBmhAnZ9qgUKCbD2hbtSYrUWAu+CIuKyEO+ADi3+ywi38Y4=; From: Ian Jackson MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Message-ID: <24584.17302.958286.788145@mariner.uk.xensource.com> Date: Wed, 20 Jan 2021 14:52:06 +0000 To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: George Dunlap , Manuel Bouyer , , Manuel Bouyer , Wei Liu , Anthony PERARD Subject: Re: [PATCH] libs/light: make it build without setresuid() In-Reply-To: <20210118181656.2abblbjg2jvhlad7@Air-de-Roger> References: <20210112181242.1570-1-bouyer@antioche.eu.org> <20210112181242.1570-16-bouyer@antioche.eu.org> <20210118181656.2abblbjg2jvhlad7@Air-de-Roger> X-Mailer: VM 8.2.0b under 24.5.1 (i686-pc-linux-gnu) Roger Pau Monné writes ("Re: [PATCH] libs/light: make it build without setresuid()"): > On Tue, Jan 12, 2021 at 07:12:36PM +0100, Manuel Bouyer wrote: > > From: Manuel Bouyer > > > > NetBSD doesn't have setresuid(). Add a configure check for it, > > and use plain setuid() if !HAVE_SETRESUID ... > LGTM from a code PoV, but I think George/Ian should take a look, since > they know exactly what this is supposed to do, and I would bet there > are some reasons why setresuid is used instead of setuid, which should > likely be taken into account in the commit message to justify why > using setuid in it's place it's fine. There is indeed a reason for using setresuid here. See the comments at the top of kill_device_model_uid_child and the commit messages for 87f9458e3400 and 0c653574d39c. This is all quite complex: https://xenproject.org/2018/08/01/killing-processes-that-dont-want-to-be-killed/ https://marc.info/?l=xen-devel&m=152215770803468 (search in that message for "libxl UID cleanup") I wrote a message to George in 2018 proving that the desired set of IDs cannot be made without setresuid. I'll c&p the relevant part below. I don't think setuid is safe - at least, if we are trying to restrict the dm. Since I think after the libxl child is forked, and has called setuid, it might be traceable (by NetBSD's equivalent of ptrace) by the dm. The dm could puppet it into pretending it had succeeded, but then hang around until the domid is reused. At the very least, this patch needs an argument, in detail, why this is OK. Also, why oh why does NetBSD not have setresuid ?? It's at least 20 years old ! Sorry, Ian. PS there is a long discussion of the history of saved set-ids, real vs effective uids, etc., here https://pubs.opengroup.org/onlinepubs/9699919799/functions/setuid.html but sadly it does not discuss setresuid. >From me to George etc. in 2018. George emailed me a draft post: > # No POSIX-compliant mousetraps? > > Although `setresuid` is implemented by both Linux and FreeBSD, it is > not in the [current POSIX > specification](http://pubs.opengroup.org/onlinepubs/9699919799/). > Looking at the official list of POSIX system interfaces, it's not > clear how to get a process to have the required tuple using only POSIX > interfaces (namely `setuid` and `setreuid`, without recourse to > `setresuid` or Linux's `CAP_SETUID`); the assumption seems to be that > `euid` must always be set to either `ruid` or `suid`. Proof that this can't be simulated by proper use of setuid, seteuid and setreuid: ruid euid suid The desired state is: reaper target reaper If the final call is seteuid: seteuid(target); reaper target reaper For this to be permitted, and nontrivial, euid was 0: Penultimate status reaper 0 reaper This state cannot be generated by setuid either euid==0 previously and setuid would have set all of the ids; or the old euid was not 0, in which case setuid() would have set only the euid, and required that one of the other ids was 0, which can see that it can't have been. This penultimate state cannot be generated by seteuid from any different state. So it must have been generated by setreuid. We must avoid setreuid setting the suid to the same as the new euid (0), which means that our setreuid call did not change the ruid either. That form of setreuid is just like euid for our purposes, and not useful. So the desired state could not be made by seteuid. Let's consider setreuid. Well, either setreuid sets the suid to the same as the new euid, or it only changes the euid. Ie, it would only do something we could have done with seteuid and the argument above applies. What abouit setuid ? Well, either setuid sets all three uids to the same thing, or it, again, sets only the euid. Ian.