public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: Jingxian Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Michael Paquier <[email protected]>
Subject: Re: [PATCH] Fix bug when calling strncmp in check_authmethod_valid
Date: Mon, 13 May 2024 10:34:48 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAMbWs4-mWCGbbE_bne5=AfqjYGDaUZmjCw2+soLjrdNA0xUDFw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAMbWs48uTLZT14aaBaEg7cpB6ercU5wm9+WfGLGC3VoP28gfCA@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAMbWs48adH1Am=tszAqyq3UtEhDCsFL22U8Jzq0EpPAA6tOd9A@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
> On 7 May 2024, at 06:46, Jingxian Li <[email protected]> wrote:
>> But, since it's been broken in all supported versions of postgres and has
>> AFAICT never been documented to exist, should we fix it or just remove it? We
>> don't support auth-options for any other methods, like clientcert to cert for
>> example. If we fix it we should also document that it works IMHO.
>
> You mentioned that auth-options are not supported for auth methods except pam,
> but I found that some methods (such as ldap and radius etc.) also requires aut-options,
> and there are no corresponding auth methods ending with space (such as "ldap " and
> radius ") present in auth_methods_host and auth_methods_local arrays.
Correct, only pam and ident were ever supported (yet not documented) and ident
was removed a long time ago.
Searching the archives I was unable to find any complaints, and this has been
broken for the entire window of supported releases, so I propose we remove it
as per the attached patch. If anyone is keen on making this work again for all
the types where it makes sense, it can be resurrected (probably with a better
implementation).
Any objections to fixing this in 17 by removing it? (cc:ing Michael from the RMT)
--
Daniel Gustafsson
Attachments:
[application/octet-stream] v2-0001-Remove-auth-options-support-from-initdb.patch (2.1K, ../[email protected]/2-v2-0001-Remove-auth-options-support-from-initdb.patch)
download | inline diff:
From 17cc0811b7eebd93f8e063a5b025180c95badf56 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Mon, 13 May 2024 09:55:48 +0200
Subject: [PATCH v2] Remove auth-options support from initdb
When --auth was added to initdb in commit e7029b212755 it had support
for auth options separated by space from the auth type, like:
--auth pam <servicename>
--auth ident sameuser
Passing an option to the ident auth type was removed in 01c1a12a5bb4
which left the pam auth-options support in place. 8a02339e9ba3 broke
this by inverting a calculation in the strncmp arguments, which went
unnoticed for a long time. The ability to pass options to the auth
type was never documented.
Rather than fixing the support for an undocumented feature which has
been broken for all supported versions, and which only supports one
out of many auth types which can take options, it is removed.
Reported-by: Jingxian Li <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/bin/initdb/initdb.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 30e17bd1d1..5e89b3c8e8 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -102,7 +102,7 @@ static const char *const auth_methods_host[] = {
"sspi",
#endif
#ifdef USE_PAM
- "pam", "pam ",
+ "pam",
#endif
#ifdef USE_BSD_AUTH
"bsd",
@@ -118,7 +118,7 @@ static const char *const auth_methods_host[] = {
static const char *const auth_methods_local[] = {
"trust", "reject", "scram-sha-256", "md5", "password", "peer", "radius",
#ifdef USE_PAM
- "pam", "pam ",
+ "pam",
#endif
#ifdef USE_BSD_AUTH
"bsd",
@@ -2512,10 +2512,6 @@ check_authmethod_valid(const char *authmethod, const char *const *valid_methods,
{
if (strcmp(authmethod, *p) == 0)
return;
- /* with space = param */
- if (strchr(authmethod, ' '))
- if (strncmp(authmethod, *p, (authmethod - strchr(authmethod, ' '))) == 0)
- return;
}
pg_fatal("invalid authentication method \"%s\" for \"%s\" connections",
--
2.39.3 (Apple Git-146)
view thread (21+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Fix bug when calling strncmp in check_authmethod_valid
In-Reply-To: <[email protected]>
* 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