Re: Logging of matching pg_hba.conf entry during auth skips trust auth, potential security issue

On Tue, Aug 15, 2023 at 04:49:47PM -0500, Shaun Thomas wrote:
> The switch statement that decodes port->hba->auth_method ends by
> simply setting status = STATUS_OK; with no supplementary output since
> it never calls set_authn_id. So in theory, a malicious user could add
> a trust line to pg_hba.conf and have unlimited unlogged access to the
> database.

That's the same as giving access to your data folder. Updating
pg_hba.conf is only the tip of the iceberg if one has write access to
your data folder.

> Unless you happen to notice that the "connection
> authenticated" line has disappeared, it would look like normal
> activity.

"trust" is not really an anthentication method because it does
nothing, it just authorizes things to go through, so you cannot
really say that it can have an authn ID (grep for "pedantic" around
here):
https://www.postgresql.org/message-id/c55788dd1773c521c862e8e0dddb367df51222be.camel@vmware.com

> Would it make sense to decouple the hba info from set_authn_id so that
> it is always logged even when new auth methods get added in the
> future? Or alternatively create a function call specifically for that
> output so it can be produced from the trust case statement and
> anywhere else that needs to tag the auth line. I personally would love
> to see if someone got in through a trust line, ESPECIALLY if it isn't
> supposed to be there. Like:
>
> 2023-08-15 13:26:03.159 PDT [692166] postgres(at)snip LOG: connection
> authenticated: identity="postgres" method=trust
> (/etc/postgresql/15/main/pg_hba.conf:1)

You mean outside the switch/case in ClientAuthentication(). Some
methods have an authn that is implementation-dependent, like ldap. I
am not sure if switching the logic would lead to a gain, like calling
once set_authn_id() vs passing up a string to set in a single call of
set_authn_id().

> I read through the discussion, and it doesn't seem like the security
> aspect of simply hiding trust auths from the log was considered. Since
> this is a new capability, I suppose nothing is really different from
> say Postgres 14 and below. Still, it never hurts to ask.

The first message from Jacob outlines the idea behind the handling of
trust. We could perhaps add one extra set_authn_id() for the uaTrust
case (not uaCert!) if that's more helpful.
--
Michael