md5_password_warnings for password auth with MD5-encrypted passwords
Hi,
While testing md5_password_warnings, I noticed that authentication
with an MD5-encrypted password emits the expected warning when the HBA
method is md5, but not when it is password.
Was this intentional, or just an oversight?
I couldn't find any discussion about this, so I put together the
attached patch. It updates the authentication code to emit the same
MD5 deprecation connection warning after successful password
authentication when the stored password is MD5-encrypted.
Thoughts?
Regards,
--
Fujii Masao
Attachments:
v1-0001-Warn-on-password-auth-with-MD5-encrypted-password.patchapplication/octet-stream; name=v1-0001-Warn-on-password-auth-with-MD5-encrypted-password.patchDownload+45-24
On Jun 23, 2026, at 09:39, Fujii Masao <masao.fujii@gmail.com> wrote:
Hi,
While testing md5_password_warnings, I noticed that authentication
with an MD5-encrypted password emits the expected warning when the HBA
method is md5, but not when it is password.Was this intentional, or just an oversight?
I couldn't find any discussion about this, so I put together the
attached patch. It updates the authentication code to emit the same
MD5 deprecation connection warning after successful password
authentication when the stored password is MD5-encrypted.Thoughts?
Regards,
--
Fujii Masao
<v1-0001-Warn-on-password-auth-with-MD5-encrypted-password.patch>
Given that the original warning emission was in md5_crypt_verify(), I think it might be a bit better to keep the two private helpers in crypt.c and add the warning emission in plain_crypt_verify(), because that function has already determined the password type and authentication result.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Tue, 23 Jun 2026 at 10:44, Chao Li <li.evan.chao@gmail.com> wrote:
On Jun 23, 2026, at 09:39, Fujii Masao <masao.fujii@gmail.com> wrote:
Hi,
While testing md5_password_warnings, I noticed that authentication
with an MD5-encrypted password emits the expected warning when the HBA
method is md5, but not when it is password.Was this intentional, or just an oversight?
I couldn't find any discussion about this, so I put together the
attached patch. It updates the authentication code to emit the same
MD5 deprecation connection warning after successful password
authentication when the stored password is MD5-encrypted.Thoughts?
Regards,
--
Fujii Masao
<v1-0001-Warn-on-password-auth-with-MD5-encrypted-password.patch>Given that the original warning emission was in md5_crypt_verify(), I
think it might be a bit better to keep the two private helpers in
crypt.c and add the warning emission in plain_crypt_verify(), because
that function has already determined the password type and
authentication result.
+1
Placing it in plain_crypt_verify() leverages the already-determined type and
result, while keeping the helpers internal to crypt.c is cleaner.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
On Tue, Jun 23, 2026 at 12:17 PM Japin Li <japinli@hotmail.com> wrote:
--
Fujii Masao
<v1-0001-Warn-on-password-auth-with-MD5-encrypted-password.patch>Given that the original warning emission was in md5_crypt_verify(), I
think it might be a bit better to keep the two private helpers in
crypt.c and add the warning emission in plain_crypt_verify(), because
that function has already determined the password type and
authentication result.+1
Placing it in plain_crypt_verify() leverages the already-determined type and
result, while keeping the helpers internal to crypt.c is cleaner.
plain_crypt_verify() looks like a generic helper to me. It's used not
only for connection authentication, but also by CREATE/ALTER ROLE and
passwordcheck. So I'm not sure it's clearer to add a
connection-authentication-specific side effect there.
Also, plain_crypt_verify() can be called after
EmitConnectionWarnings(), for example, by CREATE/ALTER ROLE.
So if plain_crypt_verify() were updated to simply call
StoreConnectionWarning() for MD5 password warnings, it could end up
calling StoreConnectionWarning() after EmitConnectionWarnings(),
resulting in the unexpected error
"StoreConnectionWarning() called after EmitConnectionWarnings()".
Regards,
--
Fujii Masao
Hello,
At Tue, 23 Jun 2026 10:39:42 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in
While testing md5_password_warnings, I noticed that authentication
with an MD5-encrypted password emits the expected warning when the HBA
method is md5, but not when it is password.Was this intentional, or just an oversight?
I couldn't find any discussion about this, so I put together the
attached patch. It updates the authentication code to emit the same
MD5 deprecation connection warning after successful password
authentication when the stored password is MD5-encrypted.
The current behavior seems reasonable to me. The warning is intended
to discourage the use of MD5 password authentication and MD5 password
storage. In the password authentication case, the authentication
protocol itself does not use MD5, and MD5 password storage is already
warned about when the verifier is created. Therefore I don't see a
strong reason to emit an additional warning during
authentication. Also, given that MD5 password authentication is
already deprecated and expected to be removed eventually, I don't see
a strong reason to change this behavior now.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Mon, Jun 22, 2026 at 9:49 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
In the password authentication case, the authentication
protocol itself does not use MD5, and MD5 password storage is already
warned about when the verifier is created.
Presumably the verifier was created a while back, though, in the case
of an upgrade. Personally I think it makes sense to warn whenever the
MD5 hash is used to authenticate.
No opinion on the patch implementation, though (cc'd Nathan who might?).
--Jacob
On Wed, Jun 24, 2026 at 07:43:21AM -0700, Jacob Champion wrote:
Presumably the verifier was created a while back, though, in the case
of an upgrade. Personally I think it makes sense to warn whenever the
MD5 hash is used to authenticate.
I'm fine with expanding the warnings to "password" auth. Something else I
considered is whether we should warn if a role with an MD5 password is
authenticated without using the password. I ultimately decided against
that, but we could expand it there, too.
No opinion on the patch implementation, though (cc'd Nathan who might?).
TBH I'm not too opinionated here, if for no other reason than all this code
should be getting deleted in the next couple of years.
--
nathan
On Fri, Jun 26, 2026 at 01:18:55PM -0500, Nathan Bossart wrote:
On Wed, Jun 24, 2026 at 07:43:21AM -0700, Jacob Champion wrote:
Presumably the verifier was created a while back, though, in the case
of an upgrade. Personally I think it makes sense to warn whenever the
MD5 hash is used to authenticate.I'm fine with expanding the warnings to "password" auth. Something else I
considered is whether we should warn if a role with an MD5 password is
authenticated without using the password. I ultimately decided against
that, but we could expand it there, too.
FWIW, emitting a WARNING for the "password" case would make more
people aware that they're doing something wrong. Folks having the
idea to allow plain password authentication should be hopefully next
to nothing these days (mostly self-managed), but the more warned the
less potential pain when this gets removed.
In short, I think that Fujii-san's patch is a good thing to have.
No opinion on the patch implementation, though (cc'd Nathan who might?).
TBH I'm not too opinionated here, if for no other reason than all this code
should be getting deleted in the next couple of years.
v20 perhaps? Let's be on the very optimistic side of the spectrum. :p
--
Michael
On Mon, Jun 29, 2026 at 12:00:31PM +0900, Michael Paquier wrote:
On Fri, Jun 26, 2026 at 01:18:55PM -0500, Nathan Bossart wrote:
TBH I'm not too opinionated here, if for no other reason than all this code
should be getting deleted in the next couple of years.v20 perhaps? Let's be on the very optimistic side of the spectrum. :p
My plan was to allow a couple more years for word to spread and warnings to
be seen, and then to remove it in ~v22. If there's consensus to do it
earlier, I'm happy to do so, but I lean towards extreme caution with the
timeline.
--
nathan
On Mon, Jun 29, 2026 at 12:00 PM Michael Paquier <michael@paquier.xyz> wrote:
I'm fine with expanding the warnings to "password" auth. Something else I
considered is whether we should warn if a role with an MD5 password is
authenticated without using the password. I ultimately decided against
that, but we could expand it there, too.
Agreed with that decision, i.e., I don't think we should warn in those cases
(e.g., a role with an MD5 password being authenticated via trust). The MD5
verifier isn't used for authentication there, and emitting the warning would
require an extra lookup of the role's password solely to generate the warning.
That seems like unnecessary overhead.
FWIW, emitting a WARNING for the "password" case would make more
people aware that they're doing something wrong. Folks having the
idea to allow plain password authentication should be hopefully next
to nothing these days (mostly self-managed), but the more warned the
less potential pain when this gets removed.
+1
In short, I think that Fujii-san's patch is a good thing to have.
Thanks! So I'm thinking to commit the patch.
Regards,
--
Fujii Masao
On Tue, Jun 30, 2026 at 9:38 AM Fujii Masao <masao.fujii@gmail.com> wrote:
In short, I think that Fujii-san's patch is a good thing to have.
Thanks! So I'm thinking to commit the patch.
I've pushed the patch. Thanks!
Regards,
--
Fujii Masao