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

Jacob Champion <pchampion@vmware.com>

From: Jacob Champion <pchampion@vmware.com>
To: "michael@paquier.xyz" <michael@paquier.xyz>, "andres@anarazel.de" <andres@anarazel.de>
Cc: "rjuju123@gmail.com" <rjuju123@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "sfrost@snowman.net" <sfrost@snowman.net>
Date: 2022-02-28T18:58:00Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Remove initialization of MyClientConnectionInfo at backend startup

  2. Allow parallel workers to retrieve some data from Port