[Bug] setup regression #2

[Bug] setup regression #2

[Achim Gratz]

The release_2.91 comes with another regression that still puzzles me.
In a nutshell, the three commits that deal with setting up the groups
during / after installation

2022-08-27 	Jon Turney	Drop setting root_scope as a side-effect of read_mounts()
2022-08-16 	Jon Turney	Defer setting group until after All Users/Just For...
2022-08-16 	Jon Turney	Drop group change while running postinstall scripts

break existing installs in certain circumstances that are not yet
completely clear.  The server installation @work (which uses exactly the
same scripting around the installation that I use for my build system
@home) changed from using the "Domain Users" group to "Administrators".
Additionally the previous access for "Everyone" has been removed and
instead SYSTEM is now part of the (Windows) ACL.  In effect certain
files have become inaccessible to the normal users (unless they are aso
Administrators), in particular (this is the part that I still don't
understand) newly created symlinks can't be read by a normal user even
when the target is fully accessible.  Even doing an ls on such a symlink
gets a "permission denied".

For the time being I've reverted the three commits and re-installed all
packages that I had updated since the new setup was rolled out.  Except
for a handful of cache files for fontconfig (which I manually removed
and recreated) that cleared up the whole thing.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [Bug] setup regression #2

[Jon Turney]

On 22/09/2022 18:14, Achim Gratz wrote:
> 
> The release_2.91 comes with another regression that still puzzles me.
> In a nutshell, the three commits that deal with setting up the groups
> during / after installation
> 
> 2022-08-27 	Jon Turney	Drop setting root_scope as a side-effect of read_mounts()
> 2022-08-16 	Jon Turney	Defer setting group until after All Users/Just For...
> 2022-08-16 	Jon Turney	Drop group change while running postinstall scripts
> 
> break existing installs in certain circumstances that are not yet
> completely clear.  The server installation @work (which uses exactly the
> same scripting around the installation that I use for my build system
> @home) changed from using the "Domain Users" group to "Administrators".
> Additionally the previous access for "Everyone" has been removed and
> instead SYSTEM is now part of the (Windows) ACL.  In effect certain
> files have become inaccessible to the normal users (unless they are aso
> Administrators), in particular (this is the part that I still don't
> understand) newly created symlinks can't be read by a normal user even
> when the target is fully accessible.  Even doing an ls on such a symlink
> gets a "permission denied".

This problem is with files created by setup, or by post-install scripts?

(I'm not sure how these commits could have caused the former, if the 
latter then reverting 45d8e84e "Drop group change while running 
postinstall scripts" would be the thing to try...)

Re: [Bug] setup regression #2

[Achim Gratz]

Jon Turney writes:
> This problem is with files created by setup, or by post-install scripts?

I think both, although the problematic symlinks were created through
alternatives.

> (I'm not sure how these commits could have caused the former, if the
> latter then reverting 45d8e84e "Drop group change while running
> postinstall scripts" would be the thing to try...)

As I said, I don't understand it either.  It seems my installations were
all using the primary group for the account that does the install (which
does have administrative rights and is separate from my normal user
account on most machines).  The primary group is either "None" for the
build machine that only uses local accounts and is not a member of any
domain or "Domain Users" otherwise.

The new code uses "Administrators", but that seems to have the side
effect of denying "normal" users access to the installation and instead
weaves in extra DACL for SYSTEM.

As long as there's an option to force it to keep the former behaviour
things should be OK, but I haven't really checked if and how this is
possible.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: [Bug] setup regression #2

[Jon Turney]

On 03/10/2022 20:23, Achim Gratz wrote:
> Jon Turney writes:
>> This problem is with files created by setup, or by post-install scripts?
> 
> I think both, although the problematic symlinks were created through
> alternatives.

That's pretty baffling.

I don't see how any of those commits would change the ownership of files 
created by setup itself.

The only relevant change seems to be in "Defer setting group until after 
All Users/Just For Me is chosen", I've made nt_sec.resetPrimaryGroup() 
explicit, but that only happens for non-admin installs...

>> (I'm not sure how these commits could have caused the former, if the
>> latter then reverting 45d8e84e "Drop group change while running
>> postinstall scripts" would be the thing to try...)
> 
> As I said, I don't understand it either.  It seems my installations were
> all using the primary group for the account that does the install (which
> does have administrative rights and is separate from my normal user
> account on most machines).  The primary group is either "None" for the
> build machine that only uses local accounts and is not a member of any
> domain or "Domain Users" otherwise.
> 
> The new code uses "Administrators", but that seems to have the side
> effect of denying "normal" users access to the installation and instead
> weaves in extra DACL for SYSTEM.
> 
> As long as there's an option to force it to keep the former behaviour
> things should be OK, but I haven't really checked if and how this is
> possible.

Unfortunately, there is no such option.

Re: [Bug] setup regression #2

[Achim Gratz]

Jon Turney writes:
> On 03/10/2022 20:23, Achim Gratz wrote:
>> Jon Turney writes:
>>> This problem is with files created by setup, or by post-install scripts?
>> I think both, although the problematic symlinks were created through
>> alternatives.
>
> That's pretty baffling.

Even more baffling is that I have another installation that are
completely fine even with their Group now switched to Administrators.
The one syxstem where I've had the problems is a server version and
might have some GPO that affect thing that an admin user does.

> I don't see how any of those commits would change the ownership of
> files created by setup itself.

The ownership is still with the user that runs the install script,
however the group has changed.

> The only relevant change seems to be in "Defer setting group until
> after All Users/Just For Me is chosen", I've made
> nt_sec.resetPrimaryGroup() explicit, but that only happens for
> non-admin installs...

I think that setup was essentially treating the install as "for this
user only" since it was created and maintained by a script that can't
affect that option and the fact it was also in group Adminsitroators
didn't actually register until now.

The DACL on the server install changed from conferring access to "Everyone" to
just the install user and SYSTEM IIRC.  It doesn't do that on the
(non-domain) build machine at home that runs Win10 Pro.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

Re: [Bug] setup regression #2

[Jon Turney]

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

On 08/10/2022 17:56, Achim Gratz wrote:
> I think that setup was essentially treating the install as "for this
> user only" since it was created and maintained by a script that can't
> affect that option and the fact it was also in group Adminsitroators
> didn't actually register until now.

Yeah, that seems possible, since some of these changes fix what are 
arguably bugs in how that works (i.e. I suspect that previously, even 
when elevated, if only the registry key 
HKEY_CURRENT_USER\\Software\\Cygwin\\setup\rootdir exists (and not the 
same key under HKLM), we're going to install for "Just Me", irrespective 
of what the UI says)

> The DACL on the server install changed from conferring access to "Everyone" to
> just the install user and SYSTEM IIRC.  It doesn't do that on the
> (non-domain) build machine at home that runs Win10 Pro.

That makes less sense to me.  We should always creating an entry in the 
DACL for 'Everyone' to hold the POSIX permissions for 'other' users.

(See win32.cc:NTSecurity::GetPosixPerms() which translates a file mode 
to a SD)

 >> As long as there's an option to force it to keep the former behaviour
 >> things should be OK, but I haven't really checked if and how this is
 >> possible.
 >
 > Unfortunately, there is no such option.

I wrote some code for this option (attached), but I have a hard time 
seeing how it's functionally different from using '-B/'--no-admin'.

So, I guess a question is, does running with that option work as 
expected in your problematic instance?

[-- Attachment #2: 0001-Add-an-option-to-not-make-files-group-owned-by-Admin.patch --]
[-- Type: text/plain, Size: 3529 bytes --]

From ae547f5b4b4421bf9b7b9f204eb3d303cc6b2673 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Wed, 2 Nov 2022 22:46:29 +0000
Subject: [PATCH setup] Add an option to not make files group owned by
 Adminstrators

Add an option that, when elevated, do not make files group owned by
Adminstrators (i.e use the primary group of the user running setup
instead).

Fixes: 495b0148
---
 res.pot       | 8 ++++++--
 res/en/res.rc | 1 +
 resource.h    | 1 +
 root.cc       | 7 ++++++-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/res.pot b/res.pot
index 64079c8..e84c34c 100644
--- a/res.pot
+++ b/res.pot
@@ -3,7 +3,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2022-08-27 12:54+0100\n"
+"POT-Creation-Date: 2022-11-08 14:36+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -11,7 +11,7 @@ msgstr ""
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "X-Accelerator-Marker: &\n"
-"X-Generator: Translate Toolkit 3.7.0\n"
+"X-Generator: Translate Toolkit 3.7.3\n"
 "X-Merge-On: location\n"
 
 #: DIALOG.IDD_SOURCE.CAPTION
@@ -1245,6 +1245,10 @@ msgstr ""
 msgid "Disable creation of desktop shortcut"
 msgstr ""
 
+#: STRINGTABLE.IDS_HELPTEXT_NO_GROUP_CHANGE
+msgid "When elevated, do not make files group owned by Adminstrators"
+msgstr ""
+
 #: STRINGTABLE.IDS_HELPTEXT_NO_REPLACEONREBOOT
 msgid "Disable replacing in-use files on next reboot"
 msgstr ""
diff --git a/res/en/res.rc b/res/en/res.rc
index ef5e8b1..dad5c47 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -683,6 +683,7 @@ BEGIN
     IDS_HELPTEXT_MIRROR_MODE "Skip package availability check when installing from local directory (requires local directory to be clean mirror!)"
     IDS_HELPTEXT_NO_ADMIN "Do not check for and enforce running as Administrator"
     IDS_HELPTEXT_NO_DESKTOP "Disable creation of desktop shortcut"
+    IDS_HELPTEXT_NO_GROUP_CHANGE "When elevated, do not make files group owned by Adminstrators"
     IDS_HELPTEXT_NO_REPLACEONREBOOT "Disable replacing in-use files on next reboot"
     IDS_HELPTEXT_NO_SHORTCUTS "Disable creation of desktop and start menu shortcuts"
     IDS_HELPTEXT_NO_STARTMENU "Disable creation of start menu shortcut"
diff --git a/resource.h b/resource.h
index cfe860b..917534f 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_NO_GROUP_CHANGE     1549
 
 // Dialogs
 
diff --git a/root.cc b/root.cc
index ccbd6ae..f81c5c9 100644
--- a/root.cc
+++ b/root.cc
@@ -37,8 +37,10 @@
 #include "propsheet.h"
 
 #include "getopt++/StringOption.h"
+#include "getopt++/BoolOption.h"
 
 StringOption RootOption ("", 'R', "root", IDS_HELPTEXT_ROOT, false);
+static BoolOption NoGroupChangeOption (false, '\0', "no-group-change", IDS_HELPTEXT_NO_GROUP_CHANGE);
 
 static ControlAdjuster::ControlInfo RootControlsInfo[] = {
   { IDC_ROOTDIR_GRP,              CP_STRETCH,           CP_TOP      },
@@ -310,7 +312,10 @@ RootPage::OnNext ()
     << (root_scope == IDC_ROOT_USER ? " user" : " system") << endLog;
 
   if (root_scope == IDC_ROOT_SYSTEM)
-    nt_sec.setAdminGroup ();
+    {
+      if (!NoGroupChangeOption)
+        nt_sec.setAdminGroup ();
+    }
   else
     nt_sec.resetPrimaryGroup ();
 
-- 
2.38.1

Re: [Bug] setup regression #2

[Achim Gratz]

Jon Turney writes:
> On 08/10/2022 17:56, Achim Gratz wrote:
>> I think that setup was essentially treating the install as "for this
>> user only" since it was created and maintained by a script that can't
>> affect that option and the fact it was also in group Adminsitroators
>> didn't actually register until now.
>
> Yeah, that seems possible, since some of these changes fix what are
> arguably bugs in how that works (i.e. I suspect that previously, even
> when elevated, if only the registry key
> HKEY_CURRENT_USER\\Software\\Cygwin\\setup\rootdir exists (and not the
> same key under HKLM), we're going to install for "Just Me",
> irrespective of what the UI says)

I haven't checked that.

>> The DACL on the server install changed from conferring access to "Everyone" to
>> just the install user and SYSTEM IIRC.  It doesn't do that on the
>> (non-domain) build machine at home that runs Win10 Pro.
>
> That makes less sense to me.  We should always creating an entry in
> the DACL for 'Everyone' to hold the POSIX permissions for 'other'
> users.

Well, it wasn't there and whichever way it ended up that way, it was an
inconvenient enough fix that I don'*t want to try it again on a
productive system.

> I wrote some code for this option (attached), but I have a hard time
> seeing how it's functionally different from using '-B/'--no-admin'.
>
> So, I guess a question is, does running with that option work as
> expected in your problematic instance?

I'm not having enough time for checking right now, but I might test this
hypothesis later on.


Regard,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [Bug] setup regression #2

[Achim Gratz]

Jon Turney writes:
> On 08/10/2022 17:56, Achim Gratz wrote:
>> I think that setup was essentially treating the install as "for this
>> user only" since it was created and maintained by a script that can't
>> affect that option and the fact it was also in group Adminsitroators
>> didn't actually register until now.
>
> Yeah, that seems possible, since some of these changes fix what are
> arguably bugs in how that works (i.e. I suspect that previously, even
> when elevated, if only the registry key
> HKEY_CURRENT_USER\\Software\\Cygwin\\setup\rootdir exists (and not the
> same key under HKLM), we're going to install for "Just Me",
> irrespective of what the UI says)

I've checked some old logs and even though the install was identified as
"system", there was no line "Changing gid to Administrators" for the
main install until setup version 2.921.

> I wrote some code for this option (attached), but I have a hard time
> seeing how it's functionally different from using '-B/'--no-admin'.

This option does nothing to prevent the use of Administrator group when
the install is identified as "system" and those rights are actually
available (which they are as the scripting needs those rights in other
places).

> So, I guess a question is, does running with that option work as
> expected in your problematic instance?

No, it does not, see above.

The problem is actually a more knotty than you seem to think:
prominently ca-certificates and man-db get their knickers in a twist
when the group during post-install is different from the group of the
installed files and I suspect some other packages will run into similar
problems depending on how fussy they are with the group permissions.
The symptom is that you see failures from chmod (for whatever reason
"Invalid argument") when these programs try to swap the existing with
the newly gerenated (temporary) files.  In the case of man-db that
results in the /var/cache/man/index.db file getting removed (and
depending on the version the PID temporaries getting left in place), for
update-ca-trust the mkstemp temporaries will be left over and the
original files left in place.

So all installs from before the change to setup are affected if the
installation wasn't done via the GUI at least.

I think it would be best to have an option to directly specify a desired
group for both the installed files and running the post-install (which
already must be in the user token).  The default should be the primary
group of the user doing the installation.  I don't think the
installation should be group-owned by "Administrators" on Windows.  If
anything it makes it much more difficult to administer the installation
from within Cygwin as there doesn't seem to be a way to change to a
different than the primary group for domain accounts yet.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: [Bug] setup regression #2

[Jon Turney]

On 13/11/2022 12:47, Achim Gratz wrote:
> The problem is actually a more knotty than you seem to think:
> prominently ca-certificates and man-db get their knickers in a twist
> when the group during post-install is different from the group of the
> installed files and I suspect some other packages will run into similar
> problems depending on how fussy they are with the group permissions.

I take it seriously, otherwise I would have just released a setup with 
those changes.

I've reverted them for the moment, since I want to do a setup release 
now for other reasons.

Taking a step back:

I believe that the intent of the code in setup is that there should only 
be two modes:

USER: install "for me", with the users primary group
SYSTEM: install "for everyone", with the administrators primary group 
(only permitted if you are an administrator)

But in fact, this got broken this long ago, so we do something slightly 
different (Please take a look at the discussion around [1]).

If the intention isn't adequate, can you describe what is needed?

[1] https://cygwin.com/pipermail/cygwin-apps/2022-July/042144.html

Re: [Bug] setup regression #2

[Achim Gratz]

Jon Turney writes:
> I believe that the intent of the code in setup is that there should
> only be two modes:
>
> USER: install "for me", with the users primary group

As I understand it, the intention here was that the user can have a
"single user installation" in a place that they have access to (say,
their home directory) while they have no permission in one of the usual
places.  In a setup where that place is a certain type of share the user
will not be able to change the group the files are owned by anyway
(standard NetApp CIFS shares are set up this way) and it may not be the
users primary group.

> SYSTEM: install "for everyone", with the administrators primary group
> (only permitted if you are an administrator)

I don't see why the fact the installation is meant to be used by
multiple users means that the install must be owned by group
Administrators.  I'm not sure this is a good idea on Windows anyway, at
least when you don't put extra (inheritable) DACL on the install
folder.

I've never tried installing into the usual place (%ProgramFiles%) as
that means that Windows enforces a number of rules that are different
from Cygwin's and change non-domain vs. in-domain machines, applied GPO
etc.

So I'd really just introduce another parameter to specify what the group
the installer uses should be and have the default depend on whether the
user doing the install has administrative rights or not.  A warning
should be issued when that group is different from the existing root
directory and of course the whole install should abort if the requested
group can't be made primary.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: [Bug] setup regression #2

[Corinna Vinschen]

On Nov 20 20:05, Achim Gratz wrote:
> Jon Turney writes:
> > I believe that the intent of the code in setup is that there should
> > only be two modes:
> >
> > USER: install "for me", with the users primary group
> 
> As I understand it, the intention here was that the user can have a
> "single user installation" in a place that they have access to (say,
> their home directory) while they have no permission in one of the usual
> places.  In a setup where that place is a certain type of share the user
> will not be able to change the group the files are owned by anyway
> (standard NetApp CIFS shares are set up this way) and it may not be the
> users primary group.
> 
> > SYSTEM: install "for everyone", with the administrators primary group
> > (only permitted if you are an administrator)
> 
> I don't see why the fact the installation is meant to be used by
> multiple users means that the install must be owned by group
> Administrators.  I'm not sure this is a good idea on Windows anyway, at
> least when you don't put extra (inheritable) DACL on the install
> folder.

The idea is that the installation tree has POSIXy permissions and
administrative users have the right to change stuff.  The administrators
group is part of the user's token if the process has been started
elevated, so, to me, this looks like a natural choice.

The other advantage is that the administrators group has a fixed SID on
all systems, while other groups depend on the environment.  That goes
for the local group "None" just as well as for the "Domain Users"
group, etc.

I'm not adamant about this, it was just what was looking like being the
right thing to do at the time.  Especially I was not hot to make the
permission set more complicated than necessary for a POSIX-like system.


Corinna

Re: [Bug] setup regression #2

[ASSI]

Corinna Vinschen writes:
> The idea is that the installation tree has POSIXy permissions and
> administrative users have the right to change stuff.  The administrators
> group is part of the user's token if the process has been started
> elevated, so, to me, this looks like a natural choice.

As I said, I haven't thought through the implications of doing that.  We
certainly haven't done a security audit or anything like that
w.r.t. group ownership of the Cygwin tree and permission of the
installed files.

> The other advantage is that the administrators group has a fixed SID on
> all systems, while other groups depend on the environment.  That goes
> for the local group "None" just as well as for the "Domain Users"
> group, etc.

Yeah, a local non-domain installation currently installs as "None"
("Kein" in german Windows) and domain ones will have "Domain Users"

> I'm not adamant about this, it was just what was looking like being the
> right thing to do at the time.  Especially I was not hot to make the
> permission set more complicated than necessary for a POSIX-like system.

Agreed.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

Re: [Bug] setup regression #2

[Corinna Vinschen]

On Nov 21 13:39, ASSI wrote:
> Corinna Vinschen writes:
> > The idea is that the installation tree has POSIXy permissions and
> > administrative users have the right to change stuff.  The administrators
> > group is part of the user's token if the process has been started
> > elevated, so, to me, this looks like a natural choice.
> 
> As I said, I haven't thought through the implications of doing that.  We
> certainly haven't done a security audit or anything like that
> w.r.t. group ownership of the Cygwin tree and permission of the
> installed files.
> 
> > The other advantage is that the administrators group has a fixed SID on
> > all systems, while other groups depend on the environment.  That goes
> > for the local group "None" just as well as for the "Domain Users"
> > group, etc.
> 
> Yeah, a local non-domain installation currently installs as "None"
> ("Kein" in german Windows) and domain ones will have "Domain Users"

...both groups using the same RID is no accident @ MSFT :)


Corinna

Re: [Bug] setup regression #2

[Jon Turney]

On 20/11/2022 19:05, Achim Gratz wrote:
> Jon Turney writes:
>> I believe that the intent of the code in setup is that there should
>> only be two modes:
>>
>> USER: install "for me", with the users primary group
> 
> As I understand it, the intention here was that the user can have a
> "single user installation" in a place that they have access to (say,
> their home directory) while they have no permission in one of the usual
> places.  In a setup where that place is a certain type of share the user
> will not be able to change the group the files are owned by anyway
> (standard NetApp CIFS shares are set up this way) and it may not be the
> users primary group.
> 
>> SYSTEM: install "for everyone", with the administrators primary group
>> (only permitted if you are an administrator)
> 
> I don't see why the fact the installation is meant to be used by
> multiple users means that the install must be owned by group
> Administrators.  I'm not sure this is a good idea on Windows anyway, at
> least when you don't put extra (inheritable) DACL on the install
> folder.

Christian,

Maybe you can offer your opinion here, since you seem to have the 
opposite, or at least a different, point of view.

> I've never tried installing into the usual place (%ProgramFiles%) as
> that means that Windows enforces a number of rules that are different
> from Cygwin's and change non-domain vs. in-domain machines, applied GPO
> etc.
> 
> So I'd really just introduce another parameter to specify what the group
> the installer uses should be and have the default depend on whether the
> user doing the install has administrative rights or not.  A warning
> should be issued when that group is different from the existing root
> directory and of course the whole install should abort if the requested
> group can't be made primary.

Re: [Bug] setup regression #2

[Christian Franke]

Jon Turney wrote:
> On 20/11/2022 19:05, Achim Gratz wrote:
>> Jon Turney writes:
>>> I believe that the intent of the code in setup is that there should
>>> only be two modes:
>>>
>>> USER: install "for me", with the users primary group
>>
>> As I understand it, the intention here was that the user can have a
>> "single user installation" in a place that they have access to (say,
>> their home directory) while they have no permission in one of the usual
>> places.  In a setup where that place is a certain type of share the user
>> will not be able to change the group the files are owned by anyway
>> (standard NetApp CIFS shares are set up this way) and it may not be the
>> users primary group.
>>
>>> SYSTEM: install "for everyone", with the administrators primary group
>>> (only permitted if you are an administrator)
>>
>> I don't see why the fact the installation is meant to be used by
>> multiple users means that the install must be owned by group
>> Administrators.  I'm not sure this is a good idea on Windows anyway, at
>> least when you don't put extra (inheritable) DACL on the install
>> folder.
>
> Christian,
>
> Maybe you can offer your opinion here, since you seem to have the 
> opposite, or at least a different, point of view.

Anything installed with "All Users" option should IMO be protected 
against modifications by any regular non-elevated user.

This is not the case if the RID=513 group ("HOST\None", 
"DOMAIN\Domain-Users") is used. Many upstream projects install 
directories and files with permissions like 0664, 0775, 0660 or 0770. 
This is safe when the group is "root". On current Cygwin, all users have 
R/W access regardless of the "other" permission bits.

Try for example: find /usr/share ! -type l -perm -020 -ls (~4000 hits on 
my installation)

Using the administrators group as discussed here would solve this but 
apparently introduces interesting new permission problems with some 
packages. Could these possibly be solved by the maintainers of the 
affected packages?

An alternative may be: Keep the group as is, but prevent using above 
permissions as far as possible. For new packages, this could possibly be 
done with an enhancement of cygport. But I'm sure that there will also 
be subtle cases where these modified permissions break things. Cygport 
could allow to opt-out then.

Ideally the protection should also be effective for the non-elevated 
user who runs setup elevated. This is automatically the case for typical 
installers because Windows changes TokenOwner to administrators group if 
run elevated. That's why I provided 
https://sourceware.org/pipermail/cygwin-apps/2022-August/042221.html

Re: [Bug] setup regression #2

[Achim Gratz]

Christian Franke writes:
> Anything installed with "All Users" option should IMO be protected
> against modifications by any regular non-elevated user.

Yes.

> This is not the case if the RID=513 group ("HOST\None",
> "DOMAIN\Domain-Users") is used. Many upstream projects install
> directories and files with permissions like 0664, 0775, 0660 or
> 0770. This is safe when the group is "root". On current Cygwin, all
> users have R/W access regardless of the "other" permission bits.

Correct.  That's why I was hoping I could use a dedicated group (either
local or domain depending the install) for "Cygwin Administrators".

> Using the administrators group as discussed here would solve this but
> apparently introduces interesting new permission problems with some
> packages. Could these possibly be solved by the maintainers of the
> affected packages?

The problem is not the Administrators group per se AFAICT, but the change
from a different group to another mid-flight.  If the group could be
specified as alluded to above, I can keep the "wrong" group for existing
installs until I get around to fix their group ownership and ensure that
any new installs can be administered by whatever group of people will be
responsible for keeping things running smoothly.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [Bug] setup regression #2

[Jon Turney]

On 20/11/2022 17:16, Jon Turney wrote:
> On 13/11/2022 12:47, Achim Gratz wrote:
>> The problem is actually a more knotty than you seem to think:
>> prominently ca-certificates and man-db get their knickers in a twist
>> when the group during post-install is different from the group of the
>> installed files and I suspect some other packages will run into similar
>> problems depending on how fussy they are with the group permissions.
> 
> I take it seriously, otherwise I would have just released a setup with 
> those changes.
> 
> I've reverted them for the moment, since I want to do a setup release 
> now for other reasons.

I've reapplied these changes in the latest RC, since I'm assuming the 
problem(s) here are resolved as of [1], but it's very difficult to tell.

[1] 
https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=a5bcfe616c7e8f78f464bf045595d8213244876a