On Fri, Feb 25, 2022 at 01:23:49PM -0800, Andres Freund wrote:
> Looks to me like authn_id isn't synchronized to parallel workers right now. So
> the function will return the wrong thing when executed as part of a parallel
> query.
Thanks for the catch. It looks like MyProcPort is left empty, and other
functions that rely on like inet_server_addr() are marked parallel-
restricted, so I've done the same in v4.
On Sat, 2022-02-26 at 14:39 +0900, Michael Paquier wrote:
> FWIW, I am not completely sure what's the use case for being able to
> see the authn of the current session through a trigger. We expose
> that when log_connections is enabled, for audit purposes. I can also
> get behind something more central so as one can get a full picture of
> the authn used by a bunch of session, particularly with complex HBA
> policies, but this looks rather limited to me in usability. Perhaps
> that's not enough to stand as an objection, though, and the patch is
> dead simple.
I'm primarily motivated by the linked thread -- if the gap between
builtin roles and authn_id are going to be used as ammo against other
security features, then let's close that gap. But I think it's fair to
say that if someone is already using triggers to exhaustively audit a
table, it'd be nice to have this info in the same place too.
> > I don't think we should add further functions not prefixed with pg_.
>
> Yep.
Fixed.
> > Perhaps a few tests for less trivial authn_ids could be worthwhile?
> > E.g. certificate DNs.
>
> Yes, src/test/ssl would handle that just fine. Now, this stuff
> already looks after authn results with log_connections=on, so that
> feels like a duplicate.
It was easy enough to add, so I added it. I suppose it does protect
against any reimplementations of pg_session_authn_id() that can't
handle longer ID strings, though I admit that's a stretch.
Thanks,
--Jacob
commit efec9f040843d1de2fc52f5ce0d020478a5bc75d
Author: Jacob Champion <pchamp...@vmware.com>
Date: Mon Feb 28 10:28:51 2022 -0800
squash! Add API to retrieve authn_id from SQL
diff --git a/src/backend/utils/adt/name.c b/src/backend/utils/adt/name.c
index b892d25c29..662a7943ed 100644
--- a/src/backend/utils/adt/name.c
+++ b/src/backend/utils/adt/name.c
@@ -258,7 +258,7 @@ namestrcmp(Name name, const char *str)
/*
- * SQL-functions CURRENT_USER, SESSION_USER, SESSION_AUTHN_ID
+ * SQL-functions CURRENT_USER, SESSION_USER, PG_SESSION_AUTHN_ID
*/
Datum
current_user(PG_FUNCTION_ARGS)
@@ -273,7 +273,7 @@ session_user(PG_FUNCTION_ARGS)
}
Datum
-session_authn_id(PG_FUNCTION_ARGS)
+pg_session_authn_id(PG_FUNCTION_ARGS)
{
if (!MyProcPort || !MyProcPort->authn_id)
PG_RETURN_NULL();
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 14194afe1c..3787b8edaf 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202202251
+#define CATALOG_VERSION_NO 202202281
#endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 51e0d24f01..45326a2fe5 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1509,8 +1509,8 @@
proname => 'session_user', provolatile => 's', prorettype => 'name',
proargtypes => '', prosrc => 'session_user' },
{ oid => '9774', descr => 'session authenticated identity',
- proname => 'session_authn_id', provolatile => 's', prorettype => 'text',
- proargtypes => '', prosrc => 'session_authn_id' },
+ proname => 'pg_session_authn_id', provolatile => 's', proparallel => 'r',
+ prorettype => 'text', proargtypes => '', prosrc => 'pg_session_authn_id' },
{ oid => '744',
proname => 'array_eq', prorettype => 'bool',
diff --git a/src/test/authentication/t/001_password.pl
b/src/test/authentication/t/001_password.pl
index 2aa28ed547..1edac8d588 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -83,7 +83,7 @@ test_role($node, 'md5_role', 'trust', 0,
log_unlike => [qr/connection authenticated:/]);
my $res = $node->safe_psql('postgres',
- "SELECT session_authn_id() IS NULL;"
+ "SELECT pg_session_authn_id() IS NULL;"
);
is($res, 't', "users with trust authentication have NULL authn_id");
@@ -97,7 +97,7 @@ test_role($node, 'md5_role', 'password', 0,
[qr/connection authenticated: identity="md5_role" method=password/]);
$res = $node->safe_psql('postgres',
- "SELECT session_authn_id();",
+ "SELECT pg_session_authn_id();",
connstr => "user=md5_role"
);
is($res, 'md5_role', "users with md5 authentication have authn_id matching
role name");
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 5c5b16fbe7..79ef7b46f1 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -443,6 +443,13 @@ $node->connect_ok(
qr/connection authenticated:
identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG" method=cert/
],);
+# Sanity-check pg_session_authn_id() for long ID strings
+my $res = $node->safe_psql('postgres',
+ "SELECT pg_session_authn_id();",
+ connstr => "$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt
sslkey=$key{'client-dn.key'}",
+);
+is($res, "CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG", "users with
cert authentication have entire DN as authn_id");
+
# same thing but with a regex
$dn_connstr = "$common_connstr dbname=certdb_dn_re";
From 8a313bb19ad9d2748edc38ce91464c123642046b Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Mon, 14 Feb 2022 08:10:53 -0800
Subject: [PATCH v4] Add API to retrieve authn_id from SQL
The authn_id field in MyProcPort is currently only accessible to the
backend itself. Add a SQL function, pg_session_authn_id(), to expose
the field to triggers that may want to make use of it.
---
src/backend/utils/adt/name.c | 12 +++++++++++-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.dat | 3 +++
src/test/authentication/t/001_password.pl | 11 +++++++++++
src/test/ssl/t/001_ssltests.pl | 7 +++++++
5 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/name.c b/src/backend/utils/adt/name.c
index e8bba3670c..662a7943ed 100644
--- a/src/backend/utils/adt/name.c
+++ b/src/backend/utils/adt/name.c
@@ -23,6 +23,7 @@
#include "catalog/namespace.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -257,7 +258,7 @@ namestrcmp(Name name, const char *str)
/*
- * SQL-functions CURRENT_USER, SESSION_USER
+ * SQL-functions CURRENT_USER, SESSION_USER, PG_SESSION_AUTHN_ID
*/
Datum
current_user(PG_FUNCTION_ARGS)
@@ -271,6 +272,15 @@ session_user(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(DirectFunctionCall1(namein, CStringGetDatum(GetUserNameFromId(GetSessionUserId(), false))));
}
+Datum
+pg_session_authn_id(PG_FUNCTION_ARGS)
+{
+ if (!MyProcPort || !MyProcPort->authn_id)
+ PG_RETURN_NULL();
+
+ PG_RETURN_TEXT_P(cstring_to_text(MyProcPort->authn_id));
+}
+
/*
* SQL-functions CURRENT_SCHEMA, CURRENT_SCHEMAS
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 14194afe1c..3787b8edaf 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202202251
+#define CATALOG_VERSION_NO 202202281
#endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7de8cfc7e9..45326a2fe5 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 => 'pg_session_authn_id', provolatile => 's', proparallel => 'r',
+ prorettype => 'text', proargtypes => '', prosrc => 'pg_session_authn_id' },
{ oid => '744',
proname => 'array_eq', prorettype => 'bool',
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 3e3079c824..1edac8d588 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 pg_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 pg_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(
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 5c5b16fbe7..79ef7b46f1 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -443,6 +443,13 @@ $node->connect_ok(
qr/connection authenticated: identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG" method=cert/
],);
+# Sanity-check pg_session_authn_id() for long ID strings
+my $res = $node->safe_psql('postgres',
+ "SELECT pg_session_authn_id();",
+ connstr => "$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt sslkey=$key{'client-dn.key'}",
+);
+is($res, "CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG", "users with cert authentication have entire DN as authn_id");
+
# same thing but with a regex
$dn_connstr = "$common_connstr dbname=certdb_dn_re";
--
2.25.1
