On Fri, Feb 14, 2025 at 5:34 PM Jacob Champion <jacob.champ...@enterprisedb.com> wrote: > (But it doesn't > seem like we're going to agree on this for now; in the meantime I'll > prepare a version of the patch that only calls > pgstat_bestart_security() once.)
v9 removes the first call, and moves the second (now only) call up and out of the if/else chain, just past client authentication. The SSL pre-auth tests have been removed. Thanks! --Jacob
1: 81a61854bdf ! 1: d8de39bc076 pgstat: report in earlier with STATE_STARTING
@@ Commit message
2) pgstat_bestart_security() reports the SSL/GSS status of the
connection. Some backends don't call this at all; others call it
- twice, once after transport establishment and once after client
- authentication.
+ after client authentication.
3) pgstat_bestart_final() reports the user and database IDs, takes the
entry out of STATE_STARTING, and reports the application_name.
@@ src/backend/utils/activity/backend_status.c: pgstat_bestart(void)
+}
+
+/*
-+ * Fill in SSL and GSS information for the pgstat entry. This is separate
from
-+ * pgstat_bestart_initial() so that backends may call it multiple times as
-+ * security details are filled in.
++ * Fill in SSL and GSS information for the pgstat entry.
+ *
+ * This should only be called from backends with a MyProcPort.
+ */
@@ src/backend/utils/init/postinit.c: InitPostgres(const char *in_dbname,
Oid dboid
+ if (!bootstrap)
+ {
+ pgstat_bestart_initial();
-+ if (MyProcPort)
-+ pgstat_bestart_security(); /* fill in any SSL/GSS
info too */
-+
+ INJECTION_POINT("init-pre-auth");
+ }
+
@@ src/backend/utils/init/postinit.c: InitPostgres(const char *in_dbname,
Oid dboid
InitializeSessionUserId(username, useroid, false);
/* ensure that auth_method is actually valid, aka authn_id is
not NULL */
@@ src/backend/utils/init/postinit.c: InitPostgres(const char *in_dbname,
Oid dboid,
- InitializeSystemUser(MyClientConnectionInfo.authn_id,
-
hba_authname(MyClientConnectionInfo.auth_method));
am_superuser = superuser();
-+
-+ /*
-+ * Authentication may have changed SSL/GSS details for the
session, so
-+ * report it again.
-+ */
-+ pgstat_bestart_security();
}
++ /* Report any SSL/GSS details for the session. */
++ if (MyProcPort != NULL)
++ {
++ Assert(!bootstrap);
++
++ pgstat_bestart_security();
++ }
++
/*
+ * Binary upgrades only allowed super-user connections
+ */
@@ src/backend/utils/init/postinit.c: InitPostgres(const char *in_dbname,
Oid dboid,
/* initialize client encoding */
InitializeClientEncoding();
@@ src/test/authentication/t/007_pre_auth.pl (new)
+$conn->quit();
+
+done_testing();
-
- ## src/test/ssl/Makefile ##
-@@
- #-------------------------------------------------------------------------
-
- EXTRA_INSTALL = contrib/sslinfo
-+EXTRA_INSTALL += src/test/modules/injection_points
-
- subdir = src/test/ssl
- top_builddir = ../../..
- include $(top_builddir)/src/Makefile.global
-
--export OPENSSL with_ssl
-+export OPENSSL enable_injection_points with_ssl
-
- # The sslfiles targets are separated into their own file due to
interactions
- # with settings in Makefile.global.
-
- ## src/test/ssl/meson.build ##
-@@ src/test/ssl/meson.build: tests += {
- 'bd': meson.current_build_dir(),
- 'tap': {
- 'env': {
-+ 'enable_injection_points': get_option('injection_points') ? 'yes' :
'no',
- 'with_ssl': ssl_library,
- 'OPENSSL': openssl.found() ? openssl.path() : '',
- },
-
- ## src/test/ssl/t/001_ssltests.pl ##
-@@ src/test/ssl/t/001_ssltests.pl: use Config qw ( %Config );
- use PostgreSQL::Test::Cluster;
- use PostgreSQL::Test::Utils;
- use Test::More;
-+use Time::HiRes qw(usleep);
-
- use FindBin;
- use lib $FindBin::RealBin;
-@@ src/test/ssl/t/001_ssltests.pl: $node->start;
- my $result = $node->safe_psql('postgres', "SHOW ssl_library");
- is($result, $ssl_server->ssl_library(), 'ssl_library parameter');
-
-+my $injection_points_unavailable = '';
-+if ($ENV{enable_injection_points} ne 'yes')
-+{
-+ $injection_points_unavailable =
-+ 'Injection points not supported by this build';
-+}
-+elsif (!$node->check_extension('injection_points'))
-+{
-+ $injection_points_unavailable =
-+ 'Extension injection_points not installed';
-+}
-+else
-+{
-+ # For ease of setup, make injection_points available for all new
databases.
-+ $node->safe_psql('template1', 'CREATE EXTENSION injection_points');
-+}
-+
- $ssl_server->configure_test_server_for_ssl($node, $SERVERHOSTADDR,
- $SERVERHOSTCIDR, 'trust');
-
-@@ src/test/ssl/t/001_ssltests.pl: command_like(
-
^\d+,t,TLSv[\d.]+,[\w-]+,\d+,_null_,_null_,_null_\r?$}mx,
- 'pg_stat_ssl view without client certificate');
-
-+# Test that pg_stat_ssl gets filled in early, prior to authentication.
Requires
-+# injection point support.
-+SKIP:
-+{
-+ skip $injection_points_unavailable, 1 if $injection_points_unavailable;
-+
-+ # Connect to the server and inject a waitpoint.
-+ my $psql =
-+ $node->background_psql('trustdb', connstr => "$common_connstr user=");
-+ $psql->query_safe(
-+ "SELECT injection_points_attach('init-pre-auth', 'wait')");
-+
-+ # From this point on, all new connections will hang during startup, just
-+ # before authentication. Use the $psql connection handle for server
-+ # interaction.
-+ my $conn = $node->background_psql(
-+ 'trustdb',
-+ connstr => $common_connstr,
-+ wait => 0);
-+
-+ # Wait for the connection to show up.
-+ my $pid;
-+ while (1)
-+ {
-+ $pid = $psql->query(
-+ "SELECT pid FROM pg_stat_activity WHERE state =
'starting' AND client_addr IS NOT NULL;");
-+ last if $pid ne "";
-+
-+ usleep(100_000);
-+ }
-+
-+ like(
-+ $psql->query(
-+ "SELECT ssl, version, cipher, bits FROM pg_stat_ssl
WHERE pid = $pid"
-+ ),
-+ qr/^t\|TLSv[\d.]+\|[\w-]+\|\d+$/,
-+ 'pg_stat_ssl view is updated prior to authentication');
-+
-+ # Detach the waitpoint and wait for the connection to complete.
-+ $psql->query_safe("SELECT injection_points_wakeup('init-pre-auth');");
-+ $conn->wait_connect();
-+
-+ $psql->query_safe("SELECT injection_points_detach('init-pre-auth');");
-+ $psql->quit();
-+ $conn->quit();
-+}
-+
- # Test min/max SSL protocol versions.
- $node->connect_ok(
- "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require
ssl_min_protocol_version=TLSv1.2 ssl_max_protocol_version=TLSv1.2",
2: e734e46009f = 2: fddaedf4280 Report external auth calls as wait events
3: 39c7d9ce42b = 3: 50d14d32699 squash! Report external auth calls as wait
events
v9-0001-pgstat-report-in-earlier-with-STATE_STARTING.patch
Description: Binary data
v9-0002-Report-external-auth-calls-as-wait-events.patch
Description: Binary data
v9-0003-squash-Report-external-auth-calls-as-wait-events.patch
Description: Binary data
