pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedlost status 'STATUS_EOF' for authentication when using 'MD5' or 'scram-sha-256'
2+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2024-01-03 c20f2aab6a 5810888c56 fb464a1ae3 a0d016393b 9b042e27eb aaf09c5923 Avoid masking EOF (no-password-supplied) conditions in auth.c.
* lost status 'STATUS_EOF' for authentication when using 'MD5' or 'scram-sha-256'
@ 2024-01-02 10:31 liulang <lang.liu@esgyn.cn>
0 siblings, 1 reply; 2+ messages in thread
From: liulang @ 2024-01-02 10:31 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
Hello:
I'm an extension writer and I wrote an extension to lock users who
tried to enter the wrong password too much. This extension is hooking
'ClientAuthentication_hook' and checking the hook's 'status' parameter
to count the times of wrong password. It’s work fine when auth is
'password', but get double count when auth is'MD5'or'scram-sha-256'.
problem reappearance:
1. *create an user without password*
2. set pg_hba.conf with ‘MD5’ or ‘scram-sha-256’
3. use psql without -W or -w or .pgpass file or env PGPASSFILE
4. It's ok when client is pgadmin or another
5. all of version pg can reappearance
when I read the source, find an incorret logical way on
CheckPWChallengeAuth at src/backend/libpq/auth.c
Like
static int
CheckPWChallengeAuth(Port *port, char **logdetail)
{
...
/* the CheckMD5Auth or CheckSCRAMAuth returns STATUS_EOF because
psql is not provide password,
* It will prompt user to type and send the password to other
backend process next time.
* The current backend will exit normally.
*/
if (port->hba->auth_method == uaMD5 && pwtype == PASSWORD_TYPE_MD5)
auth_result = CheckMD5Auth(port, shadow_pass, logdetail);
else
auth_result = CheckSCRAMAuth(port, shadow_pass, logdetail);
...
/*
* If get_role_password() returned error, return error, even if the
* authentication succeeded.
*/
/* The get_role_password returns NULL when the user without password */
if (!shadow_pass)
{
Assert(auth_result != STATUS_OK);
/* lost STATUS_EOF */
*return STATUS_ERROR;*
}
...
}
The above code does not affect the database execution,but
ClientAuthentication_hook will be confused whether the password is
incorrect or not currently entered?
so.. The CheckPWChallengeAuth should returns STATUS_EOF when It is, I think.
Try to change the code
static int
CheckPWChallengeAuth(Port *port, char **logdetail)
{
if (port->hba->auth_method == uaMD5 && pwtype == PASSWORD_TYPE_MD5)
auth_result = CheckMD5Auth(port, shadow_pass, logdetail);
else
auth_result = CheckSCRAMAuth(port, shadow_pass, logdetail);
...
if (STATUS_EOF == auth_result)
{
*/* do nothing */*
}
else if (!shadow_pass)
{
Assert(auth_result != STATUS_OK);
return STATUS_ERROR;
}
else if (auth_result == STATUS_OK)
set_authn_id(port, port->user_name);
return auth_result;
}
I don't know if this is right, please point out. thanks!
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: lost status 'STATUS_EOF' for authentication when using 'MD5' or 'scram-sha-256'
@ 2024-01-02 20:28 Tom Lane <tgl@sss.pgh.pa.us>
parent: liulang <lang.liu@esgyn.cn>
0 siblings, 0 replies; 2+ messages in thread
From: Tom Lane @ 2024-01-02 20:28 UTC (permalink / raw)
To: liulang <lang.liu@esgyn.cn>; +Cc: pgsql-bugs@lists.postgresql.org
liulang <lang.liu@esgyn.cn> writes:
> The above code does not affect the database execution,but
> ClientAuthentication_hook will be confused whether the password is
> incorrect or not currently entered?
> so.. The CheckPWChallengeAuth should returns STATUS_EOF when It is, I think.
Yeah, I think you are right. Overriding the subroutine's result
here is mistaken, even without considering whether it confuses any
ClientAuthentication_hook. The whole point, as per the comments,
is to not betray to the remote end whether or not there is a user
with a password set. But if we substitute STATUS_ERROR for
STATUS_EOF then we cause exactly that to happen: if the remote closes
the connection for send only, it can tell by whether an error comes
back whether or not the code found a password.
I think we can do it more simply than you suggest though. Just
drop the "return STATUS_ERROR" bit; the Assert is enough.
regards, tom lane
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-01-02 20:28 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-01-02 10:31 lost status 'STATUS_EOF' for authentication when using 'MD5' or 'scram-sha-256' liulang <lang.liu@esgyn.cn>
2024-01-02 20:28 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox