Remove support for password_encryption='off' / 'plain'.

Storing passwords in plaintext hasn't been a good idea for a very long
time, if ever. Now seems like a good time to finally forbid it, since we're
messing with this in PostgreSQL 10 anyway.

Remove the CREATE/ALTER USER UNENCRYPTED PASSSWORD 'foo' syntax, since
storing passwords unencrypted is no longer supported. ENCRYPTED PASSWORD
'foo' is still accepted, but ENCRYPTED is now just a noise-word, it does
the same as just PASSWORD 'foo'.

Likewise, remove the --unencrypted option from createuser, but accept
--encrypted as a no-op for backward compatibility. AFAICS, --encrypted was
a no-op even before this patch, because createuser encrypted the password
before sending it to the server even if --encrypted was not specified. It
added the ENCRYPTED keyword to the SQL command, but since the password was
already in encrypted form, it didn't make any difference. The documentation
was not clear on whether that was intended or not, but it's moot now.

Also, while password_encryption='on' is still accepted as an alias for
'md5', it is now marked as hidden, so that it is not listed as an accepted
value in error hints, for example. That's not directly related to removing
'plain', but it seems better this way.

Reviewed by Michael Paquier

Discussion: https://www.postgresql.org/message-id/16e9b768-fd78-0b12-cfc1-7b6b7f238fde@iki.fi

6 files changed, 68 insertions, 162 deletions

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index c719682274d..36d5f40f062 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -80,7 +80,6 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	ListCell   *item;
 	ListCell   *option;
 	char	   *password = NULL;	/* user password */
-	int			password_type = Password_encryption;
 	bool		issuper = false;	/* Make the user a superuser? */
 	bool		inherit = true; /* Auto inherit privileges? */
 	bool		createrole = false;		/* Can this user create roles? */
@@ -128,9 +127,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 	{
 		DefElem    *defel = (DefElem *) lfirst(option);
 
-		if (strcmp(defel->defname, "password") == 0 ||
-			strcmp(defel->defname, "encryptedPassword") == 0 ||
-			strcmp(defel->defname, "unencryptedPassword") == 0)
+		if (strcmp(defel->defname, "password") == 0)
 		{
 			if (dpassword)
 				ereport(ERROR,
@@ -138,15 +135,6 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 						 errmsg("conflicting or redundant options"),
 						 parser_errposition(pstate, defel->location)));
 			dpassword = defel;
-			if (strcmp(defel->defname, "encryptedPassword") == 0)
-			{
-				if (Password_encryption == PASSWORD_TYPE_SCRAM_SHA_256)
-					password_type = PASSWORD_TYPE_SCRAM_SHA_256;
-				else
-					password_type = PASSWORD_TYPE_MD5;
-			}
-			else if (strcmp(defel->defname, "unencryptedPassword") == 0)
-				password_type = PASSWORD_TYPE_PLAINTEXT;
 		}
 		else if (strcmp(defel->defname, "sysid") == 0)
 		{
@@ -400,7 +388,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 		/* Encrypt the password to the requested format. */
 		char	   *shadow_pass;
 
-		shadow_pass = encrypt_password(password_type, stmt->role, password);
+		shadow_pass = encrypt_password(Password_encryption, stmt->role,
+									   password);
 		new_record[Anum_pg_authid_rolpassword - 1] =
 			CStringGetTextDatum(shadow_pass);
 	}
@@ -503,7 +492,6 @@ AlterRole(AlterRoleStmt *stmt)
 	ListCell   *option;
 	char	   *rolename = NULL;
 	char	   *password = NULL;	/* user password */
-	int			password_type = Password_encryption;
 	int			issuper = -1;	/* Make the user a superuser? */
 	int			inherit = -1;	/* Auto inherit privileges? */
 	int			createrole = -1;	/* Can this user create roles? */
@@ -537,24 +525,13 @@ AlterRole(AlterRoleStmt *stmt)
 	{
 		DefElem    *defel = (DefElem *) lfirst(option);
 
-		if (strcmp(defel->defname, "password") == 0 ||
-			strcmp(defel->defname, "encryptedPassword") == 0 ||
-			strcmp(defel->defname, "unencryptedPassword") == 0)
+		if (strcmp(defel->defname, "password") == 0)
 		{
 			if (dpassword)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("conflicting or redundant options")));
 			dpassword = defel;
-			if (strcmp(defel->defname, "encryptedPassword") == 0)
-			{
-				if (Password_encryption == PASSWORD_TYPE_SCRAM_SHA_256)
-					password_type = PASSWORD_TYPE_SCRAM_SHA_256;
-				else
-					password_type = PASSWORD_TYPE_MD5;
-			}
-			else if (strcmp(defel->defname, "unencryptedPassword") == 0)
-				password_type = PASSWORD_TYPE_PLAINTEXT;
 		}
 		else if (strcmp(defel->defname, "superuser") == 0)
 		{
@@ -809,7 +786,8 @@ AlterRole(AlterRoleStmt *stmt)
 		/* Encrypt the password to the requested format. */
 		char	   *shadow_pass;
 
-		shadow_pass = encrypt_password(password_type, rolename, password);
+		shadow_pass = encrypt_password(Password_encryption, rolename,
+									   password);
 		new_record[Anum_pg_authid_rolpassword - 1] =
 			CStringGetTextDatum(shadow_pass);
 		new_record_repl[Anum_pg_authid_rolpassword - 1] = true;
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 3acc2acfe41..99feb0ce947 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -199,27 +199,11 @@ pg_be_scram_init(const char *username, const char *shadow_pass)
 				got_verifier = false;
 			}
 		}
-		else if (password_type == PASSWORD_TYPE_PLAINTEXT)
-		{
-			/*
-			 * The stored password is in plain format.  Generate a fresh SCRAM
-			 * verifier from it, and proceed with that.
-			 */
-			char	   *verifier;
-
-			verifier = pg_be_scram_build_verifier(shadow_pass);
-
-			(void) parse_scram_verifier(verifier, &state->iterations, &state->salt,
-										state->StoredKey, state->ServerKey);
-			pfree(verifier);
-
-			got_verifier = true;
-		}
 		else
 		{
 			/*
-			 * The user doesn't have SCRAM verifier, nor could we generate
-			 * one. (You cannot do SCRAM authentication with an MD5 hash.)
+			 * The user doesn't have SCRAM verifier. (You cannot do SCRAM
+			 * authentication with an MD5 hash.)
 			 */
 			state->logdetail = psprintf(_("User \"%s\" does not have a valid SCRAM verifier."),
 										state->username);
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index ab4be219431..6d3ff68607d 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -754,17 +754,13 @@ CheckPWChallengeAuth(Port *port, char **logdetail)
 	shadow_pass = get_role_password(port->user_name, logdetail);
 
 	/*
-	 * If the user does not exist, or has no password, 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, if we pretend that this user had a password of that type,
-	 * too, it "blends in" best.
-	 *
-	 * If the user had a password, but it was expired, we'll use the details
-	 * of the expired password for the authentication, but report it as
-	 * failure to the client even if correct password was given.
+	 * If the user does not exist, or has no password or it's 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 (!shadow_pass)
 		pwtype = Password_encryption;
@@ -775,21 +771,15 @@ CheckPWChallengeAuth(Port *port, char **logdetail)
 	 * 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 it's an MD5 hash, we must do MD5 authentication, and if it's
-	 * a SCRAM verifier, we must do SCRAM authentication.  If it's stored in
-	 * plaintext, we could do either one, so we opt for the more secure
-	 * mechanism, SCRAM.
+	 * a SCRAM verifier, we must do SCRAM authentication.
 	 *
 	 * If MD5 authentication is not allowed, always use SCRAM.  If the user
 	 * had an MD5 password, CheckSCRAMAuth() will fail.
 	 */
 	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 (shadow_pass)
 		pfree(shadow_pass);
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 9fe79b48946..e7a6b04fb5a 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -109,9 +109,8 @@ get_password_type(const char *shadow_pass)
  * Given a user-supplied password, convert it into a verifier of
  * 'target_type' kind.
  *
- * If the password looks like a valid MD5 hash, it is stored as it is.
- * We cannot reverse the hash, so even if the caller requested a plaintext
- * plaintext password, the MD5 hash is returned.
+ * If the password is already in encrypted form, we cannot reverse the
+ * hash, so it is stored as it is regardless of the requested type.
  */
 char *
 encrypt_password(PasswordType target_type, const char *role,
@@ -120,54 +119,30 @@ encrypt_password(PasswordType target_type, const char *role,
 	PasswordType guessed_type = get_password_type(password);
 	char	   *encrypted_password;
 
-	switch (target_type)
+	if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
 	{
-		case PASSWORD_TYPE_PLAINTEXT:
-
-			/*
-			 * We cannot convert a hashed password back to plaintext, so just
-			 * store the password as it was, whether it was hashed or not.
-			 */
-			return pstrdup(password);
+		/*
+		 * Cannot convert an already-encrypted password from one
+		 * format to another, so return it as it is.
+		 */
+		return pstrdup(password);
+	}
 
+	switch (target_type)
+	{
 		case PASSWORD_TYPE_MD5:
-			switch (guessed_type)
-			{
-				case PASSWORD_TYPE_PLAINTEXT:
-					encrypted_password = palloc(MD5_PASSWD_LEN + 1);
-
-					if (!pg_md5_encrypt(password, role, strlen(role),
-										encrypted_password))
-						elog(ERROR, "password encryption failed");
-					return encrypted_password;
+			encrypted_password = palloc(MD5_PASSWD_LEN + 1);
 
-				case PASSWORD_TYPE_SCRAM_SHA_256:
-
-					/*
-					 * cannot convert a SCRAM verifier to an MD5 hash, so fall
-					 * through to save the SCRAM verifier instead.
-					 */
-				case PASSWORD_TYPE_MD5:
-					return pstrdup(password);
-			}
-			break;
+			if (!pg_md5_encrypt(password, role, strlen(role),
+								encrypted_password))
+				elog(ERROR, "password encryption failed");
+			return encrypted_password;
 
 		case PASSWORD_TYPE_SCRAM_SHA_256:
-			switch (guessed_type)
-			{
-				case PASSWORD_TYPE_PLAINTEXT:
-					return pg_be_scram_build_verifier(password);
-
-				case PASSWORD_TYPE_MD5:
+			return pg_be_scram_build_verifier(password);
 
-					/*
-					 * cannot convert an MD5 hash to a SCRAM verifier, so fall
-					 * through to save the MD5 hash instead.
-					 */
-				case PASSWORD_TYPE_SCRAM_SHA_256:
-					return pstrdup(password);
-			}
-			break;
+		case PASSWORD_TYPE_PLAINTEXT:
+			elog(ERROR, "cannot encrypt password with 'plaintext'");
 	}
 
 	/*
@@ -197,10 +172,17 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
 {
 	int			retval;
 	char		crypt_pwd[MD5_PASSWD_LEN + 1];
-	char		crypt_pwd2[MD5_PASSWD_LEN + 1];
 
 	Assert(md5_salt_len > 0);
 
+	if (get_password_type(shadow_pass) != PASSWORD_TYPE_MD5)
+	{
+		/* incompatible password hash format. */
+		*logdetail = psprintf(_("User \"%s\" has a password that cannot be used with MD5 authentication."),
+							  role);
+		return STATUS_ERROR;
+	}
+
 	/*
 	 * Compute the correct answer for the MD5 challenge.
 	 *
@@ -208,40 +190,12 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
 	 * below: the only possible error is out-of-memory, which is unlikely, and
 	 * if it did happen adding a psprintf call would only make things worse.
 	 */
-	switch (get_password_type(shadow_pass))
+	/* stored password already encrypted, only do salt */
+	if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
+						md5_salt, md5_salt_len,
+						crypt_pwd))
 	{
-		case PASSWORD_TYPE_MD5:
-			/* stored password already encrypted, only do salt */
-			if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
-								md5_salt, md5_salt_len,
-								crypt_pwd))
-			{
-				return STATUS_ERROR;
-			}
-			break;
-
-		case PASSWORD_TYPE_PLAINTEXT:
-			/* stored password is plain, double-encrypt */
-			if (!pg_md5_encrypt(shadow_pass,
-								role,
-								strlen(role),
-								crypt_pwd2))
-			{
-				return STATUS_ERROR;
-			}
-			if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),
-								md5_salt, md5_salt_len,
-								crypt_pwd))
-			{
-				return STATUS_ERROR;
-			}
-			break;
-
-		default:
-			/* unknown password hash format. */
-			*logdetail = psprintf(_("User \"%s\" has a password that cannot be used with MD5 authentication."),
-								  role);
-			return STATUS_ERROR;
+		return STATUS_ERROR;
 	}
 
 	if (strcmp(client_pass, crypt_pwd) == 0)
@@ -259,8 +213,8 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
 /*
  * Check given password for given user, and return STATUS_OK or STATUS_ERROR.
  *
- * 'shadow_pass' is the user's correct password or password hash, as stored
- * in pg_authid.rolpassword.
+ * 'shadow_pass' is the user's correct password hash, as stored in
+ * pg_authid.rolpassword.
  * 'client_pass' is the password given by the remote user.
  *
  * In the error case, optionally store a palloc'd string at *logdetail
@@ -320,14 +274,10 @@ plain_crypt_verify(const char *role, const char *shadow_pass,
 			break;
 
 		case PASSWORD_TYPE_PLAINTEXT:
-			if (strcmp(client_pass, shadow_pass) == 0)
-				return STATUS_OK;
-			else
-			{
-				*logdetail = psprintf(_("Password does not match for user \"%s\"."),
-									  role);
-				return STATUS_ERROR;
-			}
+			/*
+			 * We never store passwords in plaintext, so this shouldn't
+			 * happen.
+			 */
 			break;
 	}
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 818d2c29d49..2cad8b25b8a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -994,13 +994,21 @@ AlterOptRoleElem:
 				}
 			| ENCRYPTED PASSWORD Sconst
 				{
-					$$ = makeDefElem("encryptedPassword",
+					/*
+					 * These days, passwords are always stored in encrypted
+					 * form, so there is no difference between PASSWORD and
+					 * ENCRYPTED PASSWORD.
+					 */
+					$$ = makeDefElem("password",
 									 (Node *)makeString($3), @1);
 				}
 			| UNENCRYPTED PASSWORD Sconst
 				{
-					$$ = makeDefElem("unencryptedPassword",
-									 (Node *)makeString($3), @1);
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("UNENCRYPTED PASSWORD is no longer supported"),
+							 errhint("Remove UNENCRYPTED to store the password in encrypted form instead."),
+							 parser_errposition(@1)));
 				}
 			| INHERIT
 				{
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 587fbce147f..cb4e621c848 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -405,20 +405,16 @@ static const struct config_enum_entry force_parallel_mode_options[] = {
 
 /*
  * password_encryption used to be a boolean, so accept all the likely
- * variants of "on" and "off", too.
+ * variants of "on", too. "off" used to store passwords in plaintext,
+ * but we don't support that anymore.
  */
 static const struct config_enum_entry password_encryption_options[] = {
-	{"plain", PASSWORD_TYPE_PLAINTEXT, false},
 	{"md5", PASSWORD_TYPE_MD5, false},
 	{"scram-sha-256", PASSWORD_TYPE_SCRAM_SHA_256, false},
-	{"off", PASSWORD_TYPE_PLAINTEXT, false},
-	{"on", PASSWORD_TYPE_MD5, false},
+	{"on", PASSWORD_TYPE_MD5, true},
 	{"true", PASSWORD_TYPE_MD5, true},
-	{"false", PASSWORD_TYPE_PLAINTEXT, true},
 	{"yes", PASSWORD_TYPE_MD5, true},
-	{"no", PASSWORD_TYPE_PLAINTEXT, true},
 	{"1", PASSWORD_TYPE_MD5, true},
-	{"0", PASSWORD_TYPE_PLAINTEXT, true},
 	{NULL, 0, false}
 };