public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
From: Christian Franke <Christian.Franke@t-online.de>
To: cygwin-apps@cygwin.com
Subject: [PATCH setup] Add new option --chown-admin
Date: Wed, 6 Jul 2022 09:14:08 +0200	[thread overview]
Message-ID: <3096f251-d7ca-073b-d7d7-751b7fe3e8c1@t-online.de> (raw)

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


             reply	other threads:[~2022-07-06  7:14 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-06  7:14 Christian Franke [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3096f251-d7ca-073b-d7d7-751b7fe3e8c1@t-online.de \
    --to=christian.franke@t-online.de \
    --cc=cygwin-apps@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).