From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from re-prd-fep-042.btinternet.com (mailomta8-re.btinternet.com [213.120.69.101]) by sourceware.org (Postfix) with ESMTPS id 192D63858D32 for ; Thu, 7 Jul 2022 11:38:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 192D63858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dronecode.org.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=dronecode.org.uk Received: from re-prd-rgout-003.btmx-prd.synchronoss.net ([10.2.54.6]) by re-prd-fep-042.btinternet.com with ESMTP id <20220707113818.UNRM3291.re-prd-fep-042.btinternet.com@re-prd-rgout-003.btmx-prd.synchronoss.net>; Thu, 7 Jul 2022 12:38:18 +0100 Authentication-Results: btinternet.com; auth=pass (PLAIN) smtp.auth=jonturney@btinternet.com; bimi=skipped X-SNCR-Rigid: 61A69BAC22337B48 X-Originating-IP: [86.139.167.41] X-OWM-Source-IP: 86.139.167.41 (GB) X-OWM-Env-Sender: jonturney@btinternet.com X-VadeSecure-score: verdict=clean score=0/300, class=clean X-RazorGate-Vade: gggruggvucftvghtrhhoucdtuddrgedvfedrudeihedggeduucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuueftkffvkffujffvgffngfevqffopdfqfgfvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefkffggfgfuvfhfhfgjtgfgsehtjeertddtfeejnecuhfhrohhmpeflohhnucfvuhhrnhgvhicuoehjohhnrdhtuhhrnhgvhiesughrohhnvggtohguvgdrohhrghdruhhkqeenucggtffrrghtthgvrhhnpeehudeuveeujeeujeegueefhedttdekvedtudeileefteetfeefjeejudekfefggfenucffohhmrghinheptgihghifihhnrdgtohhmnecukfhppeekiedrudefledrudeijedrgedunecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehhvghloheplgduledvrdduieekrddurddutdehngdpihhnvghtpeekiedrudefledrudeijedrgedupdhmrghilhhfrhhomhepjhhonhdrthhurhhnvgihsegurhhonhgvtghouggvrdhorhhgrdhukhdpnhgspghrtghpthhtohepvddprhgtphhtthhopeevhhhrihhsthhirghnrdfhrhgrnhhkvgesthdqohhnlhhinhgvrdguvgdprhgtphhtthhopegthihgfihinhdqrghpphhssegthihgfihinhdrtghomh X-RazorGate-Vade-Verdict: clean 0 X-RazorGate-Vade-Classification: clean Received: from [192.168.1.105] (86.139.167.41) by re-prd-rgout-003.btmx-prd.synchronoss.net (5.8.716.04) (authenticated as jonturney@btinternet.com) id 61A69BAC22337B48; Thu, 7 Jul 2022 12:38:18 +0100 Message-ID: <5b45ccdc-da32-ff11-037f-c00828f397c5@dronecode.org.uk> Date: Thu, 7 Jul 2022 12:38:15 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH setup] Add new option --chown-admin Content-Language: en-GB To: Christian Franke , "cygwin-apps@cygwin.com" References: <3096f251-d7ca-073b-d7d7-751b7fe3e8c1@t-online.de> <405df5c6-ce47-0254-ae4d-4a23ff3533d5@dronecode.org.uk> From: Jon Turney In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3570.6 required=5.0 tests=BAYES_00, FORGED_SPF_HELO, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: cygwin-apps@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin package maintainer discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jul 2022 11:38:23 -0000 On 06/07/2022 17:34, Christian Franke wrote: > Jon Turney wrote: >> On 06/07/2022 08:14, Christian Franke wrote: >>> If an installer is run elevated, the installed files will be >>> typically owned by the local administrator (or in some cases SYSTEM >>> or TrustedInstaller) instead of the current user. This is not the >>> case for a Cygwin "All Users" installation. The files are then not >>> protected from >> >> ... instead the files are owned by the user running setup? > > Yes, because the TokenUser is unconditionally copied to TokenOwner. Some > research with 'git blame' shows that many parts of the related code are > from the early days of UAC and elevated processes (Vista 2006, Win7 > 2009). Things may have changed since then, I don't know (or remember). > > If a process is run elevated, Windows keeps TokenUser unchanged but sets > the TokenOwner to local administrator. The --chown-admin option simply > keeps this as is, so no actual chown() is needed later. The > TokenPrimaryGroup is not changed by Windows, therefore --chown-admin > calls setAdminGroup() to mimic the usual "root root" ownership. > > Typical simple installers leave everything as is, so the installed files > are owned by local adminstrator and group "None" (S-1-5-21-*-513), see > 'ls -l "$PROGRAMFILES"'. > > >>> accidental changes by this user. >>> >>> The attached patch adds an experimental --chown-admin option which >>> allows (new) installations owned by local administrator user and group. >> >> Thanks for the patch, but... >> >>> A drawback is that files generated by postinstall scripts are still >>> owned by current user + "None" group. It should be possible to fix >>> this with some perpetual preremove+postinstall scripts. >>> >>> I also don't know whether this may break some postinstall scripts. >>> >>> BTW: 'nt_sec.setDefaultSecurity (isAdmin)' is never called with >>> 'isAdmin==true' as 'root_scope' is always 0. >> >> root_scope is set later, by the "Install For" option on the "Select >> Root Install Directory" page. >> >> To me, this looks like a (very long standing) bug that we shouldn't be >> calling setAdminGroup() here, but after root_scope has been set. > > If this bug is very old, I'm not sure whether this should be fixed. > Setting admin group to files which are owned "only" by current user is > possibly not very effective. It's true that some people might be relying on that buggy behaviour. But, please help me understand how your patch differs from adding an option which fixes that misbehaviour when supplied? (As an aside, looking at how setAdminGroup()/resetPrimaryGroup() are used, changing the token for the postinstall scripts seems unnecessary now, since we don't run mkpasswd there any more...) [1] https://cygwin.com/git/?p=cygwin-apps/setup.git;a=blob;f=postinstall.cc;h=e990f520077f360d1b39f2d0fa915fd6110f4c84;hb=HEAD#l271