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
- since-v3.diff.txt (text/plain)
- v4-0001-Add-API-to-retrieve-authn_id-from-SQL.patch (text/x-patch) patch v4-0001
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
-
Remove initialization of MyClientConnectionInfo at backend startup
- 701ac2cb1fa2 16.0 landed
-
Allow parallel workers to retrieve some data from Port
- d951052a9e02 16.0 landed