PATCH: warn about, and deprecate, clear text passwords

On 21/02/2025 23:33, Greg Sabino Mullane wrote:

There have been a few complaints lately about the fact that we
cavalierly allow clear text passwords to be sent when doing CREATE USER
or ALTER USER. These, of course, can end up in many places, such as
pg_stat_activity, pg_stat_statements, .psql_history, and the server
logs. It is a genuinely valid complaint, and for security purposes,
there is little recourse other than telling users "don't do that". The
canonical recommendation is to use psql's awesome \password feature.
Second best is to use your application/driver of choice, which hopefully
has support for not sending passwords in the clear.

Please find attached a patch to implement a new GUC called
cleartext_passwords_action as an attempt to solve these problems. It is
an enum and accepts one of three values:

1. "warn" (the new default)

This issues a warning if a clear text password is used, but allows the
change to proceed. The hint can change to recommend \password if the
current application_name is 'psql'. By keeping this as a warning, we let
people know this is a bad idea, and give people time to modify
their applications.

Examples:

ALTER USER alice PASSWORD 'mynewpass';
WARNING:  using a clear text password
DETAIL:  Sending a password using plain text is deprecated and may be
removed in a future release of PostgreSQL.
HINT:  Use a client that can change the password without sending it in
clear text

ALTER USER eve PASSWORD 'anothernewpass';
WARNING:  using a clear text password
DETAIL:  Sending a password using plain text is deprecated and may be
removed in a future release of PostgreSQL.
HINT:  If using psql, you can set the password with \password

2. "allow"
This does nothing, and thus emulates the historical behavior.

3. "disallow"
This prevents the use of plain old text completely, by throwing an error
if a password set or change is attempted. So people who want to prevent
clear text can do so right away, and at some point we can make this the
default (and people can always change to hint or allow if desired)

Bike shedding welcome. I realize the irony that 'disallow' means valid
attempts will now show up in the database logs that otherwise would not,
but I'm not sure how to work around that (or if we should).

I'm obviously +1 on this patch since I sent kinda the same patch two
weeks ago
(/messages/by-id/8f17493f-0886-406d-8573-0fadcb998b1d@dalibo.co).
The only major difference is that your patch can completely disable
plain text passwords. More options, that sounds better to me.

--
Guillaume Lelarge
Consultant
https://dalibo.com

On 22/02/2025 09:07, Guillaume Lelarge wrote:

On 21/02/2025 23:33, Greg Sabino Mullane wrote:

There have been a few complaints lately about the fact that we
cavalierly allow clear text passwords to be sent when doing CREATE
USER or ALTER USER. These, of course, can end up in many places, such
as pg_stat_activity, pg_stat_statements, .psql_history, and the server
logs. It is a genuinely valid complaint, and for security purposes,
there is little recourse other than telling users "don't do that". The
canonical recommendation is to use psql's awesome \password feature.
Second best is to use your application/driver of choice, which
hopefully has support for not sending passwords in the clear.

Please find attached a patch to implement a new GUC called
cleartext_passwords_action as an attempt to solve these problems. It
is an enum and accepts one of three values:

1. "warn" (the new default)

This issues a warning if a clear text password is used, but allows the
change to proceed. The hint can change to recommend \password if the
current application_name is 'psql'. By keeping this as a warning, we
let people know this is a bad idea, and give people time to modify
their applications.

Examples:

ALTER USER alice PASSWORD 'mynewpass';
WARNING:  using a clear text password
DETAIL:  Sending a password using plain text is deprecated and may be
removed in a future release of PostgreSQL.
HINT:  Use a client that can change the password without sending it in
clear text

ALTER USER eve PASSWORD 'anothernewpass';
WARNING:  using a clear text password
DETAIL:  Sending a password using plain text is deprecated and may be
removed in a future release of PostgreSQL.
HINT:  If using psql, you can set the password with \password

2. "allow"
This does nothing, and thus emulates the historical behavior.

3. "disallow"
This prevents the use of plain old text completely, by throwing an
error if a password set or change is attempted. So people who want to
prevent clear text can do so right away, and at some point we can make
this the default (and people can always change to hint or allow if
desired)

Bike shedding welcome. I realize the irony that 'disallow' means valid
attempts will now show up in the database logs that otherwise would
not, but I'm not sure how to work around that (or if we should).

I'm obviously +1 on this patch since I sent kinda the same patch two
weeks ago (https://www.postgresql.org/message-
id/8f17493f-0886-406d-8573-0fadcb998b1d%40dalibo.co). The only major
difference is that your patch can completely disable plain text
passwords. More options, that sounds better to me.

It applies cleanly, compiles without errors or even warnings.

I did some tests, and I only found one small issue:

set password_encryption to 'md5';
create user u4 password 'md5u1';

WARNING: using a clear text password
DETAIL: Sending a password using plain text is deprecated and may be
removed in a future release of PostgreSQL.
HINT: If using psql, you can set the password with \password
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.
CREATE ROLE

It complains that I'm using a plain text password and a MD5-encrypted
password. Can't be both. (Probably not an issue with this patch, but
rather an issue with the commit that implemented MD5-password warnings.)

If I use a real md5 password, it only complains about MD5 encrypted
password:

create user u5 password 'md58026a39c502750413402a90d9d8bae3c';

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.
CREATE ROLE

Other tests were successful.

Thanks Greg!

--
Guillaume Lelarge
Consultant
https://dalibo.com

Hi,

There have been a few complaints lately about the fact that we cavalierly allow clear text passwords to be sent when doing CREATE USER or ALTER USER. These, of course, can end up in many places, such as pg_stat_activity, pg_stat_statements, .psql_history, and the server logs. It is a genuinely valid complaint, and for security purposes, there is little recourse other than telling users "don't do that". The canonical recommendation is to use psql's awesome \password feature. Second best is to use your application/driver of choice, which hopefully has support for not sending passwords in the clear.

If the problem is that the password might be logged, wouldn't a proper
solution be not to log such queries?

I don't see how a warning and an extra GUC will improve the overall
security of the system, and I suspect very few users will voluntarily
trade convenience to security by choosing "disallow". So in its
current state the patch doesn't seem to help much.

--
Best regards,
Aleksander Alekseev

Guillaume Lelarge <guillaume.lelarge@dalibo.com> wrote:

I'm obviously +1 on this patch since I sent kinda the same patch two weeks

ago

Ha ha, my brain forgot about that one (even though I commented on it!) -
apologies for that.

set password_encryption to 'md5';
create user u4 password 'md5u1';
...

It complains that I'm using a plain text password and a MD5-encrypted

password. Can't be both. (Probably not an issue with this patch, but rather
an issue with the commit that implemented MD5-password warnings.)

This is correct - it can be both. Not only are we sending a password in
clear text, but we then encrypt it using MD5. Hence, two warnings.

If I use a real md5 password, it only complains about MD5 encrypted
password:

Right. If someone sends us something that looks like an already-encrypted
password, we just store it. See get_password_type() in
backend/libpq/crypt.c. In which case, the actual password that a client
would type in would *not* be what was sent over the wire as part of the
ALTER USER / CREATE USER, so we don't complain.

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

On Mon, Feb 24, 2025 at 5:07 AM Aleksander Alekseev <
aleksander@timescale.com> wrote:

If the problem is that the password might be logged, wouldn't a proper
solution be not to log such queries?

Yes, this has been discussed before. The short answer is that it is
extremely difficult to implement reliably because we would need to add some
artificial intelligence to our parser and/or logger to prevent passwords
from being added via syntax errors. Even preventing certain valid commands
from being logged is fraught with edge cases and difficult tradeoffs. As
mentioned, it's also not just direct logging, it's things like
pg_stat_activity and pg_stat_statements.

The real solution is to train people and applications not to send clear
text passwords. If the command fails to work, that's a pretty good way to
train them.

I don't see how a warning and an extra GUC will improve the overall
security of the system, and I suspect very few users will voluntarily
trade convenience to security by choosing "disallow". So in its current
state the patch doesn't seem to help much.

One of the main goals is to start leading people away from using insecure
methods. Like with MD5, first we warn, then we disable usage (but allow a
way to enable for stubborn users/applications). So I would say the goals
are:

* Provide a warning, so people are aware that sending passwords in the
clear is a bad idea. Right now, there is no warning. Quite the improvement.

* Provide a way to disable using clear text passwords. Also a win for
people like the OP in another thread that wants to prevent password changes
from ending up in the logs. Now they can (and may help with PCI compliance
and related things).

* Provide notice to applications/drivers/frameworks that clear text
passwords are deprecated and they should provide (or recommend) an
alternate method.

* Lay the groundwork for eventually disallowing plain text passwords
completely. A long way off, but this is the start. After a couple years, we
could switch the default from "warn" to "disallow". A few years after that,
disallow completely.

I know it is not a perfect solution, and likely never will be, but we can't
let the perfect be the enemy of the good. This seems a good, small
incremental step. The three-value solution also allows for complete
backwards compatibility, while still allowing admins to throw a warning or
error if desired.

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

On 24/02/2025 14:55, Greg Sabino Mullane wrote:

Guillaume Lelarge <guillaume.lelarge@dalibo.com
<mailto:guillaume.lelarge@dalibo.com>> wrote:

I'm obviously +1 on this patch since I sent kinda the same patch two
weeks ago

Ha ha, my brain forgot about that one (even though I commented on it!) -
apologies for that.

No need to apologize :)

set password_encryption to 'md5';
create user u4 password 'md5u1';
...

It complains that I'm using a plain text password and a MD5-
encrypted password. Can't be both. (Probably not an issue with this
patch, but rather an issue with the commit that implemented MD5-
password warnings.)

This is correct - it can be both. Not only are we sending a password in
clear text, but we then encrypt it using MD5. Hence, two warnings.

If I use a real md5 password, it only complains about MD5 encrypted
password:

Right. If someone sends us something that looks like an already-
encrypted password, we just store it. See get_password_type() in
backend/libpq/crypt.c. In which case, the actual password that a client
would type in would *not* be what was sent over the wire as part of the
ALTER USER / CREATE USER, so we don't complain.

Sounds good to me.

--
Guillaume Lelarge
Consultant
https://dalibo.com

On Mon, Feb 24, 2025 at 09:26:07AM -0500, Greg Sabino Mullane wrote:

* Lay the groundwork for eventually disallowing plain text passwords
completely. A long way off, but this is the start. After a couple years, we
could switch the default from "warn" to "disallow". A few years after that,
disallow completely.

I wonder how folks feel about the idea of removing the ability to send
passwords to the server in clear text. There may be some scenarios where
clear text is probably fine, and most of passwordcheck's checks rely on
being able to see the clear text password, but we've long encouraged folks
to "pre-encrypt" passwords. I also think it's hard to argue that sending a
clear text password is much more convenient than createuser or \password
(not to mention the PQchangePassword() function in libpq). That being
said, this seems like it has the potential to break a lot of stuff, and we
probably ought to be cautious about that, too.

This is perhaps a nitpick, but one issue with ERROR-ing for clear text
passwords is that the default logging settings seem to send the statement
to the logs, too. So, it might actually increase the likelihood of the
password showing up in the logs. I'm not sure what else could be done, but
I believe the conventional wisdom is that logs can contain sensitive
information, so maybe it's okay... It still seems weird to me to try to
help folks to avoid logging passwords by logging their passwords.

--
nathan

On Mon, 24 Feb 2025 at 15:47, Nathan Bossart <nathandbossart@gmail.com>
wrote:

This is perhaps a nitpick, but one issue with ERROR-ing for clear text

passwords is that the default logging settings seem to send the statement
to the logs, too. So, it might actually increase the likelihood of the
password showing up in the logs. I'm not sure what else could be done, but
I believe the conventional wisdom is that logs can contain sensitive
information, so maybe it's okay... It still seems weird to me to try to
help folks to avoid logging passwords by logging their passwords.

It is definitely ironic, but it’s non-routinely logging their proposed new
password which, due to the server settings, does not actually get set as
the new password, in order to prevent routinely logging their passwords.

What I mean is, after the error is thrown and the proposed password logged,
they need to re-try with a pre-encrypted password which will not be logged.
If they choose a new password, then the logged one is irrelevant, and even
if they don't, it's just one password rather than all the ones they change.
So on the whole I think this is good. And in any case I believe the
existing behaviour can still be had by configuration so we're not really
imposing anything on anybody.

On Mon, Feb 24, 2025 at 04:06:41PM -0500, Isaac Morland wrote:

And in any case I believe the existing behaviour can still be had by
configuration so we're not really imposing anything on anybody.

Well, the discussion upthread suggests "disallowing plain text passwords
completely" [0]/messages/by-id/CAKAnmmKGWxEGbvuBAxyDWmuije8agUHTY-82DR1VEkFM2vKNTg@mail.gmail.com, which means at some point we would have to impose
something, right? Again, maybe this in itself is not a deal-breaker.
After all, we'll probably have a similar problem when we eventually remove
MD5 password support down the road.

[0]: /messages/by-id/CAKAnmmKGWxEGbvuBAxyDWmuije8agUHTY-82DR1VEkFM2vKNTg@mail.gmail.com

--
nathan

On Mon, Feb 24, 2025 at 4:18 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

Well, the discussion upthread suggests "disallowing plain text passwords
completely"

Yeah, that's more of a long-term dream than a real plan. It would certainly
be no sooner than Postgres v24 or so...

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

On Mon, Feb 24, 2025 at 04:20:44PM -0500, Greg Sabino Mullane wrote:

On Mon, Feb 24, 2025 at 4:18 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:

Well, the discussion upthread suggests "disallowing plain text passwords
completely"

Yeah, that's more of a long-term dream than a real plan. It would certainly
be no sooner than Postgres v24 or so...

I noticed a nearby thread [0]/messages/by-id/3136308.1740155121@sss.pgh.pa.us in which there appears to be some budding
support for a GUC that disables sending passwords to the server in
clear-text, at least for CREATE/ALTER ROLE. Perhaps we just add that for
now. (I'm probably well over my quota for new GUCs in v18...)

IMHO a WARNING would really only be appropriate if we are definitely going
to remove support in the future, and that feels like a bit of a stretch to
me due to the level of breakage it could cause. That being said, folks did
seem on board enough with deprecating MD5 passwords for me to feel
comfortable committing it, although that might not quite be an
apples-to-apples comparison. In any case, we've long encouraged folks to
avoid sending passwords to the server in clear-text, so I think it's
reasonable to provide some way to enforce that server-side.

[0]: /messages/by-id/3136308.1740155121@sss.pgh.pa.us

--
nathan

On Tue, Feb 25, 2025 at 10:34 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:

I noticed a nearby thread [0] in which there appears to be some budding
support for a GUC that disables sending passwords to the server in
clear-text, at least for CREATE/ALTER ROLE.

Yep, that was the thread that inspired this patch!

Perhaps we just add that for now. (I'm probably well over my quota for new

GUCs in v18...)

Heh.

IMHO a WARNING would really only be appropriate if we are definitely going
to remove support in the future, and that feels like a bit of a stretch to
me due to the level of breakage it could cause. That being said, folks did
seem on board enough with deprecating MD5 passwords for me to feel
comfortable committing it, although that might not quite be an
apples-to-apples comparison. In any case, we've long encouraged folks to
avoid sending passwords to the server in clear-text, so I think it's
reasonable to provide some way to enforce that server-side.

Yes, I went back and forth on the wording for the warning, but ended up
with a slightly weasely "may be removed" rather than "will be removed". Of
course, no date is explicitly promised, so "will be removed" could be an
accurate. It might just be 75 years from now, when our neural implants make
plain text passwords a quaint relic.

This has a commitfest entry now fwiw:
https://commitfest.postgresql.org/patch/5597/

Thank you for your input on this.

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

On Tue, Feb 25, 2025 at 11:13:51AM -0500, Greg Sabino Mullane wrote:

On Tue, Feb 25, 2025 at 10:34 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:

IMHO a WARNING would really only be appropriate if we are definitely going
to remove support in the future, and that feels like a bit of a stretch to
me due to the level of breakage it could cause. That being said, folks did
seem on board enough with deprecating MD5 passwords for me to feel
comfortable committing it, although that might not quite be an
apples-to-apples comparison. In any case, we've long encouraged folks to
avoid sending passwords to the server in clear-text, so I think it's
reasonable to provide some way to enforce that server-side.

Yes, I went back and forth on the wording for the warning, but ended up
with a slightly weasely "may be removed" rather than "will be removed". Of
course, no date is explicitly promised, so "will be removed" could be an
accurate. It might just be 75 years from now, when our neural implants make
plain text passwords a quaint relic.

I think it would be good to hear some other opinions on whether we should
consider sending clear-text passwords to the server as either 1) fully
supported, 2) deprecated but with no intent to remove anytime soon, or 3)
deprecated with the intent of removal at some point in the next several
years. I personally am -1 on the warning unless we have a consensus on
(3), but I'm +1 on adding a way to enforce "pre-encryption" regardless.

--
nathan

On Mon, Mar 3, 2025 at 11:33 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:

I think it would be good to hear some other opinions on whether we should
consider sending clear-text passwords to the server as either 1) fully
supported, 2) deprecated but with no intent to remove anytime soon, or 3)
deprecated with the intent of removal at some point in the next several
years. I personally am -1 on the warning unless we have a consensus on
(3), but I'm +1 on adding a way to enforce "pre-encryption" regardless.

That's more than fair. And "deprecation" doesn't need to mean that's the
next step in the process. So warn -> deny by default (but allow if you work
at it) -> remove completely. Which is very similar to our md5 path, I
suppose. I'm certainly happy staying at that middle stage for an indefinite
amount of time for both of those, as it means that Postgres is both "secure
by default" but backwards compatible.

--
Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

On Mon, 3 Mar 2025 at 12:07, Greg Sabino Mullane <htamfids@gmail.com> wrote:

On Mon, Mar 3, 2025 at 11:33 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:

I think it would be good to hear some other opinions on whether we should
consider sending clear-text passwords to the server as either 1) fully
supported, 2) deprecated but with no intent to remove anytime soon, or 3)
deprecated with the intent of removal at some point in the next several
years. I personally am -1 on the warning unless we have a consensus on
(3), but I'm +1 on adding a way to enforce "pre-encryption" regardless.

That's more than fair. And "deprecation" doesn't need to mean that's the
next step in the process. So warn -> deny by default (but allow if you work
at it) -> remove completely. Which is very similar to our md5 path, I
suppose. I'm certainly happy staying at that middle stage for an indefinite
amount of time for both of those, as it means that Postgres is both "secure
by default" but backwards compatible.

It's too bad we didn't have this discussion a few years ago. We could have
decided that SCRAM authentication doesn't allow sending cleartext passwords
and then relied on the phase-out of MD5 passwords to phase out sending of
cleartext passwords.

On Mon, Mar 3, 2025 at 11:33 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

I think it would be good to hear some other opinions on whether we should
consider sending clear-text passwords to the server as either 1) fully
supported, 2) deprecated but with no intent to remove anytime soon, or 3)
deprecated with the intent of removal at some point in the next several
years. I personally am -1 on the warning unless we have a consensus on
(3), but I'm +1 on adding a way to enforce "pre-encryption" regardless.

I wonder if we could drum up some support for not including any
version of the password (even encrypted) in the query string. For
instance, let's say that to change your password you have to use the
new CHANGE PASSWORD command which can only be used at top level (not
inside PL code or whatever) and always takes a single parameter that
must be supplied via the extended query protocol. I suppose there's
still a potential security exposure if people are logging parameters,
but maybe it's easier to avoid logging those parameters when the
command is CHANGE PASSWORD than it is to avoid logging a query string
with sensitive information in it.

If we introduced such a mechanism, perhaps we could eventually
deprecate ALTER USER as a method of changing passwords, or at least
have the option to disallow it. Or maybe we just want to add the
option to disallow it now, as proposed here -- but I'm not totally
convinced that will meaningfully improve security if the command still
exists and might still work on some systems.

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:

I wonder if we could drum up some support for not including any
version of the password (even encrypted) in the query string. For
instance, let's say that to change your password you have to use the
new CHANGE PASSWORD command which can only be used at top level (not
inside PL code or whatever) and always takes a single parameter that
must be supplied via the extended query protocol.

How would pg_dumpall cope with transferring passwords then?

I could see insisting that plain-text passwords be supplied only
that way. But removing the ability to have encrypted passwords
in-line seems like a serious operational problem with little benefit.

regards, tom lane

On Mon, Mar 3, 2025 at 1:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

I wonder if we could drum up some support for not including any
version of the password (even encrypted) in the query string. For
instance, let's say that to change your password you have to use the
new CHANGE PASSWORD command which can only be used at top level (not
inside PL code or whatever) and always takes a single parameter that
must be supplied via the extended query protocol.

How would pg_dumpall cope with transferring passwords then?

I could see insisting that plain-text passwords be supplied only
that way. But removing the ability to have encrypted passwords
in-line seems like a serious operational problem with little benefit.

Oh, good point. I don't know. I just have heard a LOT of complaining
about passwords showing up in the log, and I'm not sure insisting that
they have to all be encrypted is going to make all of the complaining
stop.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, Mar 03, 2025 at 01:54:59PM -0500, Robert Haas wrote:

Oh, good point. I don't know. I just have heard a LOT of complaining
about passwords showing up in the log, and I'm not sure insisting that
they have to all be encrypted is going to make all of the complaining
stop.

+1. At this point, IMHO we should consider this v19 material to provide
more time for discussion on the best way to tackle this problem. Blocking
plain-text passwords in CREATE/ALTER ROLE commands may be part of it, but
as Robert notes, we might need to do more.

--
nathan