Skip to content

Commit

Permalink
Fix CheckPWChallengeAuth() to restore old beheviour in absence of use…
Browse files Browse the repository at this point in the history
…r, no passwords, or expired passwords.
  • Loading branch information
gurjeet committed Oct 9, 2023
1 parent 0e40cbf commit aba99df
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
5 changes: 2 additions & 3 deletions src/backend/commands/user.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/user.h"
#include "common/cryptohash.h"
#include "common/scram-common.h"
#include "libpq/crypt.h"
#include "libpq/scram.h"
Expand Down Expand Up @@ -132,7 +131,7 @@ have_createrole_privilege(void)
* Inspect the current passwords of a role, and return the salt that can be used
* for hashing of newer passwords.
*
* Returns false on error, and true otherwise. On error the reason is stored in
* Returns success on error, and false otherwise. On error the reason is stored in
* logdetail. On success, salt may be null which indicates that the caller is
* free to generate a new salt.
*/
Expand All @@ -158,7 +157,7 @@ get_salt(char *rolename, char **salt, const char **logdetail)
current_secrets = get_role_passwords(rolename, logdetail, &num_secrets);
if (num_secrets == 0)
{
*salt = NULL; /* No existing passwords, allow one to be generated */
*salt = NULL; /* No existing passwords, allow salt to be generated */
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/backend/libpq/auth-scram.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ scram_get_mechanisms(Port *port, StringInfo buf)
* It should be one of the mechanisms that we support, as returned by
* scram_get_mechanisms().
*
* 'passwords' are the role's stored secrets, from pg_auth_password.password.
* 'passwords' are the role's stored secrets, from pg_authid.rolpassword.
* The username was provided by the client in the startup message, and is
* available in port->user_name. If 'shadow_pass' is NULL, we still perform
* an authentication exchange, but it will fail, as if an incorrect password
Expand Down Expand Up @@ -492,7 +492,7 @@ scram_exchange(void *opaq, const char *input, int inputlen,
}

/*
* Construct a SCRAM secret, for storing in pg_auth_password.password.
* Construct a SCRAM secret, for storing in pg_authid.rolpassword.
*
* The result is palloc'd, so caller is responsible for freeing it.
*/
Expand Down
37 changes: 35 additions & 2 deletions src/backend/libpq/auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -831,31 +831,54 @@ CheckPWChallengeAuth(Port *port, const char **logdetail)
int auth_result = STATUS_ERROR;
int i, num_passwords;
char **passwords;
PasswordType pwtype;

Assert(port->hba->auth_method == uaSCRAM ||
port->hba->auth_method == uaMD5);

/* First look up the user's passwords. */
passwords = get_role_passwords(port->user_name, logdetail, &num_passwords);

/*
* If the user does not exist, or has no passwords or they're all expired,
* we still go through the motions of authentication, to avoid revealing to
* the client that the user didn't exist. If 'md5' is allowed, we choose
* whether to use 'md5' or 'scram-sha-256' authentication based on current
* password_encryption setting. The idea is that most genuine users
* probably have a password of that type, and if we pretend that this user
* had a password of that type, too, it "blends in" best.
*/
if (!passwords)
pwtype = Password_encryption;

/*
* If 'md5' authentication is allowed, decide whether to perform 'md5' or
* 'scram-sha-256' authentication based on the type of password the user
* has. If there's a SCRAM PW available then we'll do SCRAM, otherwise we
* has. If there's a SCRAM password available then we'll do SCRAM, otherwise we
* will fall back to trying to use MD5.
*
* If MD5 authentication is not allowed, always use SCRAM. If the user
* had an MD5 password, CheckSASLAuth() with the SCRAM mechanism will
* fail.
*/
if (passwords != NULL)
if (passwords == NULL)
{
if (port->hba->auth_method == uaMD5 && pwtype == PASSWORD_TYPE_MD5)
auth_result = CheckMD5Auth(port, (const char **) NULL, 0, logdetail);
else
auth_result = CheckSASLAuth(&pg_be_scram_mech, port,
(const char **) NULL, 0, logdetail);
}
else
{
for (i = 0; i < num_passwords; i++)
{
if (get_password_type(passwords[i]) == PASSWORD_TYPE_SCRAM_SHA_256)
{
scram_pw_avail = true;
break;
}
}

if (port->hba->auth_method == uaMD5 && !scram_pw_avail)
auth_result = CheckMD5Auth(port, (const char **) passwords, num_passwords, logdetail);
Expand All @@ -874,6 +897,16 @@ CheckPWChallengeAuth(Port *port, const char **logdetail)
pfree(passwords);
}

/*
* If get_role_passwords() returned error, return error, even if the
* authentication succeeded.
*/
if (!passwords)
{
Assert(auth_result != STATUS_OK);
return STATUS_ERROR;
}

if (auth_result == STATUS_OK)
set_authn_id(port, port->user_name);

Expand Down

0 comments on commit aba99df

Please sign in to comment.