Re: [PATCH setup] Add new option --chown-admin

[Christian Franke <Christian.Franke@t-online.de>]

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

As the 'root_scope' issues are now fixed, here a reworked and enhanced 
(checkbox, setup.rc entry) version of the original patch from this thread.

With the new setting enabled, setup behaves like other install tools 
when run elevated: The installation is then also protected against 
accidental modifications by the current user.

owner:group assignments of newly installed dirs/files:

adm:adm -- "All Users", "[X] Change owner of newly installed files to 
local Administrator"
usr:adm -- "All Users"
usr:def -- "Just Me"

(usr = user running setup, adm = S-1-5-32-544, def = S-1-5-21-*-513)

An alternative for the UI would be a 3rd radio button ("All Users - 
change owner of newly installed files to local Administrator"), but the 
checkbox makes this addition IMO more obvious.

The new setup.rc setting 'root-scope' is only used to read the 
chown_admin setting but this could be enhanced, e.g. warn user if 
root_scope selection differs from previous setup run.

The drawback that files generated by postinstall scripts are still owned 
by current user could be fixed with a perpetual postinstall script. I 
could provide one for base-files package if desired.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Optionally-change-owner-of-installed-files-to-local-.patch --]
[-- Type: text/plain, Size: 10277 bytes --]

From 6d996b377b4a6a908fbb4c217bda24249b4b58c1 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sun, 28 Aug 2022 19:23:44 +0200
Subject: [PATCH] Optionally change owner of installed files to local
 administrator

Could be selected via new checkbox in Root dialog or via new option
--chown-admin.  This choice and the root_scope are stored in new
setup.rc entry 'root-scope'.
---
 res/en/res.rc |  6 +++++-
 res/fr/res.rc |  6 +++++-
 resource.h    |  2 ++
 root.cc       | 45 ++++++++++++++++++++++++++++++++++++++++-----
 win32.cc      | 33 ++++++++++++++++++++++++---------
 win32.h       |  2 ++
 6 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/res/en/res.rc b/res/en/res.rc
index ef5e8b1..74d3d0c 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -108,7 +108,10 @@ BEGIN
     CONTROL         "Just &Me",IDC_ROOT_USER,"Button",BS_AUTORADIOBUTTON | 
                     WS_TABSTOP,13,130,130,8
     LTEXT           "Cygwin will be available to all users of the system.",
-                    IDC_ALLUSERS_TEXT,25,101,300,28
+                    IDC_ALLUSERS_TEXT,25,100,300,28
+    CONTROL         "&Change owner of newly installed files to local Administrator",
+                    IDC_ROOT_CHOWN_ADMIN,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,
+                    25,111,300,16
     LTEXT           "Cygwin will still be available to all users, but "
                     "Desktop Icons, Cygwin Menu Entries, and important "
                     "Installer information are only available to the current "
@@ -668,6 +671,7 @@ BEGIN
     IDS_HELPTEXT_ALLOW_UNSUPPORTED_WINDOWS "Allow old, unsupported Windows versions"
     IDS_HELPTEXT_ARCH "Architecture to install (x86_64 or x86)"
     IDS_HELPTEXT_CATEGORIES "Specify categories to install"
+    IDS_HELPTEXT_CHOWN_ADMIN "Change owner of newly installed files to local Administrator"
     IDS_HELPTEXT_COMPACTOS "Compress installed files with Compact OS (xpress4k, xpress8k, xpress16k, lzx)"
     IDS_HELPTEXT_DELETE_ORPHANS "Remove orphaned packages"
     IDS_HELPTEXT_DISABLE_ANTIVIRUS "Disable known or suspected buggy anti virus software packages during execution"
diff --git a/res/fr/res.rc b/res/fr/res.rc
index d081bb2..21ba8f9 100644
--- a/res/fr/res.rc
+++ b/res/fr/res.rc
@@ -102,7 +102,10 @@ BEGIN
     CONTROL         "Juste &Moi",IDC_ROOT_USER,"Button",BS_AUTORADIOBUTTON |
                     WS_TABSTOP,13,130,130,8
     LTEXT           "Cygwin sera disponible pour tous les utilisateurs.",
-                    IDC_ALLUSERS_TEXT,25,101,300,28
+                    IDC_ALLUSERS_TEXT,25,100,300,28
+    CONTROL         "&Change owner of newly installed files to local Administrator", // XXX: missing translation
+                    IDC_ROOT_CHOWN_ADMIN,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,
+                    25,111,300,16
     LTEXT           "Cygwin sera disponible pour tous les utilisateurs "
                     "mais les icônes et les menus uniquement pour l'utilisateur "
                     "en cours. Ne sélectionner que si vous n'avez pas les droits "
@@ -648,6 +651,7 @@ BEGIN
     IDS_HELPTEXT_ALLOW_UNSUPPORTED_WINDOWS "Autoriser les vieilles versions de Windows"
     IDS_HELPTEXT_ARCH "Architecture à installer (x86_64 ou x86)"
     IDS_HELPTEXT_CATEGORIES "Spécifie les catégories à installer"
+    // IDS_HELPTEXT_CHOWN_ADMIN "XXX: missing translation"
     // IDS_HELPTEXT_COMPACTOS "XXX: missing translation"
     IDS_HELPTEXT_DELETE_ORPHANS "Supprimer les paquets orphelins"
     IDS_HELPTEXT_DISABLE_ANTIVIRUS "Inhibe les anti-virus buggés à l'exécution"
diff --git a/resource.h b/resource.h
index cfe860b..91446b6 100644
--- a/resource.h
+++ b/resource.h
@@ -157,6 +157,7 @@
 #define IDS_HELPTEXT_HEADER              1546
 #define IDS_HELPTEXT_FOOTER              1547
 #define IDS_HELPTEXT_NO_WRITE_REGISTRY   1548
+#define IDS_HELPTEXT_CHOWN_ADMIN         1549
 
 // Dialogs
 
@@ -296,3 +297,4 @@
 #define IDC_FILE_INUSE_HELP_0             605
 #define IDC_FILE_INUSE_HELP_1             606
 #define IDC_FILE_INUSE_HELP_2             607
+#define IDC_ROOT_CHOWN_ADMIN              608
diff --git a/root.cc b/root.cc
index ccbd6ae..766edc3 100644
--- a/root.cc
+++ b/root.cc
@@ -20,6 +20,7 @@
 #include "root.h"
 
 #include "LogSingleton.h"
+#include "UserSettings.h"
 
 #include "win32.h"
 #include <shlobj.h>
@@ -36,9 +37,11 @@
 #include "mount.h"
 #include "propsheet.h"
 
+#include "getopt++/BoolOption.h"
 #include "getopt++/StringOption.h"
 
 StringOption RootOption ("", 'R', "root", IDS_HELPTEXT_ROOT, false);
+static BoolOption ChownAdminOption (false, '\0', "chown-admin", IDS_HELPTEXT_CHOWN_ADMIN);
 
 static ControlAdjuster::ControlInfo RootControlsInfo[] = {
   { IDC_ROOTDIR_GRP,              CP_STRETCH,           CP_TOP      },
@@ -47,6 +50,7 @@ static ControlAdjuster::ControlInfo RootControlsInfo[] = {
 
   { IDC_INSTALLFOR_GRP,           CP_STRETCH,		CP_STRETCH  },
   { IDC_ROOT_SYSTEM,              CP_LEFT,              CP_TOP      },
+  { IDC_ROOT_CHOWN_ADMIN,         CP_LEFT,              CP_TOP      },
   { IDC_ALLUSERS_TEXT,            CP_STRETCH,		CP_TOP      },
   { IDC_ROOT_USER,                CP_LEFT,              CP_BOTTOM   },
   { IDC_JUSTME_TEXT,              CP_STRETCH,		CP_BOTTOM   },
@@ -69,17 +73,20 @@ RootPage::check_if_enable_next (HWND h)
 }
 
 static void
-load_dialog (HWND h)
+load_dialog (HWND h, bool chown_admin)
 {
   rbset (h, su, root_scope);
   eset (h, IDC_ROOT_DIR, get_root_dir ());
+  CheckDlgButton (h, IDC_ROOT_CHOWN_ADMIN,
+		  (chown_admin ? BST_CHECKED : BST_UNCHECKED));
 }
 
-static void
+static bool
 save_dialog (HWND h)
 {
   root_scope = rbget (h, su);
   set_root_dir (egetString (h, IDC_ROOT_DIR));
+  return !!IsDlgButtonChecked (h, IDC_ROOT_CHOWN_ADMIN);
 }
 
 static int CALLBACK
@@ -228,6 +235,15 @@ RootPage::OnMessageCmd (int id, HWND hwndctl, UINT code)
     case IDC_ROOT_DIR:
     case IDC_ROOT_SYSTEM:
     case IDC_ROOT_USER:
+      switch (id)
+	{
+	case IDC_ROOT_SYSTEM:
+	  EnableWindow(GetDlgItem(IDC_ROOT_CHOWN_ADMIN), TRUE);
+	  break;
+	case IDC_ROOT_USER:
+	  EnableWindow(GetDlgItem(IDC_ROOT_CHOWN_ADMIN), FALSE);
+	  break;
+	}
       check_if_enable_next (GetHWND ());
       break;
 
@@ -260,18 +276,26 @@ RootPage::OnInit ()
     read_mounts (std::string ());
   orig_root_dir = get_root_dir();
 
+  const char *root_scope_setting = UserSettings::instance().get ("root-scope");
+
   if (!nt_sec.isRunAsAdmin())
     {
       // disable IDC_ROOT_SYSTEM if not running as admin
       EnableWindow(GetDlgItem(IDC_ROOT_SYSTEM), FALSE);
+      EnableWindow(GetDlgItem(IDC_ALLUSERS_TEXT), FALSE);
       root_scope = IDC_ROOT_USER;
     }
   else
     {
+      // FIXME: Use root_scope_setting if set?
       set_default_root_scope();
     }
+  if (root_scope == IDC_ROOT_USER)
+    EnableWindow(GetDlgItem(IDC_ROOT_CHOWN_ADMIN), FALSE);
 
-  load_dialog (GetHWND ());
+  bool chown_admin = (ChownAdminOption || (root_scope_setting
+                     && !strcmp (root_scope_setting, "AllUsers,ChownAdmin")));
+  load_dialog (GetHWND (), chown_admin);
 }
 
 void
@@ -291,7 +315,9 @@ RootPage::OnNext ()
 {
   HWND h = GetHWND ();
 
-  save_dialog (h);
+  bool chown_admin = save_dialog (h);
+  if (root_scope == IDC_ROOT_USER)
+    chown_admin = false;
 
   if (!directory_is_absolute ())
     {
@@ -307,13 +333,22 @@ RootPage::OnNext ()
     return -1;
 
   Log (LOG_PLAIN) << "root: " << get_root_dir ()
-    << (root_scope == IDC_ROOT_USER ? " user" : " system") << endLog;
+    << (root_scope == IDC_ROOT_USER ? " user" : " system")
+    << (chown_admin ? ",chown_admin" : "") << endLog;
+
+  if (chown_admin)
+    nt_sec.setAdminAsOwner ();
+  else
+    nt_sec.setUserAsOwner ();
 
   if (root_scope == IDC_ROOT_SYSTEM)
     nt_sec.setAdminGroup ();
   else
     nt_sec.resetPrimaryGroup ();
 
+  UserSettings::instance().set("root-scope",
+                               (root_scope == IDC_ROOT_USER ? "CurrentUser" :
+			       chown_admin ? "AllUsers,ChownAdmin" : "AllUsers"));
   return 0;
 }
 
diff --git a/win32.cc b/win32.cc
index ea3d53a..467a1b2 100644
--- a/win32.cc
+++ b/win32.cc
@@ -281,12 +281,35 @@ NTSecurity::setBackupPrivileges ()
     }
 }
 
+void
+NTSecurity::setUserAsOwner ()
+{
+  if (ownerSID.user.User.Sid)
+    {
+      TOKEN_OWNER owner = { ownerSID.user.User.Sid };
+      Log (LOG_TIMESTAMP) << "Changing uid to current user" << endLog;
+      if (!SetTokenInformation (token.theHANDLE (), TokenOwner, &owner,
+				sizeof owner))
+	NoteFailedAPI ("SetTokenInformation(owner)");
+    }
+}
+
+void
+NTSecurity::setAdminAsOwner ()
+{
+  TOKEN_OWNER owner = { administratorsSID.theSID () };
+  Log (LOG_TIMESTAMP) << "Changing uid to Administrator" << endLog;
+  if (!SetTokenInformation (token.theHANDLE (), TokenOwner, &owner,
+			    sizeof owner))
+    NoteFailedAPI ("SetTokenInformation(owner)");
+}
+
 void
 NTSecurity::resetPrimaryGroup ()
 {
   if (primaryGroupSID.pgrp.PrimaryGroup)
     {
-      Log (LOG_TIMESTAMP) << "Changing gid back to original" << endLog;
+      Log (LOG_TIMESTAMP) << "Changing gid to original" << endLog;
       if (!SetTokenInformation (token.theHANDLE (), TokenPrimaryGroup,
 				&primaryGroupSID, sizeof primaryGroupSID))
 	NoteFailedAPI ("SetTokenInformation");
@@ -342,14 +365,6 @@ NTSecurity::setDefaultSecurity ()
       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;
-    }
   /* Get original primary group */
   if (!GetTokenInformation (token.theHANDLE (), TokenPrimaryGroup,
 			    &primaryGroupSID, sizeof primaryGroupSID, &size))
diff --git a/win32.h b/win32.h
index bf3ff10..1d31d8c 100644
--- a/win32.h
+++ b/win32.h
@@ -127,6 +127,8 @@ public:
   PSECURITY_DESCRIPTOR GetPosixPerms (const char *fname, PSID owner_sid,
 				      PSID group_sid, mode_t mode,
 				      SECURITY_DESCRIPTOR &out_sd, acl_t &acl);
+  void setUserAsOwner ();
+  void setAdminAsOwner ();
   void resetPrimaryGroup();
   void setAdminGroup ();
   void initialiseWellKnownSIDs ();
-- 
2.37.1