On Sat, 2022-03-26 at 11:36 -0700, Andres Freund wrote:
> > I also note that exposing it as a GUC means we have zero control over
> > who/what can read it. Maybe that's not a problem, but it needs to be
> > thought about before we go down that path.
>
> Yes, I think that's a fair concern.
I like that there's no builtin way, today, for a superuser to modify
the internal value; it strengthens the use as an auditing tool. Moving
this to a PGC_SU_BACKEND GUC seems to weaken that. And it looks like
PGC_INTERNAL is skipped during the serialization, so if we chose that
option, we'd need to write new code anyway?
We'd also need to guess whether the GUC system's serialization of NULL
as an empty string is likely to cause problems for any future auth
methods. My guess is "no", to be honest, but I do like maintaining the
distinction -- it feels safer.
v8 rebases over the recent SSL changes to get the cfbot green again.
Thanks,
--Jacob
commit bd02c608e3053217056464a31dff49344ca3a5f3
Author: Jacob Champion <pchamp...@vmware.com>
Date: Tue Mar 29 16:26:52 2022 -0700
fixup! Add API to retrieve authn_id from SQL
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index ac2848b931..2b6947fee5 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -428,7 +428,7 @@ $node->connect_ok(
# 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'}",
+ connstr => "$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt " .
sslkey('client-dn.key'),
);
is($res, "CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG", "users with
cert authentication have entire DN as authn_id");
From 34e590e13240bde6a5daa0e3f866b56ad7a4c2f9 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Mon, 14 Feb 2022 08:10:53 -0800
Subject: [PATCH v8 1/3] 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.
---
doc/src/sgml/func.sgml | 26 +++++++++++++++++++++++
src/backend/utils/adt/name.c | 12 ++++++++++-
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, 58 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 3a9d62b408..454c15fde4 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -22280,6 +22280,32 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_session_authn_id</primary>
+ </indexterm>
+ <function>pg_session_authn_id</function> ()
+ <returnvalue>text</returnvalue>
+ </para>
+ <para>
+ Returns the authenticated identity for the current connection, or
+ <literal>NULL</literal> if the user has not been authenticated.
+ </para>
+ <para>
+ The authenticated identity is an immutable identifier for the user
+ presented during the connection handshake; the exact format depends on
+ the authentication method in use. (For example, when using the
+ <literal>scram-sha-256</literal> auth method, the authenticated identity
+ is simply the username. When using the <literal>cert</literal> auth
+ method, the authenticated identity is the Distinguished Name of the
+ client certificate.) Even for auth methods which use the username as
+ the authenticated identity, this function differs from
+ <literal>session_user</literal> in that its return value cannot be
+ changed after login.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
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/pg_proc.dat b/src/include/catalog/pg_proc.dat
index deb00307f6..27ab913402 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..f0bdeda52d 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -82,6 +82,10 @@ 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 +95,13 @@ 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 d8eeb085da..2b6947fee5 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -425,6 +425,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('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
From e2394ee7349474b51adc203bddca0ee911bab81a Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Wed, 23 Mar 2022 15:07:05 -0700
Subject: [PATCH v8 2/3] Allow parallel workers to use pg_session_authn_id()
Move authn_id into a new global, MyProcShared, which is intended to hold
all the information that can be shared between the backend and any
parallel workers. MyProcShared is serialized and restored using a new
parallel key.
With this change, the parallel restriction can be removed from
pg_session_authn_id().
---
src/backend/access/transam/parallel.c | 18 +++++-
src/backend/libpq/auth.c | 16 ++---
src/backend/utils/adt/name.c | 4 +-
src/backend/utils/init/miscinit.c | 71 +++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 2 +-
src/include/libpq/libpq-be.h | 35 ++++++-----
src/include/miscadmin.h | 4 ++
src/test/authentication/t/001_password.pl | 33 +++++++++++
8 files changed, 159 insertions(+), 24 deletions(-)
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index df0cd77558..c88eab0933 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -76,6 +76,7 @@
#define PARALLEL_KEY_REINDEX_STATE UINT64CONST(0xFFFFFFFFFFFF000C)
#define PARALLEL_KEY_RELMAPPER_STATE UINT64CONST(0xFFFFFFFFFFFF000D)
#define PARALLEL_KEY_UNCOMMITTEDENUMS UINT64CONST(0xFFFFFFFFFFFF000E)
+#define PARALLEL_KEY_SHAREDPORT UINT64CONST(0xFFFFFFFFFFFF000F)
/* Fixed-size parallel state. */
typedef struct FixedParallelState
@@ -212,6 +213,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
Size reindexlen = 0;
Size relmapperlen = 0;
Size uncommittedenumslen = 0;
+ Size sharedportlen = 0;
Size segsize = 0;
int i;
FixedParallelState *fps;
@@ -272,8 +274,10 @@ InitializeParallelDSM(ParallelContext *pcxt)
shm_toc_estimate_chunk(&pcxt->estimator, relmapperlen);
uncommittedenumslen = EstimateUncommittedEnumsSpace();
shm_toc_estimate_chunk(&pcxt->estimator, uncommittedenumslen);
+ sharedportlen = EstimateSharedPortSpace();
+ shm_toc_estimate_chunk(&pcxt->estimator, sharedportlen);
/* If you add more chunks here, you probably need to add keys. */
- shm_toc_estimate_keys(&pcxt->estimator, 11);
+ shm_toc_estimate_keys(&pcxt->estimator, 12);
/* Estimate space need for error queues. */
StaticAssertStmt(BUFFERALIGN(PARALLEL_ERROR_QUEUE_SIZE) ==
@@ -352,6 +356,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
char *session_dsm_handle_space;
char *entrypointstate;
char *uncommittedenumsspace;
+ char *sharedportspace;
Size lnamelen;
/* Serialize shared libraries we have loaded. */
@@ -422,6 +427,12 @@ InitializeParallelDSM(ParallelContext *pcxt)
shm_toc_insert(pcxt->toc, PARALLEL_KEY_UNCOMMITTEDENUMS,
uncommittedenumsspace);
+ /* Serialize our SharedPort. */
+ sharedportspace = shm_toc_allocate(pcxt->toc, sharedportlen);
+ SerializeSharedPort(sharedportlen, sharedportspace);
+ shm_toc_insert(pcxt->toc, PARALLEL_KEY_SHAREDPORT,
+ sharedportspace);
+
/* Allocate space for worker information. */
pcxt->worker = palloc0(sizeof(ParallelWorkerInfo) * pcxt->nworkers);
@@ -1270,6 +1281,7 @@ ParallelWorkerMain(Datum main_arg)
char *reindexspace;
char *relmapperspace;
char *uncommittedenumsspace;
+ char *sharedportspace;
StringInfoData msgbuf;
char *session_dsm_handle_space;
Snapshot tsnapshot;
@@ -1479,6 +1491,10 @@ ParallelWorkerMain(Datum main_arg)
false);
RestoreUncommittedEnums(uncommittedenumsspace);
+ /* Restore the SharedPort. */
+ sharedportspace = shm_toc_lookup(toc, PARALLEL_KEY_SHAREDPORT, false);
+ RestoreSharedPort(sharedportspace);
+
/* Attach to the leader's serializable transaction, if SERIALIZABLE. */
AttachSerializableXact(fps->serializable_xact_handle);
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index efc53f3135..bceda9755a 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -342,15 +342,15 @@ auth_failed(Port *port, int status, const char *logdetail)
* authorization will fail later.
*
* The provided string will be copied into TopMemoryContext, to match the
- * lifetime of the Port, so it is safe to pass a string that is managed by an
- * external library.
+ * lifetime of MyProcShared, so it is safe to pass a string that is managed by
+ * an external library.
*/
static void
set_authn_id(Port *port, const char *id)
{
Assert(id);
- if (port->authn_id)
+ if (MyProcShared.authn_id)
{
/*
* An existing authn_id should never be overwritten; that means two
@@ -361,17 +361,18 @@ set_authn_id(Port *port, const char *id)
ereport(FATAL,
(errmsg("authentication identifier set more than once"),
errdetail_log("previous identifier: \"%s\"; new identifier: \"%s\"",
- port->authn_id, id)));
+ MyProcShared.authn_id, id)));
}
- port->authn_id = MemoryContextStrdup(TopMemoryContext, id);
+ MyProcShared.authn_id = MemoryContextStrdup(TopMemoryContext, id);
if (Log_connections)
{
ereport(LOG,
errmsg("connection authenticated: identity=\"%s\" method=%s "
"(%s:%d)",
- port->authn_id, hba_authname(port->hba->auth_method), HbaFileName,
+ MyProcShared.authn_id,
+ hba_authname(port->hba->auth_method), HbaFileName,
port->hba->linenumber));
}
}
@@ -1908,7 +1909,8 @@ auth_peer(hbaPort *port)
*/
set_authn_id(port, pw->pw_name);
- ret = check_usermap(port->hba->usermap, port->user_name, port->authn_id, false);
+ ret = check_usermap(port->hba->usermap, port->user_name,
+ MyProcShared.authn_id, false);
return ret;
#else
diff --git a/src/backend/utils/adt/name.c b/src/backend/utils/adt/name.c
index 662a7943ed..6d497e63d9 100644
--- a/src/backend/utils/adt/name.c
+++ b/src/backend/utils/adt/name.c
@@ -275,10 +275,10 @@ session_user(PG_FUNCTION_ARGS)
Datum
pg_session_authn_id(PG_FUNCTION_ARGS)
{
- if (!MyProcPort || !MyProcPort->authn_id)
+ if (!MyProcShared.authn_id)
PG_RETURN_NULL();
- PG_RETURN_TEXT_P(cstring_to_text(MyProcPort->authn_id));
+ PG_RETURN_TEXT_P(cstring_to_text(MyProcShared.authn_id));
}
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index bdc77af719..0afab3e142 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -929,6 +929,77 @@ GetUserNameFromId(Oid roleid, bool noerr)
return result;
}
+/* ------------------------------------------------------------------------
+ * "Shared" connection state
+ *
+ * MyProcShared contains pieces of information about the client that need to be
+ * synced to parallel workers when they initialize. Over time, this list will
+ * probably grow, and may subsume some of the "user state" variables above.
+ *-------------------------------------------------------------------------
+ */
+
+SharedPort MyProcShared;
+
+/*
+ * Calculate the space needed to serialize MyProcShared.
+ */
+Size
+EstimateSharedPortSpace(void)
+{
+ Size size = 1;
+
+ if (MyProcShared.authn_id)
+ size = add_size(size, strlen(MyProcShared.authn_id) + 1);
+
+ return size;
+}
+
+/*
+ * Serialize MyProcShared for use by parallel workers.
+ */
+void
+SerializeSharedPort(Size maxsize, char *start_address)
+{
+ /*
+ * First byte is an indication of whether or not authn_id has been set to
+ * non-NULL, to differentiate that case from the empty string.
+ */
+ Assert(maxsize > 0);
+ start_address[0] = MyProcShared.authn_id ? 1 : 0;
+ start_address++;
+ maxsize--;
+
+ if (MyProcShared.authn_id)
+ {
+ Size len;
+
+ len = strlcpy(start_address, MyProcShared.authn_id, maxsize) + 1;
+ Assert(len <= maxsize);
+ maxsize -= len;
+ start_address += len;
+ }
+}
+
+/*
+ * Restore MyProcShared from its serialized representation.
+ */
+void
+RestoreSharedPort(char *sharedport)
+{
+ if (sharedport[0] == 0)
+ {
+ MyProcShared.authn_id = NULL;
+ sharedport++;
+ }
+ else
+ {
+ sharedport++;
+ MyProcShared.authn_id = MemoryContextStrdup(TopMemoryContext,
+ sharedport);
+ sharedport += strlen(sharedport) + 1;
+ }
+}
+
/*-------------------------------------------------------------------------
* Interlock-file support
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 27ab913402..a98c8abb9e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1509,7 +1509,7 @@
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',
+ proname => 'pg_session_authn_id', provolatile => 's',
prorettype => 'text', proargtypes => '', prosrc => 'pg_session_authn_id' },
{ oid => '744',
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index dd3e5efba3..911b8246ce 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -99,6 +99,27 @@ typedef struct
} pg_gssinfo;
#endif
+/*
+ * Fields from Port that need to be copied over to parallel workers go into the
+ * SharedPort. The same rules apply for allocations here as for Port (must be
+ * malloc'd or palloc'd in TopMemoryContext).
+ */
+typedef struct SharedPort
+{
+ /*
+ * Authenticated identity. The meaning of this identifier is dependent on
+ * hba->auth_method; it is the identity (if any) that the user presented
+ * during the authentication cycle, before they were assigned a database
+ * role. (It is effectively the "SYSTEM-USERNAME" of a pg_ident usermap
+ * -- though the exact string in use may be different, depending on pg_hba
+ * options.)
+ *
+ * authn_id is NULL if the user has not actually been authenticated, for
+ * example if the "trust" auth method is in use.
+ */
+ const char *authn_id;
+} SharedPort;
+
/*
* This is used by the postmaster in its communication with frontends. It
* contains all state information needed during this communication before the
@@ -159,19 +180,6 @@ typedef struct Port
*/
HbaLine *hba;
- /*
- * Authenticated identity. The meaning of this identifier is dependent on
- * hba->auth_method; it is the identity (if any) that the user presented
- * during the authentication cycle, before they were assigned a database
- * role. (It is effectively the "SYSTEM-USERNAME" of a pg_ident usermap
- * -- though the exact string in use may be different, depending on pg_hba
- * options.)
- *
- * authn_id is NULL if the user has not actually been authenticated, for
- * example if the "trust" auth method is in use.
- */
- const char *authn_id;
-
/*
* TCP keepalive and user timeout settings.
*
@@ -328,6 +336,7 @@ extern ssize_t be_gssapi_write(Port *port, void *ptr, size_t len);
#endif /* ENABLE_GSS */
extern ProtocolVersion FrontendProtocol;
+extern SharedPort MyProcShared;
/* TCP keepalives configuration. These are no-ops on an AF_UNIX socket. */
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0abc3ad540..68cc1517a0 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -481,6 +481,10 @@ extern void process_session_preload_libraries(void);
extern void pg_bindtextdomain(const char *domain);
extern bool has_rolreplication(Oid roleid);
+extern Size EstimateSharedPortSpace(void);
+extern void SerializeSharedPort(Size maxsize, char *start_address);
+extern void RestoreSharedPort(char *sharedport);
+
/* in access/transam/xlog.c */
extern bool BackupInProgress(void);
extern void CancelBackup(void);
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index f0bdeda52d..3f8629b3a6 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -74,6 +74,14 @@ $node->safe_psql('postgres',
);
$ENV{"PGPASSWORD"} = 'pass';
+# Set up a table for parallel worker testing.
+$node->safe_psql('postgres',
+ 'CREATE TABLE nulls (n) AS SELECT NULL FROM generate_series(1, 200000);'
+);
+$node->safe_psql('postgres',
+ 'GRANT SELECT ON nulls TO md5_role;'
+);
+
# For "trust" method, all users should be able to connect. These users are not
# considered to be authenticated.
reset_pg_hba($node, 'trust');
@@ -86,6 +94,19 @@ my $res =
$node->safe_psql('postgres', "SELECT pg_session_authn_id() IS NULL;");
is($res, 't', "users with trust authentication have NULL authn_id");
+# Test pg_session_authn_id() with parallel workers.
+$res = $node->safe_psql(
+ 'postgres', '
+ SET min_parallel_table_scan_size TO 0;
+ SET parallel_setup_cost TO 0;
+ SET parallel_tuple_cost TO 0;
+ SET max_parallel_workers_per_gather TO 2;
+
+ SELECT bool_and(pg_session_authn_id() IS NOT DISTINCT FROM n) FROM nulls;
+ ',
+ connstr => "user=md5_role");
+is($res, 't', "parallel workers return a null authn_id when not authenticated");
+
# For plain "password" method, all users should also be able to connect.
reset_pg_hba($node, 'password');
test_role($node, 'scram_role', 'password', 0,
@@ -102,6 +123,18 @@ $res = $node->safe_psql(
is($res, 'md5_role',
"users with md5 authentication have authn_id matching role name");
+$res = $node->safe_psql(
+ 'postgres', '
+ SET min_parallel_table_scan_size TO 0;
+ SET parallel_setup_cost TO 0;
+ SET parallel_tuple_cost TO 0;
+ SET max_parallel_workers_per_gather TO 2;
+
+ SELECT bool_and(pg_session_authn_id() IS DISTINCT FROM n) FROM nulls;
+ ',
+ connstr => "user=md5_role");
+is($res, 't', "parallel workers return a non-null authn_id when authenticated");
+
# 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
