postgrespro
diff --git a/‎doc/src/sgml/ref/create_role.sgml
Lines changed: 11 additions & 0 deletions b/‎doc/src/sgml/ref/create_role.sgml
Lines changed: 11 additions & 0 deletions
diff --git a/‎src/backend/commands/user.c
Lines changed: 45 additions & 10 deletions b/‎src/backend/commands/user.c
Lines changed: 45 additions & 10 deletions
diff --git a/‎src/backend/libpq/auth.c
Lines changed: 40 additions & 20 deletions b/‎src/backend/libpq/auth.c
Lines changed: 40 additions & 20 deletions
diff --git a/‎src/backend/libpq/crypt.c
Lines changed: 0 additions & 8 deletions b/‎src/backend/libpq/crypt.c
Lines changed: 0 additions & 8 deletions
diff --git a/‎src/test/regress/expected/password.out
Lines changed: 14 additions & 0 deletions b/‎src/test/regress/expected/password.out
Lines changed: 14 additions & 0 deletions
diff --git a/‎src/test/regress/sql/password.sql
Lines changed: 7 additions & 0 deletions b/‎src/test/regress/sql/password.sql
Lines changed: 7 additions & 0 deletions
@@ -219,6 +219,17 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
         user.  A null password can optionally be written explicitly as
         <literal>PASSWORD NULL</literal>.
        </para>
+       <note>
+         <para>
+           Specifying an empty string will also set the password to null,
+           but that was not the case before <productname>PostgreSQL</>
+           version 10. In earlier versions, an empty string could be used,
+           or not, depending on the authentication method and the exact
+           version, and libpq would refuse to use it in any case.
+           To avoid the ambiguity, specifying an empty string should be
+           avoided.
+         </para>
+       </note>
        <para>
         The password is always stored encrypted in the system catalogs. The
         <literal>ENCRYPTED</> keyword has no effect, but is accepted for
 
@@ -384,13 +384,36 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 
 	if (password)
 	{
-		/* Encrypt the password to the requested format. */
 		char	   *shadow_pass;
+		char	   *logdetail;
 
-		shadow_pass = encrypt_password(Password_encryption, stmt->role,
-									   password);
-		new_record[Anum_pg_authid_rolpassword - 1] =
-			CStringGetTextDatum(shadow_pass);
+		/*
+		 * Don't allow an empty password. Libpq treats an empty password the
+		 * same as no password at all, and won't even try to authenticate. But
+		 * other clients might, so allowing it would be confusing. By clearing
+		 * the password when an empty string is specified, the account is
+		 * consistently locked for all clients.
+		 *
+		 * Note that this only covers passwords stored in the database itself.
+		 * There are also checks in the authentication code, to forbid an
+		 * empty password from being used with authentication methods that
+		 * fetch the password from an external system, like LDAP or PAM.
+		 */
+		if (password[0] == '\0' ||
+			plain_crypt_verify(stmt->role, password, "", &logdetail) == STATUS_OK)
+		{
+			ereport(NOTICE,
+					(errmsg("empty string is not a valid password, clearing password")));
+			new_record_nulls[Anum_pg_authid_rolpassword - 1] = true;
+		}
+		else
+		{
+			/* Encrypt the password to the requested format. */
+			shadow_pass = encrypt_password(Password_encryption, stmt->role,
+										   password);
+			new_record[Anum_pg_authid_rolpassword - 1] =
+				CStringGetTextDatum(shadow_pass);
+		}
 	}
 	else
 		new_record_nulls[Anum_pg_authid_rolpassword - 1] = true;
@@ -782,13 +805,25 @@ AlterRole(AlterRoleStmt *stmt)
 	/* password */
 	if (password)
 	{
-		/* Encrypt the password to the requested format. */
 		char	   *shadow_pass;
+		char	   *logdetail;
 
-		shadow_pass = encrypt_password(Password_encryption, rolename,
-									   password);
-		new_record[Anum_pg_authid_rolpassword - 1] =
-			CStringGetTextDatum(shadow_pass);
+		/* Like in CREATE USER, don't allow an empty password. */
+		if (password[0] == '\0' ||
+			plain_crypt_verify(rolename, password, "", &logdetail) == STATUS_OK)
+		{
+			ereport(NOTICE,
+					(errmsg("empty string is not a valid password, clearing password")));
+			new_record_nulls[Anum_pg_authid_rolpassword - 1] = true;
+		}
+		else
+		{
+			/* Encrypt the password to the requested format. */
+			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;
 	}
 
 
@@ -688,6 +688,24 @@ recv_password_packet(Port *port)
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
 				 errmsg("invalid password packet size")));
 
+	/*
+	 * Don't allow an empty password. Libpq treats an empty password the same
+	 * as no password at all, and won't even try to authenticate. But other
+	 * clients might, so allowing it would be confusing.
+	 *
+	 * Note that this only catches an empty password sent by the client in
+	 * plaintext. There's also a check in CREATE/ALTER USER that prevents an
+	 * empty string from being stored as a user's password in the first place.
+	 * We rely on that for MD5 and SCRAM authentication, but we still need
+	 * this check here, to prevent an empty password from being used with
+	 * authentication methods that check the password against an external
+	 * system, like PAM, LDAP and RADIUS.
+	 */
+	if (buf.len == 1)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PASSWORD),
+				 errmsg("empty password returned by client")));
+
 	/* Do not echo password to logs, for security. */
 	elog(DEBUG5, "received password packet");
 
@@ -2081,12 +2099,6 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message **msg,
 						 */
 						goto fail;
 					}
-					if (strlen(passwd) == 0)
-					{
-						ereport(LOG,
-								(errmsg("empty password returned by client")));
-						goto fail;
-					}
 				}
 				if ((reply[i].resp = strdup(passwd)) == NULL)
 					goto fail;
@@ -2277,6 +2289,8 @@ CheckBSDAuth(Port *port, char *user)
 	 */
 	retval = auth_userokay(user, NULL, "auth-postgresql", passwd);
 
+	pfree(passwd);
+
 	if (!retval)
 		return STATUS_ERROR;
 
@@ -2407,16 +2421,12 @@ CheckLDAPAuth(Port *port)
 	if (passwd == NULL)
 		return STATUS_EOF;		/* client wouldn't send password */
 
-	if (strlen(passwd) == 0)
-	{
-		ereport(LOG,
-				(errmsg("empty password returned by client")));
-		return STATUS_ERROR;
-	}
-
 	if (InitializeLDAPConnection(port, &ldap) == STATUS_ERROR)
+	{
 		/* Error message already sent */
+		pfree(passwd);
 		return STATUS_ERROR;
+	}
 
 	if (port->hba->ldapbasedn)
 	{
@@ -2448,6 +2458,7 @@ CheckLDAPAuth(Port *port)
 			{
 				ereport(LOG,
 						(errmsg("invalid character in user name for LDAP authentication")));
+				pfree(passwd);
 				return STATUS_ERROR;
 			}
 		}
@@ -2464,6 +2475,7 @@ CheckLDAPAuth(Port *port)
 			ereport(LOG,
 					(errmsg("could not perform initial LDAP bind for ldapbinddn \"%s\" on server \"%s\": %s",
 							port->hba->ldapbinddn, port->hba->ldapserver, ldap_err2string(r))));
+			pfree(passwd);
 			return STATUS_ERROR;
 		}
 
@@ -2488,6 +2500,7 @@ CheckLDAPAuth(Port *port)
 			ereport(LOG,
 					(errmsg("could not search LDAP for filter \"%s\" on server \"%s\": %s",
 							filter, port->hba->ldapserver, ldap_err2string(r))));
+			pfree(passwd);
 			pfree(filter);
 			return STATUS_ERROR;
 		}
@@ -2508,6 +2521,7 @@ CheckLDAPAuth(Port *port)
 										  count,
 										  filter, port->hba->ldapserver, count)));
 
+			pfree(passwd);
 			pfree(filter);
 			ldap_msgfree(search_message);
 			return STATUS_ERROR;
@@ -2523,6 +2537,7 @@ CheckLDAPAuth(Port *port)
 			ereport(LOG,
 					(errmsg("could not get dn for the first entry matching \"%s\" on server \"%s\": %s",
 							filter, port->hba->ldapserver, ldap_err2string(error))));
+			pfree(passwd);
 			pfree(filter);
 			ldap_msgfree(search_message);
 			return STATUS_ERROR;
@@ -2543,6 +2558,7 @@ CheckLDAPAuth(Port *port)
 			ereport(LOG,
 					(errmsg("could not unbind after searching for user \"%s\" on server \"%s\": %s",
 							fulluser, port->hba->ldapserver, ldap_err2string(error))));
+			pfree(passwd);
 			pfree(fulluser);
 			return STATUS_ERROR;
 		}
@@ -2553,6 +2569,7 @@ CheckLDAPAuth(Port *port)
 		 */
 		if (InitializeLDAPConnection(port, &ldap) == STATUS_ERROR)
 		{
+			pfree(passwd);
 			pfree(fulluser);
 
 			/* Error message already sent */
@@ -2573,10 +2590,12 @@ CheckLDAPAuth(Port *port)
 		ereport(LOG,
 				(errmsg("LDAP login failed for user \"%s\" on server \"%s\": %s",
 						fulluser, port->hba->ldapserver, ldap_err2string(r))));
+		pfree(passwd);
 		pfree(fulluser);
 		return STATUS_ERROR;
 	}
 
+	pfree(passwd);
 	pfree(fulluser);
 
 	return STATUS_OK;
@@ -2720,17 +2739,11 @@ CheckRADIUSAuth(Port *port)
 	if (passwd == NULL)
 		return STATUS_EOF;		/* client wouldn't send password */
 
-	if (strlen(passwd) == 0)
-	{
-		ereport(LOG,
-				(errmsg("empty password returned by client")));
-		return STATUS_ERROR;
-	}
-
 	if (strlen(passwd) > RADIUS_MAX_PASSWORD_LENGTH)
 	{
 		ereport(LOG,
 				(errmsg("RADIUS authentication does not support passwords longer than %d characters", RADIUS_MAX_PASSWORD_LENGTH)));
+		pfree(passwd);
 		return STATUS_ERROR;
 	}
 
@@ -2756,9 +2769,15 @@ CheckRADIUSAuth(Port *port)
 		 *------
 		 */
 		if (ret == STATUS_OK)
+		{
+			pfree(passwd);
 			return STATUS_OK;
+		}
 		else if (ret == STATUS_EOF)
+		{
+			pfree(passwd);
 			return STATUS_ERROR;
+		}
 
 		/*
 		 * secret, port and identifiers either have length 0 (use default),
@@ -2775,6 +2794,7 @@ CheckRADIUSAuth(Port *port)
 	}
 
 	/* No servers left to try, so give up */
+	pfree(passwd);
 	return STATUS_ERROR;
 }
 
 
@@ -71,14 +71,6 @@ get_role_password(const char *role, char **logdetail)
 
 	ReleaseSysCache(roleTup);
 
-	if (*shadow_pass == '\0')
-	{
-		*logdetail = psprintf(_("User \"%s\" has an empty password."),
-							  role);
-		pfree(shadow_pass);
-		return NULL;			/* empty password */
-	}
-
 	/*
 	 * Password OK, but check to be sure we are not past rolvaliduntil
 	 */
 
@@ -75,11 +75,25 @@ SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+
  regress_passwd5 | md5e73a4b11df52a6068f8b39f90be36023
 (5 rows)
 
+-- An empty password is not allowed, in any form
+CREATE ROLE regress_passwd_empty PASSWORD '';
+NOTICE:  empty string is not a valid password, clearing password
+ALTER ROLE regress_passwd_empty PASSWORD 'md585939a5ce845f1a1b620742e3c659e0a';
+NOTICE:  empty string is not a valid password, clearing password
+ALTER ROLE regress_passwd_empty PASSWORD 'SCRAM-SHA-256$4096:hpFyHTUsSWcR7O9P$LgZFIt6Oqdo27ZFKbZ2nV+vtnYM995pDh9ca6WSi120=:qVV5NeluNfUPkwm7Vqat25RjSPLkGeoZBQs6wVv+um4=';
+NOTICE:  empty string is not a valid password, clearing password
+SELECT rolpassword FROM pg_authid WHERE rolname='regress_passwd_empty';
+ rolpassword 
+-------------
+ 
+(1 row)
+
 DROP ROLE regress_passwd1;
 DROP ROLE regress_passwd2;
 DROP ROLE regress_passwd3;
 DROP ROLE regress_passwd4;
 DROP ROLE regress_passwd5;
+DROP ROLE regress_passwd_empty;
 -- all entries should have been removed
 SELECT rolname, rolpassword
     FROM pg_authid
 
@@ -59,11 +59,18 @@ SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+
     WHERE rolname LIKE 'regress_passwd%'
     ORDER BY rolname, rolpassword;
 
+-- An empty password is not allowed, in any form
+CREATE ROLE regress_passwd_empty PASSWORD '';
+ALTER ROLE regress_passwd_empty PASSWORD 'md585939a5ce845f1a1b620742e3c659e0a';
+ALTER ROLE regress_passwd_empty PASSWORD 'SCRAM-SHA-256$4096:hpFyHTUsSWcR7O9P$LgZFIt6Oqdo27ZFKbZ2nV+vtnYM995pDh9ca6WSi120=:qVV5NeluNfUPkwm7Vqat25RjSPLkGeoZBQs6wVv+um4=';
+SELECT rolpassword FROM pg_authid WHERE rolname='regress_passwd_empty';
+
 DROP ROLE regress_passwd1;
 DROP ROLE regress_passwd2;
 DROP ROLE regress_passwd3;
 DROP ROLE regress_passwd4;
 DROP ROLE regress_passwd5;
+DROP ROLE regress_passwd_empty;
 
 -- all entries should have been removed
 SELECT rolname, rolpassword