From: | Jacob Champion <pchampion(at)vmware(dot)com> |
---|---|
To: | "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de> |
Cc: | "rjuju123(at)gmail(dot)com" <rjuju123(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "sfrost(at)snowman(dot)net" <sfrost(at)snowman(dot)net> |
Subject: | Re: [PATCH] Expose port->authn_id to extensions and triggers |
Date: | 2022-02-28 18:58:00 |
Message-ID: | 0cead4e58257beebb96f162507151271ff2f7b54.camel@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
since-v3.diff.txt | text/plain | 3.5 KB |
v4-0001-Add-API-to-retrieve-authn_id-from-SQL.patch | text/x-patch | 4.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2022-02-28 19:09:23 | Re: CREATEROLE and role ownership hierarchies |
Previous Message | Nathan Bossart | 2022-02-28 18:57:32 | Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) |