From: Jacob Champion <pchampion@vmware.com>
To: magnus@hagander.net <magnus@hagander.net>
Cc: stark@mit.edu <stark@mit.edu>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: sfrost@snowman.net <sfrost@snowman.net>
Cc: tgl@sss.pgh.pa.us <tgl@sss.pgh.pa.us>
Subject: Re: Proposal: Save user's original authenticated identity for logging
Date: Mon, 8 Mar 2021 22:16:23 +0000
Message-ID: <a2fab8e350c86aa4fe245d1af521b0bbc807f11e.camel@vmware.com> (raw)
In-Reply-To: <CABUevExVHryTasKmtJW5RtU-dBesYj4bV7ggpeVMfiPCHCvLNA@mail.gmail.com>
References: <c55788dd1773c521c862e8e0dddb367df51222be.camel@vmware.com>
<a9ee5e4e8e844d06c2bcf70c6ed3306ccb4897f1.camel@vmware.com>
<5d8b74e64fe4a610b23624277e497cc0a925db4b.camel@vmware.com>
<2870147caa759155c5c8a232bbd2399f41f76491.camel@vmware.com>
<4b2ca51542fa84e6fa37ab678624d5b400c092c0.camel@vmware.com>
<CABUevExVHryTasKmtJW5RtU-dBesYj4bV7ggpeVMfiPCHCvLNA@mail.gmail.com>
On Sat, 2021-03-06 at 18:33 +0100, Magnus Hagander wrote:
> It looks like patch 0001 has some leftover debuggnig code at the end?
> Or did you intend for that to be included permanently?
I'd intended to keep it -- it works hand-in-hand with the existing
"current_logfiles" log line on 219 and might keep someone from tearing
their hair out. But I can certainly remove it, if it's cluttering up
the logs too much.
> As for log escaping, we report port->user_name already unescaped --
> surely this shouldn't be a worse case than that?
Ah, that's a fair point. I'll remove the TODO.
> 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.
Authentication can succeed even if authorization fails, and it's useful
to see that in the logs. In most cases that looks like a failed user
mapping, but there are other corner cases where we fail the connection
after a successful authentication, such as when using krb_realm.
Currently you get little to no feedback when that happens, but with a
separate log line, it's a lot easier to piece together what's happened.
(In general, I feel pretty strongly that Postgres combines/conflates
authentication and authorization in too many places.)
> 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?
Seems reasonable; I'll consolidate them.
--Jacob
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pchampion@vmware.com, magnus@hagander.net, stark@mit.edu, sfrost@snowman.net, tgl@sss.pgh.pa.us
Subject: Re: Proposal: Save user's original authenticated identity for logging
In-Reply-To: <a2fab8e350c86aa4fe245d1af521b0bbc807f11e.camel@vmware.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox