Thread: Re: Add a warning message when using unencrypted passwords

Re: Add a warning message when using unencrypted passwords

From
Jelte Fennema-Nio
Date:
On Sat, 7 Dec 2024 at 15:40, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> > Whenever log_statement is set to all (which I understand should be done for a short period of time for
troubleshootingpurposes only), if we change the password for a user, or create a new user, the passwords would be
loggedin plain text. From a security point of view, this should not be allowed. Ideally, It should error out (or at
leastthrow 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
forthe idea! 

+1 for more clearly letting people know that what they're doing is not
recommended from a security standpoint.

Regarding warning vs error, I agree that a WARNING is probably the
right choice generally. But I think that Divya is correct: When
log_statement = 'all', an error should be thrown instead. Because when
that is the case, we know for sure that the password will be leaked to
the logs. And that error should contain something like: You should
consider this password compromised.

Throwing an error always actually has an interesting downside: We then
automatically log the statement, and thus the password to the log.
When I change the level to ERROR in your code, I get the following
(but with WARNING the STATEMENT line is not there):

2024-12-08 22:59:50.967 CET [104900] ERROR:  using a plaintext
password in a query
2024-12-08 22:59:50.967 CET [104900] DETAIL:  plaintext password may be logged.
2024-12-08 22:59:50.967 CET [104900] HINT:  Refer to the PostgreSQL
documentation for details about using encrypted password in queries.
2024-12-08 22:59:50.967 CET [104900] STATEMENT:  ALTER ROLE jelte
PASSWORD 'abc';

PS. I created a commit fest entry here:
https://commitfest.postgresql.org/51/5426/



Re: Add a warning message when using unencrypted passwords

From
Greg Sabino Mullane
Date:
Overall +1 to the idea of a warning.

Regarding warning vs error, I agree that a WARNING is probably the right choice generally. But I think that Divya is correct: When
log_statement = 'all', an error should be thrown instead.

First, it should be for 'all' AND 'ddl'. And obviously glossing over log_min_duration_statement entirely. But -1 to throwing an ERROR - that's not really an error, and not our call to make, so a WARNING is sufficient.

Cheers,
Greg

Re: Add a warning message when using unencrypted passwords

From
Daniel Gustafsson
Date:
> On 9 Dec 2024, at 14:26, Greg Sabino Mullane <htamfids@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.

--
Daniel Gustafsson




Re: Add a warning message when using unencrypted passwords

From
Guillaume Lelarge
Date:
Hi,

Le lun. 9 déc. 2024 à 14:40, Daniel Gustafsson <daniel@yesql.se> a écrit :
> On 9 Dec 2024, at 14:26, Greg Sabino Mullane <htamfids@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?


--
Guillaume.

Re: Add a warning message when using unencrypted passwords

From
Tom Lane
Date:
Guillaume Lelarge <guillaume.lelarge@dalibo.com> writes:
> v2 is attached.

This seems pretty much entirely useless to me.  The password
has already been leaked to the log (*and* the network, if
session is unencrypted), so what's the point of a warning?
And as already noted, this ignores several other hazards of
the same sort, so it's more likely to create a false sense of
security than anything else.

(In addition to the points noted, what of event triggers?
Or ~/.psql_history?)

            regards, tom lane



Re: Add a warning message when using unencrypted passwords

From
Guillaume Lelarge
Date:
On 04/02/2025 17:59, Tom Lane wrote:
> Guillaume Lelarge <guillaume.lelarge@dalibo.com> writes:
>> v2 is attached.
> 
> This seems pretty much entirely useless to me.  The password
> has already been leaked to the log (*and* the network, if
> session is unencrypted), so what's the point of a warning?
> And as already noted, this ignores several other hazards of
> the same sort, so it's more likely to create a false sense of
> security than anything else.
> 
> (In addition to the points noted, what of event triggers?
> Or ~/.psql_history?)
> 

I agree that the warning itself doesn't make the password secure. But it 
never pretends to do that. If I, as a user, see a message like this, my 
next move will be to search for a way to change my password in a secure way.

Warning users won't save everyone, but it may help some. Doing nothing 
helps no one.


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