On Fri, Feb 25, 2022 at 01:23:49PM -0800, Andres Freund wrote:
> Looks to me like authn_id isn't synchronized to parallel workers right now. So
> the function will return the wrong thing when executed as part of a parallel
> query.
Thanks for the catch. It looks like MyProcPort is left empty, and other
functions that rely on like inet_server_addr() are marked parallel-
restricted, so I've done the same in v4.
On Sat, 2022-02-26 at 14:39 +0900, Michael Paquier wrote:
> FWIW, I am not completely sure what's the use case for being able to
> see the authn of the current session through a trigger. We expose
> that when log_connections is enabled, for audit purposes. I can also
> get behind something more central so as one can get a full picture of
> the authn used by a bunch of session, particularly with complex HBA
> policies, but this looks rather limited to me in usability. Perhaps
> that's not enough to stand as an objection, though, and the patch is
> dead simple.
I'm primarily motivated by the linked thread -- if the gap between
builtin roles and authn_id are going to be used as ammo against other
security features, then let's close that gap. But I think it's fair to
say that if someone is already using triggers to exhaustively audit a
table, it'd be nice to have this info in the same place too.
> > I don't think we should add further functions not prefixed with pg_.
>
> Yep.
Fixed.
> > Perhaps a few tests for less trivial authn_ids could be worthwhile?
> > E.g. certificate DNs.
>
> Yes, src/test/ssl would handle that just fine. Now, this stuff
> already looks after authn results with log_connections=on, so that
> feels like a duplicate.
It was easy enough to add, so I added it. I suppose it does protect
against any reimplementations of pg_session_authn_id() that can't
handle longer ID strings, though I admit that's a stretch.
Thanks,
--Jacob
