Clean up password authentication code a bit.

Commit fe0a0b5, which moved code to do MD5 authentication to a separate
CheckMD5Auth() function, left behind a comment that really belongs inside
the function, too. Also move the check for db_user_namespace inside the
function, seems clearer that way.

Now that the md5 salt is passed as argument to md5_crypt_verify, it's a bit
silly that it peeks into the Port struct to see if MD5 authentication was
used. Seems more straightforward to treat it as an MD5 authentication, if
the md5 salt argument is given. And after that, md5_crypt_verify only used
the Port argument to look at port->user_name, but that is redundant,
because it is also passed as a separate 'role' argument. So remove the Port
argument altogether.

Author: hlinnaka

src/backend/libpq/auth.c

@@ -50,14 +50,14 @@ static char *recv_password_packet(Port *port);
  * MD5 authentication
  *----------------------------------------------------------------
  */
-static int CheckMD5Auth(Port *port, char **logdetail);
+static int	CheckMD5Auth(Port *port, char **logdetail);
 
 /*----------------------------------------------------------------
  * Plaintext password authentication
  *----------------------------------------------------------------
  */
 
-static int CheckPasswordAuth(Port *port, char **logdetail);
+static int	CheckPasswordAuth(Port *port, char **logdetail);
 
 /*----------------------------------------------------------------
  * Ident authentication
@@ -544,11 +544,6 @@ ClientAuthentication(Port *port)
 			break;
 
 		case uaMD5:
-			if (Db_user_namespace)
-				ereport(FATAL,
-						(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						 errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled")));
-			/* include the salt to use for computing the response */
 			status = CheckMD5Auth(port, &logdetail);
 			break;
 
@@ -714,6 +709,12 @@ CheckMD5Auth(Port *port, char **logdetail)
 	char	   *passwd;
 	int			result;
 
+	if (Db_user_namespace)
+		ereport(FATAL,
+				(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+				 errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled")));
+
+	/* include the salt to use for computing the response */
 	pg_backend_random(md5Salt, 4);
 
 	sendAuthRequest(port, AUTH_REQ_MD5, md5Salt, 4);
@@ -723,7 +724,7 @@ CheckMD5Auth(Port *port, char **logdetail)
 	if (passwd == NULL)
 		return STATUS_EOF;		/* client wouldn't send password */
 
-	result = md5_crypt_verify(port, port->user_name, passwd, md5Salt, 4, logdetail);
+	result = md5_crypt_verify(port->user_name, passwd, md5Salt, 4, logdetail);
 
 	pfree(passwd);
 
@@ -748,7 +749,7 @@ CheckPasswordAuth(Port *port, char **logdetail)
 	if (passwd == NULL)
 		return STATUS_EOF;		/* client wouldn't send password */
 
-	result = md5_crypt_verify(port, port->user_name, passwd, NULL, 0, logdetail);
+	result = md5_crypt_verify(port->user_name, passwd, NULL, 0, logdetail);
 
 	pfree(passwd);
 

src/backend/libpq/crypt.c

@@ -31,11 +31,16 @@
 
 /*
  * Check given password for given user, and return STATUS_OK or STATUS_ERROR.
+ *
+ * 'client_pass' is the password response given by the remote user.  If
+ * 'md5_salt' is not NULL, it is a response to an MD5 authentication
+ * challenge, with the given salt.  Otherwise, it is a plaintext password.
+ *
  * In the error case, optionally store a palloc'd string at *logdetail
  * that will be sent to the postmaster log (but not the client).
  */
 int
-md5_crypt_verify(const Port *port, const char *role, char *client_pass,
+md5_crypt_verify(const char *role, char *client_pass,
 				 char *md5_salt, int md5_salt_len, char **logdetail)
 {
 	int			retval = STATUS_ERROR;
@@ -88,63 +93,64 @@ md5_crypt_verify(const Port *port, const char *role, char *client_pass,
 	 * error is out-of-memory, which is unlikely, and if it did happen adding
 	 * a psprintf call would only make things worse.)
 	 */
-	switch (port->hba->auth_method)
+	if (md5_salt)
 	{
-		case uaMD5:
-			Assert(md5_salt != NULL && md5_salt_len > 0);
-			crypt_pwd = palloc(MD5_PASSWD_LEN + 1);
-			if (isMD5(shadow_pass))
+		/* MD5 authentication */
+		Assert(md5_salt_len > 0);
+		crypt_pwd = palloc(MD5_PASSWD_LEN + 1);
+		if (isMD5(shadow_pass))
+		{
+			/* stored password already encrypted, only do salt */
+			if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
+								md5_salt, md5_salt_len,
+								crypt_pwd))
 			{
-				/* stored password already encrypted, only do salt */
-				if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
-									md5_salt, md5_salt_len,
-									crypt_pwd))
-				{
-					pfree(crypt_pwd);
-					return STATUS_ERROR;
-				}
+				pfree(crypt_pwd);
+				return STATUS_ERROR;
 			}
-			else
-			{
-				/* stored password is plain, double-encrypt */
-				char	   *crypt_pwd2 = palloc(MD5_PASSWD_LEN + 1);
+		}
+		else
+		{
+			/* stored password is plain, double-encrypt */
+			char	   *crypt_pwd2 = palloc(MD5_PASSWD_LEN + 1);
 
-				if (!pg_md5_encrypt(shadow_pass,
-									port->user_name,
-									strlen(port->user_name),
-									crypt_pwd2))
-				{
-					pfree(crypt_pwd);
-					pfree(crypt_pwd2);
-					return STATUS_ERROR;
-				}
-				if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),
-									md5_salt, md5_salt_len,
-									crypt_pwd))
-				{
-					pfree(crypt_pwd);
-					pfree(crypt_pwd2);
-					return STATUS_ERROR;
-				}
+			if (!pg_md5_encrypt(shadow_pass,
+								role,
+								strlen(role),
+								crypt_pwd2))
+			{
+				pfree(crypt_pwd);
 				pfree(crypt_pwd2);
+				return STATUS_ERROR;
 			}
-			break;
-		default:
-			if (isMD5(shadow_pass))
+			if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),
+								md5_salt, md5_salt_len,
+								crypt_pwd))
 			{
-				/* Encrypt user-supplied password to match stored MD5 */
-				crypt_client_pass = palloc(MD5_PASSWD_LEN + 1);
-				if (!pg_md5_encrypt(client_pass,
-									port->user_name,
-									strlen(port->user_name),
-									crypt_client_pass))
-				{
-					pfree(crypt_client_pass);
-					return STATUS_ERROR;
-				}
+				pfree(crypt_pwd);
+				pfree(crypt_pwd2);
+				return STATUS_ERROR;
 			}
-			crypt_pwd = shadow_pass;
-			break;
+			pfree(crypt_pwd2);
+		}
+	}
+	else
+	{
+		/* Client sent password in plaintext */
+		if (isMD5(shadow_pass))
+		{
+			/* Encrypt user-supplied password to match stored MD5 */
+			crypt_client_pass = palloc(MD5_PASSWD_LEN + 1);
+			if (!pg_md5_encrypt(client_pass,
+								role,
+								strlen(role),
+								crypt_client_pass))
+			{
+				pfree(crypt_client_pass);
+				return STATUS_ERROR;
+			}
+		}
+		crypt_pwd = shadow_pass;
 	}
 
 	if (strcmp(crypt_client_pass, crypt_pwd) == 0)
@@ -167,7 +173,7 @@ md5_crypt_verify(const Port *port, const char *role, char *client_pass,
 		*logdetail = psprintf(_("Password does not match for user \"%s\"."),
 							  role);
 
-	if (port->hba->auth_method == uaMD5)
+	if (crypt_pwd != shadow_pass)
 		pfree(crypt_pwd);
 	if (crypt_client_pass != client_pass)
 		pfree(crypt_client_pass);

src/include/libpq/crypt.h

@@ -13,9 +13,9 @@
 #ifndef PG_CRYPT_H
 #define PG_CRYPT_H
 
-#include "libpq/libpq-be.h"
+#include "datatype/timestamp.h"
 
-extern int md5_crypt_verify(const Port *port, const char *role,
-				 char *client_pass, char *md5_salt, int md5_salt_len, char **logdetail);
+extern int md5_crypt_verify(const char *role, char *client_pass,
+				 char *md5_salt, int md5_salt_len, char **logdetail);
 
 #endif