agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Grigorev Jurij <ju.grigorev@ftdata.ru>
To: Daniel Gustafsson <daniel@yesql.se>
To: 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: Tue, 15 Sep 2026 02:55:55 +0000
Message-ID: <c7e77341b94e406bb77e8cd3367cae59@localhost.localdomain> (raw)
In-Reply-To: <A1C4C529-9ACC-46AF-8AA8-AD7670FE277B@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>
Hi Jacob, Daniel,
Thanks for the review! In the attached v2 I removed the explanatory
comment and call query_safe() directly. The functional change and the
reload test are otherwise unchanged.
Also left the validator-name canonicalization issue out of this patch.
Checked, the patch builds cleanly against master with --enable-cassert,
and PG_TEST_EXTRA=oauth make check in src/test/modules/oauth_validator
passes all 190 tests.
Regards,
Yuriy
________________________________________
От: Daniel Gustafsson <daniel@yesql.se>
Отправлено: 15 сентября 2026 г. 4:34:08
Кому: Jacob Champion
Копия: Григорьев Юрий; pgsql-bugs@lists.postgresql.org
Тема: Re: Postmaster crashes on SIGHUP when oauth_validator_libraries holds only whitespace
> On 14 Sep 2026, at 23:28, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>> + /*
>> + * An empty or all-whitespace setting is accepted by
>> + * SplitDirectoriesString(), which returns an empty list for it, so the
>> + * parse result has to be checked rather than the raw string.
>> + */
>
> Sometimes recording a historical bug in the comments can help prevent
> future mistakes... but I don't think this is one of those cases,
> especially since the new test prevents accidental regression.
+1
> This seems like a good time to mention that I have a checklist item to
> fix the following (shouldn't block this patch):
>
>> if (!SplitDirectoriesString(rawstring, ',', &elemlist))
>> ...
>> if (strcmp(allowed, hbaline->oauth_validator) == 0)
>
> SplitDirectoriesString() canonicalizes its outputs, which we then
> compare against the uncanonicalized hbaline->oauth_validator. That
> could lead to annoying false negatives in more complicated setups.
Right, this patch wont move the needle in the wrong direction for future fixes
AFAICT.
--
Daniel Gustafsson
Attachments:
[application/octet-stream] v2-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch (3.6K, ../c7e77341b94e406bb77e8cd3367cae59@localhost.localdomain/2-v2-0001-Fix-postmaster-crash-on-whitespace-only-oauth_valida.patch)
download | inline diff:
From c4144dc1a37ea933a2985b9340b79b9232122d8a Mon Sep 17 00:00:00 2001
From: Yuriy Grigoryev <ju.grigorev@ftdata.ru>
Date: Tue, 15 Sep 2026 09:34:36 +0700
Subject: [PATCH v2] 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 and add a TAP test that reloads an invalid
whitespace-only setting and verifies that the server remains available.
---
src/backend/libpq/auth-oauth.c | 28 +++++++++----------
.../modules/oauth_validator/t/001_server.pl | 16 +++++++++++
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index b769931ca4f..90d223eed00 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -863,19 +863,6 @@ 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 */
rawstring = pstrdup(oauth_validator_libraries_string);
@@ -891,9 +878,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..65f73cbdbb2 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -123,6 +123,22 @@ $node->reload;
$log_start =
$node->wait_for_log(qr/reloading configuration files/, $log_start);
+# 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/reloading configuration files/,
+ $log_start);
+
# Check pg_hba_file_rules() support.
my $contents = $bgconn->query_safe(
qq(SELECT rule_number, auth_method, options
--
2.39.5 (Apple Git-154)
view thread (10+ messages) latest in thread
Message-ID: <c7e77341b94e406bb77e8cd3367cae59@localhost.localdomain>
Permalink: ../c7e77341b94e406bb77e8cd3367cae59@localhost.localdomain/
Also on: postgresql.org/message-id/c7e77341b94e406bb77e8cd3367cae59@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: <c7e77341b94e406bb77e8cd3367cae59@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