public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Allow overriding the home directory via the HOME variable
@ 2015-09-16 13:06 Johannes Schindelin
  2015-10-21 18:32 ` Corinna Vinschen
  2015-12-17 18:05 ` [PATCH v2 0/2] Support deriving the current user's home directory via HOME Johannes Schindelin
  0 siblings, 2 replies; 69+ messages in thread
From: Johannes Schindelin @ 2015-09-16 13:06 UTC (permalink / raw)
  To: cygwin-patches

	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
	nsswitch.conf that let's the environment variable HOME (or
	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
	directory.

	* ntsec.xml: Document the `env` schema.

Detailed comments:

In the context of Git for Windows, it is a well-established technique
to let the `$HOME` variable define where the current user's home
directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above.  This has the additional
advantage that it is much faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/cygheap.h |  3 ++-
 winsup/cygwin/uinfo.cc  | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 winsup/doc/ntsec.xml    | 20 ++++++++++++++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/cygheap.h b/winsup/cygwin/cygheap.h
index fd84814..097d50f 100644
--- a/winsup/cygwin/cygheap.h
+++ b/winsup/cygwin/cygheap.h
@@ -414,7 +414,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index da5809f..c74954a 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -780,6 +780,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -970,6 +972,40 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }
 
+static size_t
+fetch_env(LPCWSTR key, char *buf, size_t size)
+{
+  WCHAR wbuf[32767];
+  DWORD max = sizeof wbuf / sizeof *wbuf;
+  DWORD len = GetEnvironmentVariableW (key, wbuf, max);
+
+  if (!len || len >= max)
+    return 0;
+
+  len = sys_wcstombs (buf, size, wbuf);
+  return len && len < size ? len : 0;
+}
+
+static char *
+fetch_home_env (void)
+{
+  char home[32767];
+  size_t max = sizeof home / sizeof *home, len;
+
+  if (fetch_env (L"HOME", home, max)
+      || ((len = fetch_env (L"HOMEDRIVE", home, max))
+        && fetch_env (L"HOMEPATH", home + len, max - len))
+      || fetch_env (L"USERPROFILE", home, max))
+    {
+      tmp_pathbuf tp;
+      cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_ABSOLUTE,
+	  home, tp.c_get(), NT_MAX_PATH);
+      return strdup(tp.c_get());
+    }
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -1029,6 +1065,9 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1060,6 +1099,9 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1079,6 +1121,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1143,6 +1186,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  shell = fetch_from_description (ui->usri3_comment, L"shell=\"", 7);
@@ -1223,6 +1267,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1249,6 +1295,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  gecos = fetch_from_description (ui->usri3_comment, L"gecos=\"", 7);
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index ae0a119..9cee58c 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1354,6 +1354,16 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory from the environment variable
+	      <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.</listitem>
+  </varlistentry>
 </variablelist>
 
 <para>
@@ -1487,6 +1497,16 @@ of each schema when used with <literal>db_home:</literal>
 	      for a detailed description.</listitem>
   </varlistentry>
   <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory from the environment variable
+	      <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.</listitem>
+  </varlistentry>
+  <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
 	      in the <literal>ad_attribute</literal> attribute.  The path
-- 
2.5.2.windows.2


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

* Re: [PATCH] Allow overriding the home directory via the HOME variable
  2015-09-16 13:06 [PATCH] Allow overriding the home directory via the HOME variable Johannes Schindelin
@ 2015-10-21 18:32 ` Corinna Vinschen
  2015-10-22 15:38   ` Johannes Schindelin
  2015-12-17 18:05 ` [PATCH v2 0/2] Support deriving the current user's home directory via HOME Johannes Schindelin
  1 sibling, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2015-10-21 18:32 UTC (permalink / raw)
  To: cygwin-patches

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

On Sep 16 15:06, Johannes Schindelin wrote:
> 	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
> 	nsswitch.conf that let's the environment variable HOME (or
> 	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
> 	directory.
> 
> 	* ntsec.xml: Document the `env` schema.
> 
> Detailed comments:
> 
> In the context of Git for Windows, it is a well-established technique
> to let the `$HOME` variable define where the current user's home
> directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.
> 
> The idea is that we want to share user-specific settings between
> programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
> cannot blindly activate the "db_home: windows" setting because in some
> setups, the user's home directory is set to a hidden directory via an
> UNC path (\\share\some\hidden\folder$) -- something many programs
> cannot handle correctly.

-v, please.  Which applications can't handle that?  Why do we have to
care?

> The established technique is to allow setting the user's home directory
> via the environment variables mentioned above.  This has the additional
> advantage that it is much faster than querying the Windows user database.

But it's wrong.  We discussed this a couple of times on the Cygwin ML.
The underlying functionality generically implements the passwd entries.
Your "env" setting will return the same $HOME setting in the pw_dir
field for every user account.  All user accounts will have the same home
dir as your current user.  And the value is unreliable, too.  If another
user logs in, all accounts will have another $HOME, the one from the now
logged in user.  This is so wrong and potentially dangerous that I don't
think this belongs into Cygwin, sorry.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] Allow overriding the home directory via the HOME variable
  2015-10-21 18:32 ` Corinna Vinschen
@ 2015-10-22 15:38   ` Johannes Schindelin
  2015-10-23  9:10     ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2015-10-22 15:38 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Wed, 21 Oct 2015, Corinna Vinschen wrote:

> On Sep 16 15:06, Johannes Schindelin wrote:
> > 	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
> > 	nsswitch.conf that let's the environment variable HOME (or
> > 	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
> > 	directory.
> > 
> > 	* ntsec.xml: Document the `env` schema.
> > 
> > Detailed comments:
> > 
> > In the context of Git for Windows, it is a well-established technique
> > to let the `$HOME` variable define where the current user's home
> > directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.
> > 
> > The idea is that we want to share user-specific settings between
> > programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
> > cannot blindly activate the "db_home: windows" setting because in some
> > setups, the user's home directory is set to a hidden directory via an
> > UNC path (\\share\some\hidden\folder$) -- something many programs
> > cannot handle correctly.
> 
> -v, please.  Which applications can't handle that?  Why do we have to
> care?

Oh, the first one that comes to mind is `cmd.exe`. You cannot start
`cmd.exe` with a UNC working directory without getting complaints.

> > The established technique is to allow setting the user's home directory
> > via the environment variables mentioned above.  This has the additional
> > advantage that it is much faster than querying the Windows user database.
> 
> But it's wrong.  We discussed this a couple of times on the Cygwin ML.
> The underlying functionality generically implements the passwd entries.
> Your "env" setting will return the same $HOME setting in the pw_dir
> field for every user account.

No, it will not, because most users are not administrators. So they can
only set environment variables for themselves.

In most cases, `HOME` will not even be defined, but `HOMEDRIVE` and
`HOMEPATH` will, and they will be correct.

> All user accounts will have the same home dir as your current user.  And
> the value is unreliable, too.  If another user logs in, all accounts
> will have another $HOME, the one from the now logged in user.  This is
> so wrong and potentially dangerous that I don't think this belongs into
> Cygwin, sorry.

I ask you to reconsider. I am not trying to make this the default. And I
*need* a way to heed the `HOME` variable set by the user. For
backwards-compatibility with Git for Windows 1.x, where users frequently
adjusted the `HOME` variable to fix problems with MSys not handling their
home directory properly.

The patch itself is so simple that it cannot possibly cause a maintenance
burden, and by offering this feature as opt-in, users who are in need of
that feature can easily use it -- in Cygwin, MSys2, and yes, in Git for
Windows.

It would bring a major benefit to us.

Ciao,
Johannes

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

* Re: [PATCH] Allow overriding the home directory via the HOME variable
  2015-10-22 15:38   ` Johannes Schindelin
@ 2015-10-23  9:10     ` Corinna Vinschen
  2015-10-23  9:41       ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2015-10-23  9:10 UTC (permalink / raw)
  To: cygwin-patches

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

On Oct 22 17:38, Johannes Schindelin wrote:
> Hi Corinna,
> 
> On Wed, 21 Oct 2015, Corinna Vinschen wrote:
> 
> > On Sep 16 15:06, Johannes Schindelin wrote:
> > > 	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
> > > 	nsswitch.conf that let's the environment variable HOME (or
> > > 	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
> > > 	directory.
> > > 
> > > 	* ntsec.xml: Document the `env` schema.
> > > 
> > > Detailed comments:
> > > 
> > > In the context of Git for Windows, it is a well-established technique
> > > to let the `$HOME` variable define where the current user's home
> > > directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.
> > > 
> > > The idea is that we want to share user-specific settings between
> > > programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
> > > cannot blindly activate the "db_home: windows" setting because in some
> > > setups, the user's home directory is set to a hidden directory via an
> > > UNC path (\\share\some\hidden\folder$) -- something many programs
> > > cannot handle correctly.
> > 
> > -v, please.  Which applications can't handle that?  Why do we have to
> > care?
> 
> Oh, the first one that comes to mind is `cmd.exe`. You cannot start
> `cmd.exe` with a UNC working directory without getting complaints.

Sure, but then again, why do we have to care?  Didn't you say GfW is
using bash?

> > > The established technique is to allow setting the user's home directory
> > > via the environment variables mentioned above.  This has the additional
> > > advantage that it is much faster than querying the Windows user database.
> > 
> > But it's wrong.  We discussed this a couple of times on the Cygwin ML.
> > The underlying functionality generically implements the passwd entries.
> > Your "env" setting will return the same $HOME setting in the pw_dir
> > field for every user account.
> 
> No, it will not, because most users are not administrators. So they can
> only set environment variables for themselves.

You're looking at the problem from the wrong direction.  Consider how
the mechanism works.  The setting in /etc/nsswitch.conf configures how
the passwd/group entries for all accounts are created.  Your "env"
setting fetches the environment of the *current* user and generates
the passwd entry for *all* user accounts from it.  Here's what happens:

  $ echo $USER
  corinna
  $ echo $HOME
  /home/corinna
  $ getent passwd $USER
  corinna:*:1049577:1049701:U-VINSCHEN\corinna,S-1-5-21-913048732-1697188782-3448811101-1001:/home/corinna:/bin/tcsh
  $ getent passwd Guest
  Guest:*:1049077:1049090:U-VINSCHEN\Guest,S-1-5-21-2913048732-1697188782-3448811101-501:/home/corinna:/bin/bash
  $ getent passwd Administrator
  Administrator:*:1049076:1049089:U-VINSCHEN\Administrator,S-1-5-21-2913048732-1697188782-3448811101-500:/home/corinna:/bin/bash
  [...]

This is plain wrong.  You can't seriously provide a mechanism which
fetches data from the current user to generate the output for all users
from there.

> In most cases, `HOME` will not even be defined, but `HOMEDRIVE` and
> `HOMEPATH` will, and they will be correct.

That's not the point.

> > All user accounts will have the same home dir as your current user.  And
> > the value is unreliable, too.  If another user logs in, all accounts
> > will have another $HOME, the one from the now logged in user.  This is
> > so wrong and potentially dangerous that I don't think this belongs into
> > Cygwin, sorry.
> 
> I ask you to reconsider.

I did, but the result is the same.  As is, the patch is not acceptable.

Here's what you *could* do:  Create a patch which fetches the pw_home
info from the current user environment for the current user only.  For
all other accounts, the info must be taken from elsewhere, one of the
other methods.  E.g., the env setting only affects the current user
passwd entry, not any other.  For all other accounts, the env method
provides no result, thus falling back to the next method in the row.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] Allow overriding the home directory via the HOME variable
  2015-10-23  9:10     ` Corinna Vinschen
@ 2015-10-23  9:41       ` Corinna Vinschen
  2015-10-23 12:00         ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2015-10-23  9:41 UTC (permalink / raw)
  To: cygwin-patches

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

On Oct 23 11:10, Corinna Vinschen wrote:
> On Oct 22 17:38, Johannes Schindelin wrote:
> > Hi Corinna,
> > 
> > On Wed, 21 Oct 2015, Corinna Vinschen wrote:
> > 
> > > On Sep 16 15:06, Johannes Schindelin wrote:
> > > > 	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
> > > > 	nsswitch.conf that let's the environment variable HOME (or
> > > > 	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
> > > > 	directory.
> > > > 
> > > > 	* ntsec.xml: Document the `env` schema.
> > > > 
> > > > Detailed comments:
> > > > 
> > > > In the context of Git for Windows, it is a well-established technique
> > > > to let the `$HOME` variable define where the current user's home
> > > > directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.
> > > > 
> > > > The idea is that we want to share user-specific settings between
> > > > programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
> > > > cannot blindly activate the "db_home: windows" setting because in some
> > > > setups, the user's home directory is set to a hidden directory via an
> > > > UNC path (\\share\some\hidden\folder$) -- something many programs
> > > > cannot handle correctly.
> > > 
> > > -v, please.  Which applications can't handle that?  Why do we have to
> > > care?
> > 
> > Oh, the first one that comes to mind is `cmd.exe`. You cannot start
> > `cmd.exe` with a UNC working directory without getting complaints.
> 
> Sure, but then again, why do we have to care?  Didn't you say GfW is
> using bash?

In particular, it affects all other native applications as well.  If that
home setting works for the user outside GfW/Cygwin, and given Cygwin apps
don't care, why should this suddenly be a problem for GfW?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] Allow overriding the home directory via the HOME variable
  2015-10-23  9:41       ` Corinna Vinschen
@ 2015-10-23 12:00         ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2015-10-23 12:00 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Fri, 23 Oct 2015, Corinna Vinschen wrote:

> On Oct 23 11:10, Corinna Vinschen wrote:
> > On Oct 22 17:38, Johannes Schindelin wrote:
> > > 
> > > On Wed, 21 Oct 2015, Corinna Vinschen wrote:
> > > 
> > > > On Sep 16 15:06, Johannes Schindelin wrote:
> > > > > 	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
> > > > > 	nsswitch.conf that let's the environment variable HOME (or
> > > > > 	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
> > > > > 	directory.
> > > > > 
> > > > > 	* ntsec.xml: Document the `env` schema.
> > > > > 
> > > > > Detailed comments:
> > > > > 
> > > > > In the context of Git for Windows, it is a well-established technique
> > > > > to let the `$HOME` variable define where the current user's home
> > > > > directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.
> > > > > 
> > > > > The idea is that we want to share user-specific settings between
> > > > > programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
> > > > > cannot blindly activate the "db_home: windows" setting because in some
> > > > > setups, the user's home directory is set to a hidden directory via an
> > > > > UNC path (\\share\some\hidden\folder$) -- something many programs
> > > > > cannot handle correctly.
> > > > 
> > > > -v, please.  Which applications can't handle that?  Why do we have to
> > > > care?
> > > 
> > > Oh, the first one that comes to mind is `cmd.exe`. You cannot start
> > > `cmd.exe` with a UNC working directory without getting complaints.
> > 
> > Sure, but then again, why do we have to care?  Didn't you say GfW is
> > using bash?
> 
> In particular, it affects all other native applications as well.  If that
> home setting works for the user outside GfW/Cygwin, and given Cygwin apps
> don't care, why should this suddenly be a problem for GfW?

I did say that Git for Windows uses bash to execute the shell scripts that
are part of Git. However, this says nothing about the *entry* point into
Git for Windows, which is quite often `cmd.exe`. Even worse: users are
free to provide hooks as batch scripts, in which case you encounter that
bad bug again.

The point you make about getent is a good one, I did indeed misunderstand
under what circumstances the modified code path is hit. I will change it
so that it only triggers for the current user.

Ciao,
Johannes

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

* [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2015-12-17 18:05 ` [PATCH v2 0/2] Support deriving the current user's home directory via HOME Johannes Schindelin
  2015-12-17 18:05   ` [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account Johannes Schindelin
@ 2015-12-17 18:05   ` Johannes Schindelin
  2015-12-17 20:20     ` Corinna Vinschen
  2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2015-12-17 18:05 UTC (permalink / raw)
  To: cygwin-patches

	* uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
	nsswitch.conf that let's the environment variable HOME (or
	HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
	directory of the current user.

	* ntsec.xml: Document the `env` schema.

Detailed comments:

In the context of Git for Windows, it is a well-established technique
to let the `$HOME` variable define where the current user's home
directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly, e.g. cmd.exe and other native Windows
applications that users want to employ as Git helpers.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above.  This has the additional
advantage that it is much faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/cygheap.h |  3 ++-
 winsup/cygwin/uinfo.cc  | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 winsup/doc/ntsec.xml    | 20 ++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/cygheap.h b/winsup/cygwin/cygheap.h
index fd84814..097d50f 100644
--- a/winsup/cygwin/cygheap.h
+++ b/winsup/cygwin/cygheap.h
@@ -414,7 +414,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index c9b3e09..a5d6270 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -780,6 +780,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -970,6 +972,40 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }
 
+static size_t
+fetch_env(LPCWSTR key, char *buf, size_t size)
+{
+  WCHAR wbuf[32767];
+  DWORD max = sizeof wbuf / sizeof *wbuf;
+  DWORD len = GetEnvironmentVariableW (key, wbuf, max);
+
+  if (!len || len >= max)
+    return 0;
+
+  len = sys_wcstombs (buf, size, wbuf, len);
+  return len && len < size ? len : 0;
+}
+
+static char *
+fetch_home_env (void)
+{
+  char home[32767];
+  size_t max = sizeof home / sizeof *home, len;
+
+  if (fetch_env (L"HOME", home, max)
+      || ((len = fetch_env (L"HOMEDRIVE", home, max))
+        && fetch_env (L"HOMEPATH", home + len, max - len))
+      || fetch_env (L"USERPROFILE", home, max))
+    {
+      tmp_pathbuf tp;
+      cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_ABSOLUTE,
+	  home, tp.c_get(), NT_MAX_PATH);
+      return strdup(tp.c_get());
+    }
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -1029,6 +1065,10 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1060,6 +1100,10 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1079,6 +1123,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1143,6 +1188,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  shell = fetch_from_description (ui->usri3_comment, L"shell=\"", 7);
@@ -1223,6 +1269,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1249,6 +1297,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  gecos = fetch_from_description (ui->usri3_comment, L"gecos=\"", 7);
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index ae0a119..14c37c5 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1354,6 +1354,16 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.</listitem>
+  </varlistentry>
 </variablelist>
 
 <para>
@@ -1487,6 +1497,16 @@ of each schema when used with <literal>db_home:</literal>
 	      for a detailed description.</listitem>
   </varlistentry>
   <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.</listitem>
+  </varlistentry>
+  <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
 	      in the <literal>ad_attribute</literal> attribute.  The path
-- 
2.6.3.windows.1.300.g1c25e49


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

* [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account
  2015-12-17 18:05 ` [PATCH v2 0/2] Support deriving the current user's home directory via HOME Johannes Schindelin
@ 2015-12-17 18:05   ` Johannes Schindelin
  2015-12-17 20:49     ` Corinna Vinschen
  2015-12-17 18:05   ` [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
  2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2015-12-17 18:05 UTC (permalink / raw)
  To: cygwin-patches

We should not blindly set the home directory of the SYSTEM account to
/home/SYSTEM, especially not when that value disagrees with what is
configured via the `db_home` line in the `/etc/nsswitch.conf` file.

This fixes https://github.com/git-for-windows/git/issues/435

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index a5d6270..8c51b82 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -2129,7 +2129,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home (pldap, sid, dom, domain, name,
+				       fully_qualified_name);
+	}
       switch (acc_type)
       	{
 	case SidTypeUser:
-- 
2.6.3.windows.1.300.g1c25e49

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

* [PATCH v2 0/2] Support deriving the current user's home directory via HOME
  2015-09-16 13:06 [PATCH] Allow overriding the home directory via the HOME variable Johannes Schindelin
  2015-10-21 18:32 ` Corinna Vinschen
@ 2015-12-17 18:05 ` Johannes Schindelin
  2015-12-17 18:05   ` [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account Johannes Schindelin
                     ` (2 more replies)
  1 sibling, 3 replies; 69+ messages in thread
From: Johannes Schindelin @ 2015-12-17 18:05 UTC (permalink / raw)
  To: cygwin-patches

This patch mini-series supports Git for Windows' main strategy to
determine the current user's home directory by looking at the
environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
if these variables are also unset, to USERPROFILE.

This strategy is a quick method to determine the home directory, and it
also allows users to override the home directory easily (e.g. in case
that their real home directory is a network share that is not all that
well handled by some commands such as cmd.exe's cd command).

Sorry for sending out v2 so late...!


Johannes Schindelin (2):
  Allow deriving the current user's home directory via the HOME variable
  Respect `db_home` setting even for the SYSTEM account

 winsup/cygwin/cygheap.h |  3 ++-
 winsup/cygwin/uinfo.cc  | 55 ++++++++++++++++++++++++++++++++++++++++++++++++-
 winsup/doc/ntsec.xml    | 20 ++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)

Interdiff vs v1:

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 525a90e..8c51b82 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -1066,7 +1066,8 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	    }
 	  break;
 	case NSS_SCHEME_ENV:
-	  home = fetch_home_env ();
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
 	  break;
 	}
     }
@@ -1100,7 +1101,8 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 				  dom, NULL, name, full_qualified);
 	  break;
 	case NSS_SCHEME_ENV:
-	  home = fetch_home_env ();
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
 	  break;
 	}
     }
@@ -2127,7 +2129,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home (pldap, sid, dom, domain, name,
+				       fully_qualified_name);
+	}
       switch (acc_type)
       	{
 	case SidTypeUser:
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index 9cee58c..14c37c5 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1356,8 +1356,8 @@ schemata are the following:
   </varlistentry>
   <varlistentry>
     <term><literal>env</literal></term>
-    <listitem>Derives the home directory from the environment variable
-	      <literal>HOME</literal> (falling back to
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
 	      <literal>HOMEDRIVE\HOMEPATH</literal> and
 	      <literal>USERPROFILE</literal>, in that order).  This is faster
 	      than the <term><literal>windows</literal></term> schema at the
@@ -1498,8 +1498,8 @@ of each schema when used with <literal>db_home:</literal>
   </varlistentry>
   <varlistentry>
     <term><literal>env</literal></term>
-    <listitem>Derives the home directory from the environment variable
-	      <literal>HOME</literal> (falling back to
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
 	      <literal>HOMEDRIVE\HOMEPATH</literal> and
 	      <literal>USERPROFILE</literal>, in that order).  This is faster
 	      than the <term><literal>windows</literal></term> schema at the

-- 
2.6.3.windows.1.300.g1c25e49

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2015-12-17 18:05   ` [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2015-12-17 20:20     ` Corinna Vinschen
  2022-09-21 11:58       ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2015-12-17 20:20 UTC (permalink / raw)
  To: cygwin-patches

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

Hi Johannes,

a few comments...

On Dec 17 19:05, Johannes Schindelin wrote:
> [...]
> diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> index c9b3e09..a5d6270 100644
> --- a/winsup/cygwin/uinfo.cc
> +++ b/winsup/cygwin/uinfo.cc
> [...]
> +static size_t
> +fetch_env(LPCWSTR key, char *buf, size_t size)
           ^^^
           space

> +{
> +  WCHAR wbuf[32767];

Ok, there are a couple of problems here.  First, since this buffer
is a filename buffer, use NT_MAX_PATH from winsup.h as buffer size.

But then again, please avoid allocating 64K buffers on the stack.
That's what tmp_pathbuf:w_get () is for.

> +  DWORD max = sizeof wbuf / sizeof *wbuf;
> +  DWORD len = GetEnvironmentVariableW (key, wbuf, max);

This call to GetEnvironmentVariableW looks gratuitous to me.  Why don't
you simply call getenv?  It did the entire job already, it avoids the
requirement for a local buffer, and in case of $HOME it even did the
Win32->POSIX path conversion.  If there's a really good reason for using
GetEnvironmentVariableW it begs at least for a longish comment.

> +
> +  if (!len || len >= max)
> +    return 0;
> +
> +  len = sys_wcstombs (buf, size, wbuf, len);
> +  return len && len < size ? len : 0;
> +}
> +
> +static char *
> +fetch_home_env (void)
> +{
> +  char home[32767];
> +  size_t max = sizeof home / sizeof *home, len;
> +
> +  if (fetch_env (L"HOME", home, max)
> +      || ((len = fetch_env (L"HOMEDRIVE", home, max))
> +        && fetch_env (L"HOMEPATH", home + len, max - len))
> +      || fetch_env (L"USERPROFILE", home, max))
> +    {
> +      tmp_pathbuf tp;
> +      cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_ABSOLUTE,
> +	  home, tp.c_get(), NT_MAX_PATH);
                       ^^^
                       space
> +      return strdup(tp.c_get());
                     ^^^      ^^^
                     space......s

Whoa, tp.c_get() twice to access the same space?  That's a dirty trick
which may puzzle later readers of the code and heavily depends on
knowing the internals of tmp_pathbuf.  Please use a variable and only
assign tp.c_get () once.

OTOH, the above's a case for a cygwin_create_path call, rather than
cygwin_conv_path+strdup.  Also, if there's *really* a good reason to use
GetEnvironmentVariableW, you should collapse sys_wcstombs+cygwin_conv_path+
strdup into a single cygwin_create_path (CCP_WIN_W_TO_POSIX, ...).

> [...]
> @@ -1079,6 +1123,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
>  	case NSS_SCHEME_FALLBACK:
>  	  return NULL;
>  	case NSS_SCHEME_WINDOWS:
> +	case NSS_SCHEME_ENV:
>  	  break;
>  	case NSS_SCHEME_CYGWIN:
>  	  if (pldap->fetch_ad_account (sid, false, dnsdomain))

You know that I don't exactly like the "env" idea, but if we implement
it anyway, wouldn't it make sense to add some kind of $SHELL handling as
well, for symmetry?

> [...]
> @@ -1487,6 +1497,16 @@ of each schema when used with <literal>db_home:</literal>
>  	      for a detailed description.</listitem>
>    </varlistentry>
>    <varlistentry>
> +    <term><literal>env</literal></term>
> +    <listitem>Derives the home directory of the current user from the
> +	      environment variable <literal>HOME</literal> (falling back to
> +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
> +	      <literal>USERPROFILE</literal>, in that order).  This is faster
> +	      than the <term><literal>windows</literal></term> schema at the
> +	      expense of determining only the current user's home directory
> +	      correctly.</listitem>

In both case of the documentation it might make sense to add a few words
along the lines of "This schema is skipped for any other account",
wouldn't it?


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account
  2015-12-17 18:05   ` [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account Johannes Schindelin
@ 2015-12-17 20:49     ` Corinna Vinschen
  2015-12-17 21:02       ` Corinna Vinschen
  2022-09-21 12:00       ` Johannes Schindelin
  0 siblings, 2 replies; 69+ messages in thread
From: Corinna Vinschen @ 2015-12-17 20:49 UTC (permalink / raw)
  To: cygwin-patches

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

On Dec 17 19:05, Johannes Schindelin wrote:
> We should not blindly set the home directory of the SYSTEM account to
> /home/SYSTEM, especially not when that value disagrees with what is
> configured via the `db_home` line in the `/etc/nsswitch.conf` file.
> 
> This fixes https://github.com/git-for-windows/git/issues/435
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/uinfo.cc | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> index a5d6270..8c51b82 100644
> --- a/winsup/cygwin/uinfo.cc
> +++ b/winsup/cygwin/uinfo.cc
> @@ -2129,7 +2129,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
>  	 it to a well-known group here. */
>        if (acc_type == SidTypeUser
>  	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
> -	acc_type = SidTypeWellKnownGroup;
> +	{
> +	  acc_type = SidTypeWellKnownGroup;
> +	  home = cygheap->pg.get_home (pldap, sid, dom, domain, name,
> +				       fully_qualified_name);

Uhm, that's a bit over the top, isn't it?  It will affect all S-1-5-X
accounts as well as the S-1-5-11 Windows account SIDs.  Is that really
what you want?

Using pldap here may SEGV in cygheap_pwdgrp::get_home, btw, because
it may be NULL.  cygheap_pwdgrp::get_home doesn't check pldap for
validity, it expects a valid pointer.  You could either use cldap, or
cygheap_pwdgrp::get_home would have to check pldap before using it.

However, either way there's another problem: Independently of the
configured db_home schemes, you don't want to ask the DC for info on
these builtin accounts.  The better approach might be to call the
PUSER_INFO_3 variant of cygheap_pwdgrp::get_home with a NULL ui
pointer and just check for ui in the NSS_SCHEME_DESC case.  The other
called functions fetch_windows_home and fetch_from_path both can
live with both pointers, pldap and ui being NULL.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account
  2015-12-17 20:49     ` Corinna Vinschen
@ 2015-12-17 21:02       ` Corinna Vinschen
  2022-09-21 12:00       ` Johannes Schindelin
  1 sibling, 0 replies; 69+ messages in thread
From: Corinna Vinschen @ 2015-12-17 21:02 UTC (permalink / raw)
  To: cygwin-patches

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

On Dec 17 21:49, Corinna Vinschen wrote:
> On Dec 17 19:05, Johannes Schindelin wrote:
> > We should not blindly set the home directory of the SYSTEM account to
> > /home/SYSTEM, especially not when that value disagrees with what is
> > configured via the `db_home` line in the `/etc/nsswitch.conf` file.
> > 
> > This fixes https://github.com/git-for-windows/git/issues/435
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  winsup/cygwin/uinfo.cc | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index a5d6270..8c51b82 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > @@ -2129,7 +2129,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
> >  	 it to a well-known group here. */
> >        if (acc_type == SidTypeUser
> >  	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
> > -	acc_type = SidTypeWellKnownGroup;
> > +	{
> > +	  acc_type = SidTypeWellKnownGroup;
> > +	  home = cygheap->pg.get_home (pldap, sid, dom, domain, name,
> > +				       fully_qualified_name);
> 
> Uhm, that's a bit over the top, isn't it?  It will affect all S-1-5-X
> accounts as well as the S-1-5-11 Windows account SIDs.  Is that really

s/S-1-5-11/S-1-11/, sorry.

> what you want?
> 
> Using pldap here may SEGV in cygheap_pwdgrp::get_home, btw, because
> it may be NULL.  cygheap_pwdgrp::get_home doesn't check pldap for
> validity, it expects a valid pointer.  You could either use cldap, or
> cygheap_pwdgrp::get_home would have to check pldap before using it.
> 
> However, either way there's another problem: Independently of the
> configured db_home schemes, you don't want to ask the DC for info on
> these builtin accounts.  The better approach might be to call the
> PUSER_INFO_3 variant of cygheap_pwdgrp::get_home with a NULL ui
> pointer and just check for ui in the NSS_SCHEME_DESC case.  The other
> called functions fetch_windows_home and fetch_from_path both can
> live with both pointers, pldap and ui being NULL.

Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* [PATCH v3 0/3] Support deriving the current user's home directory via HOME
  2015-12-17 18:05 ` [PATCH v2 0/2] Support deriving the current user's home directory via HOME Johannes Schindelin
  2015-12-17 18:05   ` [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account Johannes Schindelin
  2015-12-17 18:05   ` [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2022-09-21 11:51   ` Johannes Schindelin
  2022-09-21 11:51     ` [PATCH v3 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
                       ` (3 more replies)
  2 siblings, 4 replies; 69+ messages in thread
From: Johannes Schindelin @ 2022-09-21 11:51 UTC (permalink / raw)
  To: cygwin-patches

This patch mini-series supports Git for Windows' main strategy to
determine the current user's home directory by looking at the
environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
if these variables are also unset, to USERPROFILE.

This strategy is a quick method to determine the home directory, and it
also allows users to override the home directory easily (e.g. in case
that their real home directory is a network share that is not all that
well handled by some commands such as cmd.exe's cd command).

Sorry for sending out v3 sooooo late...!

Changes since v2:
- Using `getenv()` and `cygwin_create_path()` instead of the
  `GetEnvironmentVariableW()`/`cygwin_conv_path()` dance
- Adjusted the documentation to drive home that this only affects the
  _current_ user's home directory
- Using the `PUSER_INFO_3` variant of `get_home()`
- Adjusted the commit messages
- Added another patch, to support "ad-hoc cloud accounts"

Johannes Schindelin (3):
  Allow deriving the current user's home directory via the HOME variable
  Respect `db_home` setting even for SYSTEM/Microsoft accounts
  Respect `db_home: env` even when no uid can be determined

 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 54 ++++++++++++++++++++++++--
 winsup/doc/ntsec.xml                   | 22 +++++++++++
 3 files changed, 75 insertions(+), 4 deletions(-)

Range-diff:
1:  047fe1d78c ! 1:  6f8fe89d9d Allow deriving the current user's home directory via the HOME variable
    @@ Metadata
      ## Commit message ##
         Allow deriving the current user's home directory via the HOME variable

    -            * uinfo.cc (cygheap_pwdgrp::get_home): Offer an option in
    -            nsswitch.conf that let's the environment variable HOME (or
    -            HOMEDRIVE & HOMEPATH, or USERPROFILE) define the home
    -            directory of the current user.
    -
    -            * ntsec.xml: Document the `env` schema.
    -
    -    Detailed comments:
    -
    -    In the context of Git for Windows, it is a well-established technique
    -    to let the `$HOME` variable define where the current user's home
    -    directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`.
    +    This patch hails from Git for Windows (where the Cygwin runtime is used
    +    in the form of a slightly modified MSYS2 runtime), where it is a
    +    well-established technique to let the `$HOME` variable define where the
    +    current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
    +    and `$USERPROFILE`.

         The idea is that we want to share user-specific settings between
    -    programs, whether they be Cygwin, MSys2 or not.  Unfortunately, we
    +    programs, whether they be Cygwin, MSYS2 or not.  Unfortunately, we
         cannot blindly activate the "db_home: windows" setting because in some
         setups, the user's home directory is set to a hidden directory via an
         UNC path (\\share\some\hidden\folder$) -- something many programs
    -    cannot handle correctly, e.g. cmd.exe and other native Windows
    +    cannot handle correctly, e.g. `cmd.exe` and other native Windows
         applications that users want to employ as Git helpers.

         The established technique is to allow setting the user's home directory
    -    via the environment variables mentioned above.  This has the additional
    -    advantage that it is much faster than querying the Windows user database.
    +    via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or
    +    `$USERPROFILE`.  This has the additional advantage that it is much
    +    faster than querying the Windows user database.

         Of course this scheme needs to be opt-in.  For that reason, it needs
         to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

    - ## winsup/cygwin/cygheap.h ##
    -@@ winsup/cygwin/cygheap.h: public:
    + ## winsup/cygwin/local_includes/cygheap.h ##
    +@@ winsup/cygwin/local_includes/cygheap.h: public:
          NSS_SCHEME_UNIX,
          NSS_SCHEME_DESC,
          NSS_SCHEME_PATH,
    @@ winsup/cygwin/uinfo.cc: fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygps
        return ret;
      }

    -+static size_t
    -+fetch_env(LPCWSTR key, char *buf, size_t size)
    -+{
    -+  WCHAR wbuf[32767];
    -+  DWORD max = sizeof wbuf / sizeof *wbuf;
    -+  DWORD len = GetEnvironmentVariableW (key, wbuf, max);
    -+
    -+  if (!len || len >= max)
    -+    return 0;
    -+
    -+  len = sys_wcstombs (buf, size, wbuf, len);
    -+  return len && len < size ? len : 0;
    -+}
    -+
     +static char *
     +fetch_home_env (void)
     +{
    -+  char home[32767];
    -+  size_t max = sizeof home / sizeof *home, len;
    ++  tmp_pathbuf tp;
    ++  char *p, *q;
    ++  const char *home, *home_drive, *home_path;
     +
    -+  if (fetch_env (L"HOME", home, max)
    -+      || ((len = fetch_env (L"HOMEDRIVE", home, max))
    -+        && fetch_env (L"HOMEPATH", home + len, max - len))
    -+      || fetch_env (L"USERPROFILE", home, max))
    -+    {
    -+      tmp_pathbuf tp;
    -+      cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_ABSOLUTE,
    -+	  home, tp.c_get(), NT_MAX_PATH);
    -+      return strdup(tp.c_get());
    -+    }
    ++  if ((home = getenv ("HOME"))
    ++      || ((home_drive = getenv ("HOMEDRIVE"))
    ++	  && (home_path = getenv ("HOMEPATH"))
    ++	  // concatenate HOMEDRIVE and HOMEPATH
    ++          && (home = p = tp.c_get ())
    ++	  && (q = stpncpy (p, home_drive, NT_MAX_PATH))
    ++          && strlcpy (q, home_path, NT_MAX_PATH - (q - p)))
    ++      || (home = getenv ("USERPROFILE")))
    ++    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
     +
     +  return NULL;
     +}
    @@ winsup/cygwin/uinfo.cc: cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid
     +	case NSS_SCHEME_ENV:
      	  break;
      	case NSS_SCHEME_DESC:
    - 	  shell = fetch_from_description (ui->usri3_comment, L"shell=\"", 7);
    + 	  if (ui)
     @@ winsup/cygwin/uinfo.cc: cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
      		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
      	    }
    @@ winsup/cygwin/uinfo.cc: cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid
     +	case NSS_SCHEME_ENV:
      	  break;
      	case NSS_SCHEME_DESC:
    - 	  gecos = fetch_from_description (ui->usri3_comment, L"gecos=\"", 7);
    + 	  if (ui)

      ## winsup/doc/ntsec.xml ##
     @@ winsup/doc/ntsec.xml: schemata are the following:
    @@ winsup/doc/ntsec.xml: schemata are the following:
     +	      <literal>USERPROFILE</literal>, in that order).  This is faster
     +	      than the <term><literal>windows</literal></term> schema at the
     +	      expense of determining only the current user's home directory
    -+	      correctly.</listitem>
    ++	      correctly.  This schema is skipped for any other account.
    ++	      </listitem>
     +  </varlistentry>
      </variablelist>

    @@ winsup/doc/ntsec.xml: of each schema when used with <literal>db_home:</literal>
     +	      <literal>USERPROFILE</literal>, in that order).  This is faster
     +	      than the <term><literal>windows</literal></term> schema at the
     +	      expense of determining only the current user's home directory
    -+	      correctly.</listitem>
    ++	      correctly.  This schema is skipped for any other account.
    ++	      </listitem>
     +  </varlistentry>
        <varlistentry>
          <term><literal>@ad_attribute</literal></term>
2:  90c5b45fbe ! 2:  1b4ee89aa7 Respect `db_home` setting even for the SYSTEM account
    @@ Metadata
     Author: Johannes Schindelin <johannes.schindelin@gmx.de>

      ## Commit message ##
    -    Respect `db_home` setting even for the SYSTEM account
    +    Respect `db_home` setting even for SYSTEM/Microsoft accounts

    -    We should not blindly set the home directory of the SYSTEM account to
    -    /home/SYSTEM, especially not when that value disagrees with what is
    -    configured via the `db_home` line in the `/etc/nsswitch.conf` file.
    -
    -    This fixes https://github.com/git-for-windows/git/issues/435
    +    We should not blindly set the home directory of the SYSTEM account (or
    +    of Microsoft accounts) to /home/SYSTEM, especially not when that value
    +    disagrees with what is configured via the `db_home` line in the
    +    `/etc/nsswitch.conf` file.

         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

    @@ winsup/cygwin/uinfo.cc: pwdgrp::fetch_account_from_windows (fetch_user_arg_t &ar
     -	acc_type = SidTypeWellKnownGroup;
     +	{
     +	  acc_type = SidTypeWellKnownGroup;
    -+	  home = cygheap->pg.get_home (pldap, sid, dom, domain, name,
    ++	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
     +				       fully_qualified_name);
     +	}
    -       switch (acc_type)
    -       	{
    +       switch ((int) acc_type)
    + 	{
      	case SidTypeUser:
-:  ---------- > 3:  4d90319e44 Respect `db_home: env` even when no uid can be determined

base-commit: a68e99f8839e4697790077c8a77b506d528cc674
Published-As: https://github.com/dscho/msys2-runtime/releases/tag/home-env-cygwin-v3
Fetch-It-Via: git fetch https://github.com/dscho/msys2-runtime home-env-cygwin-v3

--
2.38.0.rc0.windows.1


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

* [PATCH v3 1/3] Allow deriving the current user's home directory via the HOME variable
  2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
@ 2022-09-21 11:51     ` Johannes Schindelin
  2022-09-21 11:52     ` [PATCH v3 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2022-09-21 11:51 UTC (permalink / raw)
  To: cygwin-patches

This patch hails from Git for Windows (where the Cygwin runtime is used
in the form of a slightly modified MSYS2 runtime), where it is a
well-established technique to let the `$HOME` variable define where the
current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSYS2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly, e.g. `cmd.exe` and other native Windows
applications that users want to employ as Git helpers.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or
`$USERPROFILE`.  This has the additional advantage that it is much
faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/local_includes/cygheap.h |  3 ++-
 winsup/cygwin/uinfo.cc                 | 35 ++++++++++++++++++++++++++
 winsup/doc/ntsec.xml                   | 22 ++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/local_includes/cygheap.h b/winsup/cygwin/local_includes/cygheap.h
index 6a844babdb..444ca8b55c 100644
--- a/winsup/cygwin/local_includes/cygheap.h
+++ b/winsup/cygwin/local_includes/cygheap.h
@@ -408,7 +408,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index ce997c0f82..6e673ee39e 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -754,6 +754,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -942,6 +944,26 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }

+static char *
+fetch_home_env (void)
+{
+  tmp_pathbuf tp;
+  char *p, *q;
+  const char *home, *home_drive, *home_path;
+
+  if ((home = getenv ("HOME"))
+      || ((home_drive = getenv ("HOMEDRIVE"))
+	  && (home_path = getenv ("HOMEPATH"))
+	  // concatenate HOMEDRIVE and HOMEPATH
+          && (home = p = tp.c_get ())
+	  && (q = stpncpy (p, home_drive, NT_MAX_PATH))
+          && strlcpy (q, home_path, NT_MAX_PATH - (q - p)))
+      || (home = getenv ("USERPROFILE")))
+    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -1001,6 +1023,10 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1033,6 +1059,10 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1052,6 +1082,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1116,6 +1147,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
@@ -1197,6 +1229,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1223,6 +1257,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index d089964660..c02b1f72e1 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1359,6 +1359,17 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
 </variablelist>

 <para>
@@ -1491,6 +1502,17 @@ of each schema when used with <literal>db_home:</literal>
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
   <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
--
2.38.0.rc0.windows.1



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

* [PATCH v3 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2022-09-21 11:51     ` [PATCH v3 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2022-09-21 11:52     ` Johannes Schindelin
  2022-09-21 11:52     ` [PATCH v3 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
  2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  3 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2022-09-21 11:52 UTC (permalink / raw)
  To: cygwin-patches

We should not blindly set the home directory of the SYSTEM account (or
of Microsoft accounts) to /home/SYSTEM, especially not when that value
disagrees with what is configured via the `db_home` line in the
`/etc/nsswitch.conf` file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 6e673ee39e..5e4243fa4e 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -2253,7 +2253,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+				       fully_qualified_name);
+	}
       switch ((int) acc_type)
 	{
 	case SidTypeUser:
--
2.38.0.rc0.windows.1



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

* [PATCH v3 3/3] Respect `db_home: env` even when no uid can be determined
  2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2022-09-21 11:51     ` [PATCH v3 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
  2022-09-21 11:52     ` [PATCH v3 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2022-09-21 11:52     ` Johannes Schindelin
  2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  3 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2022-09-21 11:52 UTC (permalink / raw)
  To: cygwin-patches

In particular when we cannot figure out a uid for the current user, we
should still respect the `db_home: env` setting. Such a situation occurs
for example when the domain returned by `LookupAccountSid()` is not our
machine name and at the same time our machine is no domain member: In
that case, we have nobody to ask for the POSIX offset necessary to come
up with the uid.

It is important that even in such cases, the `HOME` environment variable
can be used to override the home directory, e.g. when Git for Windows is
used by an account that was generated on the fly, e.g. for transient use
in a cloud scenario.

Reported by David Ebbo.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 5e4243fa4e..b79c2b265d 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -904,6 +904,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
 	    case L'u':
 	      if (full_qualified)
 		{
+		  if (!dom)
+		    break;
 		  w = wcpncpy (w, dom, we - w);
 		  if (w < we)
 		    *w++ = cygheap->pg.nss_separator ()[0];
@@ -914,6 +916,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
 	      w = wcpncpy (w, name, we - w);
 	      break;
 	    case L'D':
+	      if (!dom)
+		break;
 	      w = wcpncpy (w, dom, we - w);
 	      break;
 	    case L'H':
@@ -2200,6 +2204,10 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	{
 	  /* Just some fake. */
 	  sid = csid.create (99, 1, 0);
+	  if (arg.id == cygheap->user.real_uid)
+	    home = cygheap->pg.get_home ((PUSER_INFO_3) NULL,
+					 cygheap->user.sid(),
+					 NULL, NULL, false);
 	  break;
 	}
       else if (arg.id >= UNIX_POSIX_OFFSET)
@@ -2760,10 +2768,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
      logon.  Unless it's the SYSTEM account.  This conveniently allows to
      logon interactively as SYSTEM for debugging purposes. */
   else if (acc_type != SidTypeUser && sid != well_known_system_sid)
-    __small_sprintf (linebuf, "%W:*:%u:%u:U-%W\\%W,%s:/:/sbin/nologin",
+    __small_sprintf (linebuf, "%W:*:%u:%u:U-%W\\%W,%s:%s:/sbin/nologin",
 		     posix_name, uid, gid,
 		     dom, name,
-		     sid.string ((char *) sidstr));
+		     sid.string ((char *) sidstr),
+		     home ? home : "/");
   else
     __small_sprintf (linebuf, "%W:*:%u:%u:%s%sU-%W\\%W,%s:%s%W:%s",
 		     posix_name, uid, gid,
--
2.38.0.rc0.windows.1

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2015-12-17 20:20     ` Corinna Vinschen
@ 2022-09-21 11:58       ` Johannes Schindelin
  2022-10-18 17:02         ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2022-09-21 11:58 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

sorry for the blast from the past, but I am renewing my efforts to
upstream Git for Windows' patches that can be upstreamed.

On Thu, 17 Dec 2015, Corinna Vinschen wrote:

> On Dec 17 19:05, Johannes Schindelin wrote:
> > [...]
> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index c9b3e09..a5d6270 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > [...]
> > +static size_t
> > +fetch_env(LPCWSTR key, char *buf, size_t size)
>            ^^^
>            space
>
> > +{
> > +  WCHAR wbuf[32767];
>
> Ok, there are a couple of problems here.  First, since this buffer
> is a filename buffer, use NT_MAX_PATH from winsup.h as buffer size.
>
> But then again, please avoid allocating 64K buffers on the stack.
> That's what tmp_pathbuf:w_get () is for.

Excellent. I did it exactly as you suggested.

> > +  DWORD max = sizeof wbuf / sizeof *wbuf;
> > +  DWORD len = GetEnvironmentVariableW (key, wbuf, max);
>
> This call to GetEnvironmentVariableW looks gratuitous to me.  Why don't
> you simply call getenv?  It did the entire job already, it avoids the
> requirement for a local buffer, and in case of $HOME it even did the
> Win32->POSIX path conversion.  If there's a really good reason for using
> GetEnvironmentVariableW it begs at least for a longish comment.

My only worry is that `getenv("HOME")` might receive a "Cygwin-ified"
version of the value. That is, `getenv("HOME")` might return something
like `/cygdrive/c/Users/corinna` when we expect it to return
`C:\Users\corinna` instead.

I do not think that the current iteration is resilient against that.

This problem might not be a big issue with Cygwin (I don't think it
automatically converts environment variables that look like paths from
Windows to Unix-style), but it will most likely cause issues with MSYS2
(where we do precisely that with environment variables that look like
paths). Meaning: it will probably take some follow-up work to make this
work correctly, even if it is just to verify that things work when `HOME`
is in Unix-style already while calling into the runtime.

> > +
> > +  if (!len || len >= max)
> > +    return 0;
> > +
> > +  len = sys_wcstombs (buf, size, wbuf, len);
> > +  return len && len < size ? len : 0;
> > +}
> > +
> > +static char *
> > +fetch_home_env (void)
> > +{
> > +  char home[32767];
> > +  size_t max = sizeof home / sizeof *home, len;
> > +
> > +  if (fetch_env (L"HOME", home, max)
> > +      || ((len = fetch_env (L"HOMEDRIVE", home, max))
> > +        && fetch_env (L"HOMEPATH", home + len, max - len))
> > +      || fetch_env (L"USERPROFILE", home, max))
> > +    {
> > +      tmp_pathbuf tp;
> > +      cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_ABSOLUTE,
> > +	  home, tp.c_get(), NT_MAX_PATH);
>                        ^^^
>                        space
> > +      return strdup(tp.c_get());
>                      ^^^      ^^^
>                      space......s
>
> Whoa, tp.c_get() twice to access the same space?  That's a dirty trick
> which may puzzle later readers of the code and heavily depends on
> knowing the internals of tmp_pathbuf.  Please use a variable and only
> assign tp.c_get () once.
>
> OTOH, the above's a case for a cygwin_create_path call, rather than
> cygwin_conv_path+strdup.  Also, if there's *really* a good reason to use
> GetEnvironmentVariableW, you should collapse sys_wcstombs+cygwin_conv_path+
> strdup into a single cygwin_create_path (CCP_WIN_W_TO_POSIX, ...).

Right, that `cygwin_create_path()` call nicely avoids all the problems of
my original code.

>
> > [...]
> > @@ -1079,6 +1123,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
> >  	case NSS_SCHEME_FALLBACK:
> >  	  return NULL;
> >  	case NSS_SCHEME_WINDOWS:
> > +	case NSS_SCHEME_ENV:
> >  	  break;
> >  	case NSS_SCHEME_CYGWIN:
> >  	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
>
> You know that I don't exactly like the "env" idea, but if we implement
> it anyway, wouldn't it make sense to add some kind of $SHELL handling as
> well, for symmetry?

I have decided against that.

The reason: the home directory is a very different thing from the `SHELL`
variable because Windows users _do_ have a home directory even if it is
called differently in Windows speak, while they do not have any POSIX
shell available. There is `COMSPEC`, of course, but it is _not_ a POSIX
shell and cannot be used in place of `SHELL`.

For that reason, I do not believe that we need to do anything about
`SHELL`.

> > [...]
> > @@ -1487,6 +1497,16 @@ of each schema when used with <literal>db_home:</literal>
> >  	      for a detailed description.</listitem>
> >    </varlistentry>
> >    <varlistentry>
> > +    <term><literal>env</literal></term>
> > +    <listitem>Derives the home directory of the current user from the
> > +	      environment variable <literal>HOME</literal> (falling back to
> > +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
> > +	      <literal>USERPROFILE</literal>, in that order).  This is faster
> > +	      than the <term><literal>windows</literal></term> schema at the
> > +	      expense of determining only the current user's home directory
> > +	      correctly.</listitem>
>
> In both case of the documentation it might make sense to add a few words
> along the lines of "This schema is skipped for any other account",
> wouldn't it?

Yes!

(Belated) thank you very much for your review!
Dscho

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

* Re: [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account
  2015-12-17 20:49     ` Corinna Vinschen
  2015-12-17 21:02       ` Corinna Vinschen
@ 2022-09-21 12:00       ` Johannes Schindelin
  1 sibling, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2022-09-21 12:00 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Thu, 17 Dec 2015, Corinna Vinschen wrote:

> On Dec 17 19:05, Johannes Schindelin wrote:
> > We should not blindly set the home directory of the SYSTEM account to
> > /home/SYSTEM, especially not when that value disagrees with what is
> > configured via the `db_home` line in the `/etc/nsswitch.conf` file.
> >
> > This fixes https://github.com/git-for-windows/git/issues/435
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  winsup/cygwin/uinfo.cc | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index a5d6270..8c51b82 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > @@ -2129,7 +2129,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
> >  	 it to a well-known group here. */
> >        if (acc_type == SidTypeUser
> >  	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
> > -	acc_type = SidTypeWellKnownGroup;
> > +	{
> > +	  acc_type = SidTypeWellKnownGroup;
> > +	  home = cygheap->pg.get_home (pldap, sid, dom, domain, name,
> > +				       fully_qualified_name);
>
> Uhm, that's a bit over the top, isn't it?  It will affect all S-1-5-X
> accounts as well as the S-1-5-11 Windows account SIDs.  Is that really
> what you want?

Yes, it was really what I want because it's about respecting `db_home:
env`, and there _are_ apparently SIDs a user can have that fall into the
category "Microsoft account" where we want that to be respected, too ;-)

> Using pldap here may SEGV in cygheap_pwdgrp::get_home, btw, because
> it may be NULL.  cygheap_pwdgrp::get_home doesn't check pldap for
> validity, it expects a valid pointer.  You could either use cldap, or
> cygheap_pwdgrp::get_home would have to check pldap before using it.
>
> However, either way there's another problem: Independently of the
> configured db_home schemes, you don't want to ask the DC for info on
> these builtin accounts.  The better approach might be to call the
> PUSER_INFO_3 variant of cygheap_pwdgrp::get_home with a NULL ui
> pointer and just check for ui in the NSS_SCHEME_DESC case.  The other
> called functions fetch_windows_home and fetch_from_path both can
> live with both pointers, pldap and ui being NULL.

Excellent, I used the `PUSER_INFO_3` method.

Thank you,
Dscho

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-09-21 11:58       ` Johannes Schindelin
@ 2022-10-18 17:02         ` Corinna Vinschen
  2022-10-23 21:04           ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2022-10-18 17:02 UTC (permalink / raw)
  To: cygwin-patches

Hi Johannes,

On Sep 21 13:58, Johannes Schindelin wrote:
> Hi Corinna,
> 
> sorry for the blast from the past, but I am renewing my efforts to
> upstream Git for Windows' patches that can be upstreamed.
> 
> On Thu, 17 Dec 2015, Corinna Vinschen wrote:

Well, not even 7 years, so what? :)

> > On Dec 17 19:05, Johannes Schindelin wrote:
> > > +  DWORD max = sizeof wbuf / sizeof *wbuf;
> > > +  DWORD len = GetEnvironmentVariableW (key, wbuf, max);
> >
> > This call to GetEnvironmentVariableW looks gratuitous to me.  Why don't
> > you simply call getenv?  It did the entire job already, it avoids the
> > requirement for a local buffer, and in case of $HOME it even did the
> > Win32->POSIX path conversion.  If there's a really good reason for using
> > GetEnvironmentVariableW it begs at least for a longish comment.
> 
> My only worry is that `getenv("HOME")` might receive a "Cygwin-ified"
> version of the value. That is, `getenv("HOME")` might return something
> like `/cygdrive/c/Users/corinna` when we expect it to return
> `C:\Users\corinna` instead.

Haha, yeah, that's exactly what it does.  Look at environ.cc, search for
conv_envvars.  There's a list of env vars which are converted
automatically.  So getenv ("HOME") already does what you need, you just
have to adapt the code accordingly, i. e.

  if ((home = getenv ("HOME")))
    return strdup (home);
  if (((home_drive = getenv ("HOMEDRIVE")
           [...]
    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);

However, on second thought, I wonder if the HOMEDRIVE/HOMEPATH/USERPROFILE
code is really required.  AFAICS, it's just a duplication of the effort
already done in fetch_windows_home(), isn't it?

HOMEDRIVE/HOMEPATH are generated from the DB data returned in
USER_INFO_3 or via ldap anyway, and fetch_windows_home() also falls back
to fetching the user profile path, albeit from the registry.

That means, the results from the "env" method is equivalent to the
"windows" method, just after checking $HOME.  That's a bit of a downer.

Assuming the "env" method would *only* check for $HOME, the user would
have the same result by simply setting nsswitch.conf accordingly:

  home: env windows


Corinna

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-10-18 17:02         ` Corinna Vinschen
@ 2022-10-23 21:04           ` Johannes Schindelin
  2022-10-24 11:37             ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2022-10-23 21:04 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Tue, 18 Oct 2022, Corinna Vinschen wrote:

> On Sep 21 13:58, Johannes Schindelin wrote:
> > Hi Corinna,
> >
> > sorry for the blast from the past, but I am renewing my efforts to
> > upstream Git for Windows' patches that can be upstreamed.
> >
> > On Thu, 17 Dec 2015, Corinna Vinschen wrote:
>
> Well, not even 7 years, so what? :)

True :-)

> > > On Dec 17 19:05, Johannes Schindelin wrote:
> > > > +  DWORD max = sizeof wbuf / sizeof *wbuf;
> > > > +  DWORD len = GetEnvironmentVariableW (key, wbuf, max);
> > >
> > > This call to GetEnvironmentVariableW looks gratuitous to me.  Why don't
> > > you simply call getenv?  It did the entire job already, it avoids the
> > > requirement for a local buffer, and in case of $HOME it even did the
> > > Win32->POSIX path conversion.  If there's a really good reason for using
> > > GetEnvironmentVariableW it begs at least for a longish comment.
> >
> > My only worry is that `getenv("HOME")` might receive a "Cygwin-ified"
> > version of the value. That is, `getenv("HOME")` might return something
> > like `/cygdrive/c/Users/corinna` when we expect it to return
> > `C:\Users\corinna` instead.
>
> Haha, yeah, that's exactly what it does.  Look at environ.cc, search for
> conv_envvars.  There's a list of env vars which are converted
> automatically.  So getenv ("HOME") already does what you need, you just
> have to adapt the code accordingly, i. e.
>
>   if ((home = getenv ("HOME")))
>     return strdup (home);
>   if (((home_drive = getenv ("HOMEDRIVE")
>            [...]
>     return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
>
> However, on second thought, I wonder if the HOMEDRIVE/HOMEPATH/USERPROFILE
> code is really required.  AFAICS, it's just a duplication of the effort
> already done in fetch_windows_home(), isn't it?

You mean the case where _both_ `pldap` and `ui` are `NULL`, i.e. where
`get_user_profile_directory()` is called?

Correct me if I'm wrong, but that does not at all look at the environment
variables, but instead queries the registry.

And _if_ we want to do that (which I would rather want to avoid, for
simplicity and speed), shouldn't we call the
`get_user_profile_directory()` function directly instead of going through
`fetch_windows_home()`?

But if you meant to still have a non-`NULL` `pldap`, the results are
definitely not the same, not only because users can easily modify their
environment variables while they cannot easily modify what is retrieved
from the DB: [see below].

> HOMEDRIVE/HOMEPATH are generated from the DB data returned in
> USER_INFO_3 or via ldap anyway, and fetch_windows_home() also falls back
> to fetching the user profile path, albeit from the registry.

The big difference of using ldap is that retrieving the environment
variable is instantaneous whereas there is a multi-second delay if the
domain controller is temporarily unreachable.

> That means, the results from the "env" method is equivalent to the
> "windows" method, just after checking $HOME.  That's a bit of a downer.
>
> Assuming the "env" method would *only* check for $HOME, the user would
> have the same result by simply setting nsswitch.conf accordingly:
>
>   home: env windows

Except when the domain controller is (temporarily) unreachable, e.g. when
sitting in a train with poor or no internet connection. Then that latter
approach would have the "benefit" of having to wait 10-15 seconds before
the network call says "nope".

This particular issue has hit enough Git for Windows users that I found
myself being forced to implement these patches and run with them for the
past seven years.

Given the scenario of an unreachable domain controller, I hope you agree
that the `env` support added in the proposed patches _has_ merit.

Ciao,
Dscho

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-10-23 21:04           ` Johannes Schindelin
@ 2022-10-24 11:37             ` Corinna Vinschen
  2022-11-10 15:16               ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2022-10-24 11:37 UTC (permalink / raw)
  To: cygwin-patches

On Oct 23 23:04, Johannes Schindelin wrote:
> On Tue, 18 Oct 2022, Corinna Vinschen wrote:
> [...]
> > That means, the results from the "env" method is equivalent to the
> > "windows" method, just after checking $HOME.  That's a bit of a downer.
> >
> > Assuming the "env" method would *only* check for $HOME, the user would
> > have the same result by simply setting nsswitch.conf accordingly:
> >
> >   home: env windows
> 
> Except when the domain controller is (temporarily) unreachable, e.g. when
> sitting in a train with poor or no internet connection. Then that latter
> approach would have the "benefit" of having to wait 10-15 seconds before
> the network call says "nope".
> 
> This particular issue has hit enough Git for Windows users that I found
> myself being forced to implement these patches and run with them for the
> past seven years.
> 
> Given the scenario of an unreachable domain controller, I hope you agree
> that the `env` support added in the proposed patches _has_ merit.

Yes, I don't doubt an `env' method checking for $HOME even a bit.

I'm just not sure as far as HOMEDRIVE/HOMEPATH/USERPROFILE are
concerned.  Those vars should be left alone, but we can't control that,
so reading them from genuine sources is preferred.

Sure, the downside in terms of the LDAP server is clear to me

So I guess it's ok to allow the env method to read the values of those
vars from the env.  I would just feel better if we urge the
user to set $HOME and read that exclusively.


Corinna

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-10-24 11:37             ` Corinna Vinschen
@ 2022-11-10 15:16               ` Johannes Schindelin
  2022-11-10 15:22                 ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2022-11-10 15:16 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Mon, 24 Oct 2022, Corinna Vinschen wrote:

> On Oct 23 23:04, Johannes Schindelin wrote:
> > On Tue, 18 Oct 2022, Corinna Vinschen wrote:
> > [...]
> > > That means, the results from the "env" method is equivalent to the
> > > "windows" method, just after checking $HOME.  That's a bit of a downer.
> > >
> > > Assuming the "env" method would *only* check for $HOME, the user would
> > > have the same result by simply setting nsswitch.conf accordingly:
> > >
> > >   home: env windows
> >
> > Except when the domain controller is (temporarily) unreachable, e.g. when
> > sitting in a train with poor or no internet connection. Then that latter
> > approach would have the "benefit" of having to wait 10-15 seconds before
> > the network call says "nope".
> >
> > This particular issue has hit enough Git for Windows users that I found
> > myself being forced to implement these patches and run with them for the
> > past seven years.
> >
> > Given the scenario of an unreachable domain controller, I hope you agree
> > that the `env` support added in the proposed patches _has_ merit.
>
> Yes, I don't doubt an `env' method checking for $HOME even a bit.

Cool!

> I'm just not sure as far as HOMEDRIVE/HOMEPATH/USERPROFILE are
> concerned.  Those vars should be left alone, but we can't control that,
> so reading them from genuine sources is preferred.

I do not recall the exact reasons because it has been a good while since I
worked on these patches. But I do remember that we had to have a fall-back
for the many scenarios in Git for Windows where `HOME` is not even set,
and we specifically had to add HOMEDRIVE/HOMEPATH handling because
USERPROFILE alone would lead to problems (IIRC there were plenty of
corporate setups where USERPROFILE pointed to a potentially-disconnected
network drive).

> Sure, the downside in terms of the LDAP server is clear to me
>
> So I guess it's ok to allow the env method to read the values of those
> vars from the env.  I would just feel better if we urge the
> user to set $HOME and read that exclusively.

I would feel better about that, too, if it was practical.

But I cannot ask millions of Git for Windows users to please go ahead and
first configure their `HOME` variable correctly, it took much less time to
implement the patch we're discussing than asking all users individually
;-)

And since there is nothing specific about Git for Windows here, I expect
Cygwin users to benefit from this feature, too.

With this context in mind, I would like to ask to integrate the patch
as-is, including the HOMEDRIVE/HOMEPATH and USERPROFILE fall-backs.

Thanks,
Dscho

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-11-10 15:16               ` Johannes Schindelin
@ 2022-11-10 15:22                 ` Corinna Vinschen
  2022-11-18  8:18                   ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2022-11-10 15:22 UTC (permalink / raw)
  To: cygwin-patches

On Nov 10 16:16, Johannes Schindelin wrote:
> Hi Corinna,
> 
> On Mon, 24 Oct 2022, Corinna Vinschen wrote:
> 
> > On Oct 23 23:04, Johannes Schindelin wrote:
> > > On Tue, 18 Oct 2022, Corinna Vinschen wrote:
> > > [...]
> > > > That means, the results from the "env" method is equivalent to the
> > > > "windows" method, just after checking $HOME.  That's a bit of a downer.
> > > >
> > > > Assuming the "env" method would *only* check for $HOME, the user would
> > > > have the same result by simply setting nsswitch.conf accordingly:
> > > >
> > > >   home: env windows
> > >
> > > Except when the domain controller is (temporarily) unreachable, e.g. when
> > > sitting in a train with poor or no internet connection. Then that latter
> > > approach would have the "benefit" of having to wait 10-15 seconds before
> > > the network call says "nope".
> > >
> > > This particular issue has hit enough Git for Windows users that I found
> > > myself being forced to implement these patches and run with them for the
> > > past seven years.
> > >
> > > Given the scenario of an unreachable domain controller, I hope you agree
> > > that the `env` support added in the proposed patches _has_ merit.
> >
> > Yes, I don't doubt an `env' method checking for $HOME even a bit.
> 
> Cool!
> 
> > I'm just not sure as far as HOMEDRIVE/HOMEPATH/USERPROFILE are
> > concerned.  Those vars should be left alone, but we can't control that,
> > so reading them from genuine sources is preferred.
> 
> I do not recall the exact reasons because it has been a good while since I
> worked on these patches. But I do remember that we had to have a fall-back
> for the many scenarios in Git for Windows where `HOME` is not even set,
> and we specifically had to add HOMEDRIVE/HOMEPATH handling because
> USERPROFILE alone would lead to problems (IIRC there were plenty of
> corporate setups where USERPROFILE pointed to a potentially-disconnected
> network drive).
> 
> > Sure, the downside in terms of the LDAP server is clear to me
> >
> > So I guess it's ok to allow the env method to read the values of those
> > vars from the env.  I would just feel better if we urge the
> > user to set $HOME and read that exclusively.
> 
> I would feel better about that, too, if it was practical.
> 
> But I cannot ask millions of Git for Windows users to please go ahead and
> first configure their `HOME` variable correctly, it took much less time to
> implement the patch we're discussing than asking all users individually
> ;-)
> 
> And since there is nothing specific about Git for Windows here, I expect
> Cygwin users to benefit from this feature, too.
> 
> With this context in mind, I would like to ask to integrate the patch
> as-is, including the HOMEDRIVE/HOMEPATH and USERPROFILE fall-backs.

Can't do that, sorry.  Two replies before I sent a necessary change,
which needs inclusion first.


Thanks,
Corinna

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-11-10 15:22                 ` Corinna Vinschen
@ 2022-11-18  8:18                   ` Johannes Schindelin
  2022-11-21 11:41                     ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2022-11-18  8:18 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Thu, 10 Nov 2022, Corinna Vinschen wrote:

> On Nov 10 16:16, Johannes Schindelin wrote:
>
> > On Mon, 24 Oct 2022, Corinna Vinschen wrote:
> >
> > > On Oct 23 23:04, Johannes Schindelin wrote:
> > > > On Tue, 18 Oct 2022, Corinna Vinschen wrote:
> > > > [...]
> > > > > That means, the results from the "env" method is equivalent to the
> > > > > "windows" method, just after checking $HOME.  That's a bit of a downer.
> > > > >
> > > > > Assuming the "env" method would *only* check for $HOME, the user would
> > > > > have the same result by simply setting nsswitch.conf accordingly:
> > > > >
> > > > >   home: env windows
> > > >
> > > > Except when the domain controller is (temporarily) unreachable, e.g. when
> > > > sitting in a train with poor or no internet connection. Then that latter
> > > > approach would have the "benefit" of having to wait 10-15 seconds before
> > > > the network call says "nope".
> > > >
> > > > This particular issue has hit enough Git for Windows users that I found
> > > > myself being forced to implement these patches and run with them for the
> > > > past seven years.
> > > >
> > > > Given the scenario of an unreachable domain controller, I hope you agree
> > > > that the `env` support added in the proposed patches _has_ merit.
> > >
> > > Yes, I don't doubt an `env' method checking for $HOME even a bit.
> >
> > Cool!
> >
> > > I'm just not sure as far as HOMEDRIVE/HOMEPATH/USERPROFILE are
> > > concerned.  Those vars should be left alone, but we can't control that,
> > > so reading them from genuine sources is preferred.
> >
> > I do not recall the exact reasons because it has been a good while since I
> > worked on these patches. But I do remember that we had to have a fall-back
> > for the many scenarios in Git for Windows where `HOME` is not even set,
> > and we specifically had to add HOMEDRIVE/HOMEPATH handling because
> > USERPROFILE alone would lead to problems (IIRC there were plenty of
> > corporate setups where USERPROFILE pointed to a potentially-disconnected
> > network drive).
> >
> > > Sure, the downside in terms of the LDAP server is clear to me
> > >
> > > So I guess it's ok to allow the env method to read the values of those
> > > vars from the env.  I would just feel better if we urge the
> > > user to set $HOME and read that exclusively.
> >
> > I would feel better about that, too, if it was practical.
> >
> > But I cannot ask millions of Git for Windows users to please go ahead and
> > first configure their `HOME` variable correctly, it took much less time to
> > implement the patch we're discussing than asking all users individually
> > ;-)
> >
> > And since there is nothing specific about Git for Windows here, I expect
> > Cygwin users to benefit from this feature, too.
> >
> > With this context in mind, I would like to ask to integrate the patch
> > as-is, including the HOMEDRIVE/HOMEPATH and USERPROFILE fall-backs.
>
> Can't do that, sorry.  Two replies before I sent a necessary change,
> which needs inclusion first.

I am a bit confused. Do you need anything from me to move this along, i.e.
are those two replies you mention some emails I failed to address yet?

Thanks,
Dscho

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-11-18  8:18                   ` Johannes Schindelin
@ 2022-11-21 11:41                     ` Corinna Vinschen
  2023-03-28  8:21                       ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2022-11-21 11:41 UTC (permalink / raw)
  To: cygwin-patches

On Nov 18 09:18, Johannes Schindelin wrote:
> Hi Corinna,
> 
> On Thu, 10 Nov 2022, Corinna Vinschen wrote:
> > On Nov 10 16:16, Johannes Schindelin wrote:
> > > With this context in mind, I would like to ask to integrate the patch
> > > as-is, including the HOMEDRIVE/HOMEPATH and USERPROFILE fall-backs.
> >
> > Can't do that, sorry.  Two replies before I sent a necessary change,
> > which needs inclusion first.
> 
> I am a bit confused. Do you need anything from me to move this along, i.e.
> are those two replies you mention some emails I failed to address yet?

I didn't mean two different replies, but my second-last reply before
that one, i. e.
https://cygwin.com/pipermail/cygwin-patches/2022q4/012025.html

Sorry if that wasn't clear.  Basically your handling of $HOME was
wrong and I suggested a fix.


Corinna

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

* [PATCH v4 0/3] Support deriving the current user's home directory via HOME
  2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
                       ` (2 preceding siblings ...)
  2022-09-21 11:52     ` [PATCH v3 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
@ 2023-03-28  8:17     ` Johannes Schindelin
  2023-03-28  8:17       ` [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
                         ` (3 more replies)
  3 siblings, 4 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-03-28  8:17 UTC (permalink / raw)
  To: cygwin-patches

This patch mini-series supports Git for Windows' default strategy to
determine the current user's home directory by looking at the
environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
if these variables are also unset, to USERPROFILE.

This strategy is a quick method to determine the home directory,
certainly quicker than looking at LDAP, even more so when a domain
controller is unreachable and causes long hangs in Cygwin's startup.

This strategy also allows users to override the home directory easily
(e.g. in case that their real home directory is a network share that is
not all that well handled by some commands such as cmd.exe's cd
command).

Sorry for sending out v4 sooooo late...!

Changes since v3:

- Fixed the bug in v2 where `getenv("HOME")` would convert the value to
  a Unix-y path and the `fetch_home_env()` function would then try to
  convert it _again_.

- Disentangled the logic in `fetch_home_env()` instead of doing
  everything in one big, honking, unreadable `if` condition.

- Commented the code in `fetch_home_env()`.

Changes since v2:

- Using `getenv()` and `cygwin_create_path()` instead of the
  `GetEnvironmentVariableW()`/`cygwin_conv_path()` dance

- Adjusted the documentation to drive home that this only affects the
  _current_ user's home directory

- Using the `PUSER_INFO_3` variant of `get_home()`

- Adjusted the commit messages

- Added another patch, to support "ad-hoc cloud accounts"

Johannes Schindelin (3):
  Allow deriving the current user's home directory via the HOME variable
  Respect `db_home` setting even for SYSTEM/Microsoft accounts
  Respect `db_home: env` even when no uid can be determined

 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 70 ++++++++++++++++++++++++--
 winsup/doc/ntsec.xml                   | 22 ++++++++
 3 files changed, 91 insertions(+), 4 deletions(-)

Range-diff:
1:  6f8fe89d9d ! 1:  7a074997ea Allow deriving the current user's home directory via the HOME variable
    @@ winsup/cygwin/uinfo.cc: fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygps
     +static char *
     +fetch_home_env (void)
     +{
    -+  tmp_pathbuf tp;
    -+  char *p, *q;
    -+  const char *home, *home_drive, *home_path;
    ++  /* If `HOME` is set, prefer it */
    ++  const char *home = getenv ("HOME");
    ++  if (home)
    ++    return strdup (home);
    ++
    ++  /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
    ++     (without a directory separator, as `HOMEPATH` starts with one). */
    ++  const char *home_drive = getenv ("HOMEDRIVE");
    ++  if (home_drive)
    ++    {
    ++      const char *home_path = getenv ("HOMEPATH");
    ++      if (home_path)
    ++	{
    ++	  tmp_pathbuf tp;
    ++	  char *p = tp.c_get (), *q;
     +
    -+  if ((home = getenv ("HOME"))
    -+      || ((home_drive = getenv ("HOMEDRIVE"))
    -+	  && (home_path = getenv ("HOMEPATH"))
     +	  // concatenate HOMEDRIVE and HOMEPATH
    -+          && (home = p = tp.c_get ())
    -+	  && (q = stpncpy (p, home_drive, NT_MAX_PATH))
    -+          && strlcpy (q, home_path, NT_MAX_PATH - (q - p)))
    -+      || (home = getenv ("USERPROFILE")))
    ++	  q = stpncpy (p, home_drive, NT_MAX_PATH);
    ++	  strlcpy (q, home_path, NT_MAX_PATH - (q - p));
    ++	  return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, p);
    ++	}
    ++    }
    ++
    ++  /* If neither `HOME` nor `HOMEDRIVE``HOMEPATH` are set, fall back
    ++     to `USERPROFILE`; In corporate setups, this might point to a
    ++     disconnected network share, hence this is the last fall back. */
    ++  home = getenv ("USERPROFILE");
    ++  if (home)
     +    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
     +
     +  return NULL;
2:  1b4ee89aa7 = 2:  a70c77dc8f Respect `db_home` setting even for SYSTEM/Microsoft accounts
3:  4d90319e44 ! 3:  4cd6ae7307 Respect `db_home: env` even when no uid can be determined
    @@ winsup/cygwin/uinfo.cc: fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygps
     +		    break;
      		  w = wcpncpy (w, dom, we - w);
      		  if (w < we)
    - 		    *w++ = cygheap->pg.nss_separator ()[0];
    + 		    *w++ = NSS_SEPARATOR_CHAR;
     @@ winsup/cygwin/uinfo.cc: fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
      	      w = wcpncpy (w, name, we - w);
      	      break;

base-commit: a9a17f5fe51498b182d4a11ac48207b8c7ffe8ec
Published-As: https://github.com/dscho/msys2-runtime/releases/tag/home-env-cygwin-v4
Fetch-It-Via: git fetch https://github.com/dscho/msys2-runtime home-env-cygwin-v4

--
2.40.0.windows.1


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

* [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
@ 2023-03-28  8:17       ` Johannes Schindelin
  2023-03-28 10:35         ` Corinna Vinschen
  2023-03-28  8:17       ` [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-03-28  8:17 UTC (permalink / raw)
  To: cygwin-patches

This patch hails from Git for Windows (where the Cygwin runtime is used
in the form of a slightly modified MSYS2 runtime), where it is a
well-established technique to let the `$HOME` variable define where the
current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSYS2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly, e.g. `cmd.exe` and other native Windows
applications that users want to employ as Git helpers.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or
`$USERPROFILE`.  This has the additional advantage that it is much
faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 51 ++++++++++++++++++++++++++
 winsup/doc/ntsec.xml                   | 22 +++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/local_includes/cygheap.h b/winsup/cygwin/local_includes/cygheap.h
index d885ca1230..b6acdf7f18 100644
--- a/winsup/cygwin/local_includes/cygheap.h
+++ b/winsup/cygwin/local_includes/cygheap.h
@@ -358,7 +358,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 30df6db6d8..baa670478d 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -733,6 +733,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -921,6 +923,42 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }

+static char *
+fetch_home_env (void)
+{
+  /* If `HOME` is set, prefer it */
+  const char *home = getenv ("HOME");
+  if (home)
+    return strdup (home);
+
+  /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
+     (without a directory separator, as `HOMEPATH` starts with one). */
+  const char *home_drive = getenv ("HOMEDRIVE");
+  if (home_drive)
+    {
+      const char *home_path = getenv ("HOMEPATH");
+      if (home_path)
+	{
+	  tmp_pathbuf tp;
+	  char *p = tp.c_get (), *q;
+
+	  // concatenate HOMEDRIVE and HOMEPATH
+	  q = stpncpy (p, home_drive, NT_MAX_PATH);
+	  strlcpy (q, home_path, NT_MAX_PATH - (q - p));
+	  return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, p);
+	}
+    }
+
+  /* If neither `HOME` nor `HOMEDRIVE``HOMEPATH` are set, fall back
+     to `USERPROFILE`; In corporate setups, this might point to a
+     disconnected network share, hence this is the last fall back. */
+  home = getenv ("USERPROFILE");
+  if (home)
+    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -980,6 +1018,10 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1012,6 +1054,10 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1031,6 +1077,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1095,6 +1142,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
@@ -1176,6 +1224,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1202,6 +1252,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index c6871ecf05..1678ff6575 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1203,6 +1203,17 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
 </variablelist>

 <para>
@@ -1335,6 +1346,17 @@ of each schema when used with <literal>db_home:</literal>
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <term><literal>windows</literal></term> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
   <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
--
2.40.0.windows.1



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

* [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-03-28  8:17       ` [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2023-03-28  8:17       ` Johannes Schindelin
  2023-03-28 10:16         ` Corinna Vinschen
  2023-03-28  8:17       ` [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
  2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  3 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-03-28  8:17 UTC (permalink / raw)
  To: cygwin-patches

We should not blindly set the home directory of the SYSTEM account (or
of Microsoft accounts) to /home/SYSTEM, especially not when that value
disagrees with what is configured via the `db_home` line in the
`/etc/nsswitch.conf` file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index baa670478d..d493d29b3b 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+				       fully_qualified_name);
+	}
       switch ((int) acc_type)
 	{
 	case SidTypeUser:
--
2.40.0.windows.1



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

* [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-03-28  8:17       ` [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
  2023-03-28  8:17       ` [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2023-03-28  8:17       ` Johannes Schindelin
  2023-03-28 10:17         ` Corinna Vinschen
  2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  3 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-03-28  8:17 UTC (permalink / raw)
  To: cygwin-patches

In particular when we cannot figure out a uid for the current user, we
should still respect the `db_home: env` setting. Such a situation occurs
for example when the domain returned by `LookupAccountSid()` is not our
machine name and at the same time our machine is no domain member: In
that case, we have nobody to ask for the POSIX offset necessary to come
up with the uid.

It is important that even in such cases, the `HOME` environment variable
can be used to override the home directory, e.g. when Git for Windows is
used by an account that was generated on the fly, e.g. for transient use
in a cloud scenario.

Reported by David Ebbo.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index d493d29b3b..b01bcff5cb 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -883,6 +883,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
 	    case L'u':
 	      if (full_qualified)
 		{
+		  if (!dom)
+		    break;
 		  w = wcpncpy (w, dom, we - w);
 		  if (w < we)
 		    *w++ = NSS_SEPARATOR_CHAR;
@@ -893,6 +895,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
 	      w = wcpncpy (w, name, we - w);
 	      break;
 	    case L'D':
+	      if (!dom)
+		break;
 	      w = wcpncpy (w, dom, we - w);
 	      break;
 	    case L'H':
@@ -2181,6 +2185,10 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	{
 	  /* Just some fake. */
 	  sid = csid.create (99, 1, 0);
+	  if (arg.id == cygheap->user.real_uid)
+	    home = cygheap->pg.get_home ((PUSER_INFO_3) NULL,
+					 cygheap->user.sid(),
+					 NULL, NULL, false);
 	  break;
 	}
       else if (arg.id >= UNIX_POSIX_OFFSET)
@@ -2710,10 +2718,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
      logon.  Unless it's the SYSTEM account.  This conveniently allows to
      logon interactively as SYSTEM for debugging purposes. */
   else if (acc_type != SidTypeUser && sid != well_known_system_sid)
-    __small_sprintf (linebuf, "%W:*:%u:%u:U-%W\\%W,%s:/:/sbin/nologin",
+    __small_sprintf (linebuf, "%W:*:%u:%u:U-%W\\%W,%s:%s:/sbin/nologin",
 		     posix_name, uid, gid,
 		     dom, name,
-		     sid.string ((char *) sidstr));
+		     sid.string ((char *) sidstr),
+		     home ? home : "/");
   else
     __small_sprintf (linebuf, "%W:*:%u:%u:%s%sU-%W\\%W,%s:%s%W:%s",
 		     posix_name, uid, gid,
--
2.40.0.windows.1

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

* Re: [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable
  2022-11-21 11:41                     ` Corinna Vinschen
@ 2023-03-28  8:21                       ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-03-28  8:21 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Mon, 21 Nov 2022, Corinna Vinschen wrote:

> On Nov 18 09:18, Johannes Schindelin wrote:
> > Hi Corinna,
> >
> > On Thu, 10 Nov 2022, Corinna Vinschen wrote:
> > > On Nov 10 16:16, Johannes Schindelin wrote:
> > > > With this context in mind, I would like to ask to integrate the patch
> > > > as-is, including the HOMEDRIVE/HOMEPATH and USERPROFILE fall-backs.
> > >
> > > Can't do that, sorry.  Two replies before I sent a necessary change,
> > > which needs inclusion first.
> >
> > I am a bit confused. Do you need anything from me to move this along, i.e.
> > are those two replies you mention some emails I failed to address yet?
>
> I didn't mean two different replies, but my second-last reply before
> that one, i. e.
> https://cygwin.com/pipermail/cygwin-patches/2022q4/012025.html
>
> Sorry if that wasn't clear.  Basically your handling of $HOME was
> wrong and I suggested a fix.

Sorry about being so thick! I thought I had done exactly what you asked
for, but had missed that `getenv("HOME")` already converts the path to a
Unix-y form but the same is untrue when getting `HOMEDRIVE`, `HOMEPATH`
and `USERPROFILE`.

I took the opportunity to unclutter the `fetch_home_env()` function and
document it, to improve the readability by strides.

Ciao,
Johannes

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

* Re: [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-03-28  8:17       ` [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2023-03-28 10:16         ` Corinna Vinschen
  2023-04-03  6:36           ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-03-28 10:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-patches

On Mar 28 10:17, Johannes Schindelin wrote:
> We should not blindly set the home directory of the SYSTEM account (or
> of Microsoft accounts) to /home/SYSTEM, especially not when that value
                                  ^^^^^^
That should probably better be <username>, no?

> disagrees with what is configured via the `db_home` line in the
> `/etc/nsswitch.conf` file.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/uinfo.cc | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> index baa670478d..d493d29b3b 100644
> --- a/winsup/cygwin/uinfo.cc
> +++ b/winsup/cygwin/uinfo.cc
> @@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
>  	 it to a well-known group here. */
>        if (acc_type == SidTypeUser
>  	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
> -	acc_type = SidTypeWellKnownGroup;
> +	{
> +	  acc_type = SidTypeWellKnownGroup;
> +	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
> +				       fully_qualified_name);
> +	}
>        switch ((int) acc_type)
>  	{
>  	case SidTypeUser:
> --
> 2.40.0.windows.1
> 

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-03-28  8:17       ` [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
@ 2023-03-28 10:17         ` Corinna Vinschen
  2023-04-03  6:45           ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-03-28 10:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-patches

On Mar 28 10:17, Johannes Schindelin wrote:
> In particular when we cannot figure out a uid for the current user, we
> should still respect the `db_home: env` setting. Such a situation occurs
> for example when the domain returned by `LookupAccountSid()` is not our
> machine name and at the same time our machine is no domain member: In
> that case, we have nobody to ask for the POSIX offset necessary to come
> up with the uid.
> 
> It is important that even in such cases, the `HOME` environment variable
> can be used to override the home directory, e.g. when Git for Windows is
> used by an account that was generated on the fly, e.g. for transient use
> in a cloud scenario.

How does this kind of account look like?  I'd like to see the contants
of name, domain, and the SID.  Isn't that just an account closely
resembling Micorosft Accounts or AzureAD accounts?  Can't we somehow
handle them alike?

> Reported by David Ebbo.

This should be

  Reported-By: David Ebbo <email address>

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/uinfo.cc | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> index d493d29b3b..b01bcff5cb 100644
> --- a/winsup/cygwin/uinfo.cc
> +++ b/winsup/cygwin/uinfo.cc
> @@ -883,6 +883,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
>  	    case L'u':
>  	      if (full_qualified)
>  		{
> +		  if (!dom)
> +		    break;

No domain?  Really?


Corinna

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

* Re: [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-03-28  8:17       ` [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2023-03-28 10:35         ` Corinna Vinschen
  2023-03-28 12:34           ` Jon Turney
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-03-28 10:35 UTC (permalink / raw)
  To: Johannes Schindelin, Jon TURNEY; +Cc: cygwin-patches

Apart from the doc change, the patch is ok now.

On Mar 28 10:17, Johannes Schindelin wrote:
> diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
> index c6871ecf05..1678ff6575 100644
> --- a/winsup/doc/ntsec.xml
> +++ b/winsup/doc/ntsec.xml
> @@ -1203,6 +1203,17 @@ schemata are the following:
>  	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
>  	      for a more detailed description.</listitem>
>    </varlistentry>
> +  <varlistentry>
> +    <term><literal>env</literal></term>
> +    <listitem>Derives the home directory of the current user from the
> +	      environment variable <literal>HOME</literal> (falling back to
> +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
> +	      <literal>USERPROFILE</literal>, in that order).  This is faster
> +	      than the <term><literal>windows</literal></term> schema at the
> +	      expense of determining only the current user's home directory
> +	      correctly.  This schema is skipped for any other account.
> +	      </listitem>
> +  </varlistentry>
>  </variablelist>

I'd rephrase that a bit here.  This is the description of the scheme
itself, so this should be something along the lines of "utilizes the
current environment ..." and "Right now only valid for db_home, see xref
linkend="ntsec-mapping-nsswitch-home"...

However, there's something strange going on, see below.

>  <para>
> @@ -1335,6 +1346,17 @@ of each schema when used with <literal>db_home:</literal>
>  	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
>  	      for a detailed description.</listitem>
>    </varlistentry>
> +  <varlistentry>
> +    <term><literal>env</literal></term>
> +    <listitem>Derives the home directory of the current user from the
> +	      environment variable <literal>HOME</literal> (falling back to
> +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
> +	      <literal>USERPROFILE</literal>, in that order).  This is faster
> +	      than the <term><literal>windows</literal></term> schema at the
> +	      expense of determining only the current user's home directory
> +	      correctly.  This schema is skipped for any other account.
> +	      </listitem>
> +  </varlistentry>
>    <varlistentry>
>      <term><literal>@ad_attribute</literal></term>
>      <listitem>AD only: The user's home directory is set to the path given

There's something wrong here. Building the docs, I get these new error
messages:

  docbook2texi://sect4[@id='ntsec-mapping-nsswitch-passwd']/variablelist[1]/varlistentry[5]/listitem/term: element not matched by any template
  docbook2texi://sect4[@id='ntsec-mapping-nsswitch-home']/variablelist/varlistentry[5]/listitem/term: element not matched by any template
  Element term in namespace '' encountered in listitem, but no template matches.
  Element term in namespace '' encountered in listitem, but no template matches.
  Element term in namespace '' encountered in listitem, but no template matches.
  Element term in namespace '' encountered in listitem, but no template matches.
  No template matches term in listitem.
  No template matches term in listitem.

It looks like this has something to do with the <term> expression?

Jon, do you have an idea?


Corinna

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

* Re: [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-03-28 10:35         ` Corinna Vinschen
@ 2023-03-28 12:34           ` Jon Turney
  2023-03-28 13:31             ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Jon Turney @ 2023-03-28 12:34 UTC (permalink / raw)
  To: Johannes Schindelin, cygwin-patches

On 28/03/2023 11:35, Corinna Vinschen wrote:
> Apart from the doc change, the patch is ok now.

The preceding text says "Four schema are predefined, two schemata are 
variable", then we add "env" to both lists? That doesn't make much sense 
to me.  Surely it's just a "predefined schema"?  In any case that text 
should be updated.

> On Mar 28 10:17, Johannes Schindelin wrote:
>> diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
>> index c6871ecf05..1678ff6575 100644
>> --- a/winsup/doc/ntsec.xml
>> +++ b/winsup/doc/ntsec.xml
>> @@ -1203,6 +1203,17 @@ schemata are the following:
>>   	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
>>   	      for a more detailed description.</listitem>
>>     </varlistentry>
>> +  <varlistentry>
>> +    <term><literal>env</literal></term>
>> +    <listitem>Derives the home directory of the current user from the
>> +	      environment variable <literal>HOME</literal> (falling back to
>> +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
>> +	      <literal>USERPROFILE</literal>, in that order).  This is faster
>> +	      than the <term><literal>windows</literal></term> schema at the
>> +	      expense of determining only the current user's home directory
>> +	      correctly.  This schema is skipped for any other account.
>> +	      </listitem>
>> +  </varlistentry>
>>   </variablelist>
> 
> I'd rephrase that a bit here.  This is the description of the scheme
> itself, so this should be something along the lines of "utilizes the
> current environment ..." and "Right now only valid for db_home, see xref
> linkend="ntsec-mapping-nsswitch-home"...
> 
> However, there's something strange going on, see below.
> 
>>   <para>
>> @@ -1335,6 +1346,17 @@ of each schema when used with <literal>db_home:</literal>
>>   	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
>>   	      for a detailed description.</listitem>
>>     </varlistentry>
>> +  <varlistentry>
>> +    <term><literal>env</literal></term>
>> +    <listitem>Derives the home directory of the current user from the
>> +	      environment variable <literal>HOME</literal> (falling back to
>> +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
>> +	      <literal>USERPROFILE</literal>, in that order).  This is faster
>> +	      than the <term><literal>windows</literal></term> schema at the

I think drop wrapping in <term> here should fix the error below.  It's 
not valid docbook here.

>> +	      expense of determining only the current user's home directory
>> +	      correctly.  This schema is skipped for any other account.
>> +	      </listitem>
>> +  </varlistentry>
>>     <varlistentry>
>>       <term><literal>@ad_attribute</literal></term>
>>       <listitem>AD only: The user's home directory is set to the path given
> 
> There's something wrong here. Building the docs, I get these new error
> messages:
> 
>    docbook2texi://sect4[@id='ntsec-mapping-nsswitch-passwd']/variablelist[1]/varlistentry[5]/listitem/term: element not matched by any template
>    docbook2texi://sect4[@id='ntsec-mapping-nsswitch-home']/variablelist/varlistentry[5]/listitem/term: element not matched by any template
>    Element term in namespace '' encountered in listitem, but no template matches.
>    Element term in namespace '' encountered in listitem, but no template matches.
>    Element term in namespace '' encountered in listitem, but no template matches.
>    Element term in namespace '' encountered in listitem, but no template matches.
>    No template matches term in listitem.
>    No template matches term in listitem.
> 
> It looks like this has something to do with the <term> expression?
> 
> Jon, do you have an idea?



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

* Re: [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-03-28 12:34           ` Jon Turney
@ 2023-03-28 13:31             ` Corinna Vinschen
  2023-03-29  8:36               ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-03-28 13:31 UTC (permalink / raw)
  To: Jon Turney; +Cc: Johannes Schindelin, cygwin-patches

On Mar 28 13:34, Jon Turney wrote:
> On 28/03/2023 11:35, Corinna Vinschen wrote:
> > Apart from the doc change, the patch is ok now.
> 
> The preceding text says "Four schema are predefined, two schemata are
> variable", then we add "env" to both lists? That doesn't make much sense to
> me.  Surely it's just a "predefined schema"?  In any case that text should
> be updated.

Ouch, yeah, I missed that.  Thanks for catching!


Corinna

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

* Re: [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-03-28 13:31             ` Corinna Vinschen
@ 2023-03-29  8:36               ` Corinna Vinschen
  2023-04-03  6:39                 ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-03-29  8:36 UTC (permalink / raw)
  To: Jon Turney; +Cc: Johannes Schindelin, cygwin-patches

On Mar 28 15:31, Corinna Vinschen wrote:
> On Mar 28 13:34, Jon Turney wrote:
> > On 28/03/2023 11:35, Corinna Vinschen wrote:
> > > Apart from the doc change, the patch is ok now.
> > 
> > The preceding text says "Four schema are predefined, two schemata are
> > variable", then we add "env" to both lists? That doesn't make much sense to
> > me.  Surely it's just a "predefined schema"?  In any case that text should
> > be updated.
> 
> Ouch, yeah, I missed that.  Thanks for catching!

I accidentally already pushed this patch, so I took it on me to fix up
the documentation.


Corinna

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

* Re: [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-03-28 10:16         ` Corinna Vinschen
@ 2023-04-03  6:36           ` Johannes Schindelin
  2023-04-03 10:59             ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03  6:36 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Tue, 28 Mar 2023, Corinna Vinschen wrote:

> On Mar 28 10:17, Johannes Schindelin wrote:
> > We should not blindly set the home directory of the SYSTEM account (or
> > of Microsoft accounts) to /home/SYSTEM, especially not when that value
>                                   ^^^^^^
> That should probably better be <username>, no?

No, this is the actual name of the home directory when you start
`Cygwin.bat` using the SYSTEM account. To reproduce, I ran `PsExec64.exe
-s -i cmd` and then `C:\Cygwin64\Cygwin.bat` in that command prompt (after
verifying that `whoami` prints `nt authority\system`).

The Bash prompt then says `SYSTEM@<hostname>`, and `echo $HOME` reports
`/home/SYSTEM`.

Ciao,
Johannes

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

* Re: [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-03-29  8:36               ` Corinna Vinschen
@ 2023-04-03  6:39                 ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03  6:39 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: Jon Turney, cygwin-patches

Hi Corinna & Jon,

On Wed, 29 Mar 2023, Corinna Vinschen wrote:

> On Mar 28 15:31, Corinna Vinschen wrote:
> > On Mar 28 13:34, Jon Turney wrote:
> > > On 28/03/2023 11:35, Corinna Vinschen wrote:
> > > > Apart from the doc change, the patch is ok now.
> > >
> > > The preceding text says "Four schema are predefined, two schemata are
> > > variable", then we add "env" to both lists? That doesn't make much sense to
> > > me.  Surely it's just a "predefined schema"?  In any case that text should
> > > be updated.
> >
> > Ouch, yeah, I missed that.  Thanks for catching!
>
> I accidentally already pushed this patch, so I took it on me to fix up
> the documentation.

Thank you for fixing my mistake! 93b05a87c2 (Cygwin: doc: fix description
of new "env" schema for /etc/nsswitch.conf, 2023-03-29) looks good to me.

Ciao,
Johannes

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-03-28 10:17         ` Corinna Vinschen
@ 2023-04-03  6:45           ` Johannes Schindelin
  2023-04-03 13:12             ` Johannes Schindelin
  2023-04-03 13:19             ` Johannes Schindelin
  0 siblings, 2 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03  6:45 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Tue, 28 Mar 2023, Corinna Vinschen wrote:

> On Mar 28 10:17, Johannes Schindelin wrote:
> > In particular when we cannot figure out a uid for the current user, we
> > should still respect the `db_home: env` setting. Such a situation occurs
> > for example when the domain returned by `LookupAccountSid()` is not our
> > machine name and at the same time our machine is no domain member: In
> > that case, we have nobody to ask for the POSIX offset necessary to come
> > up with the uid.
> >
> > It is important that even in such cases, the `HOME` environment variable
> > can be used to override the home directory, e.g. when Git for Windows is
> > used by an account that was generated on the fly, e.g. for transient use
> > in a cloud scenario.
>
> How does this kind of account look like?  I'd like to see the contants
> of name, domain, and the SID.  Isn't that just an account closely
> resembling Micorosft Accounts or AzureAD accounts?  Can't we somehow
> handle them alike?

It took a good while to remind me what was going on there. Essentially, I
had to dig up a mail from 2016 that David Ebbo sent me via my work email
(because he was working on Kudu, the Azure shell, where this issue arose).
Sadly, David is no longer a colleague of mine (he seems to work at Google
now), so I cannot pester him about details. Besides, it might be too long
ago to remember details, anyways.

What I _can_ do is try to recreate the problem (the report said that this
happens in a Kudu console of an Azure Web App, see
https://github.com/projectkudu/kudu/wiki/Kudu-console) by creating a new
Azure Web App and opening that console and run Cygwin within it, which is
what I am going to do now.

> > Reported by David Ebbo.
>
> This should be
>
>   Reported-By: David Ebbo <email address>

Will fix. Naturally, it won't be his Microsoft email address any longer,
but the recent patches I obtained from repositories at
https://github.com/davidebbo/ have a GMail address on record.

> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index d493d29b3b..b01bcff5cb 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > @@ -883,6 +883,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
> >  	    case L'u':
> >  	      if (full_qualified)
> >  		{
> > +		  if (!dom)
> > +		    break;
>
> No domain?  Really?

Yes, I distinctly remember that I had to do that, otherwise the code would
not work as intended.

Ciao,
Johannes

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

* Re: [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-04-03  6:36           ` Johannes Schindelin
@ 2023-04-03 10:59             ` Corinna Vinschen
  2023-04-03 13:32               ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-03 10:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-patches

On Apr  3 08:36, Johannes Schindelin wrote:
> Hi Corinna,
> 
> On Tue, 28 Mar 2023, Corinna Vinschen wrote:
> 
> > On Mar 28 10:17, Johannes Schindelin wrote:
> > > We should not blindly set the home directory of the SYSTEM account (or
> > > of Microsoft accounts) to /home/SYSTEM, especially not when that value
> >                                   ^^^^^^
> > That should probably better be <username>, no?
> 
> No, this is the actual name of the home directory when you start
> `Cygwin.bat` using the SYSTEM account.

I know, but that doesn't match the beginning of your sentence:

  We should not blindly set the home directory of the SYSTEM account
  (or of Microsoft accounts)
   ^^^^^^^^^^^^^^^^^^^^^^^^


Corinna

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03  6:45           ` Johannes Schindelin
@ 2023-04-03 13:12             ` Johannes Schindelin
  2023-04-03 13:29               ` Corinna Vinschen
  2023-04-03 13:19             ` Johannes Schindelin
  1 sibling, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 13:12 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Johannes Schindelin wrote:

> On Tue, 28 Mar 2023, Corinna Vinschen wrote:
>
> > On Mar 28 10:17, Johannes Schindelin wrote:
> > > In particular when we cannot figure out a uid for the current user, we
> > > should still respect the `db_home: env` setting. Such a situation occurs
> > > for example when the domain returned by `LookupAccountSid()` is not our
> > > machine name and at the same time our machine is no domain member: In
> > > that case, we have nobody to ask for the POSIX offset necessary to come
> > > up with the uid.
> > >
> > > It is important that even in such cases, the `HOME` environment variable
> > > can be used to override the home directory, e.g. when Git for Windows is
> > > used by an account that was generated on the fly, e.g. for transient use
> > > in a cloud scenario.
> >
> > How does this kind of account look like?  I'd like to see the contants
> > of name, domain, and the SID.  Isn't that just an account closely
> > resembling Micorosft Accounts or AzureAD accounts?  Can't we somehow
> > handle them alike?
>
> [...]
>
> What I _can_ do is try to recreate the problem (the report said that this
> happens in a Kudu console of an Azure Web App, see
> https://github.com/projectkudu/kudu/wiki/Kudu-console) by creating a new
> Azure Web App and opening that console and run Cygwin within it, which is
> what I am going to do now.

So here is what is going on:

- The domain is 'IIS APPPOOL'

- The name is the name of the Azure Web App

- The sid is 'S-1-5-82-3932326390-3052311582-2886778547-4123178866-1852425102'

The program I am trying to make work as expected (i.e. to respect the
`db_home: env` line in `/etc/nsswitch.conf` in conjunction with the `HOME`
variable being set to `C:\home`) is `ssh-keygen.exe`: We want it to
default to creating the file `/cygdrive/c/home/.ssh/id_rsa`. But what it
_does_, without this patch, is to default to creating the file
`//.ssh/id_rsa` (which does not make sense because that would refer to a
file share called `id_rsa` on a server whose name is `.ssh`).

Condensed to the bare minimum reproducer, the code boils down to this:

-- snip --
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
#include <pwd.h>

int main(int argc, char **argv)
{
	uid_t uid = getuid();
	struct passwd *pw = getpwuid(uid);

	printf("uid=%u, pw_dir='%s'\n", (unsigned)uid, pw->pw_dir);

	return 0;
}
-- snap --

In the Kudu console scenario, this program prints the UID 4294967295
(which is 0xffffffff) and _without_ this patch, it prints the `pw_dir` as
being `/`, even if the `HOME` environment variable should override that
for the current user.

_With_ patch 3/3, it prints out the same `uid`, but it does print the
`pw_dir` as `/cygdrive/c/home`.

I will distill the above into a new-and-improved commit message.

Ciao,
Johannes

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03  6:45           ` Johannes Schindelin
  2023-04-03 13:12             ` Johannes Schindelin
@ 2023-04-03 13:19             ` Johannes Schindelin
  1 sibling, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 13:19 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Johannes Schindelin wrote:

> On Tue, 28 Mar 2023, Corinna Vinschen wrote:
>
> > On Mar 28 10:17, Johannes Schindelin wrote:
>
> > > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > > index d493d29b3b..b01bcff5cb 100644
> > > --- a/winsup/cygwin/uinfo.cc
> > > +++ b/winsup/cygwin/uinfo.cc
> > > @@ -883,6 +883,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
> > >  	    case L'u':
> > >  	      if (full_qualified)
> > >  		{
> > > +		  if (!dom)
> > > +		    break;
> >
> > No domain?  Really?
>
> Yes, I distinctly remember that I had to do that, otherwise the code would
> not work as intended.

Right. This is actually really easy to explain: The new call I introduced
in this very patch passes `NULL` as the `dom` parameter (because this is
in a scenario where we do indeed not have a domain to work with):

  if (arg.id == cygheap->user.real_uid)
	    home = cygheap->pg.get_home ((PUSER_INFO_3) NULL,
					 cygheap->user.sid(),
					 NULL, NULL, false);

					 ^^^^
					 this is the `dom` parameter

(see
https://github.com/dscho/msys2-runtime/commit/4cd6ae73074f327064b54a08392906dbc140714a#diff-1ffeda03bc188fa732454f52c4932977bc8233d9db4da19ab5acb0c58c7320ccR2188-R2191
for details)

Ciao,
Johannes

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03 13:12             ` Johannes Schindelin
@ 2023-04-03 13:29               ` Corinna Vinschen
  2023-04-03 13:57                 ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-03 13:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-patches

On Apr  3 15:12, Johannes Schindelin wrote:
> Hi Corinna,
> 
> On Mon, 3 Apr 2023, Johannes Schindelin wrote:
> 
> > On Tue, 28 Mar 2023, Corinna Vinschen wrote:
> >
> > > On Mar 28 10:17, Johannes Schindelin wrote:
> > > > In particular when we cannot figure out a uid for the current user, we
> > > > should still respect the `db_home: env` setting. Such a situation occurs
> > > > for example when the domain returned by `LookupAccountSid()` is not our
> > > > machine name and at the same time our machine is no domain member: In
> > > > that case, we have nobody to ask for the POSIX offset necessary to come
> > > > up with the uid.
> > > >
> > > > It is important that even in such cases, the `HOME` environment variable
> > > > can be used to override the home directory, e.g. when Git for Windows is
> > > > used by an account that was generated on the fly, e.g. for transient use
> > > > in a cloud scenario.
> > >
> > > How does this kind of account look like?  I'd like to see the contants
> > > of name, domain, and the SID.  Isn't that just an account closely
> > > resembling Micorosft Accounts or AzureAD accounts?  Can't we somehow
> > > handle them alike?
> >
> > [...]
> >
> > What I _can_ do is try to recreate the problem (the report said that this
> > happens in a Kudu console of an Azure Web App, see
> > https://github.com/projectkudu/kudu/wiki/Kudu-console) by creating a new
> > Azure Web App and opening that console and run Cygwin within it, which is
> > what I am going to do now.
> 
> So here is what is going on:
> 
> - The domain is 'IIS APPPOOL'

There's a domain, so why not pass it to the called function?>

> - The name is the name of the Azure Web App
> 
> - The sid is 'S-1-5-82-3932326390-3052311582-2886778547-4123178866-1852425102'

Oh well. These are basically the same thing as 1-5-80 service accounts.
It would be great if we could handle them gracefully instead of
special-case them in a piece of code we just reach because we don't
handle them yet.

Btw., one easy way out would be if we default to /home/<name> or
/home/<SID> rather than "/", isn't it?


Corinna

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

* Re: [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-04-03 10:59             ` Corinna Vinschen
@ 2023-04-03 13:32               ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 13:32 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Corinna Vinschen wrote:

> On Apr  3 08:36, Johannes Schindelin wrote:
> > Hi Corinna,
> >
> > On Tue, 28 Mar 2023, Corinna Vinschen wrote:
> >
> > > On Mar 28 10:17, Johannes Schindelin wrote:
> > > > We should not blindly set the home directory of the SYSTEM account (or
> > > > of Microsoft accounts) to /home/SYSTEM, especially not when that value
> > >                                   ^^^^^^
> > > That should probably better be <username>, no?
> >
> > No, this is the actual name of the home directory when you start
> > `Cygwin.bat` using the SYSTEM account.
>
> I know, but that doesn't match the beginning of your sentence:
>
>   We should not blindly set the home directory of the SYSTEM account
>   (or of Microsoft accounts)
>    ^^^^^^^^^^^^^^^^^^^^^^^^

Ah. I totally focused on the wrong aspect. Will fix the commit message.

Ciao,
Johannes

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03 13:29               ` Corinna Vinschen
@ 2023-04-03 13:57                 ` Johannes Schindelin
  2023-04-03 19:23                   ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 13:57 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Corinna Vinschen wrote:

> On Apr  3 15:12, Johannes Schindelin wrote:
>
> > On Mon, 3 Apr 2023, Johannes Schindelin wrote:
> >
> > > On Tue, 28 Mar 2023, Corinna Vinschen wrote:
> > >
> > > > On Mar 28 10:17, Johannes Schindelin wrote:
> > > > > In particular when we cannot figure out a uid for the current user, we
> > > > > should still respect the `db_home: env` setting. Such a situation occurs
> > > > > for example when the domain returned by `LookupAccountSid()` is not our
> > > > > machine name and at the same time our machine is no domain member: In
> > > > > that case, we have nobody to ask for the POSIX offset necessary to come
> > > > > up with the uid.
> > > > >
> > > > > It is important that even in such cases, the `HOME` environment variable
> > > > > can be used to override the home directory, e.g. when Git for Windows is
> > > > > used by an account that was generated on the fly, e.g. for transient use
> > > > > in a cloud scenario.
> > > >
> > > > How does this kind of account look like?  I'd like to see the contants
> > > > of name, domain, and the SID.  Isn't that just an account closely
> > > > resembling Micorosft Accounts or AzureAD accounts?  Can't we somehow
> > > > handle them alike?
> > >
> > > [...]
> > >
> > > What I _can_ do is try to recreate the problem (the report said that this
> > > happens in a Kudu console of an Azure Web App, see
> > > https://github.com/projectkudu/kudu/wiki/Kudu-console) by creating a new
> > > Azure Web App and opening that console and run Cygwin within it, which is
> > > what I am going to do now.
> >
> > So here is what is going on:
> >
> > - The domain is 'IIS APPPOOL'
>
> There's a domain, so why not pass it to the called function?>

Sorry, I was unclear. This domain _is_ used when looking for the uid, but
then we run into a code path where the UID cannot be determined (because
the domain of the account is not the machine name and the machine is no
domain member). The clause in question is here:
https://github.com/cygwin/cygwin/blob/cygwin-3.4.6/winsup/cygwin/uinfo.cc#L2303-L2310.
The Cygwin runtime then returns -1 as UID.

The _subsequent_ call to `getpwuid(-1)` is the one where we need to teach
Cygwin to respect `db_home: env`. This is the code path taken by OpenSSH.
And that code path only has an `arg.id` to work with (the `type` is
`ID_arg`), and that `arg.id` is invalid. There is no domain in that code
path that we could possibly pass to the `get_home()` method.

> > - The name is the name of the Azure Web App
> >
> > - The sid is 'S-1-5-82-3932326390-3052311582-2886778547-4123178866-1852425102'
>
> Oh well. These are basically the same thing as 1-5-80 service accounts.
> It would be great if we could handle them gracefully instead of
> special-case them in a piece of code we just reach because we don't
> handle them yet.

True, but I don't really understand how they could be handled.

> Btw., one easy way out would be if we default to /home/<name> or
> /home/<SID> rather than "/", isn't it?

The default does not really matter, as the bug fix is about respecting
whatever the user has configured via the `HOME` variable, i.e. it's all
about the case when the default needs to be overridden, whatever that
default is.

Ciao,
Johannes

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

* [PATCH v5 0/3] Support deriving the current user's home directory via HOME
  2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
                         ` (2 preceding siblings ...)
  2023-03-28  8:17       ` [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
@ 2023-04-03 14:44       ` Johannes Schindelin
  2023-04-03 14:44         ` [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
                           ` (3 more replies)
  3 siblings, 4 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 14:44 UTC (permalink / raw)
  To: cygwin-patches

This patch mini-series supports Git for Windows' default strategy to
determine the current user's home directory by looking at the
environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
if these variables are also unset, to USERPROFILE.

This strategy is a quick method to determine the home directory,
certainly quicker than looking at LDAP, even more so when a domain
controller is unreachable and causes long hangs in Cygwin's startup.

This strategy also allows users to override the home directory easily
(e.g. in case that their real home directory is a network share that is
not all that well handled by some commands such as cmd.exe's cd
command).

Changes since v4:

- Squashed in Corinna's documentation fixes (read: patch 1 should not be
  applied to Cygwin's main branch, it's presented here for backporting
  purposes).

- Fixed the commit message of the second patch that mistakenly claimed
  that Microsoft accounts would be associated with `/home/SYSTEM`.

- Completely overhauled the commit message of the third patch to motivate
  much better why this fix is needed.

Changes since v3:

- Fixed the bug in v2 where `getenv("HOME")` would convert the value to
  a Unix-y path and the `fetch_home_env()` function would then try to
  convert it _again_.

- Disentangled the logic in `fetch_home_env()` instead of doing
  everything in one big, honking, unreadable `if` condition.

- Commented the code in `fetch_home_env()`.

Changes since v2:

- Using `getenv()` and `cygwin_create_path()` instead of the
  `GetEnvironmentVariableW()`/`cygwin_conv_path()` dance

- Adjusted the documentation to drive home that this only affects the
  _current_ user's home directory

- Using the `PUSER_INFO_3` variant of `get_home()`

- Adjusted the commit messages

- Added another patch, to support "ad-hoc cloud accounts"

Johannes Schindelin (3):
  Allow deriving the current user's home directory via the HOME variable
  Respect `db_home` setting even for SYSTEM/Microsoft accounts
  Respect `db_home: env` even when no uid can be determined

 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 70 ++++++++++++++++++++++++--
 winsup/doc/ntsec.xml                   | 20 +++++++-
 3 files changed, 88 insertions(+), 5 deletions(-)

Range-diff:
1:  7a074997ea ! 1:  e26cae9439 Allow deriving the current user's home directory via the HOME variable
    @@ Commit message
         Of course this scheme needs to be opt-in.  For that reason, it needs
         to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

    +    Documentation-fixes-by: Corinna Vinschen <corinna@vinschen.de>
         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

      ## winsup/cygwin/local_includes/cygheap.h ##
    @@ winsup/cygwin/uinfo.cc: cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid
      	  if (ui)

      ## winsup/doc/ntsec.xml ##
    +@@ winsup/doc/ntsec.xml: and on non-AD machines.
    + </para>
    +
    + <para>
    +-Four schemata are predefined, two schemata are variable.  The predefined
    ++Five schemata are predefined, two schemata are variable.  The predefined
    + schemata are the following:
    + </para>
    +
     @@ winsup/doc/ntsec.xml: schemata are the following:
      	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
      	      for a more detailed description.</listitem>
        </varlistentry>
     +  <varlistentry>
     +    <term><literal>env</literal></term>
    -+    <listitem>Derives the home directory of the current user from the
    -+	      environment variable <literal>HOME</literal> (falling back to
    -+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
    -+	      <literal>USERPROFILE</literal>, in that order).  This is faster
    -+	      than the <term><literal>windows</literal></term> schema at the
    -+	      expense of determining only the current user's home directory
    -+	      correctly.  This schema is skipped for any other account.
    -+	      </listitem>
    ++    <listitem>Utilizes the user's environment.  This schema is only supported
    ++	      for setting the home directory yet.
    ++	      See <xref linkend="ntsec-mapping-nsswitch-home"></xref> for
    ++	      the description.</listitem>
     +  </varlistentry>
      </variablelist>

    @@ winsup/doc/ntsec.xml: of each schema when used with <literal>db_home:</literal>
     +	      environment variable <literal>HOME</literal> (falling back to
     +	      <literal>HOMEDRIVE\HOMEPATH</literal> and
     +	      <literal>USERPROFILE</literal>, in that order).  This is faster
    -+	      than the <term><literal>windows</literal></term> schema at the
    ++	      than the <literal>windows</literal> schema at the
     +	      expense of determining only the current user's home directory
     +	      correctly.  This schema is skipped for any other account.
     +	      </listitem>
2:  a70c77dc8f ! 2:  085d4dd8b6 Respect `db_home` setting even for SYSTEM/Microsoft accounts
    @@ Commit message
         Respect `db_home` setting even for SYSTEM/Microsoft accounts

         We should not blindly set the home directory of the SYSTEM account (or
    -    of Microsoft accounts) to /home/SYSTEM, especially not when that value
    -    disagrees with what is configured via the `db_home` line in the
    -    `/etc/nsswitch.conf` file.
    +    of Microsoft accounts) to `/home/<name>`, especially
    +    `/etc/nsswitch.conf` defines `db_home: env`, in which case we want to
    +    respect the `HOME` variable.

         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

3:  4cd6ae7307 ! 3:  cf47afceba Respect `db_home: env` even when no uid can be determined
    @@ Metadata
      ## Commit message ##
         Respect `db_home: env` even when no uid can be determined

    -    In particular when we cannot figure out a uid for the current user, we
    -    should still respect the `db_home: env` setting. Such a situation occurs
    -    for example when the domain returned by `LookupAccountSid()` is not our
    -    machine name and at the same time our machine is no domain member: In
    -    that case, we have nobody to ask for the POSIX offset necessary to come
    -    up with the uid.
    +    When we cannot figure out a uid for the current user, we should still
    +    respect the `db_home: env` setting.

    -    It is important that even in such cases, the `HOME` environment variable
    -    can be used to override the home directory, e.g. when Git for Windows is
    -    used by an account that was generated on the fly, e.g. for transient use
    -    in a cloud scenario.
    +    This is particularly important when programs like `ssh` look for the
    +    home directory of the usr, the user overrode `HOME` to "help" Cygwin
    +    determine where the home directory is. Cygwin should not ignore this.

    -    Reported by David Ebbo.
    +    One situation where we cannot determine a uid is when the domain
    +    returned by `LookupAccountSid()` is not our machine name and at the same
    +    time our machine is no domain member: In that case, we have nobody to
    +    ask for the POSIX offset necessary to come up with the uid.

    +    Azure Web Apps represent such a scenario, which can be verified e.g. in
    +    a Kudu console (for details about Kudu consoles, see
    +    https://github.com/projectkudu/kudu/wiki/Kudu-console): the domain is
    +    `IIS APPPOOL`, the account name is the name of the Azure Web App, the
    +    SID starts with 'S-1-5-82-`, and `pwdgrp::fetch_account_from_windows()`
    +    runs into the code path where "[...] the domain returned by
    +    LookupAccountSid is not our machine name, and if our machine is no
    +    domain member, we lose.  We have nobody to ask for the POSIX offset."
    +
    +    In such a scenario, OpenSSH's `getuid()` call will receive the return
    +    value -1, and the subsequent `getpwuid()` call (whose return value's
    +    `pw_dir` is used as home directory) needs to be forced to respect
    +    `db_home: env`, which this here patch does.
    +
    +    Reported-by: David Ebbo <david.ebbo@gmail.com>
         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

      ## winsup/cygwin/uinfo.cc ##

base-commit: a9a17f5fe51498b182d4a11ac48207b8c7ffe8ec
Published-As: https://github.com/dscho/msys2-runtime/releases/tag/home-env-cygwin-v5
Fetch-It-Via: git fetch https://github.com/dscho/msys2-runtime home-env-cygwin-v5

--
2.40.0.windows.1


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

* [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
@ 2023-04-03 14:44         ` Johannes Schindelin
  2023-04-03 18:36           ` Corinna Vinschen
  2023-04-03 14:45         ` [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
                           ` (2 subsequent siblings)
  3 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 14:44 UTC (permalink / raw)
  To: cygwin-patches

This patch hails from Git for Windows (where the Cygwin runtime is used
in the form of a slightly modified MSYS2 runtime), where it is a
well-established technique to let the `$HOME` variable define where the
current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSYS2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly, e.g. `cmd.exe` and other native Windows
applications that users want to employ as Git helpers.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or
`$USERPROFILE`.  This has the additional advantage that it is much
faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Documentation-fixes-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 51 ++++++++++++++++++++++++++
 winsup/doc/ntsec.xml                   | 20 +++++++++-
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/local_includes/cygheap.h b/winsup/cygwin/local_includes/cygheap.h
index d885ca1230..b6acdf7f18 100644
--- a/winsup/cygwin/local_includes/cygheap.h
+++ b/winsup/cygwin/local_includes/cygheap.h
@@ -358,7 +358,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 30df6db6d8..baa670478d 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -733,6 +733,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -921,6 +923,42 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }

+static char *
+fetch_home_env (void)
+{
+  /* If `HOME` is set, prefer it */
+  const char *home = getenv ("HOME");
+  if (home)
+    return strdup (home);
+
+  /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
+     (without a directory separator, as `HOMEPATH` starts with one). */
+  const char *home_drive = getenv ("HOMEDRIVE");
+  if (home_drive)
+    {
+      const char *home_path = getenv ("HOMEPATH");
+      if (home_path)
+	{
+	  tmp_pathbuf tp;
+	  char *p = tp.c_get (), *q;
+
+	  // concatenate HOMEDRIVE and HOMEPATH
+	  q = stpncpy (p, home_drive, NT_MAX_PATH);
+	  strlcpy (q, home_path, NT_MAX_PATH - (q - p));
+	  return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, p);
+	}
+    }
+
+  /* If neither `HOME` nor `HOMEDRIVE``HOMEPATH` are set, fall back
+     to `USERPROFILE`; In corporate setups, this might point to a
+     disconnected network share, hence this is the last fall back. */
+  home = getenv ("USERPROFILE");
+  if (home)
+    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -980,6 +1018,10 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1012,6 +1054,10 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1031,6 +1077,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1095,6 +1142,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
@@ -1176,6 +1224,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1202,6 +1252,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index c6871ecf05..687789076c 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1167,7 +1167,7 @@ and on non-AD machines.
 </para>

 <para>
-Four schemata are predefined, two schemata are variable.  The predefined
+Five schemata are predefined, two schemata are variable.  The predefined
 schemata are the following:
 </para>

@@ -1203,6 +1203,13 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Utilizes the user's environment.  This schema is only supported
+	      for setting the home directory yet.
+	      See <xref linkend="ntsec-mapping-nsswitch-home"></xref> for
+	      the description.</listitem>
+  </varlistentry>
 </variablelist>

 <para>
@@ -1335,6 +1342,17 @@ of each schema when used with <literal>db_home:</literal>
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <literal>windows</literal> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
   <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
--
2.40.0.windows.1



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

* [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-04-03 14:44         ` [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2023-04-03 14:45         ` Johannes Schindelin
  2023-04-03 18:37           ` Corinna Vinschen
  2023-04-03 14:45         ` [PATCH v5 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  3 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 14:45 UTC (permalink / raw)
  To: cygwin-patches

We should not blindly set the home directory of the SYSTEM account (or
of Microsoft accounts) to `/home/<name>`, especially
`/etc/nsswitch.conf` defines `db_home: env`, in which case we want to
respect the `HOME` variable.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index baa670478d..d493d29b3b 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+				       fully_qualified_name);
+	}
       switch ((int) acc_type)
 	{
 	case SidTypeUser:
--
2.40.0.windows.1



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

* [PATCH v5 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-04-03 14:44         ` [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
  2023-04-03 14:45         ` [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2023-04-03 14:45         ` Johannes Schindelin
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  3 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-03 14:45 UTC (permalink / raw)
  To: cygwin-patches

When we cannot figure out a uid for the current user, we should still
respect the `db_home: env` setting.

This is particularly important when programs like `ssh` look for the
home directory of the usr, the user overrode `HOME` to "help" Cygwin
determine where the home directory is. Cygwin should not ignore this.

One situation where we cannot determine a uid is when the domain
returned by `LookupAccountSid()` is not our machine name and at the same
time our machine is no domain member: In that case, we have nobody to
ask for the POSIX offset necessary to come up with the uid.

Azure Web Apps represent such a scenario, which can be verified e.g. in
a Kudu console (for details about Kudu consoles, see
https://github.com/projectkudu/kudu/wiki/Kudu-console): the domain is
`IIS APPPOOL`, the account name is the name of the Azure Web App, the
SID starts with 'S-1-5-82-`, and `pwdgrp::fetch_account_from_windows()`
runs into the code path where "[...] the domain returned by
LookupAccountSid is not our machine name, and if our machine is no
domain member, we lose.  We have nobody to ask for the POSIX offset."

In such a scenario, OpenSSH's `getuid()` call will receive the return
value -1, and the subsequent `getpwuid()` call (whose return value's
`pw_dir` is used as home directory) needs to be forced to respect
`db_home: env`, which this here patch does.

Reported-by: David Ebbo <david.ebbo@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index d493d29b3b..b01bcff5cb 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -883,6 +883,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
 	    case L'u':
 	      if (full_qualified)
 		{
+		  if (!dom)
+		    break;
 		  w = wcpncpy (w, dom, we - w);
 		  if (w < we)
 		    *w++ = NSS_SEPARATOR_CHAR;
@@ -893,6 +895,8 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
 	      w = wcpncpy (w, name, we - w);
 	      break;
 	    case L'D':
+	      if (!dom)
+		break;
 	      w = wcpncpy (w, dom, we - w);
 	      break;
 	    case L'H':
@@ -2181,6 +2185,10 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	{
 	  /* Just some fake. */
 	  sid = csid.create (99, 1, 0);
+	  if (arg.id == cygheap->user.real_uid)
+	    home = cygheap->pg.get_home ((PUSER_INFO_3) NULL,
+					 cygheap->user.sid(),
+					 NULL, NULL, false);
 	  break;
 	}
       else if (arg.id >= UNIX_POSIX_OFFSET)
@@ -2710,10 +2718,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
      logon.  Unless it's the SYSTEM account.  This conveniently allows to
      logon interactively as SYSTEM for debugging purposes. */
   else if (acc_type != SidTypeUser && sid != well_known_system_sid)
-    __small_sprintf (linebuf, "%W:*:%u:%u:U-%W\\%W,%s:/:/sbin/nologin",
+    __small_sprintf (linebuf, "%W:*:%u:%u:U-%W\\%W,%s:%s:/sbin/nologin",
 		     posix_name, uid, gid,
 		     dom, name,
-		     sid.string ((char *) sidstr));
+		     sid.string ((char *) sidstr),
+		     home ? home : "/");
   else
     __small_sprintf (linebuf, "%W:*:%u:%u:%s%sU-%W\\%W,%s:%s%W:%s",
 		     posix_name, uid, gid,
--
2.40.0.windows.1

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

* Re: [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-04-03 14:44         ` [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2023-04-03 18:36           ` Corinna Vinschen
  2023-04-04 15:12             ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-03 18:36 UTC (permalink / raw)
  To: cygwin-patches

On Apr  3 16:44, Johannes Schindelin wrote:
> This patch hails from Git for Windows (where the Cygwin runtime is used
> in the form of a slightly modified MSYS2 runtime), where it is a
> well-established technique to let the `$HOME` variable define where the
> current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
> and `$USERPROFILE`.

This patch is already merged.


Corinna

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

* Re: [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-04-03 14:45         ` [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2023-04-03 18:37           ` Corinna Vinschen
  2023-04-04 15:12             ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-03 18:37 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-patches

On Apr  3 16:45, Johannes Schindelin wrote:
> We should not blindly set the home directory of the SYSTEM account (or
> of Microsoft accounts) to `/home/<name>`, especially
> `/etc/nsswitch.conf` defines `db_home: env`, in which case we want to
> respect the `HOME` variable.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/uinfo.cc | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> index baa670478d..d493d29b3b 100644
> --- a/winsup/cygwin/uinfo.cc
> +++ b/winsup/cygwin/uinfo.cc
> @@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
>  	 it to a well-known group here. */
>        if (acc_type == SidTypeUser
>  	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
> -	acc_type = SidTypeWellKnownGroup;
> +	{
> +	  acc_type = SidTypeWellKnownGroup;
> +	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
> +				       fully_qualified_name);
> +	}
>        switch ((int) acc_type)
>  	{
>  	case SidTypeUser:

Pushed.


Thanks,
Corinna

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03 13:57                 ` Johannes Schindelin
@ 2023-04-03 19:23                   ` Corinna Vinschen
  2023-04-04 15:11                     ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-03 19:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: cygwin-patches

On Apr  3 15:57, Johannes Schindelin wrote:
> On Mon, 3 Apr 2023, Corinna Vinschen wrote:
> > > So here is what is going on:
> > >
> > > - The domain is 'IIS APPPOOL'
> >
> > There's a domain, so why not pass it to the called function?>
> 
> Sorry, I was unclear. This domain _is_ used when looking for the uid, but
> then we run into a code path where the UID cannot be determined (because
> the domain of the account is not the machine name and the machine is no
> domain member). The clause in question is here:
> https://github.com/cygwin/cygwin/blob/cygwin-3.4.6/winsup/cygwin/uinfo.cc#L2303-L2310.
> The Cygwin runtime then returns -1 as UID.
> 
> The _subsequent_ call to `getpwuid(-1)` is the one where we need to teach
> Cygwin to respect `db_home: env`. This is the code path taken by OpenSSH.
> And that code path only has an `arg.id` to work with (the `type` is
> `ID_arg`), and that `arg.id` is invalid. There is no domain in that code
> path that we could possibly pass to the `get_home()` method.

That makes a lot of sense.  However, wouldn't it be better to return
some kind of valid uid, rather than working around uid -1?

> > > - The name is the name of the Azure Web App
> > >
> > > - The sid is 'S-1-5-82-3932326390-3052311582-2886778547-4123178866-1852425102'
> >
> > Oh well. These are basically the same thing as 1-5-80 service accounts.
> > It would be great if we could handle them gracefully instead of
> > special-case them in a piece of code we just reach because we don't
> > handle them yet.
> 
> True, but I don't really understand how they could be handled.

We do something along these lines already for the AzureAD SIDs of type
S-1-12-1-what-the-heck.  If we do the same for the S-1-5-82 IIS AppPool
accounts, we may be able to handle this more sanely.  Just search for
AzureAD in uinfo.cc.

What do you think?


Corinna

> > Btw., one easy way out would be if we default to /home/<name> or
> > /home/<SID> rather than "/", isn't it?
> 
> The default does not really matter, as the bug fix is about respecting
> whatever the user has configured via the `HOME` variable, i.e. it's all
> about the case when the default needs to be overridden, whatever that
> default is.

Right, that wouldn't help then.


Corinna

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

* [PATCH v6 0/4] Support deriving the current user's home directory via HOME
  2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
                           ` (2 preceding siblings ...)
  2023-04-03 14:45         ` [PATCH v5 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
@ 2023-04-04 15:07         ` Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
                             ` (4 more replies)
  3 siblings, 5 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:07 UTC (permalink / raw)
  To: cygwin-patches

This patch series supports Git for Windows' default strategy to
determine the current user's home directory by looking at the
environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
if these variables are also unset, to USERPROFILE.

This strategy is a quick method to determine the home directory,
certainly quicker than looking at LDAP, even more so when a domain
controller is unreachable and causes long hangs in Cygwin's startup.

This strategy also allows users to override the home directory easily
(e.g. in case that their real home directory is a network share that is
not all that well handled by some commands such as cmd.exe's cd
command).

NOTE! This iteration presents patches 1 & 2 only for completeness' sake
and for backporting, as they have been applied to Cygwin's main branch
already.

Changes since v5:

- Replaced the third patch by a patch that imitates AzureAD account
  handling also for IIS APPPPOOL ones.

- Added a fourth patch to fix a bug in the first patch (which
  unfortunately was already applied in the buggy form) where _very
  early_ calls to `internal_pwsid ()` would result in completely bogus
  home directory values.

Changes since v4:

- Squashed in Corinna's documentation fixes (read: patch 1 should not be
  applied to Cygwin's main branch, it's presented here for backporting
  purposes).

- Fixed the commit message of the second patch that mistakenly claimed
  that Microsoft accounts would be associated with `/home/SYSTEM`.

- Completely overhauled the commit message of the third patch to motivate
  much better why this fix is needed.

Changes since v3:

- Fixed the bug in v2 where `getenv("HOME")` would convert the value to
  a Unix-y path and the `fetch_home_env()` function would then try to
  convert it _again_.

- Disentangled the logic in `fetch_home_env()` instead of doing
  everything in one big, honking, unreadable `if` condition.

- Commented the code in `fetch_home_env()`.

Changes since v2:

- Using `getenv()` and `cygwin_create_path()` instead of the
  `GetEnvironmentVariableW()`/`cygwin_conv_path()` dance

- Adjusted the documentation to drive home that this only affects the
  _current_ user's home directory

- Using the `PUSER_INFO_3` variant of `get_home()`

- Adjusted the commit messages

- Added another patch, to support "ad-hoc cloud accounts"

Johannes Schindelin (4):
  Allow deriving the current user's home directory via the HOME variable
  Respect `db_home` setting even for SYSTEM/Microsoft accounts
  uinfo: special-case IIS APPPOOL accounts
  Do not rely on `getenv ("HOME")`'s path conversion

 winsup/cygwin/local_includes/cygheap.h |   3 +-
 winsup/cygwin/uinfo.cc                 | 170 +++++++++++++++++++++++--
 winsup/doc/ntsec.xml                   |  20 ++-
 3 files changed, 181 insertions(+), 12 deletions(-)

Range-diff:
1:  e26cae9439 = 1:  e26cae9439 Allow deriving the current user's home directory via the HOME variable
2:  085d4dd8b6 = 2:  085d4dd8b6 Respect `db_home` setting even for SYSTEM/Microsoft accounts
3:  cf47afceba < -:  ---------- Respect `db_home: env` even when no uid can be determined
-:  ---------- > 3:  9b79624368 uinfo: special-case IIS APPPOOL accounts
-:  ---------- > 4:  8ac1548b92 Do not rely on `getenv ("HOME")`'s path conversion

base-commit: a9a17f5fe51498b182d4a11ac48207b8c7ffe8ec
Published-As: https://github.com/dscho/msys2-runtime/releases/tag/home-env-cygwin-v6
Fetch-It-Via: git fetch https://github.com/dscho/msys2-runtime home-env-cygwin-v6

--
2.40.0.windows.1


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

* [PATCH v6 1/4] Allow deriving the current user's home directory via the HOME variable
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
@ 2023-04-04 15:07           ` Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
                             ` (3 subsequent siblings)
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:07 UTC (permalink / raw)
  To: cygwin-patches

This patch hails from Git for Windows (where the Cygwin runtime is used
in the form of a slightly modified MSYS2 runtime), where it is a
well-established technique to let the `$HOME` variable define where the
current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSYS2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly, e.g. `cmd.exe` and other native Windows
applications that users want to employ as Git helpers.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or
`$USERPROFILE`.  This has the additional advantage that it is much
faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Documentation-fixes-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 51 ++++++++++++++++++++++++++
 winsup/doc/ntsec.xml                   | 20 +++++++++-
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/local_includes/cygheap.h b/winsup/cygwin/local_includes/cygheap.h
index d885ca1230..b6acdf7f18 100644
--- a/winsup/cygwin/local_includes/cygheap.h
+++ b/winsup/cygwin/local_includes/cygheap.h
@@ -358,7 +358,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 30df6db6d8..baa670478d 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -733,6 +733,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -921,6 +923,42 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }

+static char *
+fetch_home_env (void)
+{
+  /* If `HOME` is set, prefer it */
+  const char *home = getenv ("HOME");
+  if (home)
+    return strdup (home);
+
+  /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
+     (without a directory separator, as `HOMEPATH` starts with one). */
+  const char *home_drive = getenv ("HOMEDRIVE");
+  if (home_drive)
+    {
+      const char *home_path = getenv ("HOMEPATH");
+      if (home_path)
+	{
+	  tmp_pathbuf tp;
+	  char *p = tp.c_get (), *q;
+
+	  // concatenate HOMEDRIVE and HOMEPATH
+	  q = stpncpy (p, home_drive, NT_MAX_PATH);
+	  strlcpy (q, home_path, NT_MAX_PATH - (q - p));
+	  return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, p);
+	}
+    }
+
+  /* If neither `HOME` nor `HOMEDRIVE``HOMEPATH` are set, fall back
+     to `USERPROFILE`; In corporate setups, this might point to a
+     disconnected network share, hence this is the last fall back. */
+  home = getenv ("USERPROFILE");
+  if (home)
+    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -980,6 +1018,10 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1012,6 +1054,10 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1031,6 +1077,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1095,6 +1142,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
@@ -1176,6 +1224,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1202,6 +1252,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index c6871ecf05..687789076c 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1167,7 +1167,7 @@ and on non-AD machines.
 </para>

 <para>
-Four schemata are predefined, two schemata are variable.  The predefined
+Five schemata are predefined, two schemata are variable.  The predefined
 schemata are the following:
 </para>

@@ -1203,6 +1203,13 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Utilizes the user's environment.  This schema is only supported
+	      for setting the home directory yet.
+	      See <xref linkend="ntsec-mapping-nsswitch-home"></xref> for
+	      the description.</listitem>
+  </varlistentry>
 </variablelist>

 <para>
@@ -1335,6 +1342,17 @@ of each schema when used with <literal>db_home:</literal>
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <literal>windows</literal> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
   <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
--
2.40.0.windows.1



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

* [PATCH v6 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2023-04-04 15:07           ` Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 3/4] uinfo: special-case IIS APPPOOL accounts Johannes Schindelin
                             ` (2 subsequent siblings)
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:07 UTC (permalink / raw)
  To: cygwin-patches

We should not blindly set the home directory of the SYSTEM account (or
of Microsoft accounts) to `/home/<name>`, especially
`/etc/nsswitch.conf` defines `db_home: env`, in which case we want to
respect the `HOME` variable.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index baa670478d..d493d29b3b 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+				       fully_qualified_name);
+	}
       switch ((int) acc_type)
 	{
 	case SidTypeUser:
--
2.40.0.windows.1



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

* [PATCH v6 3/4] uinfo: special-case IIS APPPOOL accounts
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2023-04-04 15:07           ` Johannes Schindelin
  2023-04-04 15:07           ` [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:07 UTC (permalink / raw)
  To: cygwin-patches

The account under which Azure Web Apps run is an IIS APPOOL account that
is generated on the fly.

These are special because the virtual machines on which thes Apps run
are not domain-joined, yet the accounts are domain accounts.

To support the use case where such a Web App needs to call `ssh` (e.g.
to deploy from a Git repository that is accessible only via SSH), we do
need OpenSSH's `getpwuid (getuid ())` invocation to work.

But currently it does not. Concretely, `getuid ()` returns -1 for these
accounts, and OpenSSH fails to find the correct home directory
(_especially_ when that home directory was overridden via a `db_home:
env` line in `/etc/nsswitch.conf`).

This can be verified e.g. in a Kudu console (for details about Kudu
consoles, see https://github.com/projectkudu/kudu/wiki/Kudu-console):
the domain is `IIS APPPOOL`, the account name is the name of the Azure
Web App, the SID starts with 'S-1-5-82-`, and
`pwdgrp::fetch_account_from_windows()` runs into the code path where
"[...] the domain returned by LookupAccountSid is not our machine name,
and if our machine is no domain member, we lose.  We have nobody to ask
for the POSIX offset."

Since these IIS APPPOOL accounts are relatively similar to AzureAD
accounts in this scenario, let's imitate the latter to support also the
former.

Reported-by: David Ebbo <david.ebbo@gmail.com>
Helped-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 107 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 98 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index d493d29b3b..5e2d88bcd7 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -1485,9 +1485,9 @@ get_logon_sid ()
     }
 }

-/* Fetch special AzureAD group, which is part of the token group list but
-   *not* recognized by LookupAccountSid (ERROR_NONE_MAPPED). */
-static cygsid azure_grp_sid ("");
+/* Fetch special AzureAD and IIS APPPOOL groups, which are part of the token
+   group list but *not* recognized by LookupAccountSid (ERROR_NONE_MAPPED). */
+static cygsid azure_grp_sid (""), iis_apppool_grp_sid ("");

 static void
 get_azure_grp_sid ()
@@ -1515,6 +1515,36 @@ get_azure_grp_sid ()
     }
 }

+static void
+get_iis_apppool_grp_sid ()
+{
+  if (PSID (iis_apppool_grp_sid) == NO_SID)
+    {
+      NTSTATUS status;
+      ULONG size;
+      tmp_pathbuf tp;
+      PTOKEN_GROUPS groups = (PTOKEN_GROUPS) tp.w_get ();
+
+      status = NtQueryInformationToken (hProcToken, TokenGroups, groups,
+					2 * NT_MAX_PATH, &size);
+      if (!NT_SUCCESS (status))
+	debug_printf ("NtQueryInformationToken (TokenGroups) %y", status);
+      else
+	{
+	  for (DWORD pg = 0; pg < groups->GroupCount; ++pg)
+	    {
+	      PSID sid = groups->Groups[pg].Sid;
+	      if (sid_id_auth (sid) == 5 &&
+		  sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID)
+		{
+		  iis_apppool_grp_sid = sid;
+		  break;
+		}
+	    }
+	}
+    }
+}
+
 void *
 pwdgrp::add_account_post_fetch (char *line, bool lock)
 {
@@ -1796,6 +1826,16 @@ pwdgrp::construct_sid_from_name (cygsid &sid, wchar_t *name, wchar_t *sep)
 	}
       return false;
     }
+  if (sep && wcscmp (name, L"IIS APPPOOL\\Group") == 0)
+    {
+      get_iis_apppool_grp_sid ();
+      if (PSID (logon_sid) != NO_SID)
+	{
+	  sid = iis_apppool_grp_sid;
+	  return true;
+	}
+      return false;
+    }
   if (!sep && wcscmp (name, L"CurrentSession") == 0)
     {
       get_logon_sid ();
@@ -2018,8 +2058,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
       /* Last but not least, some validity checks on the name style. */
       if (!fq_name)
 	{
-	  /* AzureAD user must be prepended by "domain" name. */
-	  if (sid_id_auth (sid) == 12)
+	  /* AzureAD and IIS APPPOOL users must be prepended by "domain"
+	     name. */
+	  if (sid_id_auth (sid) == 12 ||
+	      (sid_id_auth (sid) == 5 &&
+	       sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID))
 	    return NULL;
 	  /* name_only account is either builtin or primary domain, or
 	     account domain on non-domain machines. */
@@ -2045,8 +2088,10 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	}
       else
 	{
-	  /* AzureAD accounts should be fully qualifed either. */
-	  if (sid_id_auth (sid) == 12)
+	  /* AzureAD and IIS APPPOOL accounts should be fully qualifed either. */
+	  if (sid_id_auth (sid) == 12 ||
+	      (sid_id_auth (sid) == 5 &&
+	       sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID))
 	    break;
 	  /* Otherwise, no fully_qualified for builtin accounts, except for
 	     NT SERVICE, for which we require the prefix.  Note that there's
@@ -2125,6 +2170,19 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	  sid = csid = azure_grp_sid;
 	  break;
 	}
+      else if (arg.id == 0x1002)
+        {
+	  /* IIS APPPOOL S-1-5-82-* user */
+	  csid = cygheap->user.saved_sid ();
+	}
+      else if (arg.id == 0x1003)
+        {
+	  /* Special IIS APPPOOL group SID */
+	  get_iis_apppool_grp_sid ();
+	  /* LookupAccountSidW will fail. */
+	  sid = csid = iis_apppool_grp_sid;
+	  break;
+	}
       else if (arg.id == 0xfffe)
 	{
 	  /* Special case "nobody" for reproducible construction of a
@@ -2253,7 +2311,9 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)

 		 Those we let pass, but no others. */
 	      bool its_ok = false;
-	      if (sid_id_auth (sid) == 12)
+	      if (sid_id_auth (sid) == 12 ||
+		  (sid_id_auth (sid) == 5 &&
+		   sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID))
 		its_ok = true;
 	      else /* Microsoft Account */
 		{
@@ -2342,7 +2402,7 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 		    posix_offset = fetch_posix_offset (td, &loc_ldap);
 		}
 	    }
-	  /* AzureAD S-1-12-1-W-X-Y-Z user */
+	  /* AzureAD S-1-12-1-W-X-Y-Z and IIS APPOOL S-1-5-82-* user */
 	  else if (sid_id_auth (sid) == 12)
 	    {
 	      uid = gid = 0x1000;
@@ -2355,6 +2415,21 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 					     name, fully_qualified_name);
 	      break;
 	    }
+	  /* IIS APPOOL S-1-5-82-* user */
+	  else if (sid_id_auth (sid) == 5 &&
+		   sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID)
+	    {
+	      uid = 0x1002;
+	      gid = 0x1003;
+	      fully_qualified_name = true;
+	      home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+					   fully_qualified_name);
+	      shell = cygheap->pg.get_shell ((PUSER_INFO_3) NULL, sid, dom,
+					     name, fully_qualified_name);
+	      gecos = cygheap->pg.get_gecos ((PUSER_INFO_3) NULL, sid, dom,
+					     name, fully_qualified_name);
+	      break;
+	    }
 	  /* If the domain returned by LookupAccountSid is not our machine
 	     name, and if our machine is no domain member, we lose.  We have
 	     nobody to ask for the POSIX offset. */
@@ -2614,6 +2689,20 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
       fully_qualified_name = true;
       acc_type = SidTypeUnknown;
     }
+  else if (sid_id_auth (sid) == 5 &&
+	   sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID)
+    {
+      /* Special IIS APPPOOL group SID which can't be resolved by
+         LookupAccountSid (ERROR_NONE_MAPPED).  This is only allowed
+	 as group entry, not as passwd entry. */
+      if (is_passwd ())
+	return NULL;
+      uid = gid = 0x1003;
+      wcpcpy (dom, L"IIS APPPOOL");
+      wcpcpy (name = namebuf, L"Group");
+      fully_qualified_name = true;
+      acc_type = SidTypeUnknown;
+    }
   else if (sid_id_auth (sid) == 5 /* SECURITY_NT_AUTHORITY */
 	   && sid_sub_auth (sid, 0) == SECURITY_LOGON_IDS_RID)
     {
--
2.40.0.windows.1



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

* [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
                             ` (2 preceding siblings ...)
  2023-04-04 15:07           ` [PATCH v6 3/4] uinfo: special-case IIS APPPOOL accounts Johannes Schindelin
@ 2023-04-04 15:07           ` Johannes Schindelin
  2023-04-06  8:37             ` Corinna Vinschen
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  4 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:07 UTC (permalink / raw)
  To: cygwin-patches

In the very early code path where `dll_crt0_1 ()` calls
`user_shared->initialize ()`, the Cygwin runtime calls `internal_pwsid ()`
to initialize the user name in preparation for reading the `fstab` file.

In case `db_home: env` is defined in `/etc/nsswitch.conf`, we need to
look at the environment variable `HOME` and use it, if set.

When all of this happens, though, the `pinfo_init ()` function has had no
change to run yet (and therefore, `environ_init ()`). At this stage,
therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()`
and we get the _verbatim_ value of `HOME`. That is, the Windows form.
But we need the "POSIX" form.

To add insult to injury, later calls to `getpwuid (getuid ())` will
receive a cached version of the home directory via
`cygheap->pg.pwd_cache.win.find_user ()` thanks to the first
`internal_pwsid ()` call caching the result via
`add_user_from_cygserver ()`, read: we will never receive the converted
`HOME` but always the Windows variant.

So, contrary to the assumptions made in 27376c60a9 (Allow deriving the
current user's home directory via the HOME variable, 2023-03-28), we
cannot assume that `getenv ("HOME")` returned a "POSIX" path.

This is a real problem. Even setting aside that common callers of
`getpwuid ()` (such as OpenSSH) are unable to handle Windows paths in the
`pw_dir` attribute, the Windows path never makes it back to the caller
unscathed. The value returned from `fetch_home_env ()` is not actually
used as-is. Instead, the `fetch_account_from_windows ()` method uses it
to write a pseudo `/etc/passwd`-formatted line that is _then_ parsed via
the `pwdgrp::parse_passwd ()` method which sees no problem with
misinterpreting the colon after the drive letter as a field separator of
that `/etc/passwd`-formatted line, and instead of a Windows path, we now
have a mere drive letter.

Let's detect when the `HOME` value is still in Windows format in
`fetch_home_env ()`, and convert it in that case.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 5e2d88bcd7..bc9e926159 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -929,7 +929,13 @@ fetch_home_env (void)
   /* If `HOME` is set, prefer it */
   const char *home = getenv ("HOME");
   if (home)
-    return strdup (home);
+    {
+      /* In the very early code path of `user_info::initialize ()`, the value
+         of the environment variable `HOME` is still in its Windows form. */
+      if (isdrive (home))
+	return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+      return strdup (home);
+    }

   /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
      (without a directory separator, as `HOMEPATH` starts with one). */
--
2.40.0.windows.1

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

* Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined
  2023-04-03 19:23                   ` Corinna Vinschen
@ 2023-04-04 15:11                     ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:11 UTC (permalink / raw)
  To: Corinna Vinschen; +Cc: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Corinna Vinschen wrote:

> On Apr  3 15:57, Johannes Schindelin wrote:
> > On Mon, 3 Apr 2023, Corinna Vinschen wrote:
> > > > So here is what is going on:
> > > >
> > > > - The domain is 'IIS APPPOOL'
> > >
> > > There's a domain, so why not pass it to the called function?>
> >
> > Sorry, I was unclear. This domain _is_ used when looking for the uid, but
> > then we run into a code path where the UID cannot be determined (because
> > the domain of the account is not the machine name and the machine is no
> > domain member). The clause in question is here:
> > https://github.com/cygwin/cygwin/blob/cygwin-3.4.6/winsup/cygwin/uinfo.cc#L2303-L2310.
> > The Cygwin runtime then returns -1 as UID.
> >
> > The _subsequent_ call to `getpwuid(-1)` is the one where we need to teach
> > Cygwin to respect `db_home: env`. This is the code path taken by OpenSSH.
> > And that code path only has an `arg.id` to work with (the `type` is
> > `ID_arg`), and that `arg.id` is invalid. There is no domain in that code
> > path that we could possibly pass to the `get_home()` method.
>
> That makes a lot of sense.  However, wouldn't it be better to return
> some kind of valid uid, rather than working around uid -1?

It would!

> > > > - The name is the name of the Azure Web App
> > > >
> > > > - The sid is 'S-1-5-82-3932326390-3052311582-2886778547-4123178866-1852425102'
> > >
> > > Oh well. These are basically the same thing as 1-5-80 service accounts.
> > > It would be great if we could handle them gracefully instead of
> > > special-case them in a piece of code we just reach because we don't
> > > handle them yet.
> >
> > True, but I don't really understand how they could be handled.
>
> We do something along these lines already for the AzureAD SIDs of type
> S-1-12-1-what-the-heck.  If we do the same for the S-1-5-82 IIS AppPool
> accounts, we may be able to handle this more sanely.  Just search for
> AzureAD in uinfo.cc.
>
> What do you think?

I implemented that, as patch 3 of 4 in the sixth iteration of the patch
series.

It is a bit more involved than I would have loved, but it does the job in
my tests (although I now need the fourth patch for it to work, which was
not the case previously, for obvious reasons).

Ciao,
Johannes

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

* Re: [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable
  2023-04-03 18:36           ` Corinna Vinschen
@ 2023-04-04 15:12             ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:12 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Corinna Vinschen wrote:

> On Apr  3 16:44, Johannes Schindelin wrote:
> > This patch hails from Git for Windows (where the Cygwin runtime is used
> > in the form of a slightly modified MSYS2 runtime), where it is a
> > well-established technique to let the `$HOME` variable define where the
> > current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
> > and `$USERPROFILE`.
>
> This patch is already merged.

Yes, I am sorry, I wanted to keep this in the patch series for
completeness' sake and also to make backporting to Git for Windows' MSYS2
runtime easier. That's why it is a part of v6, too.

Ciao,
Johannes

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

* Re: [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-04-03 18:37           ` Corinna Vinschen
@ 2023-04-04 15:12             ` Johannes Schindelin
  0 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-04 15:12 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

On Mon, 3 Apr 2023, Corinna Vinschen wrote:

> On Apr  3 16:45, Johannes Schindelin wrote:
> > We should not blindly set the home directory of the SYSTEM account (or
> > of Microsoft accounts) to `/home/<name>`, especially
> > `/etc/nsswitch.conf` defines `db_home: env`, in which case we want to
> > respect the `HOME` variable.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  winsup/cygwin/uinfo.cc | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index baa670478d..d493d29b3b 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > @@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
> >  	 it to a well-known group here. */
> >        if (acc_type == SidTypeUser
> >  	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
> > -	acc_type = SidTypeWellKnownGroup;
> > +	{
> > +	  acc_type = SidTypeWellKnownGroup;
> > +	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
> > +				       fully_qualified_name);
> > +	}
> >        switch ((int) acc_type)
> >  	{
> >  	case SidTypeUser:
>
> Pushed.

Thank you!
Johannes

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

* Re: [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion
  2023-04-04 15:07           ` [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
@ 2023-04-06  8:37             ` Corinna Vinschen
  2023-04-06  9:54               ` Johannes Schindelin
  0 siblings, 1 reply; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-06  8:37 UTC (permalink / raw)
  To: cygwin-patches

On Apr  4 17:07, Johannes Schindelin wrote:
> In the very early code path where `dll_crt0_1 ()` calls
> `user_shared->initialize ()`, the Cygwin runtime calls `internal_pwsid ()`
> to initialize the user name in preparation for reading the `fstab` file.
> 
> In case `db_home: env` is defined in `/etc/nsswitch.conf`, we need to
> look at the environment variable `HOME` and use it, if set.

I'm a bit puzzled by this.  HOME is not a Windows variable.  You're
usually not supposed to set it to Windows values, but to the POSIX
value.  I'm aware that Cygwin makes the conversion, too, for historical
reasons.

But why on earth would you set a variable you have under your own
control, and which only makes sense in a POSIX environment, to a Windows
value?  Why should we actually support this scenario for such a special
case?

> When all of this happens, though, the `pinfo_init ()` function has had no
> change to run yet (and therefore, `environ_init ()`). At this stage,

  chance

> therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()`
> and we get the _verbatim_ value of `HOME`. That is, the Windows form.
> But we need the "POSIX" form.
> [...]
> Let's detect when the `HOME` value is still in Windows format in
> `fetch_home_env ()`, and convert it in that case.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/uinfo.cc | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> index 5e2d88bcd7..bc9e926159 100644
> --- a/winsup/cygwin/uinfo.cc
> +++ b/winsup/cygwin/uinfo.cc
> @@ -929,7 +929,13 @@ fetch_home_env (void)
>    /* If `HOME` is set, prefer it */
>    const char *home = getenv ("HOME");
>    if (home)
> -    return strdup (home);
> +    {
> +      /* In the very early code path of `user_info::initialize ()`, the value
> +         of the environment variable `HOME` is still in its Windows form. */
> +      if (isdrive (home))

While the description is clear on the colon problem, shouldn't this
catch UNC paths as well?  I. e., just check for strchr(home, '\\')?

> +	return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);


> +      return strdup (home);
> +    }
> 
>    /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
>       (without a directory separator, as `HOMEPATH` starts with one). */
> --
> 2.40.0.windows.1

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

* Re: [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion
  2023-04-06  8:37             ` Corinna Vinschen
@ 2023-04-06  9:54               ` Johannes Schindelin
  2023-04-06 10:28                 ` Corinna Vinschen
  0 siblings, 1 reply; 69+ messages in thread
From: Johannes Schindelin @ 2023-04-06  9:54 UTC (permalink / raw)
  To: cygwin-patches

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

Hi Corinna,

On Thu, 6 Apr 2023, Corinna Vinschen wrote:

> On Apr  4 17:07, Johannes Schindelin wrote:
> > In the very early code path where `dll_crt0_1 ()` calls
> > `user_shared->initialize ()`, the Cygwin runtime calls `internal_pwsid ()`
> > to initialize the user name in preparation for reading the `fstab` file.
> >
> > In case `db_home: env` is defined in `/etc/nsswitch.conf`, we need to
> > look at the environment variable `HOME` and use it, if set.
>
> I'm a bit puzzled by this.  HOME is not a Windows variable.  You're
> usually not supposed to set it to Windows values, but to the POSIX
> value.  I'm aware that Cygwin makes the conversion, too, for historical
> reasons.
>
> But why on earth would you set a variable you have under your own
> control, and which only makes sense in a POSIX environment, to a Windows
> value?

This is actually a well-documented feature of Git for Windows, which
cannot handle Cygwin paths (at least not `git.exe`, which is a MINGW
program).

And since Git for Windows is also used in third-party software like Visual
Studio, it is quite conceivable that the `HOME` variable is intended to be
used by Git for Windows and Cygwin. I know that I will use it for both,
once a Cygwin runtime version is released with these patches.

And even if it is not Git for Windows, we are talking about system-wide
environment variables. And that system is Windows...

So yes, we absolutely have to expect the `HOME` variable to potentially
contain a Windows path.

> > When all of this happens, though, the `pinfo_init ()` function has had no
> > change to run yet (and therefore, `environ_init ()`). At this stage,
>
>   chance

👍

> > therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()`
> > and we get the _verbatim_ value of `HOME`. That is, the Windows form.
> > But we need the "POSIX" form.
> > [...]
> > Let's detect when the `HOME` value is still in Windows format in
> > `fetch_home_env ()`, and convert it in that case.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  winsup/cygwin/uinfo.cc | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
> > index 5e2d88bcd7..bc9e926159 100644
> > --- a/winsup/cygwin/uinfo.cc
> > +++ b/winsup/cygwin/uinfo.cc
> > @@ -929,7 +929,13 @@ fetch_home_env (void)
> >    /* If `HOME` is set, prefer it */
> >    const char *home = getenv ("HOME");
> >    if (home)
> > -    return strdup (home);
> > +    {
> > +      /* In the very early code path of `user_info::initialize ()`, the value
> > +         of the environment variable `HOME` is still in its Windows form. */
> > +      if (isdrive (home))
>
> While the description is clear on the colon problem, shouldn't this
> catch UNC paths as well?  I. e., just check for strchr(home, '\\')?

Good idea! I do not know off-hand how well things work when `HOME` is an
UNC path, but we can fix those things when (and if) they arrive.

I'll use `isdrive (home) || home[0] == '\\'` as is used elsewhere, okay?

Ciao,
Johannes

>
> > +	return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
>
>
> > +      return strdup (home);
> > +    }
> >
> >    /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
> >       (without a directory separator, as `HOMEPATH` starts with one). */
> > --
> > 2.40.0.windows.1
>

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

* Re: [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion
  2023-04-06  9:54               ` Johannes Schindelin
@ 2023-04-06 10:28                 ` Corinna Vinschen
  0 siblings, 0 replies; 69+ messages in thread
From: Corinna Vinschen @ 2023-04-06 10:28 UTC (permalink / raw)
  To: cygwin-patches

On Apr  6 11:54, Johannes Schindelin wrote:
> On Thu, 6 Apr 2023, Corinna Vinschen wrote:
> > While the description is clear on the colon problem, shouldn't this
> > catch UNC paths as well?  I. e., just check for strchr(home, '\\')?
> 
> Good idea! I do not know off-hand how well things work when `HOME` is an
> UNC path, but we can fix those things when (and if) they arrive.
> 
> I'll use `isdrive (home) || home[0] == '\\'` as is used elsewhere, okay?

I was thinking of simplifying this to the strchr test for backslash
because all paths with a backslash are handled as Windows paths.
Without actually checking the source, I'd guess the above is checking
for an absolute path in the first place.  Hopefully everybody is
aware that HOME should be a full path... :)

But yes, your above expression is ok, too.


Corinna

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

* [PATCH v7 0/4] Support deriving the current user's home directory via HOME
  2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
                             ` (3 preceding siblings ...)
  2023-04-04 15:07           ` [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
@ 2023-05-22 11:12           ` Johannes Schindelin
  2023-05-22 11:12             ` [PATCH v7 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
                               ` (4 more replies)
  4 siblings, 5 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-05-22 11:12 UTC (permalink / raw)
  To: cygwin-patches

NOTE! This iteration presents patches 1 & 2 only for completeness' sake
and for backporting, as they have been applied to Cygwin's main branch
already.

This patch series supports Git for Windows' default strategy to
determine the current user's home directory by looking at the
environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
if these variables are also unset, to USERPROFILE.

This strategy is a quick method to determine the home directory,
certainly quicker than looking at LDAP, even more so when a domain
controller is unreachable and causes long hangs in Cygwin's startup.

This strategy also allows users to override the home directory easily
(e.g. in case that their real home directory is a network share that is
not all that well handled by some commands such as cmd.exe's cd
command).

Changes since v6:

- Fixed a typo in the last commit's message.

- Support UNC paths as `HOME` values, too. (Tested, works beautifully, I
  can now share my WSL home directory with Cygwin.)

Changes since v5:

- Replaced the third patch by a patch that imitates AzureAD account
  handling also for IIS APPPPOOL ones.

- Added a fourth patch to fix a bug in the first patch (which
  unfortunately was already applied in the buggy form) where _very
  early_ calls to `internal_pwsid ()` would result in completely bogus
  home directory values.

Changes since v4:

- Squashed in Corinna's documentation fixes (read: patch 1 should not be
  applied to Cygwin's main branch, it's presented here for backporting
  purposes).

- Fixed the commit message of the second patch that mistakenly claimed
  that Microsoft accounts would be associated with `/home/SYSTEM`.

- Completely overhauled the commit message of the third patch to motivate
  much better why this fix is needed.

Changes since v3:

- Fixed the bug in v2 where `getenv("HOME")` would convert the value to
  a Unix-y path and the `fetch_home_env()` function would then try to
  convert it _again_.

- Disentangled the logic in `fetch_home_env()` instead of doing
  everything in one big, honking, unreadable `if` condition.

- Commented the code in `fetch_home_env()`.

Changes since v2:

- Using `getenv()` and `cygwin_create_path()` instead of the
  `GetEnvironmentVariableW()`/`cygwin_conv_path()` dance

- Adjusted the documentation to drive home that this only affects the
  _current_ user's home directory

- Using the `PUSER_INFO_3` variant of `get_home()`

- Adjusted the commit messages

- Added another patch, to support "ad-hoc cloud accounts"

Johannes Schindelin (4):
  Allow deriving the current user's home directory via the HOME variable
  Respect `db_home` setting even for SYSTEM/Microsoft accounts
  uinfo: special-case IIS APPPOOL accounts
  Do not rely on `getenv ("HOME")`'s path conversion

 winsup/cygwin/local_includes/cygheap.h |   3 +-
 winsup/cygwin/uinfo.cc                 | 170 +++++++++++++++++++++++--
 winsup/doc/ntsec.xml                   |  20 ++-
 3 files changed, 181 insertions(+), 12 deletions(-)

Range-diff:
1:  e26cae9439 = 1:  e26cae9439 Allow deriving the current user's home directory via the HOME variable
2:  085d4dd8b6 = 2:  085d4dd8b6 Respect `db_home` setting even for SYSTEM/Microsoft accounts
3:  9b79624368 = 3:  9b79624368 uinfo: special-case IIS APPPOOL accounts
4:  8ac1548b92 ! 4:  002d94a244 Do not rely on `getenv ("HOME")`'s path conversion
    @@ Commit message
         look at the environment variable `HOME` and use it, if set.

         When all of this happens, though, the `pinfo_init ()` function has had no
    -    change to run yet (and therefore, `environ_init ()`). At this stage,
    +    chance to run yet (and therefore, `environ_init ()`). At this stage,
         therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()`
         and we get the _verbatim_ value of `HOME`. That is, the Windows form.
         But we need the "POSIX" form.
    @@ Commit message
         Let's detect when the `HOME` value is still in Windows format in
         `fetch_home_env ()`, and convert it in that case.

    +    For good measure, interpret this "Windows format" not only to include
    +    absolute paths with drive prefixes, but also UNC paths.
    +
         Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

      ## winsup/cygwin/uinfo.cc ##
    @@ winsup/cygwin/uinfo.cc: fetch_home_env (void)
     +    {
     +      /* In the very early code path of `user_info::initialize ()`, the value
     +         of the environment variable `HOME` is still in its Windows form. */
    -+      if (isdrive (home))
    ++      if (isdrive (home) || home[0] == '\\')
     +	return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
     +      return strdup (home);
     +    }

base-commit: a9a17f5fe51498b182d4a11ac48207b8c7ffe8ec
Published-As: https://github.com/dscho/msys2-runtime/releases/tag/home-env-cygwin-v7
Fetch-It-Via: git fetch https://github.com/dscho/msys2-runtime home-env-cygwin-v7

--
2.41.0.rc0.windows.1


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

* [PATCH v7 1/4] Allow deriving the current user's home directory via the HOME variable
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
@ 2023-05-22 11:12             ` Johannes Schindelin
  2023-05-22 11:12             ` [PATCH v7 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
                               ` (3 subsequent siblings)
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-05-22 11:12 UTC (permalink / raw)
  To: cygwin-patches

This patch hails from Git for Windows (where the Cygwin runtime is used
in the form of a slightly modified MSYS2 runtime), where it is a
well-established technique to let the `$HOME` variable define where the
current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH`
and `$USERPROFILE`.

The idea is that we want to share user-specific settings between
programs, whether they be Cygwin, MSYS2 or not.  Unfortunately, we
cannot blindly activate the "db_home: windows" setting because in some
setups, the user's home directory is set to a hidden directory via an
UNC path (\\share\some\hidden\folder$) -- something many programs
cannot handle correctly, e.g. `cmd.exe` and other native Windows
applications that users want to employ as Git helpers.

The established technique is to allow setting the user's home directory
via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or
`$USERPROFILE`.  This has the additional advantage that it is much
faster than querying the Windows user database.

Of course this scheme needs to be opt-in.  For that reason, it needs
to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`.

Documentation-fixes-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/local_includes/cygheap.h |  3 +-
 winsup/cygwin/uinfo.cc                 | 51 ++++++++++++++++++++++++++
 winsup/doc/ntsec.xml                   | 20 +++++++++-
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/local_includes/cygheap.h b/winsup/cygwin/local_includes/cygheap.h
index d885ca1230..b6acdf7f18 100644
--- a/winsup/cygwin/local_includes/cygheap.h
+++ b/winsup/cygwin/local_includes/cygheap.h
@@ -358,7 +358,8 @@ public:
     NSS_SCHEME_UNIX,
     NSS_SCHEME_DESC,
     NSS_SCHEME_PATH,
-    NSS_SCHEME_FREEATTR
+    NSS_SCHEME_FREEATTR,
+    NSS_SCHEME_ENV
   };
   struct nss_scheme_t {
     nss_scheme_method	method;
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 30df6db6d8..baa670478d 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -733,6 +733,8 @@ cygheap_pwdgrp::nss_init_line (const char *line)
 		    scheme[idx].method = NSS_SCHEME_UNIX;
 		  else if (NSS_CMP ("desc"))
 		    scheme[idx].method = NSS_SCHEME_DESC;
+		  else if (NSS_CMP ("env"))
+		    scheme[idx].method = NSS_SCHEME_ENV;
 		  else if (NSS_NCMP ("/"))
 		    {
 		      const char *e = c + strcspn (c, " \t");
@@ -921,6 +923,42 @@ fetch_from_path (cyg_ldap *pldap, PUSER_INFO_3 ui, cygpsid &sid, PCWSTR str,
   return ret;
 }

+static char *
+fetch_home_env (void)
+{
+  /* If `HOME` is set, prefer it */
+  const char *home = getenv ("HOME");
+  if (home)
+    return strdup (home);
+
+  /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
+     (without a directory separator, as `HOMEPATH` starts with one). */
+  const char *home_drive = getenv ("HOMEDRIVE");
+  if (home_drive)
+    {
+      const char *home_path = getenv ("HOMEPATH");
+      if (home_path)
+	{
+	  tmp_pathbuf tp;
+	  char *p = tp.c_get (), *q;
+
+	  // concatenate HOMEDRIVE and HOMEPATH
+	  q = stpncpy (p, home_drive, NT_MAX_PATH);
+	  strlcpy (q, home_path, NT_MAX_PATH - (q - p));
+	  return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, p);
+	}
+    }
+
+  /* If neither `HOME` nor `HOMEDRIVE``HOMEPATH` are set, fall back
+     to `USERPROFILE`; In corporate setups, this might point to a
+     disconnected network share, hence this is the last fall back. */
+  home = getenv ("USERPROFILE");
+  if (home)
+    return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+
+  return NULL;
+}
+
 char *
 cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 			  PCWSTR dnsdomain, PCWSTR name, bool full_qualified)
@@ -980,6 +1018,10 @@ cygheap_pwdgrp::get_home (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		}
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1012,6 +1054,10 @@ cygheap_pwdgrp::get_home (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	  home = fetch_from_path (NULL, ui, sid, home_scheme[idx].attrib,
 				  dom, NULL, name, full_qualified);
 	  break;
+	case NSS_SCHEME_ENV:
+	  if (RtlEqualSid (sid, cygheap->user.sid ()))
+	    home = fetch_home_env ();
+	  break;
 	}
     }
   return home;
@@ -1031,6 +1077,7 @@ cygheap_pwdgrp::get_shell (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_FALLBACK:
 	  return NULL;
 	case NSS_SCHEME_WINDOWS:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_CYGWIN:
 	  if (pldap->fetch_ad_account (sid, false, dnsdomain))
@@ -1095,6 +1142,7 @@ cygheap_pwdgrp::get_shell (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
@@ -1176,6 +1224,8 @@ cygheap_pwdgrp::get_gecos (cyg_ldap *pldap, cygpsid &sid, PCWSTR dom,
 		sys_wcstombs_alloc (&gecos, HEAP_NOTHEAP, val);
 	    }
 	  break;
+	case NSS_SCHEME_ENV:
+	  break;
 	}
     }
   if (gecos)
@@ -1202,6 +1252,7 @@ cygheap_pwdgrp::get_gecos (PUSER_INFO_3 ui, cygpsid &sid, PCWSTR dom,
 	case NSS_SCHEME_CYGWIN:
 	case NSS_SCHEME_UNIX:
 	case NSS_SCHEME_FREEATTR:
+	case NSS_SCHEME_ENV:
 	  break;
 	case NSS_SCHEME_DESC:
 	  if (ui)
diff --git a/winsup/doc/ntsec.xml b/winsup/doc/ntsec.xml
index c6871ecf05..687789076c 100644
--- a/winsup/doc/ntsec.xml
+++ b/winsup/doc/ntsec.xml
@@ -1167,7 +1167,7 @@ and on non-AD machines.
 </para>

 <para>
-Four schemata are predefined, two schemata are variable.  The predefined
+Five schemata are predefined, two schemata are variable.  The predefined
 schemata are the following:
 </para>

@@ -1203,6 +1203,13 @@ schemata are the following:
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a more detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Utilizes the user's environment.  This schema is only supported
+	      for setting the home directory yet.
+	      See <xref linkend="ntsec-mapping-nsswitch-home"></xref> for
+	      the description.</listitem>
+  </varlistentry>
 </variablelist>

 <para>
@@ -1335,6 +1342,17 @@ of each schema when used with <literal>db_home:</literal>
 	      See <xref linkend="ntsec-mapping-nsswitch-desc"></xref>
 	      for a detailed description.</listitem>
   </varlistentry>
+  <varlistentry>
+    <term><literal>env</literal></term>
+    <listitem>Derives the home directory of the current user from the
+	      environment variable <literal>HOME</literal> (falling back to
+	      <literal>HOMEDRIVE\HOMEPATH</literal> and
+	      <literal>USERPROFILE</literal>, in that order).  This is faster
+	      than the <literal>windows</literal> schema at the
+	      expense of determining only the current user's home directory
+	      correctly.  This schema is skipped for any other account.
+	      </listitem>
+  </varlistentry>
   <varlistentry>
     <term><literal>@ad_attribute</literal></term>
     <listitem>AD only: The user's home directory is set to the path given
--
2.41.0.rc0.windows.1



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

* [PATCH v7 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-05-22 11:12             ` [PATCH v7 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
@ 2023-05-22 11:12             ` Johannes Schindelin
  2023-05-22 11:12             ` [PATCH v7 3/4] uinfo: special-case IIS APPPOOL accounts Johannes Schindelin
                               ` (2 subsequent siblings)
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-05-22 11:12 UTC (permalink / raw)
  To: cygwin-patches

We should not blindly set the home directory of the SYSTEM account (or
of Microsoft accounts) to `/home/<name>`, especially
`/etc/nsswitch.conf` defines `db_home: env`, in which case we want to
respect the `HOME` variable.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index baa670478d..d493d29b3b 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -2234,7 +2234,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	 it to a well-known group here. */
       if (acc_type == SidTypeUser
 	  && (sid_sub_auth_count (sid) <= 3 || sid_id_auth (sid) == 11))
-	acc_type = SidTypeWellKnownGroup;
+	{
+	  acc_type = SidTypeWellKnownGroup;
+	  home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+				       fully_qualified_name);
+	}
       switch ((int) acc_type)
 	{
 	case SidTypeUser:
--
2.41.0.rc0.windows.1



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

* [PATCH v7 3/4] uinfo: special-case IIS APPPOOL accounts
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
  2023-05-22 11:12             ` [PATCH v7 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
  2023-05-22 11:12             ` [PATCH v7 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
@ 2023-05-22 11:12             ` Johannes Schindelin
  2023-05-22 11:13             ` [PATCH v7 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
  2023-06-06 13:33             ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Corinna Vinschen
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-05-22 11:12 UTC (permalink / raw)
  To: cygwin-patches

The account under which Azure Web Apps run is an IIS APPOOL account that
is generated on the fly.

These are special because the virtual machines on which thes Apps run
are not domain-joined, yet the accounts are domain accounts.

To support the use case where such a Web App needs to call `ssh` (e.g.
to deploy from a Git repository that is accessible only via SSH), we do
need OpenSSH's `getpwuid (getuid ())` invocation to work.

But currently it does not. Concretely, `getuid ()` returns -1 for these
accounts, and OpenSSH fails to find the correct home directory
(_especially_ when that home directory was overridden via a `db_home:
env` line in `/etc/nsswitch.conf`).

This can be verified e.g. in a Kudu console (for details about Kudu
consoles, see https://github.com/projectkudu/kudu/wiki/Kudu-console):
the domain is `IIS APPPOOL`, the account name is the name of the Azure
Web App, the SID starts with 'S-1-5-82-`, and
`pwdgrp::fetch_account_from_windows()` runs into the code path where
"[...] the domain returned by LookupAccountSid is not our machine name,
and if our machine is no domain member, we lose.  We have nobody to ask
for the POSIX offset."

Since these IIS APPPOOL accounts are relatively similar to AzureAD
accounts in this scenario, let's imitate the latter to support also the
former.

Reported-by: David Ebbo <david.ebbo@gmail.com>
Helped-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 107 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 98 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index d493d29b3b..5e2d88bcd7 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -1485,9 +1485,9 @@ get_logon_sid ()
     }
 }

-/* Fetch special AzureAD group, which is part of the token group list but
-   *not* recognized by LookupAccountSid (ERROR_NONE_MAPPED). */
-static cygsid azure_grp_sid ("");
+/* Fetch special AzureAD and IIS APPPOOL groups, which are part of the token
+   group list but *not* recognized by LookupAccountSid (ERROR_NONE_MAPPED). */
+static cygsid azure_grp_sid (""), iis_apppool_grp_sid ("");

 static void
 get_azure_grp_sid ()
@@ -1515,6 +1515,36 @@ get_azure_grp_sid ()
     }
 }

+static void
+get_iis_apppool_grp_sid ()
+{
+  if (PSID (iis_apppool_grp_sid) == NO_SID)
+    {
+      NTSTATUS status;
+      ULONG size;
+      tmp_pathbuf tp;
+      PTOKEN_GROUPS groups = (PTOKEN_GROUPS) tp.w_get ();
+
+      status = NtQueryInformationToken (hProcToken, TokenGroups, groups,
+					2 * NT_MAX_PATH, &size);
+      if (!NT_SUCCESS (status))
+	debug_printf ("NtQueryInformationToken (TokenGroups) %y", status);
+      else
+	{
+	  for (DWORD pg = 0; pg < groups->GroupCount; ++pg)
+	    {
+	      PSID sid = groups->Groups[pg].Sid;
+	      if (sid_id_auth (sid) == 5 &&
+		  sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID)
+		{
+		  iis_apppool_grp_sid = sid;
+		  break;
+		}
+	    }
+	}
+    }
+}
+
 void *
 pwdgrp::add_account_post_fetch (char *line, bool lock)
 {
@@ -1796,6 +1826,16 @@ pwdgrp::construct_sid_from_name (cygsid &sid, wchar_t *name, wchar_t *sep)
 	}
       return false;
     }
+  if (sep && wcscmp (name, L"IIS APPPOOL\\Group") == 0)
+    {
+      get_iis_apppool_grp_sid ();
+      if (PSID (logon_sid) != NO_SID)
+	{
+	  sid = iis_apppool_grp_sid;
+	  return true;
+	}
+      return false;
+    }
   if (!sep && wcscmp (name, L"CurrentSession") == 0)
     {
       get_logon_sid ();
@@ -2018,8 +2058,11 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
       /* Last but not least, some validity checks on the name style. */
       if (!fq_name)
 	{
-	  /* AzureAD user must be prepended by "domain" name. */
-	  if (sid_id_auth (sid) == 12)
+	  /* AzureAD and IIS APPPOOL users must be prepended by "domain"
+	     name. */
+	  if (sid_id_auth (sid) == 12 ||
+	      (sid_id_auth (sid) == 5 &&
+	       sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID))
 	    return NULL;
 	  /* name_only account is either builtin or primary domain, or
 	     account domain on non-domain machines. */
@@ -2045,8 +2088,10 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	}
       else
 	{
-	  /* AzureAD accounts should be fully qualifed either. */
-	  if (sid_id_auth (sid) == 12)
+	  /* AzureAD and IIS APPPOOL accounts should be fully qualifed either. */
+	  if (sid_id_auth (sid) == 12 ||
+	      (sid_id_auth (sid) == 5 &&
+	       sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID))
 	    break;
 	  /* Otherwise, no fully_qualified for builtin accounts, except for
 	     NT SERVICE, for which we require the prefix.  Note that there's
@@ -2125,6 +2170,19 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 	  sid = csid = azure_grp_sid;
 	  break;
 	}
+      else if (arg.id == 0x1002)
+        {
+	  /* IIS APPPOOL S-1-5-82-* user */
+	  csid = cygheap->user.saved_sid ();
+	}
+      else if (arg.id == 0x1003)
+        {
+	  /* Special IIS APPPOOL group SID */
+	  get_iis_apppool_grp_sid ();
+	  /* LookupAccountSidW will fail. */
+	  sid = csid = iis_apppool_grp_sid;
+	  break;
+	}
       else if (arg.id == 0xfffe)
 	{
 	  /* Special case "nobody" for reproducible construction of a
@@ -2253,7 +2311,9 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)

 		 Those we let pass, but no others. */
 	      bool its_ok = false;
-	      if (sid_id_auth (sid) == 12)
+	      if (sid_id_auth (sid) == 12 ||
+		  (sid_id_auth (sid) == 5 &&
+		   sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID))
 		its_ok = true;
 	      else /* Microsoft Account */
 		{
@@ -2342,7 +2402,7 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 		    posix_offset = fetch_posix_offset (td, &loc_ldap);
 		}
 	    }
-	  /* AzureAD S-1-12-1-W-X-Y-Z user */
+	  /* AzureAD S-1-12-1-W-X-Y-Z and IIS APPOOL S-1-5-82-* user */
 	  else if (sid_id_auth (sid) == 12)
 	    {
 	      uid = gid = 0x1000;
@@ -2355,6 +2415,21 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
 					     name, fully_qualified_name);
 	      break;
 	    }
+	  /* IIS APPOOL S-1-5-82-* user */
+	  else if (sid_id_auth (sid) == 5 &&
+		   sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID)
+	    {
+	      uid = 0x1002;
+	      gid = 0x1003;
+	      fully_qualified_name = true;
+	      home = cygheap->pg.get_home ((PUSER_INFO_3) NULL, sid, dom, name,
+					   fully_qualified_name);
+	      shell = cygheap->pg.get_shell ((PUSER_INFO_3) NULL, sid, dom,
+					     name, fully_qualified_name);
+	      gecos = cygheap->pg.get_gecos ((PUSER_INFO_3) NULL, sid, dom,
+					     name, fully_qualified_name);
+	      break;
+	    }
 	  /* If the domain returned by LookupAccountSid is not our machine
 	     name, and if our machine is no domain member, we lose.  We have
 	     nobody to ask for the POSIX offset. */
@@ -2614,6 +2689,20 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
       fully_qualified_name = true;
       acc_type = SidTypeUnknown;
     }
+  else if (sid_id_auth (sid) == 5 &&
+	   sid_sub_auth (sid, 0) == SECURITY_APPPOOL_ID_BASE_RID)
+    {
+      /* Special IIS APPPOOL group SID which can't be resolved by
+         LookupAccountSid (ERROR_NONE_MAPPED).  This is only allowed
+	 as group entry, not as passwd entry. */
+      if (is_passwd ())
+	return NULL;
+      uid = gid = 0x1003;
+      wcpcpy (dom, L"IIS APPPOOL");
+      wcpcpy (name = namebuf, L"Group");
+      fully_qualified_name = true;
+      acc_type = SidTypeUnknown;
+    }
   else if (sid_id_auth (sid) == 5 /* SECURITY_NT_AUTHORITY */
 	   && sid_sub_auth (sid, 0) == SECURITY_LOGON_IDS_RID)
     {
--
2.41.0.rc0.windows.1



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

* [PATCH v7 4/4] Do not rely on `getenv ("HOME")`'s path conversion
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
                               ` (2 preceding siblings ...)
  2023-05-22 11:12             ` [PATCH v7 3/4] uinfo: special-case IIS APPPOOL accounts Johannes Schindelin
@ 2023-05-22 11:13             ` Johannes Schindelin
  2023-06-06 13:33             ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Corinna Vinschen
  4 siblings, 0 replies; 69+ messages in thread
From: Johannes Schindelin @ 2023-05-22 11:13 UTC (permalink / raw)
  To: cygwin-patches

In the very early code path where `dll_crt0_1 ()` calls
`user_shared->initialize ()`, the Cygwin runtime calls `internal_pwsid ()`
to initialize the user name in preparation for reading the `fstab` file.

In case `db_home: env` is defined in `/etc/nsswitch.conf`, we need to
look at the environment variable `HOME` and use it, if set.

When all of this happens, though, the `pinfo_init ()` function has had no
chance to run yet (and therefore, `environ_init ()`). At this stage,
therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()`
and we get the _verbatim_ value of `HOME`. That is, the Windows form.
But we need the "POSIX" form.

To add insult to injury, later calls to `getpwuid (getuid ())` will
receive a cached version of the home directory via
`cygheap->pg.pwd_cache.win.find_user ()` thanks to the first
`internal_pwsid ()` call caching the result via
`add_user_from_cygserver ()`, read: we will never receive the converted
`HOME` but always the Windows variant.

So, contrary to the assumptions made in 27376c60a9 (Allow deriving the
current user's home directory via the HOME variable, 2023-03-28), we
cannot assume that `getenv ("HOME")` returned a "POSIX" path.

This is a real problem. Even setting aside that common callers of
`getpwuid ()` (such as OpenSSH) are unable to handle Windows paths in the
`pw_dir` attribute, the Windows path never makes it back to the caller
unscathed. The value returned from `fetch_home_env ()` is not actually
used as-is. Instead, the `fetch_account_from_windows ()` method uses it
to write a pseudo `/etc/passwd`-formatted line that is _then_ parsed via
the `pwdgrp::parse_passwd ()` method which sees no problem with
misinterpreting the colon after the drive letter as a field separator of
that `/etc/passwd`-formatted line, and instead of a Windows path, we now
have a mere drive letter.

Let's detect when the `HOME` value is still in Windows format in
`fetch_home_env ()`, and convert it in that case.

For good measure, interpret this "Windows format" not only to include
absolute paths with drive prefixes, but also UNC paths.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 winsup/cygwin/uinfo.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 5e2d88bcd7..21d729d5dc 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -929,7 +929,13 @@ fetch_home_env (void)
   /* If `HOME` is set, prefer it */
   const char *home = getenv ("HOME");
   if (home)
-    return strdup (home);
+    {
+      /* In the very early code path of `user_info::initialize ()`, the value
+         of the environment variable `HOME` is still in its Windows form. */
+      if (isdrive (home) || home[0] == '\\')
+	return (char *) cygwin_create_path (CCP_WIN_A_TO_POSIX, home);
+      return strdup (home);
+    }

   /* If `HOME` is unset, fall back to `HOMEDRIVE``HOMEPATH`
      (without a directory separator, as `HOMEPATH` starts with one). */
--
2.41.0.rc0.windows.1

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

* Re: [PATCH v7 0/4] Support deriving the current user's home directory via HOME
  2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
                               ` (3 preceding siblings ...)
  2023-05-22 11:13             ` [PATCH v7 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
@ 2023-06-06 13:33             ` Corinna Vinschen
  4 siblings, 0 replies; 69+ messages in thread
From: Corinna Vinschen @ 2023-06-06 13:33 UTC (permalink / raw)
  To: cygwin-patches

On May 22 13:12, Johannes Schindelin wrote:
> NOTE! This iteration presents patches 1 & 2 only for completeness' sake
> and for backporting, as they have been applied to Cygwin's main branch
> already.
> 
> This patch series supports Git for Windows' default strategy to
> determine the current user's home directory by looking at the
> environment variable HOME, falling back to HOMEDRIVE and HOMEPATH, and
> if these variables are also unset, to USERPROFILE.
> 
> This strategy is a quick method to determine the home directory,
> certainly quicker than looking at LDAP, even more so when a domain
> controller is unreachable and causes long hangs in Cygwin's startup.
> 
> This strategy also allows users to override the home directory easily
> (e.g. in case that their real home directory is a network share that is
> not all that well handled by some commands such as cmd.exe's cd
> command).
> 
> Changes since v6:
> 
> - Fixed a typo in the last commit's message.
> 
> - Support UNC paths as `HOME` values, too. (Tested, works beautifully, I
>   can now share my WSL home directory with Cygwin.)

Pushed.


Thanks,
Corinna

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

end of thread, other threads:[~2023-06-06 13:33 UTC | newest]

Thread overview: 69+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-16 13:06 [PATCH] Allow overriding the home directory via the HOME variable Johannes Schindelin
2015-10-21 18:32 ` Corinna Vinschen
2015-10-22 15:38   ` Johannes Schindelin
2015-10-23  9:10     ` Corinna Vinschen
2015-10-23  9:41       ` Corinna Vinschen
2015-10-23 12:00         ` Johannes Schindelin
2015-12-17 18:05 ` [PATCH v2 0/2] Support deriving the current user's home directory via HOME Johannes Schindelin
2015-12-17 18:05   ` [PATCH v2 2/2] Respect `db_home` setting even for the SYSTEM account Johannes Schindelin
2015-12-17 20:49     ` Corinna Vinschen
2015-12-17 21:02       ` Corinna Vinschen
2022-09-21 12:00       ` Johannes Schindelin
2015-12-17 18:05   ` [PATCH v2 1/2] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
2015-12-17 20:20     ` Corinna Vinschen
2022-09-21 11:58       ` Johannes Schindelin
2022-10-18 17:02         ` Corinna Vinschen
2022-10-23 21:04           ` Johannes Schindelin
2022-10-24 11:37             ` Corinna Vinschen
2022-11-10 15:16               ` Johannes Schindelin
2022-11-10 15:22                 ` Corinna Vinschen
2022-11-18  8:18                   ` Johannes Schindelin
2022-11-21 11:41                     ` Corinna Vinschen
2023-03-28  8:21                       ` Johannes Schindelin
2022-09-21 11:51   ` [PATCH v3 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
2022-09-21 11:51     ` [PATCH v3 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
2022-09-21 11:52     ` [PATCH v3 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
2022-09-21 11:52     ` [PATCH v3 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
2023-03-28  8:17     ` [PATCH v4 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
2023-03-28  8:17       ` [PATCH v4 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
2023-03-28 10:35         ` Corinna Vinschen
2023-03-28 12:34           ` Jon Turney
2023-03-28 13:31             ` Corinna Vinschen
2023-03-29  8:36               ` Corinna Vinschen
2023-04-03  6:39                 ` Johannes Schindelin
2023-03-28  8:17       ` [PATCH v4 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
2023-03-28 10:16         ` Corinna Vinschen
2023-04-03  6:36           ` Johannes Schindelin
2023-04-03 10:59             ` Corinna Vinschen
2023-04-03 13:32               ` Johannes Schindelin
2023-03-28  8:17       ` [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
2023-03-28 10:17         ` Corinna Vinschen
2023-04-03  6:45           ` Johannes Schindelin
2023-04-03 13:12             ` Johannes Schindelin
2023-04-03 13:29               ` Corinna Vinschen
2023-04-03 13:57                 ` Johannes Schindelin
2023-04-03 19:23                   ` Corinna Vinschen
2023-04-04 15:11                     ` Johannes Schindelin
2023-04-03 13:19             ` Johannes Schindelin
2023-04-03 14:44       ` [PATCH v5 0/3] Support deriving the current user's home directory via HOME Johannes Schindelin
2023-04-03 14:44         ` [PATCH v5 1/3] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
2023-04-03 18:36           ` Corinna Vinschen
2023-04-04 15:12             ` Johannes Schindelin
2023-04-03 14:45         ` [PATCH v5 2/3] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
2023-04-03 18:37           ` Corinna Vinschen
2023-04-04 15:12             ` Johannes Schindelin
2023-04-03 14:45         ` [PATCH v5 3/3] Respect `db_home: env` even when no uid can be determined Johannes Schindelin
2023-04-04 15:07         ` [PATCH v6 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
2023-04-04 15:07           ` [PATCH v6 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
2023-04-04 15:07           ` [PATCH v6 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
2023-04-04 15:07           ` [PATCH v6 3/4] uinfo: special-case IIS APPPOOL accounts Johannes Schindelin
2023-04-04 15:07           ` [PATCH v6 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
2023-04-06  8:37             ` Corinna Vinschen
2023-04-06  9:54               ` Johannes Schindelin
2023-04-06 10:28                 ` Corinna Vinschen
2023-05-22 11:12           ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Johannes Schindelin
2023-05-22 11:12             ` [PATCH v7 1/4] Allow deriving the current user's home directory via the HOME variable Johannes Schindelin
2023-05-22 11:12             ` [PATCH v7 2/4] Respect `db_home` setting even for SYSTEM/Microsoft accounts Johannes Schindelin
2023-05-22 11:12             ` [PATCH v7 3/4] uinfo: special-case IIS APPPOOL accounts Johannes Schindelin
2023-05-22 11:13             ` [PATCH v7 4/4] Do not rely on `getenv ("HOME")`'s path conversion Johannes Schindelin
2023-06-06 13:33             ` [PATCH v7 0/4] Support deriving the current user's home directory via HOME Corinna Vinschen

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