agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Grigorev Jurij <ju.grigorev@ftdata.ru>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
Date: Wed, 16 Sep 2026 08:27:56 +0000
Message-ID: <0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain> (raw)
In-Reply-To: <75204002-3933-4034-8959-DADD0593A800@yesql.se>
References: <04fa84f6ebbe400f940e179ebe1070e9@localhost.localdomain>
<19B73F3F-8FAE-43B6-8746-21FE466026B8@yesql.se>
<CAOYmi+=JYJaAuPAcHYNptOQJaLHM5i6ex_wH0aS4QP9ngukX2A@mail.gmail.com>
<A1C4C529-9ACC-46AF-8AA8-AD7670FE277B@yesql.se>
<c7e77341b94e406bb77e8cd3367cae59@localhost.localdomain>
<75204002-3933-4034-8959-DADD0593A800@yesql.se>
Hi Daniel,
You're right. I would rather not treat NULL as an empty setting, since
that would turn an internal programming error into the user-facing
"must be set" error. Attached v3 adds:
Assert(oauth_validator_libraries_string != NULL);
before the pstrdup().
I also moved the whitespace-only TAP case to after the
pg_hba_file_rules() check. Its previous position made the test racy:
wait_for_log() synchronized with the postmaster reload, but not
necessarily with the process-local GUC state of the existing bgconn.
As a result, pg_hba_file_rules() could run in a backend that still had
the whitespace value loaded and return unexpected empty fields.
Moving the case after that assertion avoids making the
pg_hba_file_rules() result depend on the timing of SIGHUP processing in
bgconn. After restoring the setting, the test runs SHOW
oauth_validator_libraries through bgconn, ensuring that the backend has
processed the second SIGHUP before the later tests continue.
Thanks,
Yuriy=
Attachments:
[application/octet-stream] v3-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch (4.1K, ../0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain/2-v3-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch)
download | inline diff:
From a52d00740dd22db1203dc7377a75fbead9133981 Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Wed, 16 Sep 2026 14:56:16 +0700
Subject: [PATCH v3] Fix postmaster crash on whitespace-only
oauth_validator_libraries
check_oauth_validator() checks the raw GUC string for an empty validator
list. That does not cover a value containing only whitespace.
SplitDirectoriesString() accepts such input and returns an empty list, so
the code dereferences NIL when an OAuth HBA line has no validator option.
This can crash the postmaster while processing SIGHUP.
Check the parsed list instead. Assert that the GUC string is non-NULL
before pstrdup(); users cannot set it to NULL, but the C variable is
initialized that way. Add a TAP test that reloads an invalid
whitespace-only setting after pg_hba_file_rules() and waits until the
existing backend sees the restored GUC.
---
src/backend/libpq/auth-oauth.c | 29 ++++++++++---------
.../modules/oauth_validator/t/001_server.pl | 20 +++++++++++++
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index b769931ca4f..c01e8ae3524 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -863,20 +863,8 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
*err_msg = NULL;
- if (oauth_validator_libraries_string[0] == '\0')
- {
- ereport(elevel,
- errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth"),
- errcontext("line %d of configuration file \"%s\"",
- line_num, file_name));
- *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
- "oauth_validator_libraries", "oauth");
- return false;
- }
-
/* SplitDirectoriesString needs a modifiable copy */
+ Assert(oauth_validator_libraries_string != NULL);
rawstring = pstrdup(oauth_validator_libraries_string);
if (!SplitDirectoriesString(rawstring, ',', &elemlist))
@@ -891,9 +879,22 @@ check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg)
goto done;
}
+ if (elemlist == NIL)
+ {
+ ereport(elevel,
+ errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, file_name));
+ *err_msg = psprintf("parameter \"%s\" must be set for authentication method \"%s\"",
+ "oauth_validator_libraries", "oauth");
+ goto done;
+ }
+
if (!hbaline->oauth_validator)
{
- if (elemlist->length == 1)
+ if (list_length(elemlist) == 1)
{
hbaline->oauth_validator = pstrdup(linitial(elemlist));
goto done;
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 8941a355423..4ceb775fe20 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -134,6 +134,26 @@ is( $contents,
3|oauth|\{issuer=$issuer/param,"scope=openid postgres",validator=validator\}},
"pg_hba_file_rules recreates OAuth HBA settings");
+# An all-whitespace library list parses as an empty list. Reject it without
+# crashing the postmaster during HBA reload.
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = ' '\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+ qr/parameter "oauth_validator_libraries" must be set for authentication/,
+ $log_start);
+$bgconn->query_safe('SELECT 1');
+
+$node->append_conf('postgresql.conf',
+ "oauth_validator_libraries = 'validator'\n");
+$node->reload;
+$log_start = $node->wait_for_log(
+ qr/parameter "oauth_validator_libraries" changed to "validator"/,
+ $log_start);
+is( $bgconn->query_safe('SHOW oauth_validator_libraries'),
+ 'validator',
+ 'oauth_validator_libraries restored');
+
{
# Make sure PGOAUTHDEBUG=UNSAFE doesn't disable certificate verification.
local $ENV{PGOAUTHDEBUG} = "UNSAFE";
--
2.54.0 (Apple Git-157)
view thread (12+ messages) latest in thread
Message-ID: <0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain>
Permalink: ../0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain/
Also on: postgresql.org/message-id/0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: ju.grigorev@ftdata.ru, daniel@yesql.se, jacob.champion@enterprisedb.com, pgsql-bugs@lists.postgresql.org
Subject: Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
In-Reply-To: <0bc0874d0b774514a61d8d7b30825e0f@localhost.localdomain>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox