Subject: [PATCH] Expose port->authn_id to extensions and triggers

From: Jacob Champion <[email-masked]>
Date: Thu, 24 Feb 2022 00:15:40 +0000

Hi all,

Stephen pointed out [1] that the authenticated identity that's stored
in MyProcPort can't be retrieved by extensions or triggers. Attached is
a patch that provides both a C API and a SQL function for retrieving
it.

GetAuthenticatedIdentityString() is a mouthful but I wanted to
differentiate it from the existing GetAuthenticatedUserId(); better
names welcome. It only exists as an accessor because I wasn't sure if
extensions outside of contrib were encouraged to rely on the internal
layout of struct Port. (If they can, then that call can go away
entirely.)

Thanks,
--Jacob

[1] https://www.postgresql.org/message-id/CAOuzzgpFpuroNRabEvB9kST_TSyS2jFicBNoXvW7G2pZFixyBw%40mail.gmail.com
From 155a490d28cecf161d994e6d8824cbe967f4d469 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email-masked]>
Date: Mon, 14 Feb 2022 08:10:53 -0800
Subject: [PATCH] Add APIs to retrieve authn_id from C and SQL

The authn_id field in MyProcPort is currently only accessible to the
backend itself.  Add a getter in C, GetAuthenticatedIdentityString(),
and a corresponding SQL function, session_authn_id(), to expose the
field to extensions and triggers that may want to make use of it.
---
 src/backend/utils/adt/name.c              | 14 +++++++++++++-
 src/backend/utils/init/miscinit.c         | 14 ++++++++++++++
 src/include/catalog/catversion.h          |  2 +-
 src/include/catalog/pg_proc.dat           |  3 +++
 src/include/miscadmin.h                   |  1 +
 src/test/authentication/t/001_password.pl | 11 +++++++++++
 6 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/name.c b/src/backend/utils/adt/name.c
index e8bba3670c..d605b1e116 100644
--- a/src/backend/utils/adt/name.c
+++ b/src/backend/utils/adt/name.c
@@ -257,7 +257,7 @@ namestrcmp(Name name, const char *str)

 /*
- * SQL-functions CURRENT_USER, SESSION_USER
+ * SQL-functions CURRENT_USER, SESSION_USER, SESSION_AUTHN_ID
  */
 Datum
 current_user(PG_FUNCTION_ARGS)
@@ -271,6 +271,18 @@ session_user(PG_FUNCTION_ARGS)
 	PG_RETURN_DATUM(DirectFunctionCall1(namein, CStringGetDatum(GetUserNameFromId(GetSessionUserId(), false))));
 }
 
+Datum
+session_authn_id(PG_FUNCTION_ARGS)
+{
+	char	   *authn_id;
+
+	authn_id = GetAuthenticatedIdentityString();
+	if (!authn_id)
+		PG_RETURN_NULL();
+
+	PG_RETURN_CSTRING(authn_id);
+}
+
 
 /*
  * SQL-functions CURRENT_SCHEMA, CURRENT_SCHEMAS
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index bdc77af719..9fea7f0be3 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -929,6 +929,20 @@ GetUserNameFromId(Oid roleid, bool noerr)
 	return result;
 }
 
+/*
+ * Get a palloc'd string representing the authenticated identity of the client
+ * (see documentation for Port's authn_id field). Returns NULL if the client has
+ * not been authenticated.
+ */
+char *
+GetAuthenticatedIdentityString(void)
+{
+	if (!MyProcPort || !MyProcPort->authn_id)
+		return NULL;
+
+	return pstrdup(MyProcPort->authn_id);
+}
+
 
 /*-------------------------------------------------------------------------
  *				Interlock-file support
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 1addb568ef..ca52e6889c 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202202221
+#define CATALOG_VERSION_NO	202202231
 
 #endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7f1ee97f55..b76d357bee 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1508,6 +1508,9 @@
 { oid => '746', descr => 'session user name',
   proname => 'session_user', provolatile => 's', prorettype => 'name',
   proargtypes => '', prosrc => 'session_user' },
+{ oid => '9774', descr => 'session authenticated identity',
+  proname => 'session_authn_id', provolatile => 's', prorettype => 'cstring',
+  proargtypes => '', prosrc => 'session_authn_id' },
 
 { oid => '744',
   proname => 'array_eq', prorettype => 'bool',
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0abc3ad540..25731ce977 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -347,6 +347,7 @@ extern void SetDataDir(const char *dir);
 extern void ChangeToDataDir(void);
 
 extern char *GetUserNameFromId(Oid roleid, bool noerr);
+extern char *GetAuthenticatedIdentityString(void);
 extern Oid	GetUserId(void);
 extern Oid	GetOuterUserId(void);
 extern Oid	GetSessionUserId(void);
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 3e3079c824..2aa28ed547 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -82,6 +82,11 @@ test_role($node, 'scram_role', 'trust', 0,
 test_role($node, 'md5_role', 'trust', 0,
 	log_unlike => [qr/connection authenticated:/]);
 
+my $res = $node->safe_psql('postgres',
+	"SELECT session_authn_id() IS NULL;"
+);
+is($res, 't', "users with trust authentication have NULL authn_id");
+
 # For plain "password" method, all users should also be able to connect.
 reset_pg_hba($node, 'password');
 test_role($node, 'scram_role', 'password', 0,
@@ -91,6 +96,12 @@ test_role($node, 'md5_role', 'password', 0,
 	log_like =>
 	  [qr/connection authenticated: identity="md5_role" method=password/]);
 
+$res = $node->safe_psql('postgres',
+	"SELECT session_authn_id();",
+	connstr      => "user=md5_role"
+);
+is($res, 'md5_role', "users with md5 authentication have authn_id matching role name");
+
 # For "scram-sha-256" method, user "scram_role" should be able to connect.
 reset_pg_hba($node, 'scram-sha-256');
 test_role(
-- 
2.25.1

---

From: Michael Paquier <[email-masked]>
Date: Thu, 24 Feb 2022 20:39:48 +0900

+char *

I don't quite see the additional value that this API brings as
MyProcPort is directly accessible, and contrib modules like
postgres_fdw and sslinfo just use that to find the data of the current
backend.  Cannot a module like pgaudit, through the elog hook, do its
work with what we have already in place?

What's the use case for a given session to be able to report back
only its own authn through SQL?

I could still see a use case for that at a more global level with
beentrys, but it looked like there was not much interest the last time
I dropped this idea.
--
Michael

---

From: Jacob Champion <[email-masked]>
Date: Thu, 24 Feb 2022 16:50:59 +0000

On Thu, 2022-02-24 at 20:39 +0900, Michael Paquier wrote:
> backend.

Right -- I just didn't know if third-party modules were actually able
to rely on the internal layout of struct Port. Is that guaranteed to
remain constant for a major release line? If so, this new API is
superfluous.

> Cannot a module like pgaudit, through the elog hook, do its

Given the above, I would hope so. Stephen mentioned that pgaudit only
had access to the logged-in role, and when I proposed a miscadmin.h API
he said that would help. CC'ing him to see what he meant; I don't know
if pgaudit has additional constraints.

> only its own authn through SQL?

That's for triggers to be able to grab the current ID for
logging/auditing. Is there a better way to expose this for that use
case?

> beentrys, but it looked like there was not much interest the last time

I agree that this would be useful to have in the stats. From my outside
perspective, it seems like it's difficult to get strings of arbitrary
length in there; is that accurate?

Thanks,
--Jacob