public inbox for [email protected]  
help / color / mirror / Atom feed
Re: Proposal: Support custom authentication methods using hooks
26+ messages / 13 participants
[nested] [flat]

* Re: Proposal: Support custom authentication methods using hooks
@ 2022-02-25 19:10 Tom Lane <[email protected]>
  2022-02-25 19:49 ` Re: Proposal: Support custom authentication methods using hooks Andres Freund <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Tom Lane @ 2022-02-25 19:10 UTC (permalink / raw)
  To: Jeff Davis <[email protected]>; +Cc: samay sharma <[email protected]>; [email protected]

Jeff Davis <[email protected]> writes:
> On Fri, 2022-02-25 at 12:39 -0500, Tom Lane wrote:
>> My point is that sending cleartext passwords over the wire is an
>> insecure-by-definition protocol that we shouldn't be encouraging
>> more use of.

> We can require custom auth entries in pg_hba.conf to also specify
> local, hostssl or hostgssenc.

That's just a band-aid, though.  It does nothing for the other
reasons not to want cleartext passwords, notably that you might
be giving your password to a compromised server.

I'm happy to add support for custom auth methods if they can use
a protocol that's safer than cleartext-password.  But if that's the
only feasible option, then we're just encouraging people to use
insecure methods.

I also have in mind here that there has been discussion of giving
libpq a feature to refuse, on the client side, to send cleartext
passwords.  (I don't recall right now if that's been mentioned
publicly or only among the security team, but it's definitely
under consideration.)

So I think this proposal needs more thought.  A server-side hook
alone is not a useful improvement IMO; it's closer to being an
attractive nuisance.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
@ 2022-02-25 19:49 ` Andres Freund <[email protected]>
  1 sibling, 0 replies; 26+ messages in thread

From: Andres Freund @ 2022-02-25 19:49 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Hi,

On 2022-02-25 14:10:39 -0500, Tom Lane wrote:
> I'm happy to add support for custom auth methods if they can use
> a protocol that's safer than cleartext-password.  But if that's the
> only feasible option, then we're just encouraging people to use
> insecure methods.

It looks like scram can be used without much trouble. All the necessary
infrastructure to implement it without duplication appears to be public.  The
plugin would need to get a secret from whereever and call
  CheckSASLAuth(&pg_be_scram_mech, port, shadow_pass, logdetail);

or if it needs to do something more complicated than CheckSASLAuth(), it can
use AUTH_REQ_SASL{,_FIN CONT} itself.

Of course whether it's viable depends on what the custom auth method wants to
do. But it's not a restriction of the authentication plugin model.

Greetings,

Andres Freund






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
@ 2022-02-27 23:17 ` Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Jeff Davis @ 2022-02-27 23:17 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: samay sharma <[email protected]>; [email protected]

On Fri, 2022-02-25 at 14:10 -0500, Tom Lane wrote:
> I'm happy to add support for custom auth methods if they can use
> a protocol that's safer than cleartext-password.  But if that's the
> only feasible option, then we're just encouraging people to use
> insecure methods.

FWIW, I'd like to be able to use a token in the password field, and
then authenticate that token. So I didn't intend to send an actual
password in plaintext.

Maybe we should have a new "token" connection parameter so that libpq
can allow sending a token plaintext but refuse to send a password in
plaintext?

> I also have in mind here that there has been discussion of giving
> libpq a feature to refuse, on the client side, to send cleartext
> passwords.

I am generally in favor of that idea, but I'm not sure that will
completely resolve the issue. For instance, should it also refuse MD5?

Regards,
	Jeff Davis








^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
@ 2022-02-28 20:46   ` Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-03-01 21:14     ` Re: Proposal: Support custom authentication methods using hooks Nathan Bossart <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Stephen Frost @ 2022-02-28 20:46 UTC (permalink / raw)
  To: Jeff Davis <[email protected]>; +Cc: Tom Lane <[email protected]>; samay sharma <[email protected]>; [email protected]

Greetings,

* Jeff Davis ([email protected]) wrote:
> On Fri, 2022-02-25 at 14:10 -0500, Tom Lane wrote:
> > I also have in mind here that there has been discussion of giving
> > libpq a feature to refuse, on the client side, to send cleartext
> > passwords.
> 
> I am generally in favor of that idea, but I'm not sure that will
> completely resolve the issue. For instance, should it also refuse MD5?

md5 should be removed.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-02-28 21:18     ` Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Tom Lane @ 2022-02-28 21:18 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; +Cc: Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Stephen Frost <[email protected]> writes:
> md5 should be removed.

Really? I've always thought that the arguments against it were
overblown for our use-case.  At any rate, it's likely to be
years before we could practically do that, since it's the best
that older client libraries can manage.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
@ 2022-02-28 21:42       ` Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-03 06:15         ` Re: Proposal: Support custom authentication methods using hooks Tatsuo Ishii <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Stephen Frost @ 2022-02-28 21:42 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Greetings,

* Tom Lane ([email protected]) wrote:
> Stephen Frost <[email protected]> writes:
> > md5 should be removed.
> 
> Really? I've always thought that the arguments against it were
> overblown for our use-case.  At any rate, it's likely to be
> years before we could practically do that, since it's the best
> that older client libraries can manage.

Yes, really, it's a known-broken system which suffers from such an old
and well known attack that it's been given a name: pass-the-hash.  As
was discussed on this thread even, just the fact that it's not trivial
to break on the wire doesn't make it not-broken, particularly when we
use the username (which is rather commonly the same one used across
multiple systems..) as the salt.  Worse, md5 isn't exactly the pinnacle
of hashing techniques around these days.

The wiki page goes over it in some detail regarding LM/NTLM which
suffers the same problem (and also uses a challenge-response for the
over-the-network bits): https://en.wikipedia.org/wiki/Pass_the_hash

Further, a whole bunch of effort was put in to get scram support added
to the different libraries and language bindings and such, specifically
to allow us to get to a point where we can drop md5.  Even after it's
removed, folks will have 5 years before the release that removes it is
the oldest supported release.  I don't think we'll somehow get agreement
to remove it for v15, so it'll be 5 major versions of overlap (11->15)
by the time v16 comes out, and a total of 10 years of support for scram
before md5 is gone.

That's plenty, it's time to move on.

Keeping it around will just push out the point at which everyone will
finally be done with it, as there's really only two groups: those who
have already moved to scram, and those who won't move until they want to
upgrade to a release that doesn't have md5.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-01 06:26         ` Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Michael Paquier @ 2022-03-01 06:26 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On Mon, Feb 28, 2022 at 04:42:55PM -0500, Stephen Frost wrote:
> Keeping it around will just push out the point at which everyone will
> finally be done with it, as there's really only two groups: those who
> have already moved to scram, and those who won't move until they want to
> upgrade to a release that doesn't have md5.

FWIW, I am not sure if we are at this point yet.  An extra reason to
remove it would be that it is a support burden, but I don't have seen
in recent memory any problems related to it that required any deep
changes in the way to use it, and its code paths are independent.

The last time I played with this area is the recent error handling
improvement with cryptohashes but MD5 has actually helped here in
detecting the problem as a patched OpenSSL would complain if trying to
use MD5 as hash function when FIPS is enabled.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
@ 2022-03-01 13:31           ` Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 14:55             ` Re: Proposal: Support custom authentication methods using hooks Bruce Momjian <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Stephen Frost @ 2022-03-01 13:31 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Greetings,

* Michael Paquier ([email protected]) wrote:
> On Mon, Feb 28, 2022 at 04:42:55PM -0500, Stephen Frost wrote:
> > Keeping it around will just push out the point at which everyone will
> > finally be done with it, as there's really only two groups: those who
> > have already moved to scram, and those who won't move until they want to
> > upgrade to a release that doesn't have md5.
> 
> FWIW, I am not sure if we are at this point yet.  An extra reason to
> remove it would be that it is a support burden, but I don't have seen
> in recent memory any problems related to it that required any deep
> changes in the way to use it, and its code paths are independent.

Ongoing reports that there's a known vulnerability aren't great to have
to deal with.  We can at least point people to scram but that's not
great.

> The last time I played with this area is the recent error handling
> improvement with cryptohashes but MD5 has actually helped here in
> detecting the problem as a patched OpenSSL would complain if trying to
> use MD5 as hash function when FIPS is enabled.

Having to continue to deal with md5 as an algorithm when it's known to
be notably less secure and so much so that organizations essentially ban
its use for exactly what we're using it for, in fact, another reason to
remove it, not a reason to keep it.  Better code coverage testing of
error paths is the answer to making sure that our error handling behaves
properly.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-01 21:17             ` Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Jonathan S. Katz @ 2022-03-01 21:17 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On 3/1/22 8:31 AM, Stephen Frost wrote:
> Greetings,
> 
> * Michael Paquier ([email protected]) wrote:
>> On Mon, Feb 28, 2022 at 04:42:55PM -0500, Stephen Frost wrote:
>>> Keeping it around will just push out the point at which everyone will
>>> finally be done with it, as there's really only two groups: those who
>>> have already moved to scram, and those who won't move until they want to
>>> upgrade to a release that doesn't have md5.
>>
>> FWIW, I am not sure if we are at this point yet.  An extra reason to
>> remove it would be that it is a support burden, but I don't have seen
>> in recent memory any problems related to it that required any deep
>> changes in the way to use it, and its code paths are independent.
> 
> Ongoing reports that there's a known vulnerability aren't great to have
> to deal with.  We can at least point people to scram but that's not
> great.
> 
>> The last time I played with this area is the recent error handling
>> improvement with cryptohashes but MD5 has actually helped here in
>> detecting the problem as a patched OpenSSL would complain if trying to
>> use MD5 as hash function when FIPS is enabled.
> 
> Having to continue to deal with md5 as an algorithm when it's known to
> be notably less secure and so much so that organizations essentially ban
> its use for exactly what we're using it for, in fact, another reason to
> remove it, not a reason to keep it.  Better code coverage testing of
> error paths is the answer to making sure that our error handling behaves
> properly.

So, I generally agree with Stephen and his arguments for dropping the 
md5 mechanism.

If you're moving to a newer version of PostgreSQL, you likely have to 
update your connection drivers anyway (rebuilt against new libpq, 
supporting any changes in the protocol, etc). I would prefer more data 
to support that argument, but this is generally what you need to do.

However, we may need to step towards it. We took one step last release 
with defaulting to SCRAM. Perhaps this release we add a warning for 
anything using md5 auth that "this will be removed in a future release." 
(or specifically v16). We should also indicate in the docs that md5 is 
deprecated and will be removed.

We also need to think about upgrading: what happens if I try to upgrade 
and I still have user passwords stored in a md5 hash? What if all my 
password-based users have a md5 hash, does pg_upgrade fail?

As much as I want md5 gone, I think v16 is the earliest we can do it, 
but we can lay the groundwork for it now.

Thanks,

Jonathan


Attachments:

  [application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
@ 2022-03-02 08:24               ` Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Peter Eisentraut @ 2022-03-02 08:24 UTC (permalink / raw)
  To: Jonathan S. Katz <[email protected]>; Stephen Frost <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On 01.03.22 22:17, Jonathan S. Katz wrote:
> If you're moving to a newer version of PostgreSQL, you likely have to 
> update your connection drivers anyway (rebuilt against new libpq, 
> supporting any changes in the protocol, etc). I would prefer more data 
> to support that argument, but this is generally what you need to do.
> 
> However, we may need to step towards it. We took one step last release 
> with defaulting to SCRAM. Perhaps this release we add a warning for 
> anything using md5 auth that "this will be removed in a future release." 
> (or specifically v16). We should also indicate in the docs that md5 is 
> deprecated and will be removed.

I find that a lot of people are still purposely using md5.  Removing it 
now or in a year would be quite a disruption.

It's also worth considering that keeping the code equipped to handle 
different kinds of password hashing would help it stay in shape if we 
ever need to add support for the next SHA after 256 or whatever.







^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
@ 2022-03-02 14:16                 ` Jonathan S. Katz <[email protected]>
  2022-03-02 15:29                   ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-03 09:59                   ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Jonathan S. Katz @ 2022-03-02 14:16 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; Stephen Frost <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On 3/2/22 3:24 AM, Peter Eisentraut wrote:
> On 01.03.22 22:17, Jonathan S. Katz wrote:
>> If you're moving to a newer version of PostgreSQL, you likely have to 
>> update your connection drivers anyway (rebuilt against new libpq, 
>> supporting any changes in the protocol, etc). I would prefer more data 
>> to support that argument, but this is generally what you need to do.
>>
>> However, we may need to step towards it. We took one step last release 
>> with defaulting to SCRAM. Perhaps this release we add a warning for 
>> anything using md5 auth that "this will be removed in a future 
>> release." (or specifically v16). We should also indicate in the docs 
>> that md5 is deprecated and will be removed.
> 
> I find that a lot of people are still purposely using md5.  Removing it 
> now or in a year would be quite a disruption.

What are the reasons they are still purposely using it? The ones I have 
seen/heard are:

- Using an older driver
- On a pre-v10 PG
- Unaware of SCRAM

What I'm proposing above is to start the process of deprecating it as an 
auth method, which also allows to continue the education efforts to 
upgrae. Does that make sense?

> It's also worth considering that keeping the code equipped to handle 
> different kinds of password hashing would help it stay in shape if we 
> ever need to add support for the next SHA after 256 or whatever.

I think it's fine to keep the hashing code. The end goal is to remove 
the md5 authentication mechanism.

Thanks,

Jonathan




Attachments:

  [application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
@ 2022-03-02 15:29                   ` Peter Eisentraut <[email protected]>
  2022-03-02 15:30                     ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Peter Eisentraut @ 2022-03-02 15:29 UTC (permalink / raw)
  To: Jonathan S. Katz <[email protected]>; Stephen Frost <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]


On 02.03.22 15:16, Jonathan S. Katz wrote:
>> I find that a lot of people are still purposely using md5.  Removing 
>> it now or in a year would be quite a disruption.
> 
> What are the reasons they are still purposely using it? The ones I have 
> seen/heard are:
> 
> - Using an older driver
> - On a pre-v10 PG
> - Unaware of SCRAM

I'm not really sure, but it seems like they are content with what they 
have and don't want to bother with the new fancy stuff.

> What I'm proposing above is to start the process of deprecating it as an 
> auth method, which also allows to continue the education efforts to 
> upgrae. Does that make sense?

I'm not in favor of starting a process that will result in removal of 
the md5 method at this time.






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 15:29                   ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
@ 2022-03-02 15:30                     ` Stephen Frost <[email protected]>
  2022-03-02 15:45                       ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Stephen Frost @ 2022-03-02 15:30 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Greetings,

* Peter Eisentraut ([email protected]) wrote:
> On 02.03.22 15:16, Jonathan S. Katz wrote:
> >>I find that a lot of people are still purposely using md5.  Removing it
> >>now or in a year would be quite a disruption.
> >
> >What are the reasons they are still purposely using it? The ones I have
> >seen/heard are:
> >
> >- Using an older driver
> >- On a pre-v10 PG
> >- Unaware of SCRAM
> 
> I'm not really sure, but it seems like they are content with what they have
> and don't want to bother with the new fancy stuff.

There were lots and lots of folks who were comfortable with
recovery.conf, yet we removed that without any qualms from one major
version to the next.  md5 will have had 5 years of overlap with scram.

> >What I'm proposing above is to start the process of deprecating it as an
> >auth method, which also allows to continue the education efforts to
> >upgrae. Does that make sense?
> 
> I'm not in favor of starting a process that will result in removal of the
> md5 method at this time.

I am.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 15:29                   ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 15:30                     ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-02 15:45                       ` Jonathan S. Katz <[email protected]>
  2022-03-03 09:45                         ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Jonathan S. Katz @ 2022-03-02 15:45 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On 3/2/22 10:30 AM, Stephen Frost wrote:
> Greetings,
> 
> * Peter Eisentraut ([email protected]) wrote:
>> On 02.03.22 15:16, Jonathan S. Katz wrote:
>>>> I find that a lot of people are still purposely using md5.  Removing it
>>>> now or in a year would be quite a disruption.
>>>
>>> What are the reasons they are still purposely using it? The ones I have
>>> seen/heard are:
>>>
>>> - Using an older driver
>>> - On a pre-v10 PG
>>> - Unaware of SCRAM
>>
>> I'm not really sure, but it seems like they are content with what they have
>> and don't want to bother with the new fancy stuff.

By that argument, we should have kept "password" (plain) as an 
authentication method.

The specific use-cases I've presented are all solvable issues. The 
biggest challenging with existing users is the upgrade process, which is 
why I'd rather we begin a deprecation process and see if there are any 
ways we can make the md5 => SCRAM transition easier.

> There were lots and lots of folks who were comfortable with
> recovery.conf, yet we removed that without any qualms from one major
> version to the next.  md5 will have had 5 years of overlap with scram.

I do agree with Stephen in principle here. I encountered upgrade 
challenges in this an challenge with updating automation to handle this 
change.

>>> What I'm proposing above is to start the process of deprecating it as an
>>> auth method, which also allows to continue the education efforts to
>>> upgrae. Does that make sense?
>>
>> I'm not in favor of starting a process that will result in removal of the
>> md5 method at this time.
> 
> I am.

+1 for starting this process. It may still take a few more years, but we 
should help our users to move away from an auth method with known issues.

Thanks,

Jonathan


Attachments:

  [application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 15:29                   ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 15:30                     ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-02 15:45                       ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
@ 2022-03-03 09:45                         ` Peter Eisentraut <[email protected]>
  2022-03-03 16:12                           ` Re: Proposal: Support custom authentication methods using hooks Joshua Brindle <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Peter Eisentraut @ 2022-03-03 09:45 UTC (permalink / raw)
  To: Jonathan S. Katz <[email protected]>; Stephen Frost <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On 02.03.22 16:45, Jonathan S. Katz wrote:
> By that argument, we should have kept "password" (plain) as an 
> authentication method.

For comparison, the time between adding md5 and removing password was 16 
years.  It has been 5 years since scram was added.






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 15:29                   ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 15:30                     ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-02 15:45                       ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-03 09:45                         ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
@ 2022-03-03 16:12                           ` Joshua Brindle <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Joshua Brindle @ 2022-03-03 16:12 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Stephen Frost <[email protected]>; Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On Thu, Mar 3, 2022 at 4:45 AM Peter Eisentraut
<[email protected]> wrote:
>
> On 02.03.22 16:45, Jonathan S. Katz wrote:
> > By that argument, we should have kept "password" (plain) as an
> > authentication method.
>
> For comparison, the time between adding md5 and removing password was 16
> years.  It has been 5 years since scram was added.

It's been 7 years since this thread:
https://www.postgresql.org/message-id/[email protected]

As Jonathan and Stephen and others have said, anyone who wishes to
continue using MD5 or other plaintext methods can keep doing that for
5 more years with a supported version of PG. There is no excuse to
leave well known, flawed mechanisms in PG16.






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 21:17             ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
  2022-03-02 08:24               ` Re: Proposal: Support custom authentication methods using hooks Peter Eisentraut <[email protected]>
  2022-03-02 14:16                 ` Re: Proposal: Support custom authentication methods using hooks Jonathan S. Katz <[email protected]>
@ 2022-03-03 09:59                   ` Peter Eisentraut <[email protected]>
  1 sibling, 0 replies; 26+ messages in thread

From: Peter Eisentraut @ 2022-03-03 09:59 UTC (permalink / raw)
  To: Jonathan S. Katz <[email protected]>; Stephen Frost <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On 02.03.22 15:16, Jonathan S. Katz wrote:
> What are the reasons they are still purposely using it? The ones I have 
> seen/heard are:
> 
> - Using an older driver
> - On a pre-v10 PG
> - Unaware of SCRAM

Another reason is that SCRAM presents subtle operational issues in 
distributed systems.  As someone who is involved with products such as 
pgbouncer and bdr, I am aware that there are still unresolved problems 
and ongoing research in that area.  Maybe they can all be solved 
eventually, even if it is concluding "you can't do that anymore" in 
certain cases, but it's not all solved yet, and falling back to the 
best-method-before-this-one is a useful workaround.

I'm thinking there might be room for an authentication method between 
plain and scram that is less complicated and allows distributed systems 
to be set up more easily.  I don't know what that would be, but I don't 
think we should prohibit the consideration of "anything less than SCRAM".

I notice that a lot of internet services are promoting "application 
passwords" nowadays.  I don't know the implementation details of that, 
but it appears that the overall idea is to have instead of one 
high-value password have many frequently generated medium-value 
passwords.  We also have a recent proposal to store multiple passwords 
per user.  (Obviously that could apply to SCRAM and not-SCRAM equally.) 
That's the kind of direction I would like to explore.







^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-02 14:55             ` Bruce Momjian <[email protected]>
  2022-03-02 14:58               ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Bruce Momjian @ 2022-03-02 14:55 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

On Tue, Mar  1, 2022 at 08:31:19AM -0500, Stephen Frost wrote:
> > The last time I played with this area is the recent error handling
> > improvement with cryptohashes but MD5 has actually helped here in
> > detecting the problem as a patched OpenSSL would complain if trying to
> > use MD5 as hash function when FIPS is enabled.
> 
> Having to continue to deal with md5 as an algorithm when it's known to
> be notably less secure and so much so that organizations essentially ban
> its use for exactly what we're using it for, in fact, another reason to

Really?  I thought it was publicly-visible MD5 hashes that were the
biggest problem.  Our 32-bit salt during the connection is a problem, of
course.

> remove it, not a reason to keep it.  Better code coverage testing of
> error paths is the answer to making sure that our error handling behaves
> properly.

What is the logic to removing md5 but keeping 'password'?

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.







^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-02 14:55             ` Re: Proposal: Support custom authentication methods using hooks Bruce Momjian <[email protected]>
@ 2022-03-02 14:58               ` Stephen Frost <[email protected]>
  2022-03-02 15:01                 ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Stephen Frost @ 2022-03-02 14:58 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Greetings,

* Bruce Momjian ([email protected]) wrote:
> On Tue, Mar  1, 2022 at 08:31:19AM -0500, Stephen Frost wrote:
> > > The last time I played with this area is the recent error handling
> > > improvement with cryptohashes but MD5 has actually helped here in
> > > detecting the problem as a patched OpenSSL would complain if trying to
> > > use MD5 as hash function when FIPS is enabled.
> > 
> > Having to continue to deal with md5 as an algorithm when it's known to
> > be notably less secure and so much so that organizations essentially ban
> > its use for exactly what we're using it for, in fact, another reason to
> 
> Really?  I thought it was publicly-visible MD5 hashes that were the
> biggest problem.  Our 32-bit salt during the connection is a problem, of
> course.

Neither are good.  Not sure that we really need to spend a lot of effort
trying to figure out which issue is the biggest problem.

> > remove it, not a reason to keep it.  Better code coverage testing of
> > error paths is the answer to making sure that our error handling behaves
> > properly.
> 
> What is the logic to removing md5 but keeping 'password'?

I don't think we should keep 'password'.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-01 06:26         ` Re: Proposal: Support custom authentication methods using hooks Michael Paquier <[email protected]>
  2022-03-01 13:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-02 14:55             ` Re: Proposal: Support custom authentication methods using hooks Bruce Momjian <[email protected]>
  2022-03-02 14:58               ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-02 15:01                 ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Tom Lane @ 2022-03-02 15:01 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Michael Paquier <[email protected]>; Jeff Davis <[email protected]>; samay sharma <[email protected]>; [email protected]

Stephen Frost <[email protected]> writes:
> * Bruce Momjian ([email protected]) wrote:
>> What is the logic to removing md5 but keeping 'password'?

> I don't think we should keep 'password'.

I don't see much point in that unless we deprecate *all* the
auth methods that transmit a cleartext password.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-03 06:15         ` Tatsuo Ishii <[email protected]>
  2022-03-03 17:31           ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Tatsuo Ishii @ 2022-03-03 06:15 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]

> Yes, really, it's a known-broken system which suffers from such an old
> and well known attack that it's been given a name: pass-the-hash.  As
> was discussed on this thread even, just the fact that it's not trivial
> to break on the wire doesn't make it not-broken, particularly when we
> use the username (which is rather commonly the same one used across
> multiple systems..) as the salt.  Worse, md5 isn't exactly the pinnacle

I am not a big fan of md5 auth but saying that md5 auth uses username
as the salt is oversimplified. The md5 hashed password shored in
pg_shadow is created as md5(password + username).  But the md5 hashed
password flying over wire is using a random salt like md5(md5(password
+ username) + random_salt).

Best reagards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-02-28 21:18     ` Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-28 21:42       ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
  2022-03-03 06:15         ` Re: Proposal: Support custom authentication methods using hooks Tatsuo Ishii <[email protected]>
@ 2022-03-03 17:31           ` Stephen Frost <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Stephen Frost @ 2022-03-03 17:31 UTC (permalink / raw)
  To: Tatsuo Ishii <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]

Greetings,

* Tatsuo Ishii ([email protected]) wrote:
> > Yes, really, it's a known-broken system which suffers from such an old
> > and well known attack that it's been given a name: pass-the-hash.  As
> > was discussed on this thread even, just the fact that it's not trivial
> > to break on the wire doesn't make it not-broken, particularly when we
> > use the username (which is rather commonly the same one used across
> > multiple systems..) as the salt.  Worse, md5 isn't exactly the pinnacle
> 
> I am not a big fan of md5 auth but saying that md5 auth uses username
> as the salt is oversimplified. The md5 hashed password shored in
> pg_shadow is created as md5(password + username).  But the md5 hashed
> password flying over wire is using a random salt like md5(md5(password
> + username) + random_salt).

Err, no, it's not oversimplified at all- we do, in fact, as you say
above, use the username as the salt for what gets stored in pg_authid
(pg_shadow is just a view).  That's absolutely a problem because servers
can be compromised, backups can be compromised, and when it comes to PG
servers you don't even need to actually bother cracking the password
once you've gained access to an md5 value in pg_authid anyway.

Yes, we do use a challenge/response over the wire but that doesn't
absolve us of the fact that the hashes we store in pg_authid with the
md5 method is subject to pass-the-hash and brute-force attacks against
it.  If anything, the challenge/response over the wire is less useful
considering the common usage of TLS these days.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Proposal: Support custom authentication methods using hooks
  2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
  2022-02-27 23:17 ` Re: Proposal: Support custom authentication methods using hooks Jeff Davis <[email protected]>
  2022-02-28 20:46   ` Re: Proposal: Support custom authentication methods using hooks Stephen Frost <[email protected]>
@ 2022-03-01 21:14     ` Nathan Bossart <[email protected]>
  1 sibling, 0 replies; 26+ messages in thread

From: Nathan Bossart @ 2022-03-01 21:14 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; +Cc: Jeff Davis <[email protected]>; Tom Lane <[email protected]>; samay sharma <[email protected]>; [email protected]

On Mon, Feb 28, 2022 at 03:46:34PM -0500, Stephen Frost wrote:
> md5 should be removed.

+1

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* [PATCH v2 1/2] Do not lock tables in get_tables_to_repack().
@ 2026-06-16 06:49 ChangAo Chen <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: ChangAo Chen @ 2026-06-16 06:49 UTC (permalink / raw)

When doing a whole database repack, we build a list of repackable
tables and take a lock on them to prevent concurrent drops. But
concurrent drops can always happen after we build the list because
we process each table in a separate transaction. The
ConditionalLockRelationOid() also makes the default behavior like
SKIP_LOCKED, which is unexpected.

To remove the locks, we need to make repack_is_permitted_for_relation()
handles concurrent drops correctly: it should not report an error
when failing to search the syscache in pg_class_aclcheck(). Use
pg_class_aclcheck_ext() instead to detect a concurrent drop. Also
check the return value of get_rel_name().

While at it, replace relation_close() with table_close() to match
the table_open().
---
 src/backend/commands/repack.c | 67 ++++++++++-------------------------
 1 file changed, 19 insertions(+), 48 deletions(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index faa07d1a118..2879c8af574 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -2169,22 +2169,9 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
 
-			/*
-			 * Try to obtain a light lock on the index's table, to ensure it
-			 * doesn't go away while we collect the list.  If we cannot, just
-			 * disregard it.  Be sure to release this if we ultimately decide
-			 * not to process the table!
-			 */
-			if (!ConditionalLockRelationOid(index->indrelid, AccessShareLock))
-				continue;
-
-			/* Verify that the table still exists; skip if not */
 			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
 			if (!HeapTupleIsValid(classtup))
-			{
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
-			}
 			classForm = (Form_pg_class) GETSTRUCT(classtup);
 
 			/* Skip temp relations belonging to other sessions */
@@ -2192,7 +2179,6 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				!isTempOrTempToastNamespace(classForm->relnamespace))
 			{
 				ReleaseSysCache(classtup);
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
 
@@ -2201,10 +2187,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
 												  GetUserId()))
-			{
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
-			}
 
 			/* Use a permanent memory context for the result list */
 			oldcxt = MemoryContextSwitchTo(permcxt);
@@ -2228,45 +2211,20 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			class = (Form_pg_class) GETSTRUCT(tuple);
 
-			/*
-			 * Try to obtain a light lock on the table, to ensure it doesn't
-			 * go away while we collect the list.  If we cannot, just
-			 * disregard the table.  Be sure to release this if we ultimately
-			 * decide not to process the table!
-			 */
-			if (!ConditionalLockRelationOid(class->oid, AccessShareLock))
-				continue;
-
-			/* Verify that the table still exists */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(class->oid)))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
-				continue;
-			}
-
 			/* Can only process plain tables and matviews */
 			if (class->relkind != RELKIND_RELATION &&
 				class->relkind != RELKIND_MATVIEW)
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* Skip temp relations belonging to other sessions */
 			if (class->relpersistence == RELPERSISTENCE_TEMP &&
 				!isTempOrTempToastNamespace(class->relnamespace))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* Use a permanent memory context for the result list */
 			oldcxt = MemoryContextSwitchTo(permcxt);
@@ -2279,7 +2237,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 	}
 
 	table_endscan(scan);
-	relation_close(catalog, AccessShareLock);
+	table_close(catalog, AccessShareLock);
 
 	return rtcs;
 }
@@ -2357,15 +2315,28 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid,
 static bool
 repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid)
 {
+	bool		is_missing = false;
+
 	Assert(cmd == REPACK_COMMAND_CLUSTER || cmd == REPACK_COMMAND_REPACK);
 
-	if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK)
+	if (pg_class_aclcheck_ext(relid, userid, ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
 		return true;
 
-	ereport(WARNING,
-			errmsg("permission denied to execute %s on \"%s\", skipping it",
-				   RepackCommandAsString(cmd),
-				   get_rel_name(relid)));
+	/* Report a warning if the relation still exists. */
+	if (!is_missing)
+	{
+		char	   *relname;
+
+		relname = get_rel_name(relid);
+		if (relname != NULL)
+		{
+			ereport(WARNING,
+					errmsg("permission denied to execute %s on \"%s\", skipping it",
+						   RepackCommandAsString(cmd), relname));
+
+			pfree(relname);
+		}
+	}
 
 	return false;
 }
-- 
2.47.3


--op6mnexl7cn72cto
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename=v2-0002-fixups.patch



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* [PATCH v2 1/2] Do not lock tables in get_tables_to_repack().
@ 2026-06-16 06:49 ChangAo Chen <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: ChangAo Chen @ 2026-06-16 06:49 UTC (permalink / raw)

When doing a whole database repack, we build a list of repackable
tables and take a lock on them to prevent concurrent drops. But
concurrent drops can always happen after we build the list because
we process each table in a separate transaction. The
ConditionalLockRelationOid() also makes the default behavior like
SKIP_LOCKED, which is unexpected.

To remove the locks, we need to make repack_is_permitted_for_relation()
handles concurrent drops correctly: it should not report an error
when failing to search the syscache in pg_class_aclcheck(). Use
pg_class_aclcheck_ext() instead to detect a concurrent drop. Also
check the return value of get_rel_name().

While at it, replace relation_close() with table_close() to match
the table_open().
---
 src/backend/commands/repack.c | 67 ++++++++++-------------------------
 1 file changed, 19 insertions(+), 48 deletions(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index faa07d1a118..2879c8af574 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -2169,22 +2169,9 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			index = (Form_pg_index) GETSTRUCT(tuple);
 
-			/*
-			 * Try to obtain a light lock on the index's table, to ensure it
-			 * doesn't go away while we collect the list.  If we cannot, just
-			 * disregard it.  Be sure to release this if we ultimately decide
-			 * not to process the table!
-			 */
-			if (!ConditionalLockRelationOid(index->indrelid, AccessShareLock))
-				continue;
-
-			/* Verify that the table still exists; skip if not */
 			classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
 			if (!HeapTupleIsValid(classtup))
-			{
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
-			}
 			classForm = (Form_pg_class) GETSTRUCT(classtup);
 
 			/* Skip temp relations belonging to other sessions */
@@ -2192,7 +2179,6 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 				!isTempOrTempToastNamespace(classForm->relnamespace))
 			{
 				ReleaseSysCache(classtup);
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
 			}
 
@@ -2201,10 +2187,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, index->indrelid,
 												  GetUserId()))
-			{
-				UnlockRelationOid(index->indrelid, AccessShareLock);
 				continue;
-			}
 
 			/* Use a permanent memory context for the result list */
 			oldcxt = MemoryContextSwitchTo(permcxt);
@@ -2228,45 +2211,20 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 
 			class = (Form_pg_class) GETSTRUCT(tuple);
 
-			/*
-			 * Try to obtain a light lock on the table, to ensure it doesn't
-			 * go away while we collect the list.  If we cannot, just
-			 * disregard the table.  Be sure to release this if we ultimately
-			 * decide not to process the table!
-			 */
-			if (!ConditionalLockRelationOid(class->oid, AccessShareLock))
-				continue;
-
-			/* Verify that the table still exists */
-			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(class->oid)))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
-				continue;
-			}
-
 			/* Can only process plain tables and matviews */
 			if (class->relkind != RELKIND_RELATION &&
 				class->relkind != RELKIND_MATVIEW)
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* Skip temp relations belonging to other sessions */
 			if (class->relpersistence == RELPERSISTENCE_TEMP &&
 				!isTempOrTempToastNamespace(class->relnamespace))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* noisily skip rels which the user can't process */
 			if (!repack_is_permitted_for_relation(cmd, class->oid,
 												  GetUserId()))
-			{
-				UnlockRelationOid(class->oid, AccessShareLock);
 				continue;
-			}
 
 			/* Use a permanent memory context for the result list */
 			oldcxt = MemoryContextSwitchTo(permcxt);
@@ -2279,7 +2237,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt)
 	}
 
 	table_endscan(scan);
-	relation_close(catalog, AccessShareLock);
+	table_close(catalog, AccessShareLock);
 
 	return rtcs;
 }
@@ -2357,15 +2315,28 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid,
 static bool
 repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid)
 {
+	bool		is_missing = false;
+
 	Assert(cmd == REPACK_COMMAND_CLUSTER || cmd == REPACK_COMMAND_REPACK);
 
-	if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK)
+	if (pg_class_aclcheck_ext(relid, userid, ACL_MAINTAIN, &is_missing) == ACLCHECK_OK)
 		return true;
 
-	ereport(WARNING,
-			errmsg("permission denied to execute %s on \"%s\", skipping it",
-				   RepackCommandAsString(cmd),
-				   get_rel_name(relid)));
+	/* Report a warning if the relation still exists. */
+	if (!is_missing)
+	{
+		char	   *relname;
+
+		relname = get_rel_name(relid);
+		if (relname != NULL)
+		{
+			ereport(WARNING,
+					errmsg("permission denied to execute %s on \"%s\", skipping it",
+						   RepackCommandAsString(cmd), relname));
+
+			pfree(relname);
+		}
+	}
 
 	return false;
 }
-- 
2.47.3


--op6mnexl7cn72cto
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename=v2-0002-fixups.patch



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: Do not lock tables in get_tables_to_repack
@ 2026-07-10 14:22 Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Álvaro Herrera @ 2026-07-10 14:22 UTC (permalink / raw)
  To: cca5507 <[email protected]>; +Cc: pgsql-hackers <[email protected]>

On 2026-Jul-08, cca5507 wrote:

> -                       classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
> +                       classtup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(index->indrelid));
> 
> Do we really need to copy it? We hold a refcount on it so it won't be freed
> until we release it. Otherwise LGTM.

Yeah, I guess it doesn't matter.  I have removed the copy and updated
some comments.  I also realized that there are some places where we
weren't dealing correctly with the possibility that the relation goes
away, or is replaced with something different, so I added that too.

While looking at it I also realized that get_tables_to_repack_partitioned
is likewise not careful enough about it: we do IndexGetRelation(, false)
which fails hard if the pg_index tuple cannot be found, which is the
wrong thing to do.

At the same time, it's annoying that half of the code that clearly
belongs in that routine is actually in ExecRepack().  I moved that to
where it rightfully belongs.  (The only somewhat annoying thing is that
we have to NULL-out the Relation pointer after returning; but that's not
*too* bad IMO.)

Any opinions on this?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"No me acuerdo, pero no es cierto.  No es cierto, y si fuera cierto,
 no me acuerdo."                 (Augusto Pinochet a una corte de justicia)


^ permalink  raw  reply  [nested|flat] 26+ messages in thread


end of thread, other threads:[~2026-07-10 14:22 UTC | newest]

Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 19:10 Re: Proposal: Support custom authentication methods using hooks Tom Lane <[email protected]>
2022-02-25 19:49 ` Andres Freund <[email protected]>
2022-02-27 23:17 ` Jeff Davis <[email protected]>
2022-02-28 20:46   ` Stephen Frost <[email protected]>
2022-02-28 21:18     ` Tom Lane <[email protected]>
2022-02-28 21:42       ` Stephen Frost <[email protected]>
2022-03-01 06:26         ` Michael Paquier <[email protected]>
2022-03-01 13:31           ` Stephen Frost <[email protected]>
2022-03-01 21:17             ` Jonathan S. Katz <[email protected]>
2022-03-02 08:24               ` Peter Eisentraut <[email protected]>
2022-03-02 14:16                 ` Jonathan S. Katz <[email protected]>
2022-03-02 15:29                   ` Peter Eisentraut <[email protected]>
2022-03-02 15:30                     ` Stephen Frost <[email protected]>
2022-03-02 15:45                       ` Jonathan S. Katz <[email protected]>
2022-03-03 09:45                         ` Peter Eisentraut <[email protected]>
2022-03-03 16:12                           ` Joshua Brindle <[email protected]>
2022-03-03 09:59                   ` Peter Eisentraut <[email protected]>
2022-03-02 14:55             ` Bruce Momjian <[email protected]>
2022-03-02 14:58               ` Stephen Frost <[email protected]>
2022-03-02 15:01                 ` Tom Lane <[email protected]>
2022-03-03 06:15         ` Tatsuo Ishii <[email protected]>
2022-03-03 17:31           ` Stephen Frost <[email protected]>
2022-03-01 21:14     ` Nathan Bossart <[email protected]>
2026-06-16 06:49 [PATCH v2 1/2] Do not lock tables in get_tables_to_repack(). ChangAo Chen <[email protected]>
2026-06-16 06:49 [PATCH v2 1/2] Do not lock tables in get_tables_to_repack(). ChangAo Chen <[email protected]>
2026-07-10 14:22 Re: Do not lock tables in get_tables_to_repack Álvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox