Re: [PATCH v4 3/3] Respect `db_home: env` even when no uid can be determined

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

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