Allow SCRAM authentication, when pg_hba.conf says 'md5'.

If a user has a SCRAM verifier in pg_authid.rolpassword, there's no reason
we cannot attempt to perform SCRAM authentication instead of MD5. The worst
that can happen is that the client doesn't support SCRAM, and the
authentication will fail. But previously, it would fail for sure, because
we would not even try. SCRAM is strictly more secure than MD5, so there's
no harm in trying it. This allows for a more graceful transition from MD5
passwords to SCRAM, as user passwords can be changed to SCRAM verifiers
incrementally, without changing pg_hba.conf.

Refactor the code in auth.c to support that better. Notably, we now have to
look up the user's pg_authid entry before sending the password challenge,
also when performing MD5 authentication. Also simplify the concept of a
"doomed" authentication. Previously, if a user had a password, but it had
expired, we still performed SCRAM authentication (but always returned error
at the end) using the salt and iteration count from the expired password.
Now we construct a fake salt, like we do when the user doesn't have a
password or doesn't exist at all. That simplifies get_role_password(), and
we can don't need to distinguish the  "user has expired password", and
"user does not exist" cases in auth.c.

On second thoughts, also rename uaSASL to uaSCRAM. It refers to the
mechanism specified in pg_hba.conf, and while we use SASL for SCRAM
authentication at the protocol level, the mechanism should be called SCRAM,
not SASL. As a comparison, we have uaLDAP, even though it looks like the
plain 'password' authentication at the protocol level.

Discussion: https://www.postgresql.org/message-id/6425.1489506016@sss.pgh.pa.us
Reviewed-by: Michael Paquier

1 files changed, 16 insertions, 28 deletions

diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index ac10751ec20..34beab53342 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -31,25 +31,18 @@
 /*
  * Fetch stored password for a user, for authentication.
  *
- * Returns STATUS_OK on success.  On error, returns STATUS_ERROR, and stores
- * a palloc'd string describing the reason, for the postmaster log, in
- * *logdetail.  The error reason should *not* be sent to the client, to avoid
- * giving away user information!
- *
- * If the password is expired, it is still returned in *shadow_pass, but the
- * return code is STATUS_ERROR.  On other errors, *shadow_pass is set to
- * NULL.
+ * On error, returns NULL, and stores a palloc'd string describing the reason,
+ * for the postmaster log, in *logdetail.  The error reason should *not* be
+ * sent to the client, to avoid giving away user information!
  */
-int
-get_role_password(const char *role, char **shadow_pass, char **logdetail)
+char *
+get_role_password(const char *role, char **logdetail)
 {
-	int			retval = STATUS_ERROR;
 	TimestampTz vuntil = 0;
 	HeapTuple	roleTup;
 	Datum		datum;
 	bool		isnull;
-
-	*shadow_pass = NULL;
+	char	   *shadow_pass;
 
 	/* Get role info from pg_authid */
 	roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(role));
@@ -57,7 +50,7 @@ get_role_password(const char *role, char **shadow_pass, char **logdetail)
 	{
 		*logdetail = psprintf(_("Role \"%s\" does not exist."),
 							  role);
-		return STATUS_ERROR;	/* no such user */
+		return NULL;	/* no such user */
 	}
 
 	datum = SysCacheGetAttr(AUTHNAME, roleTup,
@@ -67,9 +60,9 @@ get_role_password(const char *role, char **shadow_pass, char **logdetail)
 		ReleaseSysCache(roleTup);
 		*logdetail = psprintf(_("User \"%s\" has no password assigned."),
 							  role);
-		return STATUS_ERROR;	/* user has no password */
+		return NULL;	/* user has no password */
 	}
-	*shadow_pass = TextDatumGetCString(datum);
+	shadow_pass = TextDatumGetCString(datum);
 
 	datum = SysCacheGetAttr(AUTHNAME, roleTup,
 							Anum_pg_authid_rolvaliduntil, &isnull);
@@ -78,30 +71,25 @@ get_role_password(const char *role, char **shadow_pass, char **logdetail)
 
 	ReleaseSysCache(roleTup);
 
-	if (**shadow_pass == '\0')
+	if (*shadow_pass == '\0')
 	{
 		*logdetail = psprintf(_("User \"%s\" has an empty password."),
 							  role);
-		pfree(*shadow_pass);
-		*shadow_pass = NULL;
-		return STATUS_ERROR;	/* empty password */
+		pfree(shadow_pass);
+		return NULL;	/* empty password */
 	}
 
 	/*
-	 * Password OK, now check to be sure we are not past rolvaliduntil
+	 * Password OK, but check to be sure we are not past rolvaliduntil
 	 */
-	if (isnull)
-		retval = STATUS_OK;
-	else if (vuntil < GetCurrentTimestamp())
+	if (!isnull && vuntil < GetCurrentTimestamp())
 	{
 		*logdetail = psprintf(_("User \"%s\" has an expired password."),
 							  role);
-		retval = STATUS_ERROR;
+		return NULL;
 	}
-	else
-		retval = STATUS_OK;
 
-	return retval;
+	return shadow_pass;
 }
 
 /*