Date: Mon, 28 Feb 2022 16:00:36 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Jacob Champion <pchampion@vmware.com>
Cc: "michael@paquier.xyz" <michael@paquier.xyz>,
	"andres@anarazel.de" <andres@anarazel.de>,
	"rjuju123@gmail.com" <rjuju123@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Expose port->authn_id to extensions and triggers
Message-ID: <20220228210036.GN10577@tamriel.snowman.net>
References: <793d990837ae5c06a558d58d62de9378ab525d83.camel@vmware.com>
 <YhduhD/SvIX5NnxY@paquier.xyz>
 <2e28b12b450f247d5c0994207030c862263e0297.camel@vmware.com>
 <20220224171538.jbf2cxzmhiwuinp4@jrouhaud>
 <a4d9baa0717ca924331cb1dd062788bd73bd14a4.camel@vmware.com>
 <d15d02c92a3e4bdb7cc7b96b0abfe97f213e4445.camel@vmware.com>
 <cdbcf8653e56340f73438e647ea5a26f021627ed.camel@vmware.com>
 <20220225212349.gf76klktl4dmm5ah@alap3.anarazel.de>
 <Yhm9BUjrCItExUJu@paquier.xyz>
 <0cead4e58257beebb96f162507151271ff2f7b54.camel@vmware.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="+0mKm/ENadSkQxF+"
Content-Disposition: inline
In-Reply-To: <0cead4e58257beebb96f162507151271ff2f7b54.camel@vmware.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: <https://www.postgresql.org/message-id/20220228210036.GN10577%40tamriel.snowman.net>
Precedence: bulk


--+0mKm/ENadSkQxF+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Jacob Champion (pchampion@vmware.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 par=
allel
> > query.
>=20
> 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.
>=20
> 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_.
> >=20
> > Yep.
>=20
> Fixed.

That's fine.

> > > Perhaps a few tests for less trivial authn_ids could be worthwhile?
> > > E.g. certificate DNs.
> >=20
> > Yes, src/test/ssl would handle that just fine.  Now, this stuff
> > already looks after authn results with log_connections=3Don, so that
> > feels like a duplicate.
>=20
> 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.
>=20
> Thanks,
> --Jacob

> commit efec9f040843d1de2fc52f5ce0d020478a5bc75d
> Author: Jacob Champion <pchampion@vmware.com>
> Date:   Mon Feb 28 10:28:51 2022 -0800
>=20
>     squash! Add API to retrieve authn_id from SQL

Bleh. :)  Squash indeed.

> Subject: [PATCH v4] Add API to retrieve authn_id from SQL
>=20
> 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

--+0mKm/ENadSkQxF+
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJiHTf0AAoJEO1sijiDR2RVdWAP/1BzzTiA3oMPgwrUez4noo0U
Qvu+m3zuLOiDrxvQH95PKVTINNJIc4R2WV+iVlwDolJSyYT/Hxskrsx2ciAIYZN4
rdhzdur3yUukGahFs2K9jrtvq9OcthKfTqea/8dUVbFHJJdnX+nfR6bJ3yUIg8Yw
YpbHJpQLygmNcWxexrMj4hytVzXUot+NXU38Pu2cihPyOSP7LK185OKnVzPQ3G0X
ovjl702pA68rHlDJaPwTRVq3Q971OUfVSJk2j1D3iwtdHWueUorbVk9CeGkOoR/Y
m8ixdGhqWMNWidC76Mw8TEmMl8wFmwBf7QWmF1oiKKUek8upmHghqRP4T+rgmecc
mhWS3Ras+vHhKPo/iCS8rCQc8ohWvMvs3kMDSC8UY6PVbqIu4u293mMMchi/smmi
re3hJo2rmrk1rq7fSUj3bcttd9DhQYDyvQ9XTydemRGBOfHxzoU8MYq6J6qTTY6d
3J1SEB8sfqjm4Lzo12hvaQywnwvfa6mv7KRecD3mkDk8SPGRwngzfYJbr83BmSiK
Gm529hMNDp03BlC+OsEgAnbcif+ueMLC8bUkl+GwRrYmsjl/Wlcw3eCbKAOjhX8V
T4sFtWeqzQ4bRyOtja2hSzzlHesnxMyZlSpaUNLIF7IecKP+5mDKGKEkaSnZ83yi
3L45e7406D+4YD73LhZS
=XrLV
-----END PGP SIGNATURE-----

--+0mKm/ENadSkQxF+--