On Fri, Aug 18, 2023 at 08:49:16AM +0900, Michael Paquier wrote:
> After sleeping on it, I think that I'd just agree with Robert's point
> to just use the same language as the message, while also agreeing with
> the patch to not set MyClientConnectionInfo.authn_id in the uaTrust
> case, only logging something under log_connections.
> 
> +        * No authentication was actually performed; this happens e.g. when 
> the
> +        * trust method is in use.
> 
> This comment should be reworded a bit, say "No authentication identity
> was set; blah ..".

Attached is a v3 to do these two things, with adjustments for two SSL
tests.  Any objections about it?

(Note: no backpatch)
--
Michael
From 13b4c6fe0388bb4eec1d8aab6a25192ff7ad0684 Mon Sep 17 00:00:00 2001
From: Jacob Champion <champio...@gmail.com>
Date: Wed, 16 Aug 2023 14:48:49 -0700
Subject: [PATCH v3] 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 |  8 ++++++--
 src/test/ssl/t/001_ssltests.pl            |  4 ++--
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 315a24bb3f..1a084d21de 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 identity was set; 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 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..4402fa7721 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -140,9 +140,13 @@ $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_unlike => [qr/connection authenticated:/]);
+	log_like => [
+		qr/connection authenticated: user="scram_role" method=trust/
+	]);
 test_conn($node, 'user=md5_role', 'trust', 0,
-	log_unlike => [qr/connection authenticated:/]);
+	log_like => [
+		qr/connection authenticated: user="md5_role" method=trust/
+	]);
 
 # SYSTEM_USER is null when not authenticated.
 $res = $node->safe_psql('postgres', "SELECT SYSTEM_USER IS NULL;");
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 76442de063..81e36a4e88 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -801,7 +801,7 @@ $node->connect_ok(
 	  . sslkey('client.key'),
 	"auth_option clientcert=verify-full succeeds with matching username and Common Name",
 	# verify-full does not provide authentication
-	log_unlike => [qr/connection authenticated:/],);
+	log_like => [qr/connection authenticated: user="ssltestuser" method=trust/],);
 
 $node->connect_fails(
 	"$common_connstr user=anotheruser sslcert=ssl/client.crt "
@@ -819,7 +819,7 @@ $node->connect_ok(
 	  . sslkey('client.key'),
 	"auth_option clientcert=verify-ca succeeds with mismatching username and Common Name",
 	# verify-full does not provide authentication
-	log_unlike => [qr/connection authenticated:/],);
+	log_like => [qr/connection authenticated: user="yetanotheruser" method=trust/],);
 
 # intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file
 switch_server_cert($node, certfile => 'server-cn-only', cafile => 'root_ca');
-- 
2.40.1

Attachment: signature.asc
Description: PGP signature

Reply via email to