On 09/12/2024 15:58, Guillaume Lelarge wrote:
Hi,
Le lun. 9 déc. 2024 à 14:40, Daniel Gustafsson <dan...@yesql.se
<mailto:dan...@yesql.se>> a écrit :
> On 9 Dec 2024, at 14:26, Greg Sabino Mullane <htamf...@gmail.com
<mailto:htamf...@gmail.com>> wrote:
> -1 to throwing an ERROR - that's not really an error, and not our
call to make, so a WARNING is sufficient.
Agreed, regardless of how bad it's considered, it's not an error.
There are
many ways sensitive data can end up in the logs and offering the
impression
there is a safety switch offers a false sense of security.
I'm fine with adding a test on whether or not we log statements. But
that completely hides the fact that people listening on the network
could also get to the password if the server doesn't use SSL. Isn't it
weird to warn about one potential leak and not the other one?
Here is patch v2. The only addition is a check to warn the user only if
the query has been logged:
(log_statement == LOGSTMT_ALL || log_statement == LOGSTMT_DDL ||
log_min_duration_statement > -1)
v2 is attached.
Regards.
--
Guillaume Lelarge
Consultant
https://dalibo.com
From 802bd6dfd9c12da032bc307863508c3575bdea19 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 v2] 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".
---
.../passwordcheck/expected/passwordcheck.out | 1 +
.../passwordcheck/expected/passwordcheck_1.out | 1 +
contrib/passwordcheck/sql/passwordcheck.sql | 1 +
doc/src/sgml/config.sgml | 18 ++++++++++++++++++
src/backend/libpq/crypt.c | 13 +++++++++++++
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 | 4 ++++
src/test/regress/sql/create_role.sql | 2 ++
src/test/regress/sql/password.sql | 5 +++++
12 files changed, 60 insertions(+)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 83472c76d2..f3fdf9c03b 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 fb12ec45cc..7db42e831c 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 21ad8d452b..92bc2de632 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 a782f10998..4493b1a458 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7781,6 +7781,24 @@ 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 and if logging is
+ configured (either with <xref linkend="guc-log-timezone"/> or with <xref
+ linkend="guc-log-min-duration-statement"/>). 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 cbb85a27cc..8be4730750 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -23,6 +23,11 @@
#include "utils/builtins.h"
#include "utils/syscache.h"
#include "utils/timestamp.h"
+#include "utils/guc.h"
+#include "tcop/tcopprot.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 +181,14 @@ encrypt_password(PasswordType target_type, const char *role,
MAX_ENCRYPTED_PASSWORD_LEN)));
}
+ if (plaintext_password_warnings &&
+ get_password_type(password) == PASSWORD_TYPE_PLAINTEXT &&
+ (log_statement == LOGSTMT_ALL || log_statement == LOGSTMT_DDL || log_min_duration_statement > -1))
+ 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 71448bb4fd..c033c03c4b 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2087,6 +2087,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 079efa1baa..7e10b15379 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 dee477428e..fb9f0309f1 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..94c4b73cbd 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -17,6 +17,7 @@ ALTER ROLE regress_passwd1 PASSWORD 'role_pwd1';
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 +68,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 +174,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.48.1
