Re: [PATCH] Expose port->authn_id to extensions and triggers

Greetings,

* Jacob Champion (pchampion(at)vmware(dot)com) wrote:
> 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.

That's probably alright.

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

Yeah, we really should make this available to trigger-based auditing
systems too and not just through log_connections which involves a great
deal more log parsing and work external to the database to figure out
who did what.

> > > I don't think we should add further functions not prefixed with pg_.
> >
> > Yep.
>
> Fixed.

That's fine.

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

> commit efec9f040843d1de2fc52f5ce0d020478a5bc75d
> Author: Jacob Champion <pchampion(at)vmware(dot)com>
> Date: Mon Feb 28 10:28:51 2022 -0800
>
> squash! Add API to retrieve authn_id from SQL

Bleh. :) Squash indeed.

> Subject: [PATCH v4] Add API to retrieve authn_id from SQL
>
> The authn_id field in MyProcPort is currently only accessible to the
> backend itself. Add a SQL function, pg_session_authn_id(), to expose
> the field to triggers that may want to make use of it.

Only did a quick look but generally looks reasonable to me.

Thanks,

Stephen