add warning upon successful md5 password auth

At 2026-02-12 03:52:33, "Nathan Bossart" <nathandbossart@gmail.com> wrote:

From a related discussion last year [0]:

On Tue, Jun 03, 2025 at 12:09:50PM -0500, Nathan Bossart wrote:

On Tue, Jun 03, 2025 at 09:43:59AM -0500, Nathan Bossart wrote:

On Tue, Jun 03, 2025 at 10:34:06AM -0400, Tom Lane wrote:

If we really want to be in peoples' face about this, the thing
to do is to print a warning every time they log in with an MD5
password. Also, to Michael's point, that really would be exactly
the same place where the eventual "sorry, not supported anymore"
message will be.

I held off on this because I was worried it might be far too noisy. That
does seem like it has the best chance of getting folks' attention, though.
If it's too noisy, users can always turn off the warnings.

Here is a draft-grade patch that adds a WARNING upon successful
authentication with an MD5 password. It's a little hacky because AFAICT we
need to wait until well after authentication (for GUCs to be set up, etc.)
before we actually emit the WARNING. When the time comes to remove MD5
password support completely, we'll need to do something like modify
CheckMD5Auth() to always return STATUS_ERROR with an appropriate logdetail
message.

Since I just added a "connection warnings" infrastructure in commit
1d92e0c2cc, I thought it might be a good time to revisit this idea.
Attached is an updated patch. I'm not sure this is v19 material. It could
make sense to wait until v20 or something. But I figured it was worth at
least having the discussion.

[0] /messages/by-id/aD8sXgfJeIGLc7-t@nathan

--

nathan

This looks like a solid patch. I’ve taken a look and don’t have any comments.
I applied it locally and the build went through without any issues.
I also ran the new TAP test case, and everything looks good on my side.

Regards,
Xiangyu Liang

On 2/11/26 8:52 PM, Nathan Bossart wrote:

Since I just added a "connection warnings" infrastructure in commit
1d92e0c2cc, I thought it might be a good time to revisit this idea.
Attached is an updated patch. I'm not sure this is v19 material. It could
make sense to wait until v20 or something. But I figured it was worth at
least having the discussion.

The patch looks good and I think it would make sense to merge it in 19,
why wait for 20? But the main question I see is if this is too noisy or
not. Some applications connected to PostgreSQL quite a lot and I am sure
we would make some users unhappy so I am not fully on board with this
patch. But on the other hand we have way too many people who still use
md5 and we really should push them towards using scram.

Andreas

On Fri, Feb 13, 2026 at 06:04:14AM +0100, Andreas Karlsson wrote:

The patch looks good and I think it would make sense to merge it in 19, why
wait for 20? But the main question I see is if this is too noisy or not.
Some applications connected to PostgreSQL quite a lot and I am sure we would
make some users unhappy so I am not fully on board with this patch. But on
the other hand we have way too many people who still use md5 and we really
should push them towards using scram.

FWIW if users are really annoyed with these warnings, they can disable them
by setting md5_password_warnings to off. But I think we really ought to do
something like $subject before we completely remove MD5 password support.

--
nathan

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Feb 13, 2026 at 06:04:14AM +0100, Andreas Karlsson wrote:

The patch looks good and I think it would make sense to merge it in 19, why
wait for 20? But the main question I see is if this is too noisy or not.
Some applications connected to PostgreSQL quite a lot and I am sure we would
make some users unhappy so I am not fully on board with this patch. But on
the other hand we have way too many people who still use md5 and we really
should push them towards using scram.

FWIW if users are really annoyed with these warnings, they can disable them
by setting md5_password_warnings to off. But I think we really ought to do
something like $subject before we completely remove MD5 password support.

+1. We need something like this to be there for at least a year or
two before we can consider removing MD5 passwords entirely. As long
as the warnings can be turned off, I think it's all right and indeed
necessary to have them on-by-default.

regards, tom lane

PS: I've not read the patch, so this isn't an endorsement of details.

On 2/13/26 6:26 PM, Nathan Bossart wrote:

On Fri, Feb 13, 2026 at 06:04:14AM +0100, Andreas Karlsson wrote:

The patch looks good and I think it would make sense to merge it in 19, why
wait for 20? But the main question I see is if this is too noisy or not.
Some applications connected to PostgreSQL quite a lot and I am sure we would
make some users unhappy so I am not fully on board with this patch. But on
the other hand we have way too many people who still use md5 and we really
should push them towards using scram.

FWIW if users are really annoyed with these warnings, they can disable them
by setting md5_password_warnings to off. But I think we really ought to do
something like $subject before we completely remove MD5 password support.

After thinking more on the subject I have come around. I think warning
spam (that can be disabled) is fine and why not introduce it directly in 19?

As for the patch itself I think it looks good, but I am not a fan of the
test code. Why not simply write like the below?

test_conn($node, 'user=md5_role', 'md5', 0,
log_like =>
[qr/connection authenticated: identity="md5_role" method=md5/],
expected_stderr =>
[qr/authenticated with an MD5-encrypted password/])

Andreas

On Tue, Feb 17, 2026 at 07:08:17AM +0100, Andreas Karlsson wrote:

After thinking more on the subject I have come around. I think warning spam
(that can be disabled) is fine and why not introduce it directly in 19?

WFM

As for the patch itself I think it looks good, but I am not a fan of the
test code. Why not simply write like the below?

test_conn($node, 'user=md5_role', 'md5', 0,
log_like =>
[qr/connection authenticated: identity="md5_role" method=md5/],
expected_stderr =>
[qr/authenticated with an MD5-encrypted password/])

No good reason. I've updated the patch.

--
nathan

On 2/17/26 6:05 PM, Nathan Bossart wrote:

On Tue, Feb 17, 2026 at 07:08:17AM +0100, Andreas Karlsson wrote:

As for the patch itself I think it looks good, but I am not a fan of the
test code. Why not simply write like the below?

test_conn($node, 'user=md5_role', 'md5', 0,
log_like =>
[qr/connection authenticated: identity="md5_role" method=md5/],
expected_stderr =>
[qr/authenticated with an MD5-encrypted password/])

No good reason. I've updated the patch.

Nice, the patch looks good as-is for me now.

Andreas

On Tue, Feb 17, 2026 at 06:11:45PM +0100, Andreas Karlsson wrote:

Nice, the patch looks good as-is for me now.

Thanks for reviewing. I'm going to let this sit for a little while longer
in case anyone else has feedback, but my current plan is to commit it for
v19.

--
nathan

Committed.

--
nathan