Thread: PATCH: warn about, and deprecate, clear text passwords
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
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
HINT: If using psql, you can set the password with \password
2. "allow"
This does nothing, and thus emulates the historical behavior.
3. "disallow"
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).
Cheers,
Greg
--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support
Attachment
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. -- 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 doingCREATE 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, thereis little recourse other than telling users "don't do that". The canonical recommendation is to use psql's awesome \passwordfeature. Second best is to use your application/driver of choice, which hopefully has support for not sending passwordsin 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], 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] https://postgr.es/m/CAKAnmmKGWxEGbvuBAxyDWmuije8agUHTY-82DR1VEkFM2vKNTg%40mail.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] 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] https://postgr.es/m/3136308.1740155121%40sss.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
I'd rather not sit on this another year, if we can help it. We really should be warning people about this practice. The exact wording of the hint can be up for debate (or postponed - we technically don't have to say anything other than 'bad idea').
Having the ability to disable clear text passwords seems an immediate win for those that want to enable it. Sure, we could be doing more, but I don't see any of the proposed future changes interfering with this patch.
Cheers,
Greg
--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support
On Fri, Mar 14, 2025 at 12:50 PM Greg Sabino Mullane <htamfids@gmail.com> wrote:
I'd rather not sit on this another year, if we can help it. We really should be warning people about this practice. The exact wording of the hint can be up for debate (or postponed - we technically don't have to say anything other than 'bad idea').Having the ability to disable clear text passwords seems an immediate win for those that want to enable it. Sure, we could be doing more, but I don't see any of the proposed future changes interfering with this patch.
I agree. This is a clear win that can easily be turned on by packagers/distributors with little consequence to everyone else.
My only suggestion would be to have the GUC name be closer to other password-related settings. Looking at the sample file I see
password_encryption
md5_password_warnings
So perhaps something like password_cleartext_action would fit in a little better and make it easier to spot while going through the file.
Roberto
On Fri, Mar 14, 2025 at 2:50 PM Greg Sabino Mullane <htamfids@gmail.com> wrote: > I'd rather not sit on this another year, if we can help it. We really should be warning people about this practice. Theexact wording of the hint can be up for debate (or postponed - we technically don't have to say anything other than 'badidea'). > > Having the ability to disable clear text passwords seems an immediate win for those that want to enable it. Sure, we couldbe doing more, but I don't see any of the proposed future changes interfering with this patch. I don't know, I think Nathan's idea of giving ourselves more time to decide what to do is a pretty good one. It seems clear from the discussion so far that there are multiple ideas about what to do here, and it's not stupid to want to give ourselves a bit of time to think that through before committing to anything in particular. From my point of view, the warning text here for what is proposed here almost might almost be: WARNING: you just caused a problem for somebody else The user has no particular reason to care about the fact that the password they just typed ended up in the log. That is a concern for the DBA, not the user, and even if they care about the DBA's feelings, they only get the warning after it's too late to do otherwise. Blocking ALTER USER commands containing such passwords is better, because now doing the wrong thing actually doesn't work at all, and so they have to change their behavior if they want to get anything done. But we don't seem to be too sure whether we can really get away with that amount of breakage; and we also don't seem to be sure whether it really solves the problem; and we're getting very close to feature freeze. -- Robert Haas EDB: http://www.enterprisedb.com
On Sunday, March 16, 2025, Robert Haas <robertmhaas@gmail.com> wrote:
WARNING: you just caused a problem for somebody else
The user has no particular reason to care about the fact that the
password they just typed ended up in the log.
It could also be:
warning: your password is known to Big Brother
hint: use psql \password to supply a private password, or see “docs/wiki page” for more details and a way to pre-compute and send a private password via SQL.
Sure, we can’t make them drink, but let’s at least show them where we put the water trough. Some of them will care but be unaware.
We can make it an error later and do nothing, removing the choice but to figure out the proper way of changing their password.
David J.
On Sun, Mar 16, 2025 at 11:36 PM David G. Johnston <david.g.johnston@gmail.com> wrote: > It could also be: > > warning: your password is known to Big Brother > hint: use psql \password to supply a private password, or see “docs/wiki page” for more details and a way to pre-computeand send a private password via SQL. OK, that's actually a fair point. It's still true, though, that all the complaints that I hear about this are of the form "someone MIGHT do something that puts their password in a log file" and a warning doesn't stop that. Granted, other people may hear different complaints than I do. -- Robert Haas EDB: http://www.enterprisedb.com
The user has no particular reason to care about the fact that the password they just typed ended up in the log. That is a concern for
the DBA, not the user, and even if they care about the DBA's feelings, they only get the warning after it's too late to do otherwise.
Can't the same be said about other warnings, esp. md5?
Attached is a rebase of the patch.
I'm a little confused at some of the pushback - this patch is 100% backwards compatible, addresses a specific requested concern by allowing a DBA to disallow clear text passwords, and adds a warning to what is clearly a bad practice that we should be discouraging.
Robert - would you be more inclined to accept this if we kept the three states, but made the default "allow"? That would still allow people to bump it stronger manually, but would have no effect on everyone else. That would give us time to tweak the wording and/or examine other approaches. Although any other approaches would still leave the need to do something with passwords via ALTER USER / CREATE USER in the interim.
Cheers,
Greg
--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support
Attachment
On Wed, Mar 19, 2025 at 09:24:19AM -0400, Greg Sabino Mullane wrote: > I'm a little confused at some of the pushback - this patch is 100% backwards > compatible, addresses a specific requested concern by allowing a DBA to > disallow clear text passwords, and adds a warning to what is clearly a bad > practice that we should be discouraging. > > Robert - would you be more inclined to accept this if we kept the three states, > but made the default "allow"? That would still allow people to bump it stronger > manually, but would have no effect on everyone else. That would give us time to > tweak the wording and/or examine other approaches. Although any other > approaches would still leave the need to do something with passwords via ALTER > USER / CREATE USER in the interim. You are getting pushback because this complex user change is still being debated in mid-March, when the feature freeze is only a few weeks away. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Do not let urgent matters crowd out time for investment in the future.
On Wed, Mar 19, 2025 at 9:24 AM Greg Sabino Mullane <htamfids@gmail.com> wrote: >> The user has no particular reason to care about the fact that the password they just typed ended up in the log. That isa concern for >> the DBA, not the user, and even if they care about the DBA's feelings, they only get the warning after it's too late todo otherwise. > > Can't the same be said about other warnings, esp. md5? Absolutely. Warnings are sometimes the right thing, but they often suck. If something is really a bad idea, "ERROR: bad idea" is vastly superior to "WARNING: what you just already did was a bad idea". If we don't actually know for sure that it's a bad idea, then it's generally better not to emit a warning at all, for fear of log-spamming people who know what they're doing. > Robert - would you be more inclined to accept this if we kept the three states, but made the default "allow"? That wouldstill allow people to bump it stronger manually, but would have no effect on everyone else. That would give us timeto tweak the wording and/or examine other approaches. Although any other approaches would still leave the need to dosomething with passwords via ALTER USER / CREATE USER in the interim. I mean, I do think that is probably a better idea, but I personally have zero intention of committing this patch regardless. I have seen a lot of problems in this area working at EDB and my educated guess is that this solves 0% of them. Now, if enough other people show up to say "but this would solve 100% of my problems," well then fair enough. But I think it's entirely reasonable for me to look at the combination of "this is a class of problem that affects me" and "this proposed solution would not help me" and be skeptical. I think you'd feel the same if the situation were reversed. If I came along and proposed some solution to a PG problem and you agreed that the problem was a problem but my proposed solution seemed useless, I assume you'd also -1 that patch. -- Robert Haas EDB: http://www.enterprisedb.com
On Wed, Mar 19, 2025 at 10:06:58AM -0400, Robert Haas wrote: > On Wed, Mar 19, 2025 at 9:24 AM Greg Sabino Mullane <htamfids@gmail.com> wrote: >>> The user has no particular reason to care about the fact that the >>> password they just typed ended up in the log. That is a concern for the >>> DBA, not the user, and even if they care about the DBA's feelings, they >>> only get the warning after it's too late to do otherwise. >> >> Can't the same be said about other warnings, esp. md5? > > Absolutely. Warnings are sometimes the right thing, but they often > suck. If something is really a bad idea, "ERROR: bad idea" is vastly > superior to "WARNING: what you just already did was a bad idea". If we > don't actually know for sure that it's a bad idea, then it's generally > better not to emit a warning at all, for fear of log-spamming people > who know what they're doing. FWIW I primarily intended the MD5 password warning to alert folks that the ability to use MD5 passwords will go away at some point in the future. If they want to continue to use MD5 passwords for now, they are free to do so. They can even turn off the warnings. One of the main reasons I'm not totally sold on a clear-text password warning is because we don't have agreement on removing that ability anytime soon, not to mention Bruce's point about the debate extending into mid-March. -- nathan