Re: Proposal: Save user's original authenticated identity for logging

On Fri, Feb 26, 2021 at 8:45 PM Jacob Champion <pchampion(at)vmware(dot)com> wrote:
>
> On Thu, 2021-02-11 at 20:32 +0000, Jacob Champion wrote:
> > v2 just updates the patchset to remove the Windows TODO and fill in the
> > patch notes; no functional changes. The question about escaping log
> > contents remains.
>
> v3 rebases onto latest master, for SSL test conflicts.
>
> Note:
> - Since the 0001 patch from [1] is necessary for the new Kerberos tests
> in 0003, I won't make a separate commitfest entry for it.
> - 0002 would be subsumed by [2] if it's committed.

It looks like patch 0001 has some leftover debuggnig code at the end?
Or did you intend for that to be included permanently?

As for log escaping, we report port->user_name already unescaped --
surely this shouldn't be a worse case than that?

I wonder if it wouldn't be better to keep the log line on the existing
"connection authorized" line, just as a separate field. I'm kind of
split on it though, because I guess it might make that line very long.
But it's also a lot more convenient to parse it on a single line than
across multiple lines potentially overlapping with other sessions.

With this we store the same value as the authn and as
port->gss->princ, and AFAICT it's only used once. Seems we could just
use the new field for the gssapi usage as well? Especially since that
usage only seems to be there in order to do the gssapi specific
logging of, well, the same thing.

Same goes for peer_user? In fact, if we're storing it in the Port, why
are we even passing it as a separate parameter to check_usermap --
shouldn't that one always use this same value? ISTM that it could be
quite confusing if the logged value is different from whatever we
apply to the user mapping?

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/