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

[Jon Turney <jon.turney@dronecode.org.uk>]

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

On 23/08/2022 18:27, Christian Franke wrote:
> Jon Turney wrote:
>> On 12/07/2022 13:50, Jon Turney wrote:
>>> [Replying to the right list this time...]
>>> On 09/07/2022 13:21, Christian Franke wrote:
>> [...]
>>>>
>>>> The UserSettings ctor has a somewhat hidden side effect which sets 
>>>> root_scope correctly:
>>>>
>>>>   UserSettings::UserSettings(...);
>>>>    open_settings("setup.rc", ...);
>>>>     io_stream::open("cygfile:///etc/setup/setup.rc", ...);
>>>> io_stream_cygfile::io_stream_cygfile("/etc/setup/setup.rc", ...);
>>>>       get_root_dir_now();
>>>>        read_mounts("");
>>>>         read_mounts_nt("");
>>>>          root_scope = isuser ? IDC_ROOT_USER : IDC_ROOT_SYSTEM;
>>>>
>>>> Conclusion: Regression introduced Feb 24, 2012 (befc9dd).
>>>>
>>>
>>> Thanks for tracking this down.
>>>
>>> That just seems... fractally wrong.
>>
>> I kind of lost track of this.  Is there anything else needed to fix 
>> the original problem here?  Or is it solved by the change to defer 
>> setting the group until after root_scope is known?
> 
> The group seems to be correctly set now.
> 
> An old problem still remains: root_scope always ends up as 
> IDC_ROOT_SYSTEM if setup is run elevated, regardless off GUI setting. 
> Apply the temporary patch from here to see what happens:
> https://sourceware.org/pipermail/cygwin-apps/2022-July/042151.html

Ah, right, I remember now.

I think having read_mounts() have a side effect of setting root_scope is 
just nonsense now (it might have made some sense back in the day when 
the mount table was also stored in the registry).

So, how about the attached?

(We should perhaps also set the installation root directory to something 
other than C:\cygwin64 if a non-admin user, since they are unlikely to 
be able to write there in current windows versions, but that's difficult 
from a sequencing point of view in that dialog, and for backwards 
compatibility)

> Possibly difficult to fix, in particular in conjunction with later 
> changes via [< Back] button. An easier approach: Remove the GUI setting 
> and connect root_scope to -B option.

Looking for that option is not correct, because we might be running 
without -B, but from an already elevated shell.  I think checking 
nt_sec.isRunAsAdmin() would be the correct test.

[-- Attachment #2: 0001-Drop-setting-root_scope-as-a-side-effect-of-read_mou.patch --]
[-- Type: text/plain, Size: 3589 bytes --]

From 8bb9d2d647c4a9c49e696efb14d976682ad28647 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Wed, 24 Aug 2022 13:31:49 +0100
Subject: [PATCH setup] Drop setting root_scope as a side-effect of
 read_mounts()

Default root_scope as appropriate, allowing GUI to override it
Only enable 'Install For All Users' if we have Admin privs
Drop some comment cruft
---
 mount.cc | 30 +++++++++++++++++++++++-------
 mount.h  |  9 +--------
 root.cc  | 12 ++++++++++++
 3 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/mount.cc b/mount.cc
index 0136396..a38f52c 100644
--- a/mount.cc
+++ b/mount.cc
@@ -140,10 +140,6 @@ create_install_root ()
 						      : "LOCAL_MACHINE\\")
 		      << buf << "\\rootdir = \"" << get_root_dir () << "\""
 		      << endLog;
-
-  // The mount table is already in the right shape at this point.
-  // Reading it again is not necessary.
-  //read_mounts (std::string ());
 }
 
 inline char *
@@ -316,8 +312,6 @@ read_mounts (const std::string val)
     }
   got_usr_bin = got_usr_lib = false;
 
-  root_scope = (nt_sec.isRunAsAdmin ())? IDC_ROOT_SYSTEM : IDC_ROOT_USER;
-
   if (val.size ())
     {
       /* Cygwin rootdir always < MAX_PATH. */
@@ -353,7 +347,6 @@ read_mounts (const std::string val)
 	    {
 	      m->native = std::string (aBuffer);
 	      m->posix = "/";
-	      root_scope = isuser ? IDC_ROOT_USER : IDC_ROOT_SYSTEM;
 	      root_here = m++;
 	      from_fstab (m, root_here->native);
 	      add_usr_mnts (m);
@@ -376,6 +369,29 @@ read_mounts (const std::string val)
     }
 }
 
+// set default root_scope: USER if only HKEY_CURRENT_USER registry key exists,
+// otherwise SYSTEM.
+void set_default_root_scope()
+{
+  root_scope = IDC_ROOT_SYSTEM;
+
+  char buf[10000];
+  for (int isuser = 0; isuser <= 1; isuser++)
+    {
+      snprintf (buf, sizeof(buf), "Software\\%s\\%s",
+                CYGWIN_INFO_CYGWIN_REGISTRY_NAME,
+                CYGWIN_INFO_CYGWIN_SETUP_REGISTRY_NAME);
+      HKEY key = isuser ? HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE;
+      if (RegOpenKeyEx (key, buf, 0, KEY_ALL_ACCESS | SETUP_KEY_WOW64,
+                        &key) == ERROR_SUCCESS)
+        {
+          RegCloseKey (key);
+          root_scope = isuser ? IDC_ROOT_USER : IDC_ROOT_SYSTEM;
+          break;
+        }
+    }
+}
+
 void
 set_root_dir (const std::string val)
 {
diff --git a/mount.h b/mount.h
index a7d7e39..c451a02 100644
--- a/mount.h
+++ b/mount.h
@@ -15,11 +15,6 @@
 
 #ifndef SETUP_MOUNT_H
 #define SETUP_MOUNT_H
-
-/* Finds the existing root mount, or returns NULL.  istext is set to
-   nonzero if the existing mount is a text mount, else zero for
-   binary. */
-
 #include <string>
 #include "String++.h"
 
@@ -27,9 +22,7 @@
 
 void create_install_root ();
 void read_mounts (const std::string);
-
-/* Sets the cygdrive flags.  Used to make the automounted drives' binary/text
-mode consistent with the standard Cygwin mounts. */
+void set_default_root_scope();
 
 std::string cygpath (const std::string&);
 void set_root_dir (const std::string);
diff --git a/root.cc b/root.cc
index 9f072d7..ccbd6ae 100644
--- a/root.cc
+++ b/root.cc
@@ -259,6 +259,18 @@ RootPage::OnInit ()
   if (!get_root_dir ().size())
     read_mounts (std::string ());
   orig_root_dir = get_root_dir();
+
+  if (!nt_sec.isRunAsAdmin())
+    {
+      // disable IDC_ROOT_SYSTEM if not running as admin
+      EnableWindow(GetDlgItem(IDC_ROOT_SYSTEM), FALSE);
+      root_scope = IDC_ROOT_USER;
+    }
+  else
+    {
+      set_default_root_scope();
+    }
+
   load_dialog (GetHWND ());
 }
 
-- 
2.37.2