On Wed, Jun 22, 2022 at 8:10 AM Joe Conway <mail@joeconway.com> wrote:
> On the contrary, I would argue that not having the identifier for the
> external "user" available is a security concern. Ideally you want to be
> able to trace actions inside Postgres to the actual user that invoked them.
If auditing is also the use case for SYSTEM_USER, you'll probably want
to review the arguments for making it available to parallel workers
that were made in the other thread [1].
Initial comments on the patch:
> In case port->authn_id is NULL then the patch is returning the SESSION_USER for the SYSTEM_USER. Perhaps it should
returnNULL instead.
If the spec says that SYSTEM_USER "represents the operating system
user", but we don't actually know who that user was (authn_id is
NULL), then I think SYSTEM_USER should also be NULL so as not to
mislead auditors.
> --- a/src/backend/utils/init/miscinit.c
> +++ b/src/backend/utils/init/miscinit.c
> @@ -473,6 +473,7 @@ static Oid AuthenticatedUserId = InvalidOid;
> static Oid SessionUserId = InvalidOid;
> static Oid OuterUserId = InvalidOid;
> static Oid CurrentUserId = InvalidOid;
> +static const char *SystemUser = NULL;
>
> /* We also have to remember the superuser state of some of these levels */
> static bool AuthenticatedUserIsSuperuser = false;
What's the rationale for introducing a new global for this? A downside
is that now there are two sources of truth, for a security-critical
attribute of the connection.
--Jacob
[1] https://www.postgresql.org/message-id/flat/793d990837ae5c06a558d58d62de9378ab525d83.camel%40vmware.com