On Wed, Aug 16, 2023 at 6:27 AM Shaun Thomas
<shaun.tho...@enterprisedb.com> wrote:
>
> > We could do something like a LOG "connection: method=%s user=%s
> > (%s:%d)", without the "authenticated" and "identity" terms from
> > set_authn_id(). Just to drop an idea.
>
> That would be my inclination as well. Heck, just slap a log message
> right in the specific case statements that don't have actual auth as
> defined by set_authn_id. This assumes anyone really cares about it
> that much, of course. :D
Maybe something like the attached?
- I made the check more generic, rather than hardcoding it inside the
trust statement, because my OAuth proposal would add a method that
only calls set_authn_id() some of the time.
- I used the phrasing "connection not authenticated" in the hopes that
it's a bit more greppable than just "connection", especially in
combination with the existing "connection authenticated" lines.
(I'm reminded that we're reflecting an unauthenticated username as-is
into the logs, but I also don't think this makes things any worse than
they are today with the "authorized" lines.)
--Jacob
From deef6a6daa161e729968d731d0588c87ca45599a Mon Sep 17 00:00:00 2001
From: Jacob Champion <champio...@gmail.com>
Date: Wed, 16 Aug 2023 14:48:49 -0700
Subject: [PATCH] log_connections: add entries for "trust" connections
Allow DBAs to audit unexpected "trust" connections. Previously, you had
to notice the absence of a "connection authenticated" line between the
"connection received" and "connection authorized" entries. Now it's
called out explicitly.
---
src/backend/libpq/auth.c | 15 +++++++++++++++
src/test/authentication/t/001_password.pl | 6 ++++++
2 files changed, 21 insertions(+)
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index e7571aaddb..db3d2b5289 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -645,6 +645,21 @@ ClientAuthentication(Port *port)
#endif
}
+ if (Log_connections
+ && (status == STATUS_OK) && !MyClientConnectionInfo.authn_id)
+ {
+ /*
+ * No authentication was actually performed; this happens e.g. when the
+ * trust method is in use. For audit purposes, log a breadcrumb to
+ * explain where in the HBA this happened.
+ */
+ ereport(LOG,
+ errmsg("connection not authenticated: user=\"%s\" method=%s "
+ "(%s:%d)",
+ port->user_name, hba_authname(port->hba->auth_method),
+ port->hba->sourcefile, port->hba->linenumber));
+ }
+
if (ClientAuthentication_hook)
(*ClientAuthentication_hook) (port, status);
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 12552837a8..5438aa3184 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -140,8 +140,14 @@ $node->safe_psql('postgres', "CREATE database regex_testdb;");
# considered to be authenticated.
reset_pg_hba($node, 'all', 'all', 'trust');
test_conn($node, 'user=scram_role', 'trust', 0,
+ log_like => [
+ qr/connection not authenticated: user="scram_role" method=trust/
+ ],
log_unlike => [qr/connection authenticated:/]);
test_conn($node, 'user=md5_role', 'trust', 0,
+ log_like => [
+ qr/connection not authenticated: user="md5_role" method=trust/
+ ],
log_unlike => [qr/connection authenticated:/]);
# SYSTEM_USER is null when not authenticated.
--
2.39.2
