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: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