public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [PATCH setup] Add new option --chown-admin
@ 2022-07-06  7:14 Christian Franke
  2022-07-06 13:53 ` Jon Turney
  0 siblings, 1 reply; 18+ messages in thread
From: Christian Franke @ 2022-07-06  7:14 UTC (permalink / raw)
  To: cygwin-apps

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

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 
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.

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.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Add-new-option-chown-admin.patch --]
[-- Type: text/plain, Size: 4531 bytes --]

From 1dfc8d63a8438e42544b06cfdf225f222107eed2 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Wed, 6 Jul 2022 07:41:18 +0200
Subject: [PATCH] Add new option --chown-admin

If specified and the process token owner is the local administrator,
the owner is preserved and the primary group is set to the local
administrator group.
---
 main.cc  |  3 ++-
 win32.cc | 41 ++++++++++++++++++++++++++++++-----------
 win32.h  |  2 +-
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/main.cc b/main.cc
index 3a8c5ea..81c4a65 100644
--- a/main.cc
+++ b/main.cc
@@ -99,6 +99,7 @@ static StringOption Arch ("", 'a', "arch", IDS_HELPTEXT_ARCH, false);
 static BoolOption UnattendedOption (false, 'q', "quiet-mode", IDS_HELPTEXT_QUIET_MODE);
 static BoolOption PackageManagerOption (false, 'M', "package-manager", IDS_HELPTEXT_PACKAGE_MANAGER);
 static BoolOption NoAdminOption (false, 'B', "no-admin", IDS_HELPTEXT_NO_ADMIN);
+static BoolOption ChownAdminOption (false, '\0', "chown-admin" /*, TODO: IDS_HELPTEXT_... */);
 static BoolOption WaitOption (false, 'W', "wait", IDS_HELPTEXT_WAIT);
 static BoolOption HelpOption (false, 'h', "help", IDS_HELPTEXT_HELP);
 static BoolOption VersionOption (false, 'V', "version", IDS_HELPTEXT_VERSION);
@@ -359,7 +360,7 @@ WinMain (HINSTANCE h,
       }
 
     /* Set default DACL and Group. */
-    nt_sec.setDefaultSecurity ((root_scope == IDC_ROOT_SYSTEM));
+    nt_sec.setDefaultSecurity ((root_scope == IDC_ROOT_SYSTEM), ChownAdminOption);
 
     /*
        If --symlink-type option isn't given, look for winsymlinks in CYGWIN
diff --git a/win32.cc b/win32.cc
index 55072a9..5dc9616 100644
--- a/win32.cc
+++ b/win32.cc
@@ -308,7 +308,7 @@ NTSecurity::setAdminGroup ()
 }
 
 void
-NTSecurity::setDefaultSecurity (bool isAdmin)
+NTSecurity::setDefaultSecurity (bool isAdmin, bool keepAdmin)
 {
   /* Get the processes access token. */
   if (!OpenProcessToken (GetCurrentProcess (),
@@ -335,21 +335,40 @@ NTSecurity::setDefaultSecurity (bool isAdmin)
   /* Set the default DACL to all permissions for everyone as a fallback. */
   setDefaultDACL ();
 
-  /* Get the user */
-  if (!GetTokenInformation (token.theHANDLE (), TokenUser, &ownerSID,
+  /* Get the owner */
+  if (!GetTokenInformation (token.theHANDLE (), TokenOwner, &ownerSID,
 			    sizeof ownerSID, &size))
     {
-      NoteFailedAPI ("GetTokenInformation(user)");
+      NoteFailedAPI ("GetTokenInformation(owner)");
       return;
     }
-  /* Make it the owner */
-  TOKEN_OWNER owner = { ownerSID.user.User.Sid };
-  if (!SetTokenInformation (token.theHANDLE (), TokenOwner, &owner,
-			    sizeof owner))
+
+  bool ownerIsAdmin = !!EqualSid (ownerSID.user.User.Sid, administratorsSID.theSID ());
+
+  if (keepAdmin && ownerIsAdmin)
+    Log (LOG_TIMESTAMP) << "Default owner is Administrator" << endLog;
+  else
     {
-      NoteFailedAPI ("SetTokenInformation(owner)");
-      return;
+      /* Get the user */
+      if (!GetTokenInformation (token.theHANDLE (), TokenUser, &ownerSID,
+				sizeof ownerSID, &size))
+	{
+	  NoteFailedAPI ("GetTokenInformation(user)");
+	  return;
+	}
+      /* Make it the owner */
+      TOKEN_OWNER owner = { ownerSID.user.User.Sid };
+      if (!SetTokenInformation (token.theHANDLE (), TokenOwner, &owner,
+				sizeof owner))
+	{
+	  NoteFailedAPI ("SetTokenInformation(owner)");
+	  return;
+	}
+      Log (LOG_TIMESTAMP) << "Default owner changed "
+			  << (ownerIsAdmin ? "from Administrator " : "")
+			  << "to current user" << endLog;
     }
+
   /* Get original primary group.  The token's primary group will be reset
      to the original group right before we call the postinstall scripts.
      This is necessary, otherwise, if the installing user is a domain user,
@@ -365,7 +384,7 @@ NTSecurity::setDefaultSecurity (bool isAdmin)
   /* Try to set the primary group to the Administrators group, but only if
      "Install for all users" has been chosen.  If it doesn't work, we're
      no admin and that's all there's to say about it. */
-  if (isAdmin)
+  if (isAdmin || (keepAdmin && ownerIsAdmin))
     setAdminGroup ();
 }
 
diff --git a/win32.h b/win32.h
index 02c1d06..560512b 100644
--- a/win32.h
+++ b/win32.h
@@ -130,7 +130,7 @@ public:
   void resetPrimaryGroup();
   void setAdminGroup ();
   void initialiseWellKnownSIDs ();
-  void setDefaultSecurity(bool isAdmin);
+  void setDefaultSecurity(bool isAdmin, bool keepAdmin);
   bool isRunAsAdmin ();
   bool hasSymlinkCreationRights ();
 private:
-- 
2.36.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-11-30 18:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06  7:14 [PATCH setup] Add new option --chown-admin Christian Franke
2022-07-06 13:53 ` Jon Turney
2022-07-06 16:34   ` Christian Franke
2022-07-07 11:38     ` Jon Turney
2022-07-07 14:45       ` Christian Franke
2022-07-07 14:59         ` Christian Franke
     [not found]         ` <d7d51d1c-f6d5-2fac-3e6d-86714efd0734@dronecode.org.uk>
     [not found]           ` <32655945-5075-0823-2a1d-b72caa4b7791@t-online.de>
2022-07-12 12:50             ` Jon Turney
2022-08-23 15:20               ` Jon Turney
2022-08-23 17:27                 ` Christian Franke
2022-08-26 13:27                   ` Jon Turney
2022-08-26 15:02                     ` Christian Franke
2022-08-28 17:33                       ` Christian Franke
2022-09-02 13:56                         ` Jon Turney
2022-09-02 15:17                           ` Christian Franke
2022-09-15 17:45                             ` Jon Turney
2022-10-04 12:05                               ` Christian Franke
2022-11-29 21:37                                 ` Jon Turney
2022-11-30 18:49                                   ` Christian Franke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).