Hello,
We've got a long tradition of telling people not to use unencrypted
passwords in CREATE ROLE and ALTER ROLE because the queries may be logged.
We try to encourage them to use \password in psql, and related techniques
on other tools. Users usually want us to stop logging passwords, but this
is not that easy to do (if it's at all possible and interesting). A few
days ago, I read Divya Sharma's interview on postgresql.life [1] and she
said:
> Whenever log_statement is set to all (which I understand should be done
for a short period of time for troubleshooting purposes only), if we change
the password for a user, or create a new user, the passwords would be
logged in plain text. From a security point of view, this should not be
allowed. Ideally, It should error out (or at least throw a warning) saying
“while log_statement is set to ‘all’, you shouldn’t change passwords/create
new user with passwords”.
While I dislike the idea of throwing an error, I found the idea of a
warning message really great. So kudos to her for the idea!
I thought about it, and tried to write a patch. I've mostly copied the
"Deprecate MD5 passwords" patch/commit from Nathan Bossart. My patch works
on current HEAD. Documentation and tests are dealt with.
Here is a quick demo:
postgres=# show plaintext_password_warnings;
plaintext_password_warnings
-----------------------------
on
(1 row)
postgres=# create user foo password 'bar';
WARNING: using a plaintext password in a query
DETAIL: plaintext password may be logged.
HINT: Refer to the PostgreSQL documentation for details about using
encrypted password in queries.
CREATE ROLE
postgres=# alter role foo password 'bar2';
WARNING: using a plaintext password in a query
DETAIL: plaintext password may be logged.
HINT: Refer to the PostgreSQL documentation for details about using
encrypted password in queries.
ALTER ROLE
postgres=# set plaintext_password_warnings to off;
SET
postgres=# alter role foo password 'bar3';
ALTER ROLE
postgres=# set plaintext_password_warnings to on;
SET
postgres=# \password foo
Enter new password for user "foo":
Enter it again:
As I'm writing this email, I'm thinking we could transform the boolean GUC
into an enum GUC, allowing the user to get an error or a log message, or no
message at all (old behaviour), whatever fits better for him/her.
I'm interested in any comments about this. I didn't create a commitfest
entry yet, I'm mostly waiting on your comments.
Thanks.
Regards.
[1] https://postgresql.life/post/divya_sharma/
--
Guillaume.
From 6415a0dcf0a2735ee160c648b9452f3aa5d52e9c Mon Sep 17 00:00:00 2001
From: Guillaume Lelarge <guillaume.lela...@dalibo.com>
Date: Sat, 7 Dec 2024 10:22:37 +0100
Subject: [PATCH] Add a warning when using plain text passwords
With CREATE/ALTER ROLE... PASSWORD, plain text passwords may be logged
and can be discovered if someone is listening on the network. We used to
tell users not to use plain text passwords, but it would be better to
warn the user of the issue if the user tries to add/change a password
using a plain text one in the query.
CREATE ROLE and ALTER ROLE now emit warnings when using plain text
passwords. The warnings can be disabled by setting the
plaintext_password_warnings parameter to "off".
---
contrib/passwordcheck/expected/passwordcheck.out | 1 +
.../passwordcheck/expected/passwordcheck_1.out | 1 +
contrib/passwordcheck/sql/passwordcheck.sql | 1 +
doc/src/sgml/config.sgml | 16 ++++++++++++++++
src/backend/libpq/crypt.c | 10 ++++++++++
src/backend/utils/misc/guc_tables.c | 9 +++++++++
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/libpq/crypt.h | 3 +++
src/test/regress/expected/create_role.out | 2 ++
src/test/regress/expected/password.out | 7 +++++++
src/test/regress/sql/create_role.sql | 2 ++
src/test/regress/sql/password.sql | 5 +++++
12 files changed, 58 insertions(+)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..0f0f3dad9e 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,3 +1,4 @@
+SET plaintext_password_warnings = off;
SET md5_password_warnings = off;
LOAD 'passwordcheck';
CREATE USER regress_passwordcheck_user1;
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..b124011752 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -1,3 +1,4 @@
+SET plaintext_password_warnings = off;
SET md5_password_warnings = off;
LOAD 'passwordcheck';
CREATE USER regress_passwordcheck_user1;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..4aece2bed1 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,3 +1,4 @@
+SET plaintext_password_warnings = off;
SET md5_password_warnings = off;
LOAD 'passwordcheck';
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e0c8325a39..c162da4a88 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7921,6 +7921,22 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</listitem>
</varlistentry>
+ <varlistentry id="guc-plaintext-password-warnings" xreflabel="plaintext_password_warnings">
+ <term><varname>plaintext_password_warnings</varname> (<type>boolean</type>)
+ <indexterm>
+ <primary><varname>plaintext_password_warnings</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Controls whether a <literal>WARNING</literal> about plaintext password
+ is produced when a <command>CREATE ROLE</command> or
+ <command>ALTER ROLE</command> statement uses a plaintext password.
+ The default value is <literal>on</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-md5-password-warnings" xreflabel="md5_password_warnings">
<term><varname>md5_password_warnings</varname> (<type>boolean</type>)
<indexterm>
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index d37c70901b..97fc69e3d3 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -24,6 +24,9 @@
#include "utils/syscache.h"
#include "utils/timestamp.h"
+/* Enables usage warnings for plaintext passwords. */
+bool plaintext_password_warnings = true;
+
/* Enables deprecation warnings for MD5 passwords. */
bool md5_password_warnings = true;
@@ -176,6 +179,13 @@ encrypt_password(PasswordType target_type, const char *role,
MAX_ENCRYPTED_PASSWORD_LEN)));
}
+ if (plaintext_password_warnings &&
+ get_password_type(password) == PASSWORD_TYPE_PLAINTEXT)
+ ereport(WARNING,
+ (errmsg("using a plaintext password in a query"),
+ errdetail("plaintext password may be logged."),
+ errhint("Refer to the PostgreSQL documentation for details about using encrypted password in queries.")));
+
if (md5_password_warnings &&
get_password_type(encrypted_password) == PASSWORD_TYPE_MD5)
ereport(WARNING,
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8cf1afbad2..c43831bf70 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2086,6 +2086,15 @@ struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
+ {
+ {"plaintext_password_warnings", PGC_USERSET, CONN_AUTH_AUTH,
+ gettext_noop("Enables usage warnings for plaintext passwords."),
+ },
+ &plaintext_password_warnings,
+ true,
+ NULL, NULL, NULL
+ },
+
{
{"md5_password_warnings", PGC_USERSET, CONN_AUTH_AUTH,
gettext_noop("Enables deprecation warnings for MD5 passwords."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index a2ac7575ca..8265ffc9a0 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -96,6 +96,7 @@
#authentication_timeout = 1min # 1s-600s
#password_encryption = scram-sha-256 # scram-sha-256 or md5
#scram_iterations = 4096
+#plaintext_password_warnings = on
#md5_password_warnings = on
# GSSAPI using Kerberos
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index db7ea7bd1f..2f834bdcf0 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -25,6 +25,9 @@
*/
#define MAX_ENCRYPTED_PASSWORD_LEN (512)
+/* Enables usage warnings for plaintext passwords. */
+extern PGDLLIMPORT bool plaintext_password_warnings;
+
/* Enables deprecation warnings for MD5 passwords. */
extern PGDLLIMPORT bool md5_password_warnings;
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 46d4f9efe9..546fa2f086 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -63,8 +63,10 @@ GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION;
CREATE ROLE regress_login LOGIN;
CREATE ROLE regress_inherit INHERIT;
CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
+SET plaintext_password_warnings = off;
CREATE ROLE regress_encrypted_password ENCRYPTED PASSWORD 'foo';
CREATE ROLE regress_password_null PASSWORD NULL;
+SET plaintext_password_warnings = on;
-- ok, backwards compatible noise words should be ignored
CREATE ROLE regress_noiseword SYSID 12345;
NOTICE: SYSID can no longer be specified
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index 9bb3ab2818..1def5a2bc4 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -14,9 +14,13 @@ SET password_encryption = 'scram-sha-256'; -- ok
SET password_encryption = 'md5';
CREATE ROLE regress_passwd1;
ALTER ROLE regress_passwd1 PASSWORD 'role_pwd1';
+WARNING: using a plaintext password in a query
+DETAIL: plaintext password may be logged.
+HINT: Refer to the PostgreSQL documentation for details about using encrypted password in queries.
WARNING: setting an MD5-encrypted password
DETAIL: MD5 password support is deprecated and will be removed in a future release of PostgreSQL.
HINT: Refer to the PostgreSQL documentation for details about migrating to another password type.
+SET plaintext_password_warnings = off;
CREATE ROLE regress_passwd2;
ALTER ROLE regress_passwd2 PASSWORD 'role_pwd2';
WARNING: setting an MD5-encrypted password
@@ -67,11 +71,13 @@ WARNING: setting an MD5-encrypted password
DETAIL: MD5 password support is deprecated and will be removed in a future release of PostgreSQL.
HINT: Refer to the PostgreSQL documentation for details about migrating to another password type.
-- already encrypted, use as they are
+SET plaintext_password_warnings = on;
ALTER ROLE regress_passwd1 PASSWORD 'md5cd3578025fe2c3d7ed1b9a9b26238b70';
WARNING: setting an MD5-encrypted password
DETAIL: MD5 password support is deprecated and will be removed in a future release of PostgreSQL.
HINT: Refer to the PostgreSQL documentation for details about migrating to another password type.
ALTER ROLE regress_passwd3 PASSWORD 'SCRAM-SHA-256$4096:VLK4RMaQLCvNtQ==$6YtlR4t69SguDiwFvbVgVZtuz6gpJQQqUMZ7IQJK5yI=:ps75jrHeYU4lXCcXI4O8oIdJ3eO8o2jirjruw9phBTo=';
+SET plaintext_password_warnings = off;
SET password_encryption = 'scram-sha-256';
-- create SCRAM secret
ALTER ROLE regress_passwd4 PASSWORD 'foo';
@@ -171,3 +177,4 @@ SELECT rolname, rolpassword
---------+-------------
(0 rows)
+SET plaintext_password_warnings = on;
diff --git a/src/test/regress/sql/create_role.sql b/src/test/regress/sql/create_role.sql
index 4491a28a8a..600d428e8e 100644
--- a/src/test/regress/sql/create_role.sql
+++ b/src/test/regress/sql/create_role.sql
@@ -48,8 +48,10 @@ GRANT CREATE ON DATABASE regression TO regress_createrole WITH GRANT OPTION;
CREATE ROLE regress_login LOGIN;
CREATE ROLE regress_inherit INHERIT;
CREATE ROLE regress_connection_limit CONNECTION LIMIT 5;
+SET plaintext_password_warnings = off;
CREATE ROLE regress_encrypted_password ENCRYPTED PASSWORD 'foo';
CREATE ROLE regress_password_null PASSWORD NULL;
+SET plaintext_password_warnings = on;
-- ok, backwards compatible noise words should be ignored
CREATE ROLE regress_noiseword SYSID 12345;
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index e7a9804545..618d5368a8 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -12,6 +12,7 @@ SET password_encryption = 'scram-sha-256'; -- ok
SET password_encryption = 'md5';
CREATE ROLE regress_passwd1;
ALTER ROLE regress_passwd1 PASSWORD 'role_pwd1';
+SET plaintext_password_warnings = off;
CREATE ROLE regress_passwd2;
ALTER ROLE regress_passwd2 PASSWORD 'role_pwd2';
SET password_encryption = 'scram-sha-256';
@@ -46,8 +47,10 @@ SET password_encryption = 'md5';
-- encrypt with MD5
ALTER ROLE regress_passwd2 PASSWORD 'foo';
-- already encrypted, use as they are
+SET plaintext_password_warnings = on;
ALTER ROLE regress_passwd1 PASSWORD 'md5cd3578025fe2c3d7ed1b9a9b26238b70';
ALTER ROLE regress_passwd3 PASSWORD 'SCRAM-SHA-256$4096:VLK4RMaQLCvNtQ==$6YtlR4t69SguDiwFvbVgVZtuz6gpJQQqUMZ7IQJK5yI=:ps75jrHeYU4lXCcXI4O8oIdJ3eO8o2jirjruw9phBTo=';
+SET plaintext_password_warnings = off;
SET password_encryption = 'scram-sha-256';
-- create SCRAM secret
@@ -118,3 +121,5 @@ SELECT rolname, rolpassword
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
ORDER BY rolname, rolpassword;
+
+SET plaintext_password_warnings = on;
--
2.47.1
