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